[BackgroundSync] Remove BackgroundSyncRegistrationHandle

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 <utility>
 
 #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 "base/time/default_clock.h"
 #include "build/build_config.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_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/browser/storage_partition_impl.h"
 #include "content/common/service_worker/service_worker_type_converters.h"
 #include "content/public/browser/background_sync_controller.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/common/background_sync.mojom.h"
 
 #if defined(OS_ANDROID)
 #include "content/browser/android/background_sync_network_observer_android.h"
 #endif
 
 namespace content {
 
-class BackgroundSyncManager::RefCountedRegistration
-    : public base::RefCounted<RefCountedRegistration> {
- public:
-  BackgroundSyncRegistration* value() { return &registration_; }
-  const BackgroundSyncRegistration* value() const { return &registration_; }
-
- private:
-  friend class base::RefCounted<RefCountedRegistration>;
-  ~RefCountedRegistration() = default;
-
-  BackgroundSyncRegistration registration_;
-};
-
 namespace {
 
 // The key used to index the background sync data in ServiceWorkerStorage.
 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>())));
+                 base::Passed(scoped_ptr<BackgroundSyncRegistration>())));
 }
 
 // Returns nullptr if the controller cannot be accessed for any reason.
 BackgroundSyncController* GetBackgroundSyncControllerOnUIThread(
     const scoped_refptr<ServiceWorkerContextWrapper>& service_worker_context) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
   if (!service_worker_context)
     return nullptr;
   StoragePartitionImpl* storage_partition_impl =
@@ skipping 139 matching lines @@
 void BackgroundSyncManager::GetRegistrations(
     int64_t sw_registration_id,
     const StatusAndRegistrationsCallback& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
   if (disabled_) {
     base::ThreadTaskRunnerHandle::Get()->PostTask(
         FROM_HERE,
         base::Bind(
             callback, BACKGROUND_SYNC_STATUS_STORAGE_ERROR,
-            base::Passed(
-                scoped_ptr<ScopedVector<BackgroundSyncRegistrationHandle>>(
-                    new ScopedVector<BackgroundSyncRegistrationHandle>()))));
+            base::Passed(scoped_ptr<ScopedVector<BackgroundSyncRegistration>>(
+                new ScopedVector<BackgroundSyncRegistration>()))));
     return;
   }
 
   op_scheduler_.ScheduleOperation(
       base::Bind(&BackgroundSyncManager::GetRegistrationsImpl,
                  weak_ptr_factory_.GetWeakPtr(), sw_registration_id,
                  MakeStatusAndRegistrationsCompletion(callback)));
 }
 
-// Given a HandleId |handle_id|, return a new handle for the same
-// registration.
-scoped_ptr<BackgroundSyncRegistrationHandle>
-BackgroundSyncManager::DuplicateRegistrationHandle(
-    BackgroundSyncRegistrationHandle::HandleId handle_id) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
-
-  scoped_refptr<RefCountedRegistration>* ref_registration =
-      registration_handle_ids_.Lookup(handle_id);
-  if (!ref_registration)
-    return scoped_ptr<BackgroundSyncRegistrationHandle>();
-  return CreateRegistrationHandle(ref_registration->get());
-}
-
 void BackgroundSyncManager::OnRegistrationDeleted(int64_t sw_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(), sw_registration_id,
@@ skipping 116 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());
 
-        scoped_refptr<RefCountedRegistration> ref_registration(
-            new RefCountedRegistration());
-        registrations->registration_map[registration_key] = ref_registration;
-        BackgroundSyncRegistration* registration = ref_registration->value();
+        BackgroundSyncRegistration* registration =
+            &registrations->registration_map[registration_key];
 
         BackgroundSyncRegistrationOptions* options = registration->options();
         options->tag = registration_proto.tag();
         options->network_state = registration_proto.network_state();
 
         registration->set_id(registration_proto.id());
         registration->set_num_attempts(registration_proto.num_attempts());
         registration->set_delay_until(
             base::Time::FromInternalValue(registration_proto.delay_until()));
       }
