diff options
| -rw-r--r-- | base/debug/trace_event_impl.cc | 185 | ||||
| -rw-r--r-- | base/debug/trace_event_impl.h | 27 | ||||
| -rw-r--r-- | base/debug/trace_event_unittest.cc | 83 |
3 files changed, 193 insertions, 102 deletions
diff --git a/base/debug/trace_event_impl.cc b/base/debug/trace_event_impl.cc index ab282e7..176f67c 100644 --- a/base/debug/trace_event_impl.cc +++ b/base/debug/trace_event_impl.cc @@ -910,62 +910,71 @@ void TraceLog::GetKnownCategoryGroups( void TraceLog::SetEnabled(const CategoryFilter& category_filter, Options options) { - AutoLock lock(lock_); + std::vector<EnabledStateObserver*> observer_list; + { + AutoLock lock(lock_); - if (enable_count_++ > 0) { - if (options != trace_options_) { - DLOG(ERROR) << "Attemting to re-enable tracing with a different " - << "set of options."; + if (enable_count_++ > 0) { + if (options != trace_options_) { + DLOG(ERROR) << "Attemting to re-enable tracing with a different " + << "set of options."; + } + + category_filter_.Merge(category_filter); + EnableIncludedCategoryGroups(); + return; } - category_filter_.Merge(category_filter); - EnableIncludedCategoryGroups(); - return; - } + if (options != trace_options_) { + trace_options_ = options; + logged_events_.reset(GetTraceBuffer()); + } - if (options != trace_options_) { - trace_options_ = options; - logged_events_.reset(GetTraceBuffer()); - } + if (dispatching_to_observer_list_) { + DLOG(ERROR) << + "Cannot manipulate TraceLog::Enabled state from an observer."; + return; + } - if (dispatching_to_observer_list_) { - DLOG(ERROR) << - "Cannot manipulate TraceLog::Enabled state from an observer."; - return; - } + num_traces_recorded_++; - num_traces_recorded_++; + category_filter_ = CategoryFilter(category_filter); + EnableIncludedCategoryGroups(); - dispatching_to_observer_list_ = true; - FOR_EACH_OBSERVER(EnabledStateChangedObserver, enabled_state_observer_list_, - OnTraceLogWillEnable()); - dispatching_to_observer_list_ = false; + // Not supported in split-dll build. http://crbug.com/237249 + #if !defined(CHROME_SPLIT_DLL) + if (options & ENABLE_SAMPLING) { + sampling_thread_.reset(new TraceSamplingThread); + sampling_thread_->RegisterSampleBucket( + &g_trace_state0, + "bucket0", + Bind(&TraceSamplingThread::DefaultSampleCallback)); + sampling_thread_->RegisterSampleBucket( + &g_trace_state1, + "bucket1", + Bind(&TraceSamplingThread::DefaultSampleCallback)); + sampling_thread_->RegisterSampleBucket( + &g_trace_state2, + "bucket2", + Bind(&TraceSamplingThread::DefaultSampleCallback)); + if (!PlatformThread::Create( + 0, sampling_thread_.get(), &sampling_thread_handle_)) { + DCHECK(false) << "failed to create thread"; + } + } + #endif - category_filter_ = CategoryFilter(category_filter); - EnableIncludedCategoryGroups(); + dispatching_to_observer_list_ = true; + observer_list = enabled_state_observer_list_; + } + // Notify observers outside the lock in case they trigger trace events. + for (size_t i = 0; i < observer_list.size(); ++i) + observer_list[i]->OnTraceLogEnabled(); - // Not supported in split-dll build. http://crbug.com/237249 -#if !defined(CHROME_SPLIT_DLL) - if (options & ENABLE_SAMPLING) { - sampling_thread_.reset(new TraceSamplingThread); - sampling_thread_->RegisterSampleBucket( - &g_trace_state0, - "bucket0", - Bind(&TraceSamplingThread::DefaultSampleCallback)); - sampling_thread_->RegisterSampleBucket( - &g_trace_state1, - "bucket1", - Bind(&TraceSamplingThread::DefaultSampleCallback)); - sampling_thread_->RegisterSampleBucket( - &g_trace_state2, - "bucket2", - Bind(&TraceSamplingThread::DefaultSampleCallback)); - if (!PlatformThread::Create( - 0, sampling_thread_.get(), &sampling_thread_handle_)) { - DCHECK(false) << "failed to create thread"; - } + { + AutoLock lock(lock_); + dispatching_to_observer_list_ = false; } -#endif } const CategoryFilter& TraceLog::GetCurrentCategoryFilter() { @@ -975,39 +984,49 @@ const CategoryFilter& TraceLog::GetCurrentCategoryFilter() { } void TraceLog::SetDisabled() { - AutoLock lock(lock_); - DCHECK(enable_count_ > 0); - if (--enable_count_ != 0) - return; + std::vector<EnabledStateObserver*> observer_list; + { + AutoLock lock(lock_); + DCHECK(enable_count_ > 0); + if (--enable_count_ != 0) + return; - if (dispatching_to_observer_list_) { - DLOG(ERROR) - << "Cannot manipulate TraceLog::Enabled state from an observer."; - return; - } + if (dispatching_to_observer_list_) { + DLOG(ERROR) + << "Cannot manipulate TraceLog::Enabled state from an observer."; + return; + } - if (sampling_thread_.get()) { - // Stop the sampling thread. - sampling_thread_->Stop(); - lock_.Release(); - PlatformThread::Join(sampling_thread_handle_); - lock_.Acquire(); - sampling_thread_handle_ = PlatformThreadHandle(); - sampling_thread_.reset(); + if (sampling_thread_.get()) { + // Stop the sampling thread. + sampling_thread_->Stop(); + lock_.Release(); + PlatformThread::Join(sampling_thread_handle_); + lock_.Acquire(); + sampling_thread_handle_ = PlatformThreadHandle(); + sampling_thread_.reset(); + } + + category_filter_.Clear(); + watch_category_ = NULL; + watch_event_name_ = ""; + for (int i = 0; i < g_category_index; i++) + SetCategoryGroupEnabled(i, false); + AddThreadNameMetadataEvents(); + + dispatching_to_observer_list_ = true; + observer_list = enabled_state_observer_list_; } - dispatching_to_observer_list_ = true; - FOR_EACH_OBSERVER(EnabledStateChangedObserver, - enabled_state_observer_list_, - OnTraceLogWillDisable()); - dispatching_to_observer_list_ = false; + // Dispatch to observers outside the lock in case the observer triggers a + // trace event. + for (size_t i = 0; i < observer_list.size(); ++i) + observer_list[i]->OnTraceLogDisabled(); - category_filter_.Clear(); - watch_category_ = NULL; - watch_event_name_ = ""; - for (int i = 0; i < g_category_index; i++) - SetCategoryGroupEnabled(i, false); - AddThreadNameMetadataEvents(); + { + AutoLock lock(lock_); + dispatching_to_observer_list_ = false; + } } int TraceLog::GetNumTracesRecorded() { @@ -1017,13 +1036,17 @@ int TraceLog::GetNumTracesRecorded() { return num_traces_recorded_; } -void TraceLog::AddEnabledStateObserver(EnabledStateChangedObserver* listener) { - enabled_state_observer_list_.AddObserver(listener); +void TraceLog::AddEnabledStateObserver(EnabledStateObserver* listener) { + enabled_state_observer_list_.push_back(listener); } -void TraceLog::RemoveEnabledStateObserver( - EnabledStateChangedObserver* listener) { - enabled_state_observer_list_.RemoveObserver(listener); +void TraceLog::RemoveEnabledStateObserver(EnabledStateObserver* listener) { + std::vector<EnabledStateObserver*>::iterator it = + std::find(enabled_state_observer_list_.begin(), + enabled_state_observer_list_.end(), + listener); + if (it != enabled_state_observer_list_.end()) + enabled_state_observer_list_.erase(it); } float TraceLog::GetBufferPercentFull() const { @@ -1320,6 +1343,10 @@ void TraceLog::SetTimeOffset(TimeDelta offset) { time_offset_ = offset; } +size_t TraceLog::GetObserverCountForTest() const { + return enabled_state_observer_list_.size(); +} + bool CategoryFilter::IsEmptyOrContainsLeadingOrTrailingWhitespace( const std::string& str) { return str.empty() || diff --git a/base/debug/trace_event_impl.h b/base/debug/trace_event_impl.h index 384e09f..a7ebaf2 100644 --- a/base/debug/trace_event_impl.h +++ b/base/debug/trace_event_impl.h @@ -332,21 +332,18 @@ class BASE_EXPORT TraceLog { // Enabled state listeners give a callback when tracing is enabled or // disabled. This can be used to tie into other library's tracing systems // on-demand. - class EnabledStateChangedObserver { + class EnabledStateObserver { public: - // Called just before the tracing system becomes - // enabled. TraceLog::IsEnabled will return false at this point and trace - // macros and methods called within the observer will deadlock. - virtual void OnTraceLogWillEnable() { } - - // Called just before the tracing system disables. TraceLog::IsEnabled is - // still false at this point TRACE macros will still be capturing - // data. However, trace macros and methods called within the observer will - // deadlock. - virtual void OnTraceLogWillDisable() { } + // Called just after the tracing system becomes enabled, outside of the + // |lock_|. TraceLog::IsEnabled() is true at this point. + virtual void OnTraceLogEnabled() = 0; + + // Called just after the tracing system disables, outside of the |lock_|. + // TraceLog::IsEnabled() is false at this point. + virtual void OnTraceLogDisabled() = 0; }; - void AddEnabledStateObserver(EnabledStateChangedObserver* listener); - void RemoveEnabledStateObserver(EnabledStateChangedObserver* listener); + void AddEnabledStateObserver(EnabledStateObserver* listener); + void RemoveEnabledStateObserver(EnabledStateObserver* listener); float GetBufferPercentFull() const; @@ -458,6 +455,8 @@ class BASE_EXPORT TraceLog { // that should be reported. void SetTimeOffset(TimeDelta offset); + size_t GetObserverCountForTest() const; + private: // This allows constructor and destructor to be private and usable only // by the Singleton class. @@ -538,7 +537,7 @@ class BASE_EXPORT TraceLog { scoped_ptr<TraceBuffer> logged_events_; EventCallback event_callback_; bool dispatching_to_observer_list_; - ObserverList<EnabledStateChangedObserver> enabled_state_observer_list_; + std::vector<EnabledStateObserver*> enabled_state_observer_list_; base::hash_map<int, std::string> thread_names_; base::hash_map<int, std::stack<TimeTicks> > thread_event_start_times_; diff --git a/base/debug/trace_event_unittest.cc b/base/debug/trace_event_unittest.cc index d2d38c7..8685ceb 100644 --- a/base/debug/trace_event_unittest.cc +++ b/base/debug/trace_event_unittest.cc @@ -819,10 +819,10 @@ TEST_F(TraceEventTestFixture, DataCaptured) { } class MockEnabledStateChangedObserver : - public base::debug::TraceLog::EnabledStateChangedObserver { + public base::debug::TraceLog::EnabledStateObserver { public: - MOCK_METHOD0(OnTraceLogWillEnable, void()); - MOCK_METHOD0(OnTraceLogWillDisable, void()); + MOCK_METHOD0(OnTraceLogEnabled, void()); + MOCK_METHOD0(OnTraceLogDisabled, void()); }; TEST_F(TraceEventTestFixture, EnabledObserverFiresOnEnable) { @@ -831,11 +831,12 @@ TEST_F(TraceEventTestFixture, EnabledObserverFiresOnEnable) { MockEnabledStateChangedObserver observer; TraceLog::GetInstance()->AddEnabledStateObserver(&observer); - EXPECT_CALL(observer, OnTraceLogWillEnable()) + EXPECT_CALL(observer, OnTraceLogEnabled()) .Times(1); TraceLog::GetInstance()->SetEnabled(CategoryFilter("*"), TraceLog::RECORD_UNTIL_FULL); testing::Mock::VerifyAndClear(&observer); + EXPECT_TRUE(TraceLog::GetInstance()->IsEnabled()); // Cleanup. TraceLog::GetInstance()->RemoveEnabledStateObserver(&observer); @@ -851,13 +852,14 @@ TEST_F(TraceEventTestFixture, EnabledObserverDoesntFireOnSecondEnable) { testing::StrictMock<MockEnabledStateChangedObserver> observer; TraceLog::GetInstance()->AddEnabledStateObserver(&observer); - EXPECT_CALL(observer, OnTraceLogWillEnable()) + EXPECT_CALL(observer, OnTraceLogEnabled()) .Times(0); - EXPECT_CALL(observer, OnTraceLogWillDisable()) + EXPECT_CALL(observer, OnTraceLogDisabled()) .Times(0); TraceLog::GetInstance()->SetEnabled(CategoryFilter("*"), TraceLog::RECORD_UNTIL_FULL); testing::Mock::VerifyAndClear(&observer); + EXPECT_TRUE(TraceLog::GetInstance()->IsEnabled()); // Cleanup. TraceLog::GetInstance()->RemoveEnabledStateObserver(&observer); @@ -875,9 +877,9 @@ TEST_F(TraceEventTestFixture, EnabledObserverDoesntFireOnNestedDisable) { testing::StrictMock<MockEnabledStateChangedObserver> observer; TraceLog::GetInstance()->AddEnabledStateObserver(&observer); - EXPECT_CALL(observer, OnTraceLogWillEnable()) + EXPECT_CALL(observer, OnTraceLogEnabled()) .Times(0); - EXPECT_CALL(observer, OnTraceLogWillDisable()) + EXPECT_CALL(observer, OnTraceLogDisabled()) .Times(0); TraceLog::GetInstance()->SetDisabled(); testing::Mock::VerifyAndClear(&observer); @@ -896,7 +898,7 @@ TEST_F(TraceEventTestFixture, EnabledObserverFiresOnDisable) { MockEnabledStateChangedObserver observer; TraceLog::GetInstance()->AddEnabledStateObserver(&observer); - EXPECT_CALL(observer, OnTraceLogWillDisable()) + EXPECT_CALL(observer, OnTraceLogDisabled()) .Times(1); TraceLog::GetInstance()->SetDisabled(); testing::Mock::VerifyAndClear(&observer); @@ -905,6 +907,69 @@ TEST_F(TraceEventTestFixture, EnabledObserverFiresOnDisable) { TraceLog::GetInstance()->RemoveEnabledStateObserver(&observer); } +// Tests the IsEnabled() state of TraceLog changes before callbacks. +class AfterStateChangeEnabledStateObserver + : public base::debug::TraceLog::EnabledStateObserver { + public: + AfterStateChangeEnabledStateObserver() {} + virtual ~AfterStateChangeEnabledStateObserver() {} + + // base::debug::TraceLog::EnabledStateObserver overrides: + virtual void OnTraceLogEnabled() OVERRIDE { + EXPECT_TRUE(TraceLog::GetInstance()->IsEnabled()); + } + + virtual void OnTraceLogDisabled() OVERRIDE { + EXPECT_FALSE(TraceLog::GetInstance()->IsEnabled()); + } +}; + +TEST_F(TraceEventTestFixture, ObserversFireAfterStateChange) { + ManualTestSetUp(); + + AfterStateChangeEnabledStateObserver observer; + TraceLog::GetInstance()->AddEnabledStateObserver(&observer); + + TraceLog::GetInstance()->SetEnabled(CategoryFilter("*"), + TraceLog::RECORD_UNTIL_FULL); + EXPECT_TRUE(TraceLog::GetInstance()->IsEnabled()); + + TraceLog::GetInstance()->SetDisabled(); + EXPECT_FALSE(TraceLog::GetInstance()->IsEnabled()); + + TraceLog::GetInstance()->RemoveEnabledStateObserver(&observer); +} + +// Tests that a state observer can remove itself during a callback. +class SelfRemovingEnabledStateObserver + : public base::debug::TraceLog::EnabledStateObserver { + public: + SelfRemovingEnabledStateObserver() {} + virtual ~SelfRemovingEnabledStateObserver() {} + + // base::debug::TraceLog::EnabledStateObserver overrides: + virtual void OnTraceLogEnabled() OVERRIDE {} + + virtual void OnTraceLogDisabled() OVERRIDE { + TraceLog::GetInstance()->RemoveEnabledStateObserver(this); + } +}; + +TEST_F(TraceEventTestFixture, SelfRemovingObserver) { + ManualTestSetUp(); + ASSERT_EQ(0u, TraceLog::GetInstance()->GetObserverCountForTest()); + + SelfRemovingEnabledStateObserver observer; + TraceLog::GetInstance()->AddEnabledStateObserver(&observer); + EXPECT_EQ(1u, TraceLog::GetInstance()->GetObserverCountForTest()); + + TraceLog::GetInstance()->SetEnabled(CategoryFilter("*"), + TraceLog::RECORD_UNTIL_FULL); + TraceLog::GetInstance()->SetDisabled(); + // The observer removed itself on disable. + EXPECT_EQ(0u, TraceLog::GetInstance()->GetObserverCountForTest()); +} + bool IsNewTrace() { bool is_new_trace; TRACE_EVENT_IS_NEW_TRACE(&is_new_trace); |