BackgroundSyncManager tracks client registrations

content/browser/background_sync/background_sync_manager.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/browser/background_sync/background_sync_manager.h"
 
 #include "base/barrier_closure.h"
 #include "base/bind.h"
 #include "base/location.h"
 #include "base/single_thread_task_runner.h"
 #include "base/thread_task_runner_handle.h"
 #include "content/browser/background_sync/background_sync_metrics.h"
 #include "content/browser/background_sync/background_sync_network_observer.h"
 #include "content/browser/background_sync/background_sync_power_observer.h"
+#include "content/browser/background_sync/background_sync_registration_handle.h"
 #include "content/browser/background_sync/background_sync_registration_options.h"
 #include "content/browser/service_worker/service_worker_context_wrapper.h"
 #include "content/browser/service_worker/service_worker_storage.h"
 #include "content/public/browser/browser_thread.h"
 
 #if defined(OS_ANDROID)
 #include "content/browser/android/background_sync_launcher_android.h"
 #endif
 
+namespace content {
+
 namespace {
+
 const char kBackgroundSyncUserDataKey[] = "BackgroundSyncUserData";
+
+void PostErrorResponse(
+    BackgroundSyncStatus status,
+    const BackgroundSyncManager::StatusAndRegistrationCallback& callback) {
+  base::ThreadTaskRunnerHandle::Get()->PostTask(
+      FROM_HERE,
+      base::Bind(
+          callback, status,
+          base::Passed(scoped_ptr<BackgroundSyncRegistrationHandle>().Pass())));
 }
 
-namespace content {
+}  // namespace
 
 BackgroundSyncManager::BackgroundSyncRegistrations::
     BackgroundSyncRegistrations()
     : next_id(BackgroundSyncRegistration::kInitialId) {
 }
 
 BackgroundSyncManager::BackgroundSyncRegistrations::
     ~BackgroundSyncRegistrations() {
 }
 
 // static
 scoped_ptr<BackgroundSyncManager> BackgroundSyncManager::Create(
     const scoped_refptr<ServiceWorkerContextWrapper>& service_worker_context) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
   BackgroundSyncManager* sync_manager =
       new BackgroundSyncManager(service_worker_context);
   sync_manager->Init();
   return make_scoped_ptr(sync_manager);
 }
 
 BackgroundSyncManager::~BackgroundSyncManager() {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
+  // This could race in the extremely unlikely case that the
+  // BackgroundSyncManager is deleted before a dispatched sync request is able
+  // to call BackgroundSyncServiceImpl::TrackRegistration.
+  DCHECK(registration_handle_ids_.IsEmpty());
+
   service_worker_context_->RemoveObserver(this);
 }
 
 BackgroundSyncManager::RegistrationKey::RegistrationKey(
     const BackgroundSyncRegistration& registration)
     : RegistrationKey(registration.options()->tag,
                       registration.options()->periodicity) {
 }
 
 BackgroundSyncManager::RegistrationKey::RegistrationKey(
@@ skipping 17 matching lines @@
   BackgroundSyncMetrics::RegistrationCouldFire registration_could_fire =
       AreOptionConditionsMet(options)
           ? BackgroundSyncMetrics::REGISTRATION_COULD_FIRE
           : BackgroundSyncMetrics::REGISTRATION_COULD_NOT_FIRE;
 
   if (disabled_) {
     BackgroundSyncMetrics::CountRegister(
         options.periodicity, registration_could_fire,
         BackgroundSyncMetrics::REGISTRATION_IS_NOT_DUPLICATE,
         BACKGROUND_SYNC_STATUS_STORAGE_ERROR);
-    base::ThreadTaskRunnerHandle::Get()->PostTask(
-        FROM_HERE, base::Bind(callback, BACKGROUND_SYNC_STATUS_STORAGE_ERROR,
-                              BackgroundSyncRegistration()));
+    PostErrorResponse(BACKGROUND_SYNC_STATUS_STORAGE_ERROR, callback);
     return;
   }
 
   op_scheduler_.ScheduleOperation(
       base::Bind(&BackgroundSyncManager::RegisterImpl,
                  weak_ptr_factory_.GetWeakPtr(), sw_registration_id, options,
                  MakeStatusAndRegistrationCompletion(callback)));
 }
 
-void BackgroundSyncManager::Unregister(
-    int64 sw_registration_id,
-    const std::string& sync_registration_tag,
-    SyncPeriodicity periodicity,
-    BackgroundSyncRegistration::RegistrationId sync_registration_id,
-    const StatusCallback& callback) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
-
-  if (disabled_) {
-    BackgroundSyncMetrics::CountUnregister(
-        periodicity, BACKGROUND_SYNC_STATUS_STORAGE_ERROR);
-    base::ThreadTaskRunnerHandle::Get()->PostTask(
-        FROM_HERE, base::Bind(callback, BACKGROUND_SYNC_STATUS_STORAGE_ERROR));
-    return;
-  }
-
-  RegistrationKey registration_key(sync_registration_tag, periodicity);
-
-  op_scheduler_.ScheduleOperation(base::Bind(
-      &BackgroundSyncManager::UnregisterImpl, weak_ptr_factory_.GetWeakPtr(),
-      sw_registration_id, registration_key, sync_registration_id, periodicity,
-      MakeStatusCompletion(callback)));
-}
-
 void BackgroundSyncManager::GetRegistration(
     int64 sw_registration_id,
     const std::string& sync_registration_tag,
     SyncPeriodicity periodicity,
     const StatusAndRegistrationCallback& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
   if (disabled_) {
-    base::ThreadTaskRunnerHandle::Get()->PostTask(
-        FROM_HERE, base::Bind(callback, BACKGROUND_SYNC_STATUS_STORAGE_ERROR,
-                              BackgroundSyncRegistration()));
+    PostErrorResponse(BACKGROUND_SYNC_STATUS_STORAGE_ERROR, callback);
     return;
   }
 
   RegistrationKey registration_key(sync_registration_tag, periodicity);
 
   op_scheduler_.ScheduleOperation(base::Bind(
       &BackgroundSyncManager::GetRegistrationImpl,
       weak_ptr_factory_.GetWeakPtr(), sw_registration_id, registration_key,
       MakeStatusAndRegistrationCompletion(callback)));
 }
 
 void BackgroundSyncManager::GetRegistrations(
     int64 sw_registration_id,
     SyncPeriodicity periodicity,
     const StatusAndRegistrationsCallback& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
   if (disabled_) {
     base::ThreadTaskRunnerHandle::Get()->PostTask(
-        FROM_HERE, base::Bind(callback, BACKGROUND_SYNC_STATUS_STORAGE_ERROR,
-                              std::vector<BackgroundSyncRegistration>()));
+        FROM_HERE,
+        base::Bind(
+            callback, BACKGROUND_SYNC_STATUS_STORAGE_ERROR,
+            base::Passed(
+                scoped_ptr<ScopedVector<BackgroundSyncRegistrationHandle>>()
+                    .Pass())));
     return;
   }
 
   op_scheduler_.ScheduleOperation(
       base::Bind(&BackgroundSyncManager::GetRegistrationsImpl,
                  weak_ptr_factory_.GetWeakPtr(), sw_registration_id,
                  periodicity, MakeStatusAndRegistrationsCompletion(callback)));
 }
 