@@ skipping 80 matching lines @@
         BACKGROUND_SYNC_STATUS_NO_SERVICE_WORKER);
     PostErrorResponse(BACKGROUND_SYNC_STATUS_NO_SERVICE_WORKER, callback);
     return;
   }
 
   BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
                           base::Bind(&NotifyBackgroundSyncRegisteredOnUIThread,
                                      service_worker_context_,
                                      sw_registration->pattern().GetOrigin()));
 
-  RefCountedRegistration* existing_registration_ref =
+  BackgroundSyncRegistration* existing_registration =
       LookupActiveRegistration(sw_registration_id, RegistrationKey(options));
-  if (existing_registration_ref) {
-    DCHECK(existing_registration_ref->value()->options()->Equals(options));
-    BackgroundSyncRegistration* existing_registration =
-        existing_registration_ref->value();
+  if (existing_registration) {
+    DCHECK(existing_registration->options()->Equals(options));
 
     BackgroundSyncMetrics::RegistrationCouldFire registration_could_fire =
         AreOptionConditionsMet(options)
             ? BackgroundSyncMetrics::REGISTRATION_COULD_FIRE
             : BackgroundSyncMetrics::REGISTRATION_COULD_NOT_FIRE;
     BackgroundSyncMetrics::CountRegisterSuccess(
         registration_could_fire,
         BackgroundSyncMetrics::REGISTRATION_IS_DUPLICATE);
 
     if (existing_registration->IsFiring()) {
       existing_registration->set_sync_state(
           BackgroundSyncState::REREGISTERED_WHILE_FIRING);
     }
 
+    scoped_ptr<BackgroundSyncRegistration> out_registration(
+        new BackgroundSyncRegistration(*existing_registration));
+
     base::ThreadTaskRunnerHandle::Get()->PostTask(
-        FROM_HERE,
-        base::Bind(
-            callback, BACKGROUND_SYNC_STATUS_OK,
-            base::Passed(CreateRegistrationHandle(existing_registration_ref))));
+        FROM_HERE, base::Bind(callback, BACKGROUND_SYNC_STATUS_OK,
+                              base::Passed(std::move(out_registration))));
     return;
   }
 
-  scoped_refptr<RefCountedRegistration> new_ref_registration(
-      new RefCountedRegistration());
-  BackgroundSyncRegistration* new_registration = new_ref_registration->value();
+  BackgroundSyncRegistration new_registration;
 
-  *new_registration->options() = options;
+  *new_registration.options() = options;
 
   BackgroundSyncRegistrations* registrations =
       &active_registrations_[sw_registration_id];
-  new_registration->set_id(registrations->next_id++);
+  new_registration.set_id(registrations->next_id++);
 
   AddActiveRegistration(sw_registration_id,
                         sw_registration->pattern().GetOrigin(),
-                        new_ref_registration);
+                        new_registration);
 
   StoreRegistrations(
       sw_registration_id,
       base::Bind(&BackgroundSyncManager::RegisterDidStore,
                  weak_ptr_factory_.GetWeakPtr(), sw_registration_id,
-                 new_ref_registration, callback));
+                 new_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 38 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));
 }
 
