Manage ownership of TraceLog observers in TraceLog if needed

There are many TraceLog observers that are just tracing clients which
add a feature to tracing. These features are expected to live as long
as tracing is possible. So, make TraceLog own such observers instead
of creating singletons everywhere.

BUG=859260

Change-Id: Iebdaf128f11373f19a27415b66b256ef9a2c72f3
Reviewed-on: https://chromium-review.googlesource.com/1195847Reviewed-by: oysteine <oysteine@chromium.org>
Reviewed-by: Alexei Filippov <alph@chromium.org>
Reviewed-by: agrieve <agrieve@chromium.org>
Commit-Queue: Siddhartha S <ssid@chromium.org>
Cr-Commit-Position: refs/heads/master@{#587369}

base/android/trace_event_binding.cc

@@ -7,7 +7,6 @@
 #include <set>
 
 #include "base/android/jni_string.h"
-#include "base/lazy_instance.h"
 #include "base/macros.h"
 #include "base/trace_event/trace_event.h"
 #include "base/trace_event/trace_event_impl.h"
@@ -58,8 +57,6 @@ class TraceEnabledObserver
     }
 };
 
-base::LazyInstance<TraceEnabledObserver>::Leaky g_trace_enabled_state_observer_;
-
 }  // namespace
 
 static void JNI_TraceEvent_RegisterEnabledObserver(
@@ -67,8 +64,8 @@ static void JNI_TraceEvent_RegisterEnabledObserver(
     const JavaParamRef<jclass>& clazz) {
   bool enabled = trace_event::TraceLog::GetInstance()->IsEnabled();
   base::android::Java_TraceEvent_setEnabled(env, enabled);
-  trace_event::TraceLog::GetInstance()->AddEnabledStateObserver(
-      g_trace_enabled_state_observer_.Pointer());
+  trace_event::TraceLog::GetInstance()->AddOwnedEnabledStateObserver(
+      std::make_unique<TraceEnabledObserver>());
 }
 
 static void JNI_TraceEvent_StartATrace(JNIEnv* env,

base/trace_event/trace_event_unittest.cc

@@ -1273,6 +1273,21 @@ TEST_F(TraceEventTestFixture, EnabledObserverFiresOnDisable) {
   TraceLog::GetInstance()->RemoveEnabledStateObserver(&observer);
 }
 
+TEST_F(TraceEventTestFixture, EnabledObserverOwnedByTraceLog) {
+  auto observer = std::make_unique<MockEnabledStateChangedObserver>();
+  EXPECT_CALL(*observer, OnTraceLogEnabled()).Times(1);
+  EXPECT_CALL(*observer, OnTraceLogDisabled()).Times(1);
+  TraceLog::GetInstance()->AddOwnedEnabledStateObserver(std::move(observer));
+  TraceLog::GetInstance()->SetEnabled(TraceConfig(kRecordAllCategoryFilter, ""),
+                                      TraceLog::RECORDING_MODE);
+  TraceLog::GetInstance()->SetDisabled();
+  TraceLog::ResetForTesting();
+  // These notifications won't be sent.
+  TraceLog::GetInstance()->SetEnabled(TraceConfig(kRecordAllCategoryFilter, ""),
+                                      TraceLog::RECORDING_MODE);
+  TraceLog::GetInstance()->SetDisabled();
+}
+
 // Tests the IsEnabled() state of TraceLog changes before callbacks.
 class AfterStateChangeEnabledStateObserver
     : public TraceLog::EnabledStateObserver {

base/trace_event/trace_log.cc

@@ -776,6 +776,13 @@ void TraceLog::RemoveEnabledStateObserver(EnabledStateObserver* listener) {
     enabled_state_observer_list_.erase(it);
 }
 
+void TraceLog::AddOwnedEnabledStateObserver(
+    std::unique_ptr<EnabledStateObserver> listener) {
+  AutoLock lock(lock_);
+  enabled_state_observer_list_.push_back(listener.get());
+  owned_enabled_state_observer_copy_.push_back(std::move(listener));
+}
+
 bool TraceLog::HasEnabledStateObserver(EnabledStateObserver* listener) const {
   AutoLock lock(lock_);
   return ContainsValue(enabled_state_observer_list_, listener);

base/trace_event/trace_log.h

@@ -127,6 +127,11 @@ class BASE_EXPORT TraceLog : public MemoryDumpProvider {
   };
   void AddEnabledStateObserver(EnabledStateObserver* listener);
   void RemoveEnabledStateObserver(EnabledStateObserver* listener);
+  // Adds an observer that is owned by TraceLog. This is useful for agents that
+  // implement tracing feature that needs to stay alive as long as TraceLog
+  // does.
+  void AddOwnedEnabledStateObserver(
+      std::unique_ptr<EnabledStateObserver> listener);
   bool HasEnabledStateObserver(EnabledStateObserver* listener) const;
 
   // Asynchronous enabled state listeners. When tracing is enabled or disabled,
@@ -477,6 +482,10 @@ class BASE_EXPORT TraceLog : public MemoryDumpProvider {
   std::vector<EnabledStateObserver*> enabled_state_observer_list_;
   std::map<AsyncEnabledStateObserver*, RegisteredAsyncObserver>
       async_observers_;
+  // Manages ownership of the owned observers. The owned observers will also be
+  // added to |enabled_state_observer_list_|.
+  std::vector<std::unique_ptr<EnabledStateObserver>>
+      owned_enabled_state_observer_copy_;
 
   std::string process_name_;
   std::unordered_map<int, std::string> process_labels_;