+const BackgroundSyncRegistration*
+BackgroundSyncManager::GetRegistrationForHandle(
+    BackgroundSyncRegistrationHandle::HandleId handle_id) const {
+  scoped_refptr<const RefCountedRegistration>* ref_registration =
+      registration_handle_ids_.Lookup(handle_id);
+  if (!ref_registration)
+    return nullptr;
+  return (*ref_registration)->value();
+}
+
+void BackgroundSyncManager::ReleaseHandleId(
+    BackgroundSyncRegistrationHandle::HandleId handle_id) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  DCHECK(registration_handle_ids_.Lookup(handle_id));
+  registration_handle_ids_.Remove(handle_id);
+}
+
 void BackgroundSyncManager::OnRegistrationDeleted(int64 registration_id,
                                                   const GURL& pattern) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
   // Operations already in the queue will either fail when they write to storage
   // or return stale results based on registrations loaded in memory. This is
   // inconsequential since the service worker is gone.
   op_scheduler_.ScheduleOperation(base::Bind(
       &BackgroundSyncManager::OnRegistrationDeletedImpl,
       weak_ptr_factory_.GetWeakPtr(), registration_id, MakeEmptyCompletion()));
@@ skipping 77 matching lines @@
         const BackgroundSyncRegistrationProto& registration_proto =
             registrations_proto.registration(i);
 
         if (registration_proto.id() >= registrations->next_id) {
           corruption_detected = true;
           break;
         }
 
         RegistrationKey registration_key(registration_proto.tag(),
                                          registration_proto.periodicity());
-        BackgroundSyncRegistration* registration =
-            &registrations->registration_map[registration_key];
+
+        scoped_refptr<RefCountedRegistration> ref_registration(
+            new RefCountedRegistration());
+        registrations->registration_map[registration_key] = ref_registration;
+        BackgroundSyncRegistration* registration = ref_registration->value();
 
         BackgroundSyncRegistrationOptions* options = registration->options();
         options->tag = registration_proto.tag();
         options->periodicity = registration_proto.periodicity();
         options->min_period = registration_proto.min_period();
         options->network_state = registration_proto.network_state();
         options->power_state = registration_proto.power_state();
 
         registration->set_id(registration_proto.id());
         registration->set_sync_state(registration_proto.sync_state());