-BackgroundSyncManager::RefCountedRegistration*
-BackgroundSyncManager::LookupActiveRegistration(
+BackgroundSyncRegistration* BackgroundSyncManager::LookupActiveRegistration(
     int64_t sw_registration_id,
     const RegistrationKey& registration_key) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
   SWIdToRegistrationsMap::iterator it =
       active_registrations_.find(sw_registration_id);
   if (it == active_registrations_.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.get();
+  return &key_and_registration_iter->second;
 }
 
 void BackgroundSyncManager::StoreRegistrations(
     int64_t sw_registration_id,
     const ServiceWorkerStorage::StatusCallback& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
   // Serialize the data.
   const BackgroundSyncRegistrations& registrations =
       active_registrations_[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->value();
+        key_and_registration.second;
     BackgroundSyncRegistrationProto* registration_proto =
         registrations_proto.add_registration();
     registration_proto->set_id(registration.id());
     registration_proto->set_tag(registration.options()->tag);
     registration_proto->set_network_state(
         registration.options()->network_state);
     registration_proto->set_num_attempts(registration.num_attempts());
     registration_proto->set_delay_until(
         registration.delay_until().ToInternalValue());
   }
   std::string serialized;
   bool success = registrations_proto.SerializeToString(&serialized);
   DCHECK(success);
 
   StoreDataInBackend(sw_registration_id, registrations.origin,
                      kBackgroundSyncUserDataKey, serialized, callback);
 }
 
 void BackgroundSyncManager::RegisterDidStore(
     int64_t sw_registration_id,
-    const scoped_refptr<RefCountedRegistration>& new_registration_ref,
+    const BackgroundSyncRegistration& new_registration,
     const StatusAndRegistrationCallback& callback,
     ServiceWorkerStatusCode status) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
-  const BackgroundSyncRegistration* new_registration =
-      new_registration_ref->value();
-
   if (status == SERVICE_WORKER_ERROR_NOT_FOUND) {
     // The service worker registration is gone.
     BackgroundSyncMetrics::CountRegisterFailure(
         BACKGROUND_SYNC_STATUS_STORAGE_ERROR);
     active_registrations_.erase(sw_registration_id);
     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::CountRegisterFailure(
         BACKGROUND_SYNC_STATUS_STORAGE_ERROR);
-    DisableAndClearManager(base::Bind(
-        callback, BACKGROUND_SYNC_STATUS_STORAGE_ERROR,
-        base::Passed(scoped_ptr<BackgroundSyncRegistrationHandle>())));
+    DisableAndClearManager(
+        base::Bind(callback, BACKGROUND_SYNC_STATUS_STORAGE_ERROR,
+                   base::Passed(scoped_ptr<BackgroundSyncRegistration>())));
     return;
   }
 
   BackgroundSyncMetrics::RegistrationCouldFire registration_could_fire =
-      AreOptionConditionsMet(*new_registration->options())
+      AreOptionConditionsMet(*new_registration.options())
           ? BackgroundSyncMetrics::REGISTRATION_COULD_FIRE
           : BackgroundSyncMetrics::REGISTRATION_COULD_NOT_FIRE;
   BackgroundSyncMetrics::CountRegisterSuccess(
       registration_could_fire,
       BackgroundSyncMetrics::REGISTRATION_IS_NOT_DUPLICATE);
 
   FireReadyEvents();
+
+  scoped_ptr<BackgroundSyncRegistration> out_registration(
+      new BackgroundSyncRegistration(new_registration));
+
   base::ThreadTaskRunnerHandle::Get()->PostTask(
-      FROM_HERE,
-      base::Bind(
-          callback, BACKGROUND_SYNC_STATUS_OK,
-          base::Passed(CreateRegistrationHandle(new_registration_ref.get()))));
+      FROM_HERE, base::Bind(callback, BACKGROUND_SYNC_STATUS_OK,
+                            base::Passed(std::move(out_registration))));
 }
 
 void BackgroundSyncManager::RemoveActiveRegistration(
     int64_t sw_registration_id,
     const RegistrationKey& registration_key) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   DCHECK(LookupActiveRegistration(sw_registration_id, registration_key));
 
   BackgroundSyncRegistrations* registrations =
       &active_registrations_[sw_registration_id];
 
   registrations->registration_map.erase(registration_key);
 }
 
 void BackgroundSyncManager::AddActiveRegistration(
     int64_t sw_registration_id,
     const GURL& origin,
-    const scoped_refptr<RefCountedRegistration>& sync_registration) {
+    const BackgroundSyncRegistration& sync_registration) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  DCHECK(sync_registration->value()->IsValid());
+  DCHECK(sync_registration.IsValid());
 
   BackgroundSyncRegistrations* registrations =
       &active_registrations_[sw_registration_id];
   registrations->origin = origin;
 
