Protect TraceLog::enabled_state_observer_list_ with a dedicated lock

base/trace_event/trace_event_unittest.cc

Index: base/trace_event/trace_event_unittest.cc
diff --git a/base/trace_event/trace_event_unittest.cc b/base/trace_event/trace_event_unittest.cc
index 6a1d8a4991d28952f25e900e7378c48cd6ef80f9..9b2eb1318ebd4af49be19493eb0e79d861257102 100644
--- a/base/trace_event/trace_event_unittest.cc
+++ b/base/trace_event/trace_event_unittest.cc
@@ -1136,35 +1136,6 @@ TEST_F(TraceEventTestFixture, ObserversFireAfterStateChange) {
   TraceLog::GetInstance()->RemoveEnabledStateObserver(&observer);
 }
 
-// Tests that a state observer can remove itself during a callback.
-class SelfRemovingEnabledStateObserver
-    : public TraceLog::EnabledStateObserver {
- public:
-  SelfRemovingEnabledStateObserver() {}
-  ~SelfRemovingEnabledStateObserver() override {}
-
-  // TraceLog::EnabledStateObserver overrides:
-  void OnTraceLogEnabled() override {}
-
-  void OnTraceLogDisabled() override {
-    TraceLog::GetInstance()->RemoveEnabledStateObserver(this);
-  }
-};
-
-TEST_F(TraceEventTestFixture, SelfRemovingObserver) {

[Primiano Tucci (use gerrit), 2016/04/27 13:50:21]

Hmm why are we removing this test? I assume it was introduced to capture some
use case no?

[Xiaocheng, 2016/04/27 14:18:13]

I haven't seen any real use case in the code base that needs to modifies
TraceLog::enabled_state_observer_list_ during the callback. Guess it's just a
corner case?

If we add a lock to protect the observer list when dispatching to them, this
test case has to be removed. Otherwise the call back would try to acquire the
same lock again (via RemoveEnabledStateObserver), which causes a deadlock.

-  ASSERT_EQ(0u, TraceLog::GetInstance()->GetObserverCountForTest());
-
-  SelfRemovingEnabledStateObserver observer;
-  TraceLog::GetInstance()->AddEnabledStateObserver(&observer);
-  EXPECT_EQ(1u, TraceLog::GetInstance()->GetObserverCountForTest());
-
-  TraceLog::GetInstance()->SetEnabled(TraceConfig(kRecordAllCategoryFilter, ""),
-                                      TraceLog::RECORDING_MODE);
-  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);