@@ skipping 32 matching lines @@
   BackgroundSyncMetrics::RegistrationCouldFire registration_could_fire =
       AreOptionConditionsMet(options)
           ? BackgroundSyncMetrics::REGISTRATION_COULD_FIRE
           : BackgroundSyncMetrics::REGISTRATION_COULD_NOT_FIRE;
 
   if (disabled_) {
     BackgroundSyncMetrics::CountRegister(
         options.periodicity, registration_could_fire,
         BackgroundSyncMetrics::REGISTRATION_IS_NOT_DUPLICATE,
         BACKGROUND_SYNC_STATUS_STORAGE_ERROR);
-    base::ThreadTaskRunnerHandle::Get()->PostTask(
-        FROM_HERE, base::Bind(callback, BACKGROUND_SYNC_STATUS_STORAGE_ERROR,
-                              BackgroundSyncRegistration()));
+    PostErrorResponse(BACKGROUND_SYNC_STATUS_STORAGE_ERROR, callback);
     return;
   }
 
   ServiceWorkerRegistration* sw_registration =
       service_worker_context_->GetLiveRegistration(sw_registration_id);
   if (!sw_registration || !sw_registration->active_version()) {
     BackgroundSyncMetrics::CountRegister(
         options.periodicity, registration_could_fire,
         BackgroundSyncMetrics::REGISTRATION_IS_NOT_DUPLICATE,
         BACKGROUND_SYNC_STATUS_NO_SERVICE_WORKER);
-    base::ThreadTaskRunnerHandle::Get()->PostTask(
-        FROM_HERE,
-        base::Bind(callback, BACKGROUND_SYNC_STATUS_NO_SERVICE_WORKER,
-                   BackgroundSyncRegistration()));
+    PostErrorResponse(BACKGROUND_SYNC_STATUS_NO_SERVICE_WORKER, callback);
     return;
   }
 
   if (!sw_registration->active_version()->HasWindowClients()) {
-    base::ThreadTaskRunnerHandle::Get()->PostTask(
-        FROM_HERE, base::Bind(callback, BACKGROUND_SYNC_STATUS_NOT_ALLOWED,
-                              BackgroundSyncRegistration()));
+    PostErrorResponse(BACKGROUND_SYNC_STATUS_NOT_ALLOWED, callback);
     return;
   }
 
-  BackgroundSyncRegistration* existing_registration =
+  RefCountedRegistration* existing_registration_ref =
       LookupRegistration(sw_registration_id, RegistrationKey(options));
-  if (existing_registration &&
-      existing_registration->options()->Equals(options)) {
+  if (existing_registration_ref &&
+      existing_registration_ref->value()->options()->Equals(options)) {
+    BackgroundSyncRegistration* existing_registration =
+        existing_registration_ref->value();
     if (existing_registration->sync_state() == SYNC_STATE_FAILED) {
       existing_registration->set_sync_state(SYNC_STATE_PENDING);
       StoreRegistrations(
           sw_registration_id,
           base::Bind(&BackgroundSyncManager::RegisterDidStore,
                      weak_ptr_factory_.GetWeakPtr(), sw_registration_id,
-                     *existing_registration, callback));
+                     make_scoped_refptr(existing_registration_ref), callback));
       return;
     }
 
     // Record the duplicated registration
     BackgroundSyncMetrics::CountRegister(
         existing_registration->options()->periodicity, registration_could_fire,
         BackgroundSyncMetrics::REGISTRATION_IS_DUPLICATE,
         BACKGROUND_SYNC_STATUS_OK);
 
     base::ThreadTaskRunnerHandle::Get()->PostTask(
-        FROM_HERE, base::Bind(callback, BACKGROUND_SYNC_STATUS_OK,
-                              *existing_registration));
+        FROM_HERE,
+        base::Bind(
+            callback, BACKGROUND_SYNC_STATUS_OK,
+            base::Passed(
+                CreateRegistrationHandle(*existing_registration_ref).Pass())));
     return;
   }
 
-  BackgroundSyncRegistration new_registration;
-  *new_registration.options() = options;
+  scoped_refptr<RefCountedRegistration> new_ref_registration(

[michaeln, 2015/08/21 02:39:24]

Is it possible for there to be handle to a registration matching these criteria
in the  registration_handle_ids_ map? And if so should its underlying
BackgroundSyncRegistration be updated and reinserted with AddRegistrationToMap?

Something that might help is to have an authoritative collection of 'live'
RefCountedRegistration instances, keyed by persistent id i guess, rawptrs to not
influence lifetimes? Their may be refs to these objects from the
registration_handle_ids_ and the sw_to_registrations_map_, and those refs
determine the object lifetimes.

You can see something like that at work in the sw code base. The
constructors/destructors add/rmv ptrs from the map. See AddLiveVersion() and
RemoveLiveVersion() for example.

[jkarlin, 2015/08/25 17:32:58]

Yes, but it will have a different registration id. No, the handle's registration
shouldn't be updated, it should keep pointing at the out-of-date registration.
sw_to_registrations_map_ is the collection of live registrations. Once it leaves
that map it may still be referenced by clients but it's no longer live (won't be
run).

+      new RefCountedRegistration());
+  BackgroundSyncRegistration* new_registration = new_ref_registration->value();
+
+  *new_registration->options() = options;
 
   BackgroundSyncRegistrations* registrations =
       &sw_to_registrations_map_[sw_registration_id];
-  new_registration.set_id(registrations->next_id++);
+  new_registration->set_id(registrations->next_id++);
 
   AddRegistrationToMap(sw_registration_id,
                        sw_registration->pattern().GetOrigin(),
-                       new_registration);
+                       new_ref_registration);
 
   StoreRegistrations(
       sw_registration_id,
       base::Bind(&BackgroundSyncManager::RegisterDidStore,
                  weak_ptr_factory_.GetWeakPtr(), sw_registration_id,
-                 new_registration, callback));
+                 new_ref_registration, callback));
 }
 
 void BackgroundSyncManager::DisableAndClearManager(
     const base::Closure& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
   if (disabled_) {
     base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE,
                                                   base::Bind(callback));
     return;
@@ skipping 37 matching lines @@
 void BackgroundSyncManager::DisableAndClearManagerClearedOne(
     const base::Closure& barrier_closure,
     ServiceWorkerStatusCode status) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
   // The status doesn't matter at this point, there is nothing else to be done.
   base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE,
                                                 base::Bind(barrier_closure));
 }
 