-  RegistrationKey registration_key(*sync_registration->value());
+  RegistrationKey registration_key(sync_registration);
   registrations->registration_map[registration_key] = sync_registration;
 }
 
 void BackgroundSyncManager::StoreDataInBackend(
     int64_t 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::DispatchSyncEvent(
-    BackgroundSyncRegistrationHandle::HandleId handle_id,
+    const std::string& tag,
     const scoped_refptr<ServiceWorkerVersion>& active_version,
     BackgroundSyncEventLastChance last_chance,
     const ServiceWorkerVersion::StatusCallback& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   DCHECK(active_version);
 
   if (active_version->running_status() != ServiceWorkerVersion::RUNNING) {
     active_version->RunAfterStartWorker(
         base::Bind(&BackgroundSyncManager::DispatchSyncEvent,
-                   weak_ptr_factory_.GetWeakPtr(), handle_id, active_version,
+                   weak_ptr_factory_.GetWeakPtr(), tag, active_version,
                    last_chance, callback),
         callback);
     return;
   }
 
   int request_id = active_version->StartRequestWithCustomTimeout(
       ServiceWorkerMetrics::EventType::SYNC, callback,
       parameters_->max_sync_event_duration,
       ServiceWorkerVersion::CONTINUE_ON_TIMEOUT);
   base::WeakPtr<BackgroundSyncServiceClient> client =
       active_version->GetMojoServiceForRequest<BackgroundSyncServiceClient>(
           request_id);
 
-  // The ServiceWorkerVersion doesn't know when the client (javascript) is done
-  // with the registration so don't give it a BackgroundSyncRegistrationHandle.
-  // Once the render process gets the handle_id it can create its own handle
-  // (with a new unique handle id).
   client->Sync(
-      handle_id, last_chance,
+      tag, last_chance,
       base::Bind(&OnSyncEventFinished, active_version, request_id, callback));
 }
 
 void BackgroundSyncManager::ScheduleDelayedTask(const base::Closure& callback,
                                                 base::TimeDelta delay) {
   base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(FROM_HERE, callback,
                                                        delay);
 }
 
 void BackgroundSyncManager::HasMainFrameProviderHost(
     const GURL& origin,
     const BoolCallback& callback) {
   service_worker_context_->HasMainFrameProviderHost(origin, callback);
 }
 
-scoped_ptr<BackgroundSyncRegistrationHandle>
-BackgroundSyncManager::CreateRegistrationHandle(
-    const scoped_refptr<RefCountedRegistration>& registration) {
-  scoped_refptr<RefCountedRegistration>* ptr =
-      new scoped_refptr<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(
-      weak_ptr_factory_.GetWeakPtr(), handle_id));
-}
-
-BackgroundSyncRegistration* BackgroundSyncManager::GetRegistrationForHandle(
-    BackgroundSyncRegistrationHandle::HandleId handle_id) const {
-  scoped_refptr<RefCountedRegistration>* ref_registration =
-      registration_handle_ids_.Lookup(handle_id);
-  if (!ref_registration)
-    return nullptr;
-  return (*ref_registration)->value();
-}
-
-void BackgroundSyncManager::ReleaseRegistrationHandle(
-    BackgroundSyncRegistrationHandle::HandleId handle_id) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  DCHECK(registration_handle_ids_.Lookup(handle_id));
-  registration_handle_ids_.Remove(handle_id);
-}
-
 void BackgroundSyncManager::GetRegistrationsImpl(
     int64_t sw_registration_id,
     const StatusAndRegistrationsCallback& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
-  scoped_ptr<ScopedVector<BackgroundSyncRegistrationHandle>> out_registrations(
-      new ScopedVector<BackgroundSyncRegistrationHandle>());
+  scoped_ptr<ScopedVector<BackgroundSyncRegistration>> out_registrations(
+      new ScopedVector<BackgroundSyncRegistration>());
 
   if (disabled_) {
     base::ThreadTaskRunnerHandle::Get()->PostTask(
         FROM_HERE, base::Bind(callback, BACKGROUND_SYNC_STATUS_STORAGE_ERROR,
                               base::Passed(std::move(out_registrations))));
     return;
   }
 
   SWIdToRegistrationsMap::iterator it =
       active_registrations_.find(sw_registration_id);
 
   if (it != active_registrations_.end()) {
     const BackgroundSyncRegistrations& registrations = it->second;
     for (const auto& tag_and_registration : registrations.registration_map) {
-      RefCountedRegistration* registration = tag_and_registration.second.get();
-      out_registrations->push_back(
-          CreateRegistrationHandle(registration).release());
+      const BackgroundSyncRegistration& registration =
+          tag_and_registration.second;
+      BackgroundSyncRegistration* out_registration =
+          new BackgroundSyncRegistration(registration);

[iclelland, 2016/03/08 16:48:14]

These are allocated here, but never released in the ServiceImpl as they're being
transferred to the the mojo array.

[jkarlin, 2016/03/08 19:37:36]

That weak_clear had to go, thanks!

+      out_registrations->push_back(out_registration);
     }
   }
 
   base::ThreadTaskRunnerHandle::Get()->PostTask(
       FROM_HERE, base::Bind(callback, BACKGROUND_SYNC_STATUS_OK,
                             base::Passed(std::move(out_registrations))));
 }
 
 bool BackgroundSyncManager::AreOptionConditionsMet(
     const BackgroundSyncRegistrationOptions& options) {
@@ skipping 15 matching lines @@
 }
 
 void BackgroundSyncManager::RunInBackgroundIfNecessary() {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   base::TimeDelta soonest_wakeup_delta = base::TimeDelta::Max();
 
   for (const auto& sw_id_and_registrations : active_registrations_) {
     for (const auto& key_and_registration :
          sw_id_and_registrations.second.registration_map) {
       const BackgroundSyncRegistration& registration =
-          *key_and_registration.second->value();
+          key_and_registration.second;
       if (registration.sync_state() == BackgroundSyncState::PENDING) {
         if (clock_->Now() >= registration.delay_until()) {
           soonest_wakeup_delta = base::TimeDelta();
         } else {
           base::TimeDelta delay_delta =
               registration.delay_until() - clock_->Now();
           if (delay_delta < soonest_wakeup_delta)
             soonest_wakeup_delta = delay_delta;
         }
       }
@@ skipping 45 matching lines @@
     return;
   }
 
   // Find the registrations that are ready to run.
   std::vector<std::pair<int64_t, RegistrationKey>> sw_id_and_keys_to_fire;
 
   for (auto& sw_id_and_registrations : active_registrations_) {
     const int64_t 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->value();
+      BackgroundSyncRegistration* registration = &key_and_registration.second;
       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(BackgroundSyncState::FIRING);
       }
     }
   }
