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

content/browser/service_worker/service_worker_mojo_event_dispatcher.cc

Index: content/browser/service_worker/service_worker_mojo_event_dispatcher.cc
diff --git a/content/browser/service_worker/service_worker_mojo_event_dispatcher.cc b/content/browser/service_worker/service_worker_mojo_event_dispatcher.cc
new file mode 100644
index 0000000000000000000000000000000000000000..f25ef02e85badd94be3afa51e03d0a4408a1db8f
--- /dev/null
+++ b/content/browser/service_worker/service_worker_mojo_event_dispatcher.cc
@@ -0,0 +1,123 @@
+// Copyright 2015 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/browser/service_worker/service_worker_mojo_event_dispatcher.h"
+
+#include "content/public/browser/render_process_host.h"
+#include "content/public/common/service_registry.h"
+#include "third_party/mojo/src/mojo/public/cpp/bindings/interface_ptr_info.h"
+
+namespace content {
+
+namespace {
+
+// Converts mojo enum content::ServiceWorkerEventStatus values to Chromium
+// content::ServiceWorkerStatusCode values.
+// TODO(iclelland): Make these enums equivalent so that conversion can be a
+// static cast.
+ServiceWorkerStatusCode statusCodeFromMojoStatus(
+    ServiceWorkerEventStatus status) {
+  ServiceWorkerStatusCode status_code = SERVICE_WORKER_OK;
+  if (status == SERVICE_WORKER_EVENT_STATUS_COMPLETED) {
+    status_code = SERVICE_WORKER_OK;
+  } else if (status == SERVICE_WORKER_EVENT_STATUS_REJECTED) {
+    status_code = SERVICE_WORKER_ERROR_EVENT_WAITUNTIL_REJECTED;
+  } else if (status == SERVICE_WORKER_EVENT_STATUS_ABORT) {
+    status_code = SERVICE_WORKER_ERROR_ABORT;
+  } else {
+    NOTREACHED();
+  }
+  return status_code;
+}
+
+}  // namespace
+
+ServiceWorkerMojoEventDispatcher::ServiceWorkerMojoEventDispatcher() {

[jkarlin, 2015/06/16 18:59:59]

DCHECK_CURRENTLY_ON(BrowserThread::IO);

[iclelland, 2015/06/17 12:39:24]

Done.

+}
+
+void ServiceWorkerMojoEventDispatcher::DispatchSyncEvent(
+    int render_process_id,
+    int thread_id,
+    const ServiceWorkerVersion::StatusCallback& callback) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+
+  if (!background_sync_client_.get()) {
+    // We have not connected to the mojo service before. Switch to the UI thread
+    // to connect, and then dispatch the event once the connection is
+    // established.
+    BrowserThread::PostTask(
+        BrowserThread::UI, FROM_HERE,
+        base::Bind(&ServiceWorkerMojoEventDispatcher::Connect<
+                       BackgroundSyncServiceClient>,
+                   this, render_process_id,
+                   base::Bind(&ServiceWorkerMojoEventDispatcher::
+                                  BindChannelAndDispatchSyncEvent,
+                              this, thread_id, callback)));

[jkarlin, 2015/06/16 19:00:00]

nit: prefer you return here and remove the else {} below.

[iclelland, 2015/06/17 12:39:24]

Done.

+  } else {
+    DispatchSyncEventInternal(thread_id, callback);
+  }
+}
+
+ServiceWorkerMojoEventDispatcher::~ServiceWorkerMojoEventDispatcher() {
+}
+
+// Establishes a connection to a mojo service, and then transfers the local
+// endpoint of the message pipe to the IO thread so that it can be called from
+// there in the future without switching threads.
+// This method must be called on the UI thread, as it accesses the service
+// registry member of the RenderProcessHost.
+template <typename MojoServiceType>
+void ServiceWorkerMojoEventDispatcher::Connect(

[jkarlin, 2015/06/16 19:00:00]

This should be called ConnectOnUIThread and should either be static or an
unnamed-namespace function. 

[iclelland, 2015/06/17 12:39:24]

Done.

+    int render_process_id,
+    const base::Callback<void(mojo::InterfacePtrInfo<MojoServiceType>)>&
+        callback) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  RenderProcessHost* host = RenderProcessHost::FromID(render_process_id);

[jkarlin, 2015/06/16 19:00:00]

There is no guarantee that the render process still exists at this point, check
the pointer.

[iclelland, 2015/06/17 12:39:24]

Done.

+  content::ServiceRegistry* registry = host->GetServiceRegistry();
+  if (!registry)
+    return;
+
+  mojo::InterfacePtr<MojoServiceType> background_sync_client;
+  registry->ConnectToRemoteService(mojo::GetProxy(&background_sync_client));

[jkarlin, 2015/06/16 19:00:00]

ConnectToRemoveService isn't documented, but appears to have weird behavior if
the remove provider isn't yet connected. Do you need to check for null on what's
returned?

[iclelland, 2015/06/17 12:39:24]

Yes; see cleanup patch. My understanding (from
content/common/mojo/service_registry_impl.cc and
third_party/mojo/src/mojo/public/cpp/system/handle.h) is that the InterfacePtr
will not be valid if the connection fails, and I'm checking for that in the
callback.

+  BrowserThread::PostTask(
+      BrowserThread::IO, FROM_HERE,
+      base::Bind(callback,
+                 base::Passed(background_sync_client.PassInterface())));
+}
+
+void ServiceWorkerMojoEventDispatcher::BindChannelAndDispatchSyncEvent(
+    int thread_id,
+    const ServiceWorkerVersion::StatusCallback& callback,
+    mojo::InterfacePtrInfo<BackgroundSyncServiceClient> ptri) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  if (!background_sync_client_.get()) {

[jkarlin, 2015/06/16 18:59:59]

DCHECK(background_sync_client_.get())?

[iclelland, 2015/06/17 12:39:24]

No -- this is checking whether the interface has been bound yet or not. If it
has not been bound, then we check whether a valid info struct was passed in from
Connect, and connect in that case. (See cleanup thread)

+    background_sync_client_.Bind(ptri.Pass());
+  }
+  DispatchSyncEventInternal(thread_id, callback);
+}
+
+void ServiceWorkerMojoEventDispatcher::DispatchSyncEventInternal(
+    int thread_id,
+    const ServiceWorkerVersion::StatusCallback& callback) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+
+  // TODO(iclelland): Replace this with the real event registration details
+  // crbug.com/482066
+  content::SyncRegistrationPtr TEMP_null_event(

[jkarlin, 2015/06/16 19:00:00]

tmp_null_event, lowercase

[iclelland, 2015/06/17 12:39:24]

I didn't expect this identifier to last too long, but yeah. I've removed the
TEMP_, and let the TODO stand for the fact that this is a temporary situation.
Done.

+      content::SyncRegistration::New());
+
+  background_sync_client_->Sync(
+      TEMP_null_event.Pass(), thread_id,
+      base::Bind(&ServiceWorkerMojoEventDispatcher::OnEventFinished, this,
+                 callback));
+}
+
+void ServiceWorkerMojoEventDispatcher::OnEventFinished(
+    const ServiceWorkerVersion::StatusCallback& callback,
+    ServiceWorkerEventStatus result) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  callback.Run(statusCodeFromMojoStatus(result));
+}
+
+}  // namespace content