[BackgroundSync] Add browser side support for SyncRegistration.done

content/child/background_sync/background_sync_provider_thread_proxy.cc

 // 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/child/background_sync/background_sync_provider_thread_proxy.h"
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/lazy_instance.h"
 #include "base/location.h"
@@ skipping 193 matching lines @@
           new CallbackThreadAdapter<blink::WebSyncGetPermissionStatusCallbacks>(
               make_scoped_ptr(callbacks), WorkerThread::GetCurrentId())));
 }
 
 void BackgroundSyncProviderThreadProxy::releaseRegistration(int64_t handle_id) {
   main_thread_task_runner_->PostTask(
       FROM_HERE, base::Bind(&BackgroundSyncProvider::releaseRegistration,
                             base::Unretained(sync_provider_), handle_id));
 }
 
+void BackgroundSyncProviderThreadProxy::notifyWhenDone(
+    int64_t handle_id,
+    blink::WebSyncNotifyWhenDoneCallbacks* callbacks) {

[michaeln, 2015/09/17 21:44:12]

I'm very wary of the thread affinity all blink::WebFoo classes. Is it safe to
delete |callbacks| on the main thread if the worker should exit prior to done
happening?

I'm not sure if deletion on the main thread may be possible with the
CallbackThreadAdapter even when the worker is still running? The bind closure
owns the callbackptr, actually the refcounted bindstate buried inside the
closure owns it. The bindstate is refcounted to as copies of the closure are
made. If at some point the closure is not handled as a move-only type, i think
deletion on the wrong thread could happen even if the worker thread is still
running at done time.

(hopefully soon, per-thread BackgroundSyncProviderIntfPtr to the rescue)

[jkarlin, 2015/09/18 12:03:20]

Yes, good catch. chasej@ is making good headway into the mojo-per-thread CL.

[michaeln, 2015/09/18 19:18:52]

Glad to hear that. I hope it helps to establish safe best practices for using
mojo in workers. Its just so easy to get introduce threading bugs, having a good
example w/o bugs to follow will be nice.

Can you point me at a cl or other artifacts of work in progress?

+  DCHECK(callbacks);
+
+  main_thread_task_runner_->PostTask(
+      FROM_HERE,
+      base::Bind(
+          &BackgroundSyncProvider::notifyWhenDone,
+          base::Unretained(sync_provider_), handle_id,
+          new CallbackThreadAdapter<blink::WebSyncNotifyWhenDoneCallbacks>(
+              make_scoped_ptr(callbacks), WorkerThread::GetCurrentId())));
+}
+
 void BackgroundSyncProviderThreadProxy::DuplicateRegistrationHandle(
     int64 handle_id,
     const BackgroundSyncService::DuplicateRegistrationHandleCallback&
         callback) {
   main_thread_task_runner_->PostTask(
       FROM_HERE,
       base::Bind(&BackgroundSyncProvider::DuplicateRegistrationHandle,
                  base::Unretained(sync_provider_), handle_id,
                  base::Bind(&DuplicateRegistrationHandleCallbackOnMainThread,
                             WorkerThread::GetCurrentId(), callback)));
 }
 
 void BackgroundSyncProviderThreadProxy::WillStopCurrentWorkerThread() {
   delete this;
 }
 
 BackgroundSyncProviderThreadProxy::BackgroundSyncProviderThreadProxy(
     base::SingleThreadTaskRunner* main_thread_task_runner,
     BackgroundSyncProvider* sync_provider)
     : main_thread_task_runner_(main_thread_task_runner),
       sync_provider_(sync_provider) {
   g_sync_provider_tls.Pointer()->Set(this);
 }
 
 BackgroundSyncProviderThreadProxy::~BackgroundSyncProviderThreadProxy() {
   g_sync_provider_tls.Pointer()->Set(nullptr);
 }
 
 }  // namespace content