-BackgroundSyncRegistration* BackgroundSyncManager::LookupRegistration(
+BackgroundSyncManager::RefCountedRegistration*
+BackgroundSyncManager::LookupRegistration(
     int64 sw_registration_id,
     const RegistrationKey& registration_key) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
   SWIdToRegistrationsMap::iterator it =
       sw_to_registrations_map_.find(sw_registration_id);
   if (it == sw_to_registrations_map_.end())
     return nullptr;
 
   BackgroundSyncRegistrations& registrations = it->second;
   DCHECK_LE(BackgroundSyncRegistration::kInitialId, registrations.next_id);
   DCHECK(!registrations.origin.is_empty());
 
   auto key_and_registration_iter =
       registrations.registration_map.find(registration_key);
   if (key_and_registration_iter == registrations.registration_map.end())
     return nullptr;
 
-  return &key_and_registration_iter->second;
+  return key_and_registration_iter->second.get();
 }
 
 void BackgroundSyncManager::StoreRegistrations(
     int64 sw_registration_id,
     const ServiceWorkerStorage::StatusCallback& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
   // Serialize the data.
   const BackgroundSyncRegistrations& registrations =
       sw_to_registrations_map_[sw_registration_id];
   BackgroundSyncRegistrationsProto registrations_proto;
   registrations_proto.set_next_registration_id(registrations.next_id);
   registrations_proto.set_origin(registrations.origin.spec());
 
   for (const auto& key_and_registration : registrations.registration_map) {
     const BackgroundSyncRegistration& registration =
-        key_and_registration.second;
+        *key_and_registration.second->value();
     BackgroundSyncRegistrationProto* registration_proto =
         registrations_proto.add_registration();
     registration_proto->set_id(registration.id());
     registration_proto->set_sync_state(registration.sync_state());
     registration_proto->set_tag(registration.options()->tag);
     registration_proto->set_periodicity(registration.options()->periodicity);
     registration_proto->set_min_period(registration.options()->min_period);
     registration_proto->set_network_state(
         registration.options()->network_state);
     registration_proto->set_power_state(registration.options()->power_state);
   }
   std::string serialized;
   bool success = registrations_proto.SerializeToString(&serialized);
   DCHECK(success);
 
   StoreDataInBackend(sw_registration_id, registrations.origin,
                      kBackgroundSyncUserDataKey, serialized, callback);
 }
 
 void BackgroundSyncManager::RegisterDidStore(
     int64 sw_registration_id,
-    const BackgroundSyncRegistration& new_registration,
+    const scoped_refptr<RefCountedRegistration>& new_registration_ref,
     const StatusAndRegistrationCallback& callback,
     ServiceWorkerStatusCode status) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
+  const BackgroundSyncRegistration* new_registration =
+      new_registration_ref->value();
+
   // For UMA, determine here whether the sync could fire immediately
   BackgroundSyncMetrics::RegistrationCouldFire registration_could_fire =
-      AreOptionConditionsMet(*new_registration.options())
+      AreOptionConditionsMet(*new_registration->options())
           ? BackgroundSyncMetrics::REGISTRATION_COULD_FIRE
           : BackgroundSyncMetrics::REGISTRATION_COULD_NOT_FIRE;
 
   if (status == SERVICE_WORKER_ERROR_NOT_FOUND) {
     // The service worker registration is gone.
     BackgroundSyncMetrics::CountRegister(
-        new_registration.options()->periodicity, registration_could_fire,
+        new_registration->options()->periodicity, registration_could_fire,
         BackgroundSyncMetrics::REGISTRATION_IS_NOT_DUPLICATE,
         BACKGROUND_SYNC_STATUS_STORAGE_ERROR);
     sw_to_registrations_map_.erase(sw_registration_id);
-    base::ThreadTaskRunnerHandle::Get()->PostTask(
-        FROM_HERE, base::Bind(callback, BACKGROUND_SYNC_STATUS_STORAGE_ERROR,
-                              BackgroundSyncRegistration()));
+    PostErrorResponse(BACKGROUND_SYNC_STATUS_STORAGE_ERROR, callback);
     return;
   }
 
   if (status != SERVICE_WORKER_OK) {
     LOG(ERROR) << "BackgroundSync failed to store registration due to backend "
                   "failure.";
     BackgroundSyncMetrics::CountRegister(
-        new_registration.options()->periodicity, registration_could_fire,
+        new_registration->options()->periodicity, registration_could_fire,
         BackgroundSyncMetrics::REGISTRATION_IS_NOT_DUPLICATE,
         BACKGROUND_SYNC_STATUS_STORAGE_ERROR);
-    DisableAndClearManager(base::Bind(callback,
-                                      BACKGROUND_SYNC_STATUS_STORAGE_ERROR,
-                                      BackgroundSyncRegistration()));
+    DisableAndClearManager(base::Bind(
+        callback, BACKGROUND_SYNC_STATUS_STORAGE_ERROR,
+        base::Passed(scoped_ptr<BackgroundSyncRegistrationHandle>().Pass())));
     return;
   }
 
   BackgroundSyncMetrics::CountRegister(
-      new_registration.options()->periodicity, registration_could_fire,
+      new_registration->options()->periodicity, registration_could_fire,
       BackgroundSyncMetrics::REGISTRATION_IS_NOT_DUPLICATE,
       BACKGROUND_SYNC_STATUS_OK);
   FireReadyEvents();
   base::ThreadTaskRunnerHandle::Get()->PostTask(
       FROM_HERE,
-      base::Bind(callback, BACKGROUND_SYNC_STATUS_OK, new_registration));
+      base::Bind(callback, BACKGROUND_SYNC_STATUS_OK,
+                 base::Passed(
+                     CreateRegistrationHandle(*new_registration_ref).Pass())));
 }
 
 void BackgroundSyncManager::RemoveRegistrationFromMap(
     int64 sw_registration_id,
     const RegistrationKey& registration_key) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   DCHECK(LookupRegistration(sw_registration_id, registration_key));
 
   BackgroundSyncRegistrations* registrations =
       &sw_to_registrations_map_[sw_registration_id];
 
   registrations->registration_map.erase(registration_key);
 }
 
 void BackgroundSyncManager::AddRegistrationToMap(
     int64 sw_registration_id,
     const GURL& origin,
-    const BackgroundSyncRegistration& sync_registration) {
+    const scoped_refptr<RefCountedRegistration>& sync_registration) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  DCHECK(sync_registration.IsValid());
+  DCHECK(sync_registration->value()->IsValid());
 
   BackgroundSyncRegistrations* registrations =
       &sw_to_registrations_map_[sw_registration_id];
   registrations->origin = origin;
 
