Use mojo to connect to BackgroundSyncManager object

content/child/blink_platform_impl.cc

Index: content/child/blink_platform_impl.cc
diff --git a/content/child/blink_platform_impl.cc b/content/child/blink_platform_impl.cc
index c8f41688a6fb8017f9075e160af300c0d5673640..ed82c3b17cea393c5bfca2540e5c6ab578189c8c 100644
--- a/content/child/blink_platform_impl.cc
+++ b/content/child/blink_platform_impl.cc
@@ -38,7 +38,6 @@
 #include "content/app/resources/grit/content_resources.h"
 #include "content/app/strings/grit/content_strings.h"
 #include "content/child/background_sync/background_sync_provider.h"
-#include "content/child/background_sync/background_sync_provider_thread_proxy.h"
 #include "content/child/child_thread_impl.h"
 #include "content/child/content_child_helpers.h"
 #include "content/child/geofencing/web_geofencing_provider_impl.h"
@@ -459,8 +458,9 @@ void BlinkPlatformImpl::InternalInit() {
     push_dispatcher_ = ChildThreadImpl::current()->push_dispatcher();
     permission_client_.reset(new PermissionDispatcher(
         ChildThreadImpl::current()->service_registry()));
-    sync_provider_.reset(new BackgroundSyncProvider(
-        ChildThreadImpl::current()->service_registry()));
+    main_thread_sync_provider_.reset(

[davidben, 2015/10/14 21:52:16]

This is a scoped_ptr, but the method is GetOrCreateBlah. That seems wrong. What
if it was a 'Get' and you didn't create a new one? (I realize this is hanging
off a singleton, but still.)

[chasej, 2015/10/15 17:40:26]

I see how this approach looks wrong/confusing.  I've addressed it in my reply to
your other comment.

+        BackgroundSyncProvider::GetOrCreateThreadSpecificInstance(
+            main_thread_task_runner_.get()));
   }
 }
 
@@ -1213,14 +1213,8 @@ blink::WebPermissionClient* BlinkPlatformImpl::permissionClient() {
 }
 
 blink::WebSyncProvider* BlinkPlatformImpl::backgroundSyncProvider() {
-  if (!sync_provider_.get())
-    return nullptr;
-
-  if (IsMainThread())
-    return sync_provider_.get();
-
-  return BackgroundSyncProviderThreadProxy::GetThreadInstance(
-      main_thread_task_runner_.get(), sync_provider_.get());
+  return BackgroundSyncProvider::GetOrCreateThreadSpecificInstance(
+      main_thread_task_runner_.get());

[davidben, 2015/10/14 21:52:16]

I'm not familiar with Mojo, but why GetOrCreateThreadSpecificInstance and not
main_thread_sync_provider_? It doesn't look like the latter's even used.

[chasej, 2015/10/15 17:40:26]

The goal is to have a provider instance for each thread (main and workers), and
let mojo handle all the thread hopping. The creation/access to the provider goes
through GetOrCreate, as that is similar regardless of calling thread type. 
However, destruction has to be handled differently for the main thread vs
workers. The main_thread_sync_provider_ was used only to ensure the instance for
the main thread was cleaned up.

There are a number of classes that use some variation of the
ThreadSpecificInstance approach.  I've found another class (WebFileSystemImpl),
that uses a more explicit approach to solve this problem.  I've copied that
approach, and instead added DeleteThreadSpecificInstance(), which allows removal
of the main_thread_sync_provider_.  I think this is better because all
creation/deletion is encapsulated in BackgroundSyncProvider, and
BlinkPlatformImpl now uses symmetric methods for creation and deletion.

 }
 
 WebThemeEngine* BlinkPlatformImpl::themeEngine() {