BackgroundSyncManager tracks client registrations

content/renderer/background_sync/background_sync_client_impl.cc

Index: content/renderer/background_sync/background_sync_client_impl.cc
diff --git a/content/renderer/background_sync/background_sync_client_impl.cc b/content/renderer/background_sync/background_sync_client_impl.cc
index 2eb1c40019da279279e19a5813eef0786524426b..7ee5bd932f2e944e995a3bb94e7143bbeb5e2e7a 100644
--- a/content/renderer/background_sync/background_sync_client_impl.cc
+++ b/content/renderer/background_sync/background_sync_client_impl.cc
@@ -4,35 +4,70 @@
 
 #include "content/renderer/background_sync/background_sync_client_impl.h"
 
+#include "content/child/background_sync/background_sync_provider_thread_proxy.h"
 #include "content/child/background_sync/background_sync_type_converters.h"
 #include "content/renderer/service_worker/service_worker_context_client.h"
+#include "third_party/WebKit/public/platform/Platform.h"
+#include "third_party/WebKit/public/platform/WebThread.h"
+#include "third_party/WebKit/public/platform/modules/background_sync/WebSyncProvider.h"
 #include "third_party/WebKit/public/platform/modules/background_sync/WebSyncRegistration.h"
 
 namespace content {
 
 // static
 void BackgroundSyncClientImpl::Create(
+    int64 service_worker_registration_id,
     mojo::InterfaceRequest<BackgroundSyncServiceClient> request) {
-  new BackgroundSyncClientImpl(request.Pass());
+  new BackgroundSyncClientImpl(service_worker_registration_id, request.Pass());
 }
 
 BackgroundSyncClientImpl::~BackgroundSyncClientImpl() {}
 
 BackgroundSyncClientImpl::BackgroundSyncClientImpl(
+    int64 service_worker_registration_id,
     mojo::InterfaceRequest<BackgroundSyncServiceClient> request)
-    : binding_(this, request.Pass()) {}
+    : service_worker_registration_id_(service_worker_registration_id),
+      binding_(this, request.Pass()) {}
 
-void BackgroundSyncClientImpl::Sync(content::SyncRegistrationPtr registration,
+void BackgroundSyncClientImpl::Sync(int64 handle_id,
                                     const SyncCallback& callback) {
+  DCHECK(!blink::Platform::current()->mainThread()->isCurrentThread());
+  // Get a registration for the given handle_id from the provider. This way
+  // the provider knows about the handle and can delete it once Blink releases
+  // it.
+  // TODO(jkarlin): Change the WebSyncPlatform to support
+  // DuplicateRegistrationHandle and then this cast can go.
+  BackgroundSyncProviderThreadProxy* provider =
+      static_cast<BackgroundSyncProviderThreadProxy*>(
+          blink::Platform::current()->backgroundSyncProvider());
+  DCHECK(provider);
+
+  provider->DuplicateRegistrationHandle(

[michaeln, 2015/08/28 02:53:11]

Eeek, the thread and msg round trips are kinda ugly. This is akin to putting a
sync ipc in the loop since msg queues have to flush in both directions prior to
proceeding with the event handleing.

I think can leak bgsync handle references for the lifetime of the renderer
process. Suppose...

* DuplicateRegistrationHandle is called.
* Browser-side does its work stashes the handleptr in
BackgroundSyncServiceImpl.active_handles_ and puts the goods in transit.
* Worker thread terminates.
* Main thread receives the goods and calls
DuplicateRegistrationHandleCallbackOnMainThread
* But... DuplicateRegistrationHandleCallbackOnWorkerThread is never called
* And... releaseRegistration(handle_id) is never called
* The BackgroundSyncServiceImpl eventually gets deleted when the renderer exits
and that does clean up the handle.

[jkarlin, 2015/09/02 23:51:41]

Agreed. On the plus side, this isn't a latency-sensitive event. In fact, it's
whole purpose is to fire when convenient for the UA. That said, I'd rather avoid
the round-trip if possible. TODO added.
This can also happen to register, getRegistration, and getRegistrations. This
leak will be taken care of once we move to the per-service-worker mojo channel
instead of the per-renderer registration. I've created bug
https://code.google.com/p/chromium/issues/detail?id=527601 to do the mojo work.

+      handle_id, service_worker_registration_id_,
+      base::Bind(&BackgroundSyncClientImpl::SyncDidGetRegistration,
+                 base::Unretained(this), callback));
+}
+
+void BackgroundSyncClientImpl::SyncDidGetRegistration(
+    const SyncCallback& callback,
+    BackgroundSyncError error,
+    const SyncRegistrationPtr& registration) {
+  if (error != BACKGROUND_SYNC_ERROR_NONE) {
+    callback.Run(SERVICE_WORKER_EVENT_STATUS_ABORTED);
+    return;
+  }
+
   ServiceWorkerContextClient* client =
       ServiceWorkerContextClient::ThreadSpecificInstance();
   if (!client) {
     callback.Run(SERVICE_WORKER_EVENT_STATUS_ABORTED);
     return;
   }
-  scoped_ptr<blink::WebSyncRegistration> reg =
+
+  scoped_ptr<blink::WebSyncRegistration> web_registration =
       mojo::ConvertTo<scoped_ptr<blink::WebSyncRegistration>>(registration);
-  client->DispatchSyncEvent(*reg, callback);
+
+  client->DispatchSyncEvent(*web_registration, callback);
 }
 
 }  // namespace content