-  RegistrationKey registration_key(sync_registration);
+  RegistrationKey registration_key(*sync_registration->value());
   registrations->registration_map[registration_key] = sync_registration;
 }
 
 void BackgroundSyncManager::StoreDataInBackend(
     int64 sw_registration_id,
     const GURL& origin,
     const std::string& backend_key,
     const std::string& data,
     const ServiceWorkerStorage::StatusCallback& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
   service_worker_context_->StoreRegistrationUserData(
       sw_registration_id, origin, backend_key, data, callback);
 }
 
 void BackgroundSyncManager::GetDataFromBackend(
     const std::string& backend_key,
     const ServiceWorkerStorage::GetUserDataForAllRegistrationsCallback&
         callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
   service_worker_context_->GetUserDataForAllRegistrations(backend_key,
                                                           callback);
 }
 
 void BackgroundSyncManager::FireOneShotSync(
-    const BackgroundSyncRegistration& registration,
+    const RefCountedRegistration& registration,
     const scoped_refptr<ServiceWorkerVersion>& active_version,
     const ServiceWorkerVersion::StatusCallback& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
-  active_version->DispatchSyncEvent(
-      mojo::ConvertTo<SyncRegistrationPtr>(registration), callback);
+  // The ServiceWorkerVersion doesn't know when the client (javascript) is done
+  // with the registration so don't give it a BackgroundSyncRegistrationHandle.
+  // The render process must call BackgroundSyncServiceImpl::TrackRegistration
+  // which creates the corresponding BackgroundSyncRegistrationHandle. In the
+  // unlikely event that the render process is killed before TrackRegistration
+  // is called, the manager will hold the reference until it is destroyed.
+  // The ~BackgroundSyncManager DCHECKS verify that this is uncommon.
+  SyncRegistrationPtr sync_registration =
+      mojo::ConvertTo<SyncRegistrationPtr>(*registration.value());
+  scoped_refptr<const RefCountedRegistration>* ptr =
+      new scoped_refptr<const RefCountedRegistration>(&registration);
+  sync_registration->id = registration_handle_ids_.Add(ptr);
+
+  active_version->DispatchSyncEvent(sync_registration.Pass(), callback);
+}
+
+scoped_ptr<BackgroundSyncRegistrationHandle>
+BackgroundSyncManager::CreateRegistrationHandle(
+    const RefCountedRegistration& registration) {
+  scoped_refptr<const RefCountedRegistration>* ptr =
+      new scoped_refptr<const RefCountedRegistration>(&registration);
+
+  // Registration handles have unique handle ids. The handle id maps to an
+  // internal RefCountedRegistration (which has the persistent registration id)
+  // via
+  // registration_reference_ids_.
+  BackgroundSyncRegistrationHandle::HandleId handle_id =
+      registration_handle_ids_.Add(ptr);
+
+  return make_scoped_ptr(new BackgroundSyncRegistrationHandle(this, handle_id));
+}
+
+void BackgroundSyncManager::Unregister(
+    int64 sw_registration_id,
+    SyncPeriodicity periodicity,
+    BackgroundSyncRegistrationHandle::HandleId handle_id,
+    const StatusCallback& callback) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+
+  if (disabled_) {
+    BackgroundSyncMetrics::CountUnregister(
+        periodicity, BACKGROUND_SYNC_STATUS_STORAGE_ERROR);
+    base::ThreadTaskRunnerHandle::Get()->PostTask(
+        FROM_HERE, base::Bind(callback, BACKGROUND_SYNC_STATUS_STORAGE_ERROR));
+    return;
+  }
+
+  op_scheduler_.ScheduleOperation(
+      base::Bind(&BackgroundSyncManager::UnregisterImpl,

[michaeln, 2015/08/21 02:39:24]

What prevents ReleaseHandleId(xxx) from happening prior to the scheduled op
running? You could create a new handle and bind that instance into this closure,
Pass() it as a param in place of 'handle_id'. Passing ids is good for crossing
process boundaries, but once your inside a process, use the smart scopers to
manage lifetimes.

[jkarlin, 2015/08/25 17:32:58]

Thanks for that catch. Resolved by reverting UnregisterImpl.

+                 weak_ptr_factory_.GetWeakPtr(), sw_registration_id,
+                 periodicity, handle_id, MakeStatusCompletion(callback)));
 }
 
 void BackgroundSyncManager::UnregisterImpl(
     int64 sw_registration_id,
-    const RegistrationKey& registration_key,
-    BackgroundSyncRegistration::RegistrationId sync_registration_id,
     SyncPeriodicity periodicity,
+    BackgroundSyncRegistrationHandle::HandleId handle_id,
     const StatusCallback& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
   if (disabled_) {
     BackgroundSyncMetrics::CountUnregister(
         periodicity, BACKGROUND_SYNC_STATUS_STORAGE_ERROR);
     base::ThreadTaskRunnerHandle::Get()->PostTask(
         FROM_HERE, base::Bind(callback, BACKGROUND_SYNC_STATUS_STORAGE_ERROR));
     return;
   }
 
-  const BackgroundSyncRegistration* existing_registration =
+  scoped_refptr<const RefCountedRegistration>* handle_ref =
+      registration_handle_ids_.Lookup(handle_id);
+  DCHECK(handle_ref);
+
+  const BackgroundSyncRegistration* handle_registration =
+      (*handle_ref)->value();
+
+  RegistrationKey registration_key(handle_registration->options()->tag,
+                                   periodicity);
+
+  const RefCountedRegistration* existing_registration =
       LookupRegistration(sw_registration_id, registration_key);
+
   if (!existing_registration ||
-      existing_registration->id() != sync_registration_id) {
+      existing_registration->value()->id() != handle_registration->id()) {
     BackgroundSyncMetrics::CountUnregister(periodicity,
                                            BACKGROUND_SYNC_STATUS_NOT_FOUND);
     base::ThreadTaskRunnerHandle::Get()->PostTask(
         FROM_HERE, base::Bind(callback, BACKGROUND_SYNC_STATUS_NOT_FOUND));
     return;
   }
 
   RemoveRegistrationFromMap(sw_registration_id, registration_key);
 
   StoreRegistrations(sw_registration_id,
@@ skipping 33 matching lines @@
       FROM_HERE, base::Bind(callback, BACKGROUND_SYNC_STATUS_OK));
 }
 
 void BackgroundSyncManager::GetRegistrationImpl(
     int64 sw_registration_id,
     const RegistrationKey& registration_key,
     const StatusAndRegistrationCallback& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
   if (disabled_) {
-    base::ThreadTaskRunnerHandle::Get()->PostTask(
-        FROM_HERE, base::Bind(callback, BACKGROUND_SYNC_STATUS_STORAGE_ERROR,
-                              BackgroundSyncRegistration()));
+    PostErrorResponse(BACKGROUND_SYNC_STATUS_STORAGE_ERROR, callback);
     return;
   }
 
-  const BackgroundSyncRegistration* out_registration =
+  const RefCountedRegistration* registration =
       LookupRegistration(sw_registration_id, registration_key);
-  if (!out_registration) {
-    base::ThreadTaskRunnerHandle::Get()->PostTask(
-        FROM_HERE, base::Bind(callback, BACKGROUND_SYNC_STATUS_NOT_FOUND,
-                              BackgroundSyncRegistration()));
+  if (!registration) {
+    PostErrorResponse(BACKGROUND_SYNC_STATUS_NOT_FOUND, callback);
     return;
   }
 
   base::ThreadTaskRunnerHandle::Get()->PostTask(
       FROM_HERE,
-      base::Bind(callback, BACKGROUND_SYNC_STATUS_OK, *out_registration));
+      base::Bind(callback, BACKGROUND_SYNC_STATUS_OK,
+                 base::Passed(CreateRegistrationHandle(*registration).Pass())));
 }
 
 void BackgroundSyncManager::GetRegistrationsImpl(
     int64 sw_registration_id,
     SyncPeriodicity periodicity,
     const StatusAndRegistrationsCallback& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
-  std::vector<BackgroundSyncRegistration> out_registrations;
+  scoped_ptr<ScopedVector<BackgroundSyncRegistrationHandle>> out_registrations(
+      new ScopedVector<BackgroundSyncRegistrationHandle>());
 
   if (disabled_) {
     base::ThreadTaskRunnerHandle::Get()->PostTask(
         FROM_HERE, base::Bind(callback, BACKGROUND_SYNC_STATUS_STORAGE_ERROR,
-                              out_registrations));
+                              base::Passed(out_registrations.Pass())));
     return;
   }
 
   SWIdToRegistrationsMap::iterator it =
       sw_to_registrations_map_.find(sw_registration_id);
 
   if (it != sw_to_registrations_map_.end()) {
     const BackgroundSyncRegistrations& registrations = it->second;
     for (const auto& tag_and_registration : registrations.registration_map) {
-      const BackgroundSyncRegistration& registration =
-          tag_and_registration.second;
-      if (registration.options()->periodicity == periodicity)
-        out_registrations.push_back(registration);
+      const RefCountedRegistration& registration =
+          *tag_and_registration.second.get();
+      if (registration.value()->options()->periodicity == periodicity) {
+        out_registrations->push_back(
+            CreateRegistrationHandle(registration).release());
+      }
     }
   }
 
   base::ThreadTaskRunnerHandle::Get()->PostTask(
-      FROM_HERE,
-      base::Bind(callback, BACKGROUND_SYNC_STATUS_OK, out_registrations));
+      FROM_HERE, base::Bind(callback, BACKGROUND_SYNC_STATUS_OK,
+                            base::Passed(out_registrations.Pass())));
 }
 
 bool BackgroundSyncManager::AreOptionConditionsMet(
     const BackgroundSyncRegistrationOptions& options) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   return network_observer_->NetworkSufficient(options.network_state) &&
          power_observer_->PowerSufficient(options.power_state);
 }
 
 bool BackgroundSyncManager::IsRegistrationReadyToFire(
@@ skipping 13 matching lines @@
 }
 
 void BackgroundSyncManager::SchedulePendingRegistrations() {
 #if defined(OS_ANDROID)
   bool keep_browser_alive_for_one_shot = false;
 
   for (const auto& sw_id_and_registrations : sw_to_registrations_map_) {
     for (const auto& key_and_registration :
          sw_id_and_registrations.second.registration_map) {
       const BackgroundSyncRegistration& registration =
-          key_and_registration.second;
+          *key_and_registration.second->value();
       if (registration.sync_state() == SYNC_STATE_PENDING) {
         if (registration.options()->periodicity == SYNC_ONE_SHOT) {
           keep_browser_alive_for_one_shot = true;
         } else {
           // TODO(jkarlin): Support keeping the browser alive for periodic
           // syncs.
         }
       }
     }
   }
@@ skipping 29 matching lines @@
     return;
   }
 
   // Find the registrations that are ready to run.
   std::vector<std::pair<int64, RegistrationKey>> sw_id_and_keys_to_fire;
 
   for (auto& sw_id_and_registrations : sw_to_registrations_map_) {
     const int64 service_worker_id = sw_id_and_registrations.first;
     for (auto& key_and_registration :
          sw_id_and_registrations.second.registration_map) {
-      BackgroundSyncRegistration* registration = &key_and_registration.second;
+      BackgroundSyncRegistration* registration =
+          key_and_registration.second->value();
       if (IsRegistrationReadyToFire(*registration)) {
         sw_id_and_keys_to_fire.push_back(
             std::make_pair(service_worker_id, key_and_registration.first));
         // The state change is not saved to persistent storage because
         // if the sync event is killed mid-sync then it should return to
         // SYNC_STATE_PENDING.
         registration->set_sync_state(SYNC_STATE_FIRING);
       }
     }
   }
