Make tracing work in service manager

services/tracing/public/cpp/provider.cc

Index: services/tracing/public/cpp/provider.cc
diff --git a/services/tracing/public/cpp/provider.cc b/services/tracing/public/cpp/provider.cc
index 47e0ab0c0fc6d74e1777fd84f2bcbb549f913c22..22c1bbf675c6219abfdf4ed4d46da7b7260fe62d 100644
--- a/services/tracing/public/cpp/provider.cc
+++ b/services/tracing/public/cpp/provider.cc
@@ -42,6 +42,30 @@ Provider::~Provider() {
   StopTracing();
 }
 
+void Provider::InitializeWithFactoryLocked(mojom::FactoryPtr* factory) {
+  mojom::ProviderPtr provider;
+  Bind(GetProxy(&provider));
+  (*factory)->CreateRecorder(std::move(provider));
+#ifdef NDEBUG
+  if (base::CommandLine::ForCurrentProcess()->HasSwitch(
+          tracing::kEarlyTracing)) {
+    ForceEnableTracing();
+  }
+#else
+  ForceEnableTracing();
+#endif
+}
+
+void Provider::InitializeWithFactory(mojom::FactoryPtr* factory) {
+  {
+    base::AutoLock lock(g_singleton_lock.Get());
+    if (g_tracing_singleton_created)
+      return;
+    g_tracing_singleton_created = true;
+  }
+  InitializeWithFactoryLocked(factory);

[Ken Rockot(use gerrit already), 2016/10/18 20:30:59]

Is the intent that this is called under the lock? If so, it's not; if not, why
the "Locked" suffix?

[chiniforooshan, 2016/10/18 20:46:18]

Done. Yeah, "Locked" was not a good suffix. This is called only by the first
caller who obtains the lock and not subsequent calls. Changed it to
InitializeWithFactoryInternal. Please let me know if you have a better name
suggestion.

+}
+
 void Provider::Initialize(service_manager::Connector* connector,
                           const std::string& url) {
   {
@@ -50,24 +74,12 @@ void Provider::Initialize(service_manager::Connector* connector,
       return;
     g_tracing_singleton_created = true;
   }
-
+  mojom::FactoryPtr factory;
+  connector->ConnectToInterface("service:tracing", &factory);
+  InitializeWithFactoryLocked(&factory);

[Ken Rockot(use gerrit already), 2016/10/18 20:30:59]

Should this really be calling the Locked variant?

[chiniforooshan, 2016/10/18 20:46:18]

Done. Changed it to InitializeWithFactoryInternal. Please let me know if you
have a better name suggestion.

   // This will only set the name for the first app in a loaded mojo file. It's
   // up to something like CoreServices to name its own child threads.
   base::PlatformThread::SetName(url);
-
-  mojom::FactoryPtr factory;
-  connector->ConnectToInterface("service:tracing", &factory);
-  mojom::ProviderPtr provider;
-  Bind(GetProxy(&provider));
-  factory->CreateRecorder(std::move(provider));
-#ifdef NDEBUG
-  if (base::CommandLine::ForCurrentProcess()->HasSwitch(
-          tracing::kEarlyTracing)) {
-    ForceEnableTracing();
-  }
-#else
-  ForceEnableTracing();
-#endif
 }
 
 void Provider::Bind(mojom::ProviderRequest request) {