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

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
new file mode 100644
index 0000000000000000000000000000000000000000..a339292f7de0dadc3b57d2a314ec86d43f397baf
--- /dev/null
+++ b/content/renderer/background_sync/background_sync_client_impl.cc
@@ -0,0 +1,76 @@
+// 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/renderer/background_sync/background_sync_client_impl.h"
+
+#include "base/bind.h"
+#include "base/location.h"
+#include "base/memory/weak_ptr.h"
+#include "content/child/worker_task_runner.h"
+#include "content/renderer/service_worker/service_worker_context_client.h"
+
+namespace content {
+
+// static
+void BackgroundSyncClientImpl::Create(
+    const scoped_refptr<base::SingleThreadTaskRunner>& main_thread_task_runner,
+    mojo::InterfaceRequest<BackgroundSyncServiceClient> request) {
+  new BackgroundSyncClientImpl(main_thread_task_runner, request.Pass());
+}
+
+BackgroundSyncClientImpl::~BackgroundSyncClientImpl() {
+}
+
+BackgroundSyncClientImpl::BackgroundSyncClientImpl(
+    const scoped_refptr<base::SingleThreadTaskRunner>& main_thread_task_runner,
+    mojo::InterfaceRequest<BackgroundSyncServiceClient> request)
+    : binding_(this, request.Pass()),
+      main_thread_task_runner_(main_thread_task_runner),
+      weak_ptr_factory_(this) {

[jkarlin, 2015/06/12 21:08:15]

DCHECK that we're on main_thread_task_runner's thread

[iclelland, 2015/06/16 15:59:16]

Done.

+}
+
+void BackgroundSyncClientImpl::Sync(content::SyncRegistrationPtr registration,
+                                    int thread_id,
+                                    const SyncCallback& callback) {

[jkarlin, 2015/06/12 21:08:14]

DCHECK that this is called on the main thread task runner thread

[iclelland, 2015/06/16 15:59:16]

Done.

+  WorkerTaskRunner* worker_task_runner = WorkerTaskRunner::Instance();
+  if (!worker_task_runner) {
+    callback.Run(SERVICE_WORKER_EVENT_STATUS_ABORT);
+    return;
+  }
+  // TODO(iclelland): If the worker is dead, GetTaskRunnerFor(thread_id) will
+  // still return a task runner. Detect that state and call callback with an
+  // abort status.
+  worker_task_runner->GetTaskRunnerFor(thread_id)->PostTask(
+      FROM_HERE, base::Bind(&BackgroundSyncClientImpl::DispatchSyncForWorker,
+                            base::Unretained(this),

[jkarlin, 2015/06/12 21:08:14]

This base::Unretained makes me nervous. Can you guarantee this object's
survival?

Instead, you could make DispatchSyncForWorker and OnSyncComplete anonymous
functions or statics. They live on different threads and shouldn't have access
to member variables anyway.

[iclelland, 2015/06/16 15:59:16]

OnSyncComplete needs to have access to at least main_thread_task_runner_, but I
can pass that along.

+                            base::Passed(registration.Pass()), callback));
+}
+
+void BackgroundSyncClientImpl::DispatchSyncForWorker(
+    content::SyncRegistrationPtr registration,
+    const SyncCallback& callback) {
+  ServiceWorkerContextClient* client =
+      ServiceWorkerContextClient::ThreadSpecificInstance();
+  if (!client) {
+    callback.Run(SERVICE_WORKER_EVENT_STATUS_ABORT);

[jkarlin, 2015/06/12 21:08:15]

Isn't callback supposed to be run on the main thread?

[iclelland, 2015/06/16 15:59:16]

Yes. Fixed, thanks.

+    return;
+  }
+  client->DispatchSyncEvent(
+      base::Bind(&BackgroundSyncClientImpl::OnSyncComplete,
+                 weak_ptr_factory_.GetWeakPtr(), callback));

[jkarlin, 2015/06/12 21:08:14]

You can't use weak_ptr_factory_ here as you're on a different thread.

[iclelland, 2015/06/16 15:59:16]

Done.

+}
+
+void BackgroundSyncClientImpl::OnSyncComplete(const SyncCallback& callback,
+                                              ServiceWorkerEventStatus status) {
+  if (!main_thread_task_runner_->RunsTasksOnCurrentThread()) {

[jkarlin, 2015/06/12 21:08:15]

This pattern is being phased out in Chromium. Better to be clear about what
thread you're on.

You can instead simply post task regardless.

[iclelland, 2015/06/16 15:59:16]

At this point, all I know is that I'm on *a* worker thread; there's no specific
thread id here than I can check against, unless I pass long the original
thread_id from Sync, but it would only be for the purpose of a DCHECK here.
But I'm posting back to this function; I suppose I could post the callback task
to the main thread directly instead.

+    // Re-run on main thread
+    main_thread_task_runner_->PostTask(
+        FROM_HERE, base::Bind(&BackgroundSyncClientImpl::OnSyncComplete,
+                              base::Unretained(this), callback, status));
+    return;
+  }
+  callback.Run(status);
+}
+
+}  // namespace content