Protect TraceLog::enabled_state_observer_list_ with a dedicated lock

base/trace_event/trace_log.h

Index: base/trace_event/trace_log.h
diff --git a/base/trace_event/trace_log.h b/base/trace_event/trace_log.h
index 3f09674829381f30825e9ceea2734b0cf7111fc9..1f287d823e74d18d465ee38de46c03ea58934144 100644
--- a/base/trace_event/trace_log.h
+++ b/base/trace_event/trace_log.h
@@ -107,12 +107,18 @@ class BASE_EXPORT TraceLog : public MemoryDumpProvider {
    public:
     virtual ~EnabledStateObserver() = default;
 
-    // Called just after the tracing system becomes enabled, outside of the
-    // |lock_|. TraceLog::IsEnabled() is true at this point.
+    // Called just after the tracing system becomes enabled, outside of |lock_|
+    // but inside of |observer_list_lock_|. TraceLog::IsEnabled() is true at
+    // this point. Any subclass MUST NOT access |enabled_state_observer_list_|
+    // in the callback (e.g., via any |TraceLog::FooEnabledStateObserver()|),
+    // which would otherwise cause a deadlock.
     virtual void OnTraceLogEnabled() = 0;
 
-    // Called just after the tracing system disables, outside of the |lock_|.
-    // TraceLog::IsEnabled() is false at this point.
+    // Called just after the tracing system disables, outside of |lock_| but
+    // inside of |observer_list_lock_|. TraceLog::IsEnabled() is false at this
+    // point. Any subclass MUST NOT access |enabled_state_observer_list_| in the
+    // callback (e.g., via any |TraceLog::FooEnabledStateObserver()|), which
+    // would otherwise cause a deadlock.
     virtual void OnTraceLogDisabled() = 0;
   };
   void AddEnabledStateObserver(EnabledStateObserver* listener);
@@ -430,7 +436,7 @@ class BASE_EXPORT TraceLog : public MemoryDumpProvider {
   static const InternalTraceOptions kInternalEnableArgumentFilter;
 
   // This lock protects TraceLog member accesses (except for members protected
-  // by thread_info_lock_) from arbitrary threads.
+  // by thread_info_lock_ or observer_list_lock_) from arbitrary threads.
   mutable Lock lock_;
   // This lock protects accesses to thread_names_, thread_event_start_times_
   // and thread_colors_.
@@ -440,7 +446,13 @@ class BASE_EXPORT TraceLog : public MemoryDumpProvider {
   std::unique_ptr<TraceBuffer> logged_events_;
   std::vector<std::unique_ptr<TraceEvent>> metadata_events_;
   subtle::AtomicWord /* EventCallback */ event_callback_;
+
+  // This lock protects accesses to enabled_state_observer_list_.
+  mutable Lock observer_list_lock_;
   bool dispatching_to_observer_list_;
+  // TODO(xiaochengh): Replace these raw pointers by weak pointers and direct
+  // callbacks by task posting, so that we can get rid of the current nasty
+  // and complicated handling of threading issues.
   std::vector<EnabledStateObserver*> enabled_state_observer_list_;
 
   std::string process_name_;