@@ skipping 15 matching lines @@
                  weak_ptr_factory_.GetWeakPtr(), 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_t service_worker_id = sw_id_and_key.first;
-    const RefCountedRegistration* registration =
+    const BackgroundSyncRegistration* registration =
         LookupActiveRegistration(service_worker_id, sw_id_and_key.second);
     DCHECK(registration);
 
     service_worker_context_->FindReadyRegistrationForId(
         service_worker_id, active_registrations_[service_worker_id].origin,
         base::Bind(&BackgroundSyncManager::FireReadyEventsDidFindRegistration,
                    weak_ptr_factory_.GetWeakPtr(), sw_id_and_key.second,
-                   registration->value()->id(), events_fired_barrier_closure,
+                   registration->id(), events_fired_barrier_closure,
                    events_completed_barrier_closure));
   }
 }
 
 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;
   }
 
-  RefCountedRegistration* registration = LookupActiveRegistration(
+  BackgroundSyncRegistration* registration = LookupActiveRegistration(
       service_worker_registration->id(), registration_key);
   DCHECK(registration);
 
-  // Create a handle and keep it until the sync event completes. The client can
-  // acquire its own handle for longer-term use.
-  scoped_ptr<BackgroundSyncRegistrationHandle> registration_handle =
-      CreateRegistrationHandle(registration);
-
   num_firing_registrations_ += 1;
 
-  BackgroundSyncRegistrationHandle::HandleId handle_id =
-      registration_handle->handle_id();
-
   BackgroundSyncEventLastChance last_chance =
-      registration->value()->num_attempts() ==
-              parameters_->max_sync_attempts - 1
+      registration->num_attempts() == parameters_->max_sync_attempts - 1
           ? BackgroundSyncEventLastChance::IS_LAST_CHANCE
           : BackgroundSyncEventLastChance::IS_NOT_LAST_CHANCE;
 
   HasMainFrameProviderHost(
       service_worker_registration->pattern().GetOrigin(),
       base::Bind(&BackgroundSyncMetrics::RecordEventStarted));
 
   DispatchSyncEvent(
-      handle_id, service_worker_registration->active_version(), last_chance,
+      registration->options()->tag,
+      service_worker_registration->active_version(), last_chance,
       base::Bind(&BackgroundSyncManager::EventComplete,
                  weak_ptr_factory_.GetWeakPtr(), service_worker_registration,
-                 service_worker_registration->id(),
-                 base::Passed(std::move(registration_handle)),
+                 service_worker_registration->id(), registration_key,
                  event_completed_callback));
 
   base::ThreadTaskRunnerHandle::Get()->PostTask(
       FROM_HERE, base::Bind(event_fired_callback));
 }
 
 void BackgroundSyncManager::FireReadyEventsAllEventsFiring(
     const base::Closure& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
   RunInBackgroundIfNecessary();
   base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE,
                                                 base::Bind(callback));
 }
 
 // |service_worker_registration| is just to keep the registration alive
 // while the event is firing.
 void BackgroundSyncManager::EventComplete(
     const scoped_refptr<ServiceWorkerRegistration>& service_worker_registration,
     int64_t service_worker_id,
-    scoped_ptr<BackgroundSyncRegistrationHandle> registration_handle,
+    const RegistrationKey& registration_key,
     const base::Closure& callback,
     ServiceWorkerStatusCode status_code) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
   if (disabled_) {
     base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE,
                                                   base::Bind(callback));
     return;
   }
 
   op_scheduler_.ScheduleOperation(base::Bind(
       &BackgroundSyncManager::EventCompleteImpl, weak_ptr_factory_.GetWeakPtr(),
-      service_worker_id, base::Passed(std::move(registration_handle)),
-      status_code, MakeClosureCompletion(callback)));
+      service_worker_id, registration_key, status_code,
+      MakeClosureCompletion(callback)));
 }
 
 void BackgroundSyncManager::EventCompleteImpl(
     int64_t service_worker_id,
-    scoped_ptr<BackgroundSyncRegistrationHandle> registration_handle,
+    const RegistrationKey& registration_key,
     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 =
-      registration_handle->registration();
+      LookupActiveRegistration(service_worker_id, registration_key);
+  if (!registration) {

[iclelland, 2016/03/08 16:48:14]

Is this a failure case?

[jkarlin, 2016/03/08 19:37:36]

This is what happens if the ServiceWorkerRegistration is removed mid-sync. 

+    base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE,
+                                                  base::Bind(callback));
+    return;
+  }
+
   DCHECK(registration);

