Introduce TraceLog::AsyncEnabledStateObserver

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..3e7086ab4bfe8949786979a789327560f5d78472 100644
--- a/base/trace_event/trace_log.h
+++ b/base/trace_event/trace_log.h
@@ -103,6 +103,8 @@ class BASE_EXPORT TraceLog : public MemoryDumpProvider {
   // 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.
+  // TODO(xiaochengh): Deprecate this class. Migrate all its subclasses under

[Primiano Tucci (use gerrit), 2016/05/09 17:18:29]

I will be happy if we can remove all callers to this, not 100% sure you will be
able to do that.
I wonder if there are cases where we register observers before we have a message
loop (I am thinking specifically to the chain
TracingControllerImpl::GetInstance() -> MemoryDumpManager->Initialize() ->
AddEnabledStateObserver).

The truth is that most of the observer out there are indefinitely lived
(MemoryDumpManager,TraceNetLog, TraceEventSystemStatsMonitor,
android/trace_event_binding.cc) and don't really need this.
I think this is really just for BlameContext, V8 sampling profiler. 

What I am saying is: Makes sense what you are introducing here, not sure you
will be able to deprecated this. Try, if you succeed, good. If you don't, remove
the todo :)

[Xiaocheng, 2016/05/10 09:33:35]

Yeah, a deprecation of this class seems too much.

A better idea seems to be having two observer interfaces, synchronous and
asynchronous. Only some of the existing observer classes need to be migrated to
the async interface.

+  // ThreadSafeEnabledStateObserver.
   class BASE_EXPORT EnabledStateObserver {
    public:
     virtual ~EnabledStateObserver() = default;
@@ -119,6 +121,28 @@ class BASE_EXPORT TraceLog : public MemoryDumpProvider {
   void RemoveEnabledStateObserver(EnabledStateObserver* listener);
   bool HasEnabledStateObserver(EnabledStateObserver* listener) const;
 
+  // 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.

[Primiano Tucci (use gerrit), 2016/05/09 17:18:29]

I think the important information to convey to this comment is:
All observer calls are posted on the task runner where AddEnabledStateObserver
was called

[Xiaocheng, 2016/05/10 09:33:34]

Done.

+  class BASE_EXPORT ThreadSafeEnabledStateObserver {
+   public:
+    virtual ~ThreadSafeEnabledStateObserver() = default;
+
+    // Called just after the tracing system becomes enabled.
+    // TraceLog::IsEnabled() is true at this point.
+    virtual void OnTraceLogEnabled() = 0;
+
+    // Called just after the tracing system becomes disabled.
+    // TraceLog::IsEnabled() is false at this point.
+    virtual void OnTraceLogDisabled() = 0;
+  };
+  // Assigns a unique id to the listener, and returns the id to the caller.
+  int AddThreadSafeEnabledStateObserver(

[Primiano Tucci (use gerrit), 2016/05/09 17:18:29]

instea of returning an int, can we just make the TSESO* pointer the actual key
(i.e. map<TSESO*, RegisteredTSO>). So that:
when you register you can just deref the weakptr to get the key (and this is
fine as you will be on the right thread).
To unregister you just ask the class to pass you "this".

[Xiaocheng, 2016/05/10 09:33:35]

That's neat. Done.

+      WeakPtr<ThreadSafeEnabledStateObserver> listener);
+  void RemoveThreadSafeEnabledStateObserver(int listener_id);
+  bool HasThreadSafeEnabledStateObserver(int listener_id) const;
+  size_t GetThreadSafeObserverCountForTest() const;

[Primiano Tucci (use gerrit), 2016/05/09 17:18:29]

this doesn't seem to be used in the current CL

[Xiaocheng, 2016/05/10 09:33:35]

Removed. It's not even useful in test.

+
   TraceLogStatus GetStatus() const;
   bool BufferIsFull() const;
 
@@ -365,6 +389,7 @@ class BASE_EXPORT TraceLog : public MemoryDumpProvider {
 
   class ThreadLocalEventBuffer;
   class OptionalAutoLock;
+  struct RegisteredThreadSafeObserver;
 
   TraceLog();
   ~TraceLog() override;
@@ -443,6 +468,8 @@ class BASE_EXPORT TraceLog : public MemoryDumpProvider {
   bool dispatching_to_observer_list_;
   std::vector<EnabledStateObserver*> enabled_state_observer_list_;
 
+  std::map<int, RegisteredThreadSafeObserver> thread_safe_observers_;
+
   std::string process_name_;
   base::hash_map<int, std::string> process_labels_;
   int process_sort_index_;