@@ skipping 12 matching lines @@
         sw_id_and_keys_to_fire.size(), base::Bind(callback));
 
     // Record the total time taken after all events have run to completion.
     base::Closure events_completed_barrier_closure =
         base::BarrierClosure(sw_id_and_keys_to_fire.size(),
                              base::Bind(&OnAllSyncEventsCompleted, start_time,
                                         sw_id_and_keys_to_fire.size()));
 
     for (const auto& sw_id_and_key : sw_id_and_keys_to_fire) {
       int64 service_worker_id = sw_id_and_key.first;
-      const BackgroundSyncRegistration* registration =
+      const RefCountedRegistration* registration =
           LookupRegistration(service_worker_id, sw_id_and_key.second);
+      DCHECK(registration);
 
       service_worker_context_->FindRegistrationForId(
           service_worker_id, sw_to_registrations_map_[service_worker_id].origin,
           base::Bind(&BackgroundSyncManager::FireReadyEventsDidFindRegistration,
                      weak_ptr_factory_.GetWeakPtr(), sw_id_and_key.second,
-                     registration->id(), events_fired_barrier_closure,
+                     registration->value()->id(), events_fired_barrier_closure,
                      events_completed_barrier_closure));
     }
   }
 
   SchedulePendingRegistrations();
 }
 
 void BackgroundSyncManager::FireReadyEventsDidFindRegistration(
     const RegistrationKey& registration_key,
     BackgroundSyncRegistration::RegistrationId registration_id,
     const base::Closure& event_fired_callback,
     const base::Closure& event_completed_callback,
     ServiceWorkerStatusCode service_worker_status,
     const scoped_refptr<ServiceWorkerRegistration>&
         service_worker_registration) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   if (service_worker_status != SERVICE_WORKER_OK) {
     base::ThreadTaskRunnerHandle::Get()->PostTask(
         FROM_HERE, base::Bind(event_fired_callback));
     base::ThreadTaskRunnerHandle::Get()->PostTask(
         FROM_HERE, base::Bind(event_completed_callback));
     return;
   }
 