[iclelland, 2016/03/08 16:48:13]

No longer necessary

[jkarlin, 2016/03/08 19:37:36]

Done.

   DCHECK_NE(BackgroundSyncState::PENDING, registration->sync_state());
 
   registration->set_num_attempts(registration->num_attempts() + 1);
 
   num_firing_registrations_ -= 1;

[iclelland, 2016/03/08 16:48:14]

Since we're not guarding this with the DCHECK any more, this looks like it might
need to be decremented in the "if (!registration)" block as well, to keep the
accounting accurate.

[jkarlin, 2016/03/08 19:37:36]

Good catch, thanks!

 
   // The event ran to completion, we should count it, no matter what happens
   // from here.
   ServiceWorkerRegistration* sw_registration =
       service_worker_context_->GetLiveRegistration(service_worker_id);
   if (sw_registration) {
     HasMainFrameProviderHost(
         sw_registration->pattern().GetOrigin(),
         base::Bind(&BackgroundSyncMetrics::RecordEventResult,
                    status_code == SERVICE_WORKER_OK));
@@ skipping 13 matching lines @@
     registration->set_sync_state(BackgroundSyncState::PENDING);
     registration->set_delay_until(clock_->Now() +
                                   parameters_->initial_retry_delay *
                                       pow(parameters_->retry_delay_factor,
                                           registration->num_attempts() - 1));
     registration_completed = false;
   }
 
   if (registration_completed) {
     RegistrationKey key(*registration);
-    RefCountedRegistration* active_registration =
+    BackgroundSyncRegistration* active_registration =
         LookupActiveRegistration(service_worker_id, key);
     if (active_registration &&
-        active_registration->value()->id() == registration->id()) {
+        active_registration->id() == registration->id()) {
       RemoveActiveRegistration(service_worker_id, key);
     }
   }
 
   StoreRegistrations(
       service_worker_id,
       base::Bind(&BackgroundSyncManager::EventCompleteDidStore,
                  weak_ptr_factory_.GetWeakPtr(), service_worker_id, callback));
 }
 
