[Background Sync] Use Mojo IPC to fire background sync events

content/browser/service_worker/service_worker_version.cc

Index: content/browser/service_worker/service_worker_version.cc
diff --git a/content/browser/service_worker/service_worker_version.cc b/content/browser/service_worker/service_worker_version.cc
index 9964b8a46697cfd1c3822473c21ebc2fb8dfb3f4..cb80921ba917f0584aaa4aa821718125ade5eca1 100644
--- a/content/browser/service_worker/service_worker_version.cc
+++ b/content/browser/service_worker/service_worker_version.cc
@@ -36,6 +36,7 @@
 #include "content/public/common/content_client.h"
 #include "content/public/common/content_switches.h"
 #include "content/public/common/result_codes.h"
+#include "content/public/common/service_registry.h"
 #include "net/http/http_response_headers.h"
 #include "net/http/http_response_info.h"
 
@@ -737,12 +738,76 @@ void ServiceWorkerVersion::DispatchSyncEvent(const StatusCallback& callback) {
     return;
   }
 
-  int request_id = AddRequest(callback, &sync_callbacks_, REQUEST_SYNC);
-  ServiceWorkerStatusCode status = embedded_worker_->SendMessage(
-      ServiceWorkerMsg_SyncEvent(request_id));
-  if (status != SERVICE_WORKER_OK) {
-    sync_callbacks_.Remove(request_id);
-    RunSoon(base::Bind(callback, status));
+  // We can only get the embedded worker's process and thread ids on the IO
+  // thread, but we need to be on the UI thread to get the the mojo service
+  // registry from the RenderProcessHost.
+  BrowserThread::PostTask(
+      BrowserThread::UI, FROM_HERE,
+      base::Bind(&ServiceWorkerVersion::DispatchSyncEventOnUIThread,
+                 /* IS this the right one? */ base::Unretained(this),

[jkarlin, 2015/06/10 12:21:33]

No, this is not safe. base::Unretained should rarely be used. See
https://www.chromium.org/developers/design-documents/threading for more
information. 

If you use the class I suggested in a comment in the header file, you'll be
calling that and since it's RefCountedThreadSafe it'll work fine. The function
you call will want a callback for when the event finishes. You can give that
callback a 'this' pointer since it's RefCounted and will be run on the IO
thread.

+                 embedded_worker_->process_id(), embedded_worker_->thread_id(),
+                 callback));
+}
+
+void ServiceWorkerVersion::DispatchSyncEventOnUIThread(
+    int render_process_id,
+    int thread_id,
+    const StatusCallback& callback) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  // TODO(iclelland): Replace this with the real event registration details
+  content::SyncRegistrationPtr TEMP_null_event(
+      content::SyncRegistration::New());
+
+  RenderProcessHost* host = RenderProcessHost::FromID(render_process_id);
+  content::ServiceRegistry* registry = host->GetServiceRegistry();
+  if (!registry)
+    return;
+
+  if (!background_sync_client_.get())
+    registry->ConnectToRemoteService(mojo::GetProxy(&background_sync_client_));
+  background_sync_client_->Sync(
+      TEMP_null_event.Pass(), thread_id,
+      base::Bind(&ServiceWorkerVersion::OnSyncEventFinished,
+                 weak_factory_.GetWeakPtr(), callback));
+}
+
+void ServiceWorkerVersion::OnSyncEventFinished(
+    const StatusCallback& callback,
+    BackgroundSyncEventStatus result) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  TRACE_EVENT0("ServiceWorker", "ServiceWorkerVersion::OnSyncEventFinished")
+  BrowserThread::PostTask(
+      BrowserThread::IO, FROM_HERE,
+      base::Bind(&ServiceWorkerVersion::FinishSyncEventOnIOThread,
+                 /* IS this the right one? */ base::Unretained(this), callback,
+                 result));
+}
+
+void ServiceWorkerVersion::FinishSyncEventOnIOThread(
+    const StatusCallback& callback,
+    BackgroundSyncEventStatus result) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  ServiceWorkerStatusCode status = SERVICE_WORKER_OK;
+  if (!base::CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kEnableServiceWorkerSync)) {
+    // Avoid potential race condition where flag is disabled after a sync event
+    // was dispatched
+    status = SERVICE_WORKER_ERROR_ABORT;
+  } else if (result == BACKGROUND_SYNC_EVENT_STATUS_REJECTED) {
+    status = SERVICE_WORKER_ERROR_EVENT_WAITUNTIL_REJECTED;
+  } else if (result == BACKGROUND_SYNC_EVENT_STATUS_ABORT) {
+    status = SERVICE_WORKER_ERROR_ABORT;
+  }
+
+  scoped_refptr<ServiceWorkerVersion> protect(this);
+  callback.Run(status);
+
+  RestartTick(&idle_time_);
+  // TODO(iclelland) factor this out of RemoveCallbackAndStopIfRedundant
+  if (is_redundant()) {
+    // The stop should be already scheduled, but try to stop immediately, in
+    // order to release worker resources soon.
+    StopWorkerIfIdle();
   }
 }
 