-  BackgroundSyncRegistration* registration =
+  const RefCountedRegistration* registration =
       LookupRegistration(service_worker_registration->id(), registration_key);
+  DCHECK(registration);
 
   FireOneShotSync(
       *registration, service_worker_registration->active_version(),
       base::Bind(&BackgroundSyncManager::EventComplete,

[michaeln, 2015/08/21 02:39:24]

I think we might be able to improve  upon the lifetime management. As coded,
this introduces a leak that isn't all that unlikely (if the worker thread
doesn't actually process the event... leak).

What if...

This method creates a handle and binds it to the EventComplete callback so the
handle will be valid during event dispatch to the renderer. During event
dispatch the renderer is expected to invoke the
track|adpot|acquire|copy|duplicate|take (or whatever name is settled upon)
method.

[jkarlin, 2015/08/25 17:32:58]

This is a good idea. In my implementation the renderer creates a duplicate
handle (with a unique handle id) and doesn't fire the event until the handle is
made. There shouldn't be any race issues.

                  weak_ptr_factory_.GetWeakPtr(), service_worker_registration,
                  service_worker_registration->id(), registration_key,
                  registration_id, event_completed_callback));
 
   base::ThreadTaskRunnerHandle::Get()->PostTask(
       FROM_HERE, base::Bind(event_fired_callback));
 }
 
 // |service_worker_registration| is just to keep the registration alive
 // while the event is firing.
