Use mojo to connect to BackgroundSyncManager object

content/child/blink_platform_impl.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/child/blink_platform_impl.h"
 
 #include <math.h>
 
 #include <vector>
 
@@ skipping 20 matching lines @@
 #include "base/trace_event/memory_allocator_dump_guid.h"
 #include "base/trace_event/memory_dump_manager.h"
 #include "base/trace_event/trace_event.h"
 #include "blink/public/resources/grit/blink_image_resources.h"
 #include "blink/public/resources/grit/blink_resources.h"
 #include "components/mime_util/mime_util.h"
 #include "components/scheduler/child/webthread_impl_for_worker_scheduler.h"
 #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"
 #include "content/child/navigator_connect/service_port_provider.h"
 #include "content/child/notifications/notification_dispatcher.h"
 #include "content/child/notifications/notification_manager.h"
 #include "content/child/permissions/permission_dispatcher.h"
 #include "content/child/permissions/permission_dispatcher_thread_proxy.h"
 #include "content/child/push_messaging/push_dispatcher.h"
 #include "content/child/push_messaging/push_provider.h"
@@ skipping 400 matching lines @@
   // ChildThread may not exist in some tests.
   if (ChildThreadImpl::current()) {
     geofencing_provider_.reset(new WebGeofencingProviderImpl(
         ChildThreadImpl::current()->thread_safe_sender()));
     thread_safe_sender_ = ChildThreadImpl::current()->thread_safe_sender();
     notification_dispatcher_ =
         ChildThreadImpl::current()->notification_dispatcher();
     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()));
   }
 }
 
 void BlinkPlatformImpl::UpdateWebThreadTLS(blink::WebThread* thread) {
   DCHECK(!current_thread_slot_.Get());
   current_thread_slot_.Set(thread);
 }
 
 BlinkPlatformImpl::~BlinkPlatformImpl() {
 }
@@ skipping 732 matching lines @@
     return nullptr;
 
   if (IsMainThread())
     return permission_client_.get();
 
   return PermissionDispatcherThreadProxy::GetThreadInstance(
       main_thread_task_runner_.get(), permission_client_.get());
 }
 
 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() {
   return &native_theme_engine_;
 }
 
 WebFallbackThemeEngine* BlinkPlatformImpl::fallbackThemeEngine() {
   return &fallback_theme_engine_;
 }
 
@@ skipping 158 matching lines @@
   return WebString::fromUTF8(ui::KeycodeConverter::DomKeyToKeyString(
       static_cast<ui::DomKey>(dom_key)));
 }
 
 int BlinkPlatformImpl::domKeyEnumFromString(const WebString& key_string) {
   return static_cast<int>(
       ui::KeycodeConverter::KeyStringToDomKey(key_string.utf8().data()));
 }
 
 }  // namespace content