@@ skipping 75 matching lines @@
                                                       Params... parameters) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
   callback.Run(parameters...);
   op_scheduler_.CompleteOperationAndRunNext();
 }
 
 void BackgroundSyncManager::CompleteStatusAndRegistrationCallback(
     StatusAndRegistrationCallback callback,
     BackgroundSyncStatus status,
-    scoped_ptr<BackgroundSyncRegistrationHandle> registration_handle) {
+    scoped_ptr<BackgroundSyncRegistration> registration) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
-  callback.Run(status, std::move(registration_handle));
+  callback.Run(status, std::move(registration));
   op_scheduler_.CompleteOperationAndRunNext();
 }
 
 void BackgroundSyncManager::CompleteStatusAndRegistrationsCallback(
     StatusAndRegistrationsCallback callback,
     BackgroundSyncStatus status,
-    scoped_ptr<ScopedVector<BackgroundSyncRegistrationHandle>>
-        registration_handles) {
+    scoped_ptr<ScopedVector<BackgroundSyncRegistration>> registration) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
-  callback.Run(status, std::move(registration_handles));
+  callback.Run(status, std::move(registration));
   op_scheduler_.CompleteOperationAndRunNext();
 }
 
 base::Closure BackgroundSyncManager::MakeEmptyCompletion() {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
   return MakeClosureCompletion(base::Bind(base::DoNothing));
 }
 
 base::Closure BackgroundSyncManager::MakeClosureCompletion(
@@ skipping 29 matching lines @@
 BackgroundSyncManager::MakeStatusCompletion(const StatusCallback& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
   return base::Bind(
       &BackgroundSyncManager::CompleteOperationCallback<StatusCallback,
                                                         BackgroundSyncStatus>,
       weak_ptr_factory_.GetWeakPtr(), callback);
 }
 
 }  // namespace content