Use mojo to connect to BackgroundSyncManager object

content/child/background_sync/background_sync_provider.cc

Index: content/child/background_sync/background_sync_provider.cc
diff --git a/content/child/background_sync/background_sync_provider.cc b/content/child/background_sync/background_sync_provider.cc
index bbf4ee41bcb741f1b391e520da1d5ce56c6b4a3e..b8580b5eb673f38707724865b2a9e6706294d5d4 100644
--- a/content/child/background_sync/background_sync_provider.cc
+++ b/content/child/background_sync/background_sync_provider.cc
@@ -5,16 +5,21 @@
 #include "content/child/background_sync/background_sync_provider.h"
 
 #include "base/bind.h"
+#include "base/lazy_instance.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/single_thread_task_runner.h"
+#include "base/threading/thread_local.h"
 #include "content/child/background_sync/background_sync_type_converters.h"
+#include "content/child/child_thread_impl.h"
 #include "content/child/service_worker/web_service_worker_registration_impl.h"
-#include "content/child/worker_task_runner.h"
 #include "content/public/common/background_sync.mojom.h"
 #include "content/public/common/permission_status.mojom.h"
-#include "content/public/common/service_registry.h"
 #include "third_party/WebKit/public/platform/modules/background_sync/WebSyncError.h"
 #include "third_party/WebKit/public/platform/modules/background_sync/WebSyncRegistration.h"
 
+using base::LazyInstance;
+using base::ThreadLocalPointer;
+
 namespace content {
 namespace {
 
@@ -26,15 +31,47 @@ int64 GetServiceWorkerRegistrationId(
              service_worker_registration)->registration_id();
 }
 
+void ConnectToServiceOnMainThread(
+    mojo::InterfaceRequest<BackgroundSyncService> request) {
+  DCHECK(ChildThreadImpl::current());
+  ChildThreadImpl::current()->service_registry()->ConnectToRemoteService(
+      request.Pass());
+}
+
+LazyInstance<ThreadLocalPointer<BackgroundSyncProvider>>::Leaky

[jkarlin, 2015/10/08 15:43:14]

Why Leaky?

[chasej, 2015/10/09 14:58:29]

This was copied from the previous implementation of
BackgroundSyncProviderThreadProxy.  I've removed it as it's not clear why it
would be needed now (or in the previous implementation).  There's hundreds of
usages of LazyInstance<ThreadLocalPointer<>>, many with Leaky, many without. 
Based on a random sampling, I couldn't tell why Leaky is used in some cases, but
not others.

[jkarlin, 2015/10/09 15:13:26]

I found this: "NOTE: Both Singleton and base::LazyInstance provide "leaky"
traits to leak the global on shutdown. This is often advisable (except
potentially in library code where the code may be dynamically loaded into
another process's address space or when data needs to be flushed on process
shutdown) in order to not to slow down shutdown. There are valgrind suppressions
for these "leaky" traits." in
https://www.chromium.org/developers/coding-style/important-abstractions-and-d...

Looks like leaky is preferable.

+    g_sync_provider_tls = LAZY_INSTANCE_INITIALIZER;
+
 }  // namespace
 
-BackgroundSyncProvider::BackgroundSyncProvider(
-    ServiceRegistry* service_registry)
-    : service_registry_(service_registry), background_sync_service_(14) {
-  DCHECK(service_registry);
+BackgroundSyncProvider::~BackgroundSyncProvider() {
+  g_sync_provider_tls.Pointer()->Set(nullptr);
 }
 
-BackgroundSyncProvider::~BackgroundSyncProvider() {
+// static
+BackgroundSyncProvider*
+BackgroundSyncProvider::GetOrCreateThreadSpecificInstance(
+    base::SingleThreadTaskRunner* main_thread_task_runner) {
+  if (g_sync_provider_tls.Pointer()->Get())
+    return g_sync_provider_tls.Pointer()->Get();
+
+  bool on_main_thread = main_thread_task_runner &&

[jkarlin, 2015/10/08 15:43:14]

I don't see how main_thread_task_runner can be nullptr. Remove this check and
DCHECK(main_thread_task_runner) at the start of the method instead.

[chasej, 2015/10/09 14:58:29]

Done. Also reworked the flag variables to more clearly capture the conditions.

+                        main_thread_task_runner->BelongsToCurrentThread();
+  if (!on_main_thread && !WorkerThread::GetCurrentId()) {
+    // On a worker thread, this could happen if GetThreadInstance is called
+    // very late (say by a garbage collected SyncRegistration).
+    return nullptr;
+  }
+
+  BackgroundSyncProvider* instance =
+      new BackgroundSyncProvider(main_thread_task_runner);
+
+  if (!on_main_thread) {
+    // For worker threads, use the observer interface to clean up when workers
+    // are stopped.
+    WorkerThread::AddObserver(instance);
+  }
+
+  return instance;
 }
 
 void BackgroundSyncProvider::registerBackgroundSync(
@@ -163,6 +200,17 @@ void BackgroundSyncProvider::DuplicateRegistrationHandle(
                                                              callback);
 }
 
+void BackgroundSyncProvider::WillStopCurrentWorkerThread() {
+  delete this;
+}
+
+BackgroundSyncProvider::BackgroundSyncProvider(
+    const scoped_refptr<base::SingleThreadTaskRunner> main_task_runner)
+    : main_thread_task_runner_(main_task_runner) {
+  DCHECK(main_task_runner);
+  g_sync_provider_tls.Pointer()->Set(this);
+}
+
 void BackgroundSyncProvider::RegisterCallback(
     scoped_ptr<blink::WebSyncRegistrationCallbacks> callbacks,
     BackgroundSyncError error,
@@ -376,8 +424,11 @@ void BackgroundSyncProvider::NotifyWhenDoneCallback(
 BackgroundSyncServicePtr&
 BackgroundSyncProvider::GetBackgroundSyncServicePtr() {
   if (!background_sync_service_.get()) {
-    service_registry_->ConnectToRemoteService(
-        mojo::GetProxy(&background_sync_service_));
+    mojo::InterfaceRequest<BackgroundSyncService> request =
+        mojo::GetProxy(&background_sync_service_);
+    main_thread_task_runner_->PostTask(
+        FROM_HERE,
+        base::Bind(&ConnectToServiceOnMainThread, base::Passed(&request)));
   }
   return background_sync_service_;
 }