@@ -1096,8 +1161,6 @@ void ServiceWorkerVersion::OnStopped(
                     SERVICE_WORKER_ERROR_FAILED,
                     SERVICE_WORKER_FETCH_EVENT_RESULT_FALLBACK,
                     ServiceWorkerResponse());
-  RunIDMapCallbacks(&sync_callbacks_,
-                    SERVICE_WORKER_ERROR_FAILED);
   RunIDMapCallbacks(&notification_click_callbacks_,
                     SERVICE_WORKER_ERROR_FAILED);
   RunIDMapCallbacks(&push_callbacks_,
@@ -1154,8 +1217,6 @@ bool ServiceWorkerVersion::OnMessageReceived(const IPC::Message& message) {
                         OnInstallEventFinished)
     IPC_MESSAGE_HANDLER(ServiceWorkerHostMsg_FetchEventFinished,
                         OnFetchEventFinished)
-    IPC_MESSAGE_HANDLER(ServiceWorkerHostMsg_SyncEventFinished,
-                        OnSyncEventFinished)
     IPC_MESSAGE_HANDLER(ServiceWorkerHostMsg_NotificationClickEventFinished,
                         OnNotificationClickEventFinished)
     IPC_MESSAGE_HANDLER(ServiceWorkerHostMsg_PushEventFinished,
@@ -1327,33 +1388,6 @@ void ServiceWorkerVersion::OnFetchEventFinished(
   RemoveCallbackAndStopIfRedundant(&fetch_callbacks_, request_id);
 }
 
-void ServiceWorkerVersion::OnSyncEventFinished(
-    int request_id,
-    blink::WebServiceWorkerEventResult result) {
-  TRACE_EVENT1("ServiceWorker",
-               "ServiceWorkerVersion::OnSyncEventFinished",
-               "Request id", request_id);
-  StatusCallback* callback = sync_callbacks_.Lookup(request_id);
-  if (!callback) {
-    NOTREACHED() << "Got unexpected message: " << request_id;
-    return;
-  }
-
-  ServiceWorkerStatusCode status = SERVICE_WORKER_OK;
-  if (!base::CommandLine::ForCurrentProcess()->HasSwitch(
-          switches::kEnableServiceWorkerSync)) {
-    // Avoid potential race condition where flag is disabled after a sync event
-    // was dispatched
-    status = SERVICE_WORKER_ERROR_ABORT;
-  } else if (result == blink::WebServiceWorkerEventResultRejected) {
-    status = SERVICE_WORKER_ERROR_EVENT_WAITUNTIL_REJECTED;
-  }
-
-  scoped_refptr<ServiceWorkerVersion> protect(this);
-  callback->Run(status);
-  RemoveCallbackAndStopIfRedundant(&sync_callbacks_, request_id);
-}
-
 void ServiceWorkerVersion::OnNotificationClickEventFinished(
     int request_id) {
   TRACE_EVENT1("ServiceWorker",
@@ -1898,7 +1932,6 @@ bool ServiceWorkerVersion::HasInflightRequests() const {
     !activate_callbacks_.IsEmpty() ||
     !install_callbacks_.IsEmpty() ||
     !fetch_callbacks_.IsEmpty() ||
-    !sync_callbacks_.IsEmpty() ||
     !notification_click_callbacks_.IsEmpty() ||
     !push_callbacks_.IsEmpty() ||
     !geofencing_callbacks_.IsEmpty() ||
@@ -1979,9 +2012,6 @@ bool ServiceWorkerVersion::OnRequestTimeout(const RequestInfo& info) {
           &fetch_callbacks_, info.id, SERVICE_WORKER_ERROR_TIMEOUT,
           /* The other args are ignored for non-OK status. */
           SERVICE_WORKER_FETCH_EVENT_RESULT_FALLBACK, ServiceWorkerResponse());
-    case REQUEST_SYNC:
-      return RunIDMapCallback(&sync_callbacks_, info.id,
-                              SERVICE_WORKER_ERROR_TIMEOUT);
     case REQUEST_NOTIFICATION_CLICK:
       return RunIDMapCallback(&notification_click_callbacks_, info.id,
                               SERVICE_WORKER_ERROR_TIMEOUT);