@@ skipping 21 matching lines @@
     ServiceWorkerStatusCode status_code,
     const base::Closure& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
   if (disabled_) {
     base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE,
                                                   base::Bind(callback));
     return;
   }
 
-  BackgroundSyncRegistration* registration =
+  RefCountedRegistration* registration_ref =
       LookupRegistration(service_worker_id, key);
-  if (!registration || registration->id() != sync_registration_id) {
+  if (!registration_ref ||
+      registration_ref->value()->id() != sync_registration_id) {
     base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE,
                                                   base::Bind(callback));
     return;
   }
 
+  BackgroundSyncRegistration* registration = registration_ref->value();
+
   // The event ran to completion, we should count it, no matter what happens
   // from here.
   BackgroundSyncMetrics::RecordEventResult(registration->options()->periodicity,
                                            status_code == SERVICE_WORKER_OK);
 
   if (registration->options()->periodicity == SYNC_ONE_SHOT) {
     if (status_code != SERVICE_WORKER_OK) {
       // TODO(jkarlin) Fire the sync event on the next page load controlled by
       // this registration. (crbug.com/479665)
       registration->set_sync_state(SYNC_STATE_FAILED);
@@ skipping 71 matching lines @@
 
   FireReadyEvents();
 }
 
 void BackgroundSyncManager::OnPowerChanged() {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
   FireReadyEvents();
 }
 
+// TODO(jkarlin): Figure out how to pass scoped_ptrs with this.
 template <typename CallbackT, typename... Params>
 void BackgroundSyncManager::CompleteOperationCallback(const CallbackT& callback,
                                                       Params... parameters) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
   callback.Run(parameters...);
   op_scheduler_.CompleteOperationAndRunNext();
 }
 
+void BackgroundSyncManager::CompleteStatusAndRegistrationCallback(
+    StatusAndRegistrationCallback callback,
+    BackgroundSyncStatus status,
+    scoped_ptr<BackgroundSyncRegistrationHandle> result) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+
+  callback.Run(status, result.Pass());
+  op_scheduler_.CompleteOperationAndRunNext();
+}
+
+void BackgroundSyncManager::CompleteStatusAndRegistrationsCallback(
+    StatusAndRegistrationsCallback callback,
+    BackgroundSyncStatus status,
+    scoped_ptr<ScopedVector<BackgroundSyncRegistrationHandle>> results) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+
+  callback.Run(status, results.Pass());
+  op_scheduler_.CompleteOperationAndRunNext();
+}
+
 base::Closure BackgroundSyncManager::MakeEmptyCompletion() {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
   return MakeClosureCompletion(base::Bind(base::DoNothing));
 }
 
 base::Closure BackgroundSyncManager::MakeClosureCompletion(
     const base::Closure& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
   return base::Bind(
       &BackgroundSyncManager::CompleteOperationCallback<base::Closure>,
       weak_ptr_factory_.GetWeakPtr(), callback);
 }
 
 BackgroundSyncManager::StatusAndRegistrationCallback
 BackgroundSyncManager::MakeStatusAndRegistrationCompletion(
     const StatusAndRegistrationCallback& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
-  return base::Bind(&BackgroundSyncManager::CompleteOperationCallback<
-                        StatusAndRegistrationCallback, BackgroundSyncStatus,
-                        const BackgroundSyncRegistration&>,
-                    weak_ptr_factory_.GetWeakPtr(), callback);
+  return base::Bind(
+      &BackgroundSyncManager::CompleteStatusAndRegistrationCallback,
+      weak_ptr_factory_.GetWeakPtr(), callback);
 }
 
 BackgroundSyncManager::StatusAndRegistrationsCallback
 BackgroundSyncManager::MakeStatusAndRegistrationsCompletion(
     const StatusAndRegistrationsCallback& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
-  return base::Bind(&BackgroundSyncManager::CompleteOperationCallback<
-                        StatusAndRegistrationsCallback, BackgroundSyncStatus,
-                        const std::vector<BackgroundSyncRegistration>&>,
-                    weak_ptr_factory_.GetWeakPtr(), callback);
+  return base::Bind(
+      &BackgroundSyncManager::CompleteStatusAndRegistrationsCallback,
+      weak_ptr_factory_.GetWeakPtr(), callback);
 }
 
 BackgroundSyncManager::StatusCallback
 BackgroundSyncManager::MakeStatusCompletion(const StatusCallback& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
   return base::Bind(
       &BackgroundSyncManager::CompleteOperationCallback<StatusCallback,
                                                         BackgroundSyncStatus>,
       weak_ptr_factory_.GetWeakPtr(), callback);
 }
 
 }  // namespace content