BackgroundSyncManager tracks client registrations

content/browser/background_sync/background_sync_manager.h

Index: content/browser/background_sync/background_sync_manager.h
diff --git a/content/browser/background_sync/background_sync_manager.h b/content/browser/background_sync/background_sync_manager.h
index 84353f906328115ca2abc077f8c52b52aaf39be4..1567b7b3e476462f97601ad50ff2f2eeb64a8a15 100644
--- a/content/browser/background_sync/background_sync_manager.h
+++ b/content/browser/background_sync/background_sync_manager.h
@@ -15,6 +15,7 @@
 #include "base/memory/weak_ptr.h"
 #include "content/browser/background_sync/background_sync.pb.h"
 #include "content/browser/background_sync/background_sync_registration.h"
+#include "content/browser/background_sync/background_sync_registration_handle.h"
 #include "content/browser/background_sync/background_sync_status.h"
 #include "content/browser/cache_storage/cache_storage_scheduler.h"
 #include "content/browser/service_worker/service_worker_context_observer.h"
@@ -50,10 +51,10 @@ class CONTENT_EXPORT BackgroundSyncManager
   using StatusCallback = base::Callback<void(BackgroundSyncStatus)>;
   using StatusAndRegistrationCallback =
       base::Callback<void(BackgroundSyncStatus,
-                          const BackgroundSyncRegistration&)>;
-  using StatusAndRegistrationsCallback =
-      base::Callback<void(BackgroundSyncStatus,
-                          const std::vector<BackgroundSyncRegistration>&)>;
+                          scoped_ptr<BackgroundSyncRegistrationHandle>)>;
+  using StatusAndRegistrationsCallback = base::Callback<void(
+      BackgroundSyncStatus,
+      scoped_ptr<ScopedVector<BackgroundSyncRegistrationHandle>>)>;
 
   static scoped_ptr<BackgroundSyncManager> Create(
       const scoped_refptr<ServiceWorkerContextWrapper>& service_worker_context);
@@ -70,17 +71,6 @@ class CONTENT_EXPORT BackgroundSyncManager
                 const BackgroundSyncRegistrationOptions& options,
                 const StatusAndRegistrationCallback& callback);
 
-  // Removes the background sync with tag |sync_registration_tag|, periodicity
-  // |periodicity|, and id |sync_registration_id|. Calls |callback| with
-  // BACKGROUND_SYNC_STATUS_NOT_FOUND if no match is found. Calls |callback|
-  // with BACKGROUND_SYNC_STATUS_OK on success.
-  void Unregister(
-      int64 sw_registration_id,
-      const std::string& sync_registration_tag,
-      SyncPeriodicity periodicity,
-      BackgroundSyncRegistration::RegistrationId sync_registration_id,
-      const StatusCallback& callback);
-
   // Finds the background sync registration associated with
   // |sw_registration_id| with periodicity |periodicity|. Calls
   // |callback| with BACKGROUND_SYNC_STATUS_NOT_FOUND if it doesn't exist. Calls
@@ -100,6 +90,25 @@ class CONTENT_EXPORT BackgroundSyncManager
   void OnStorageWiped() override;
 
  protected:
+  // A registration might be referenced by the client longer than
+  // the BackgroundSyncManager needs to keep track of it (e.g., the event has
+  // finished firing). The BackgroundSyncManager reference counts its
+  // registrations internally and every BackgroundSyncRegistrationHandle has a
+  // unique handle id which maps to a locally maintained (in
+  // client_registration_ids_) scoped_refptr.
+  class RefCountedRegistration
+      : public base::RefCounted<RefCountedRegistration> {
+   public:
+    BackgroundSyncRegistration* value() { return &registration_; }
+    const BackgroundSyncRegistration* value() const { return &registration_; }
+
+   private:
+    friend class base::RefCounted<RefCountedRegistration>;
+    virtual ~RefCountedRegistration() = default;
+
+    BackgroundSyncRegistration registration_;
+  };
+
   explicit BackgroundSyncManager(
       const scoped_refptr<ServiceWorkerContextWrapper>& context);
 
@@ -118,11 +127,13 @@ class CONTENT_EXPORT BackgroundSyncManager
       const ServiceWorkerStorage::GetUserDataForAllRegistrationsCallback&
           callback);
   virtual void FireOneShotSync(
-      const BackgroundSyncRegistration& registration,
+      const RefCountedRegistration& registration,
       const scoped_refptr<ServiceWorkerVersion>& active_version,
       const ServiceWorkerVersion::StatusCallback& callback);
 
  private:
+  friend class BackgroundSyncRegistrationHandle;
+
   class RegistrationKey {
    public:
     explicit RegistrationKey(const BackgroundSyncRegistration& registration);
@@ -141,7 +152,7 @@ class CONTENT_EXPORT BackgroundSyncManager
 
   struct BackgroundSyncRegistrations {
     using RegistrationMap =
-        std::map<RegistrationKey, BackgroundSyncRegistration>;
+        std::map<RegistrationKey, scoped_refptr<RefCountedRegistration>>;
 
     BackgroundSyncRegistrations();
     ~BackgroundSyncRegistrations();
@@ -154,6 +165,18 @@ class CONTENT_EXPORT BackgroundSyncManager
   using PermissionStatusCallback = base::Callback<void(bool)>;
   using SWIdToRegistrationsMap = std::map<int64, BackgroundSyncRegistrations>;

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

hmmm... what if...


// Used to dispatch the sync event, bound to the event completion callback.
// At dtor time, will release the underlying reg if it hasn't been adpoted.
// Or if it's been adopted, dtor does nothing.
scoped_ptr<AdoptableBgSyncRegHandle> CreateAdoptableRegistrationHandle(reg);

// Called by the client-side when processing the event. Returns a normal
// handle having the same id, or nullptr is there is no adpotable handle
// with that id.
scoped_ptr<BgSyncRegHandle> AdoptRegistrationHandle(handle_id);

// Just like you have now, used for responding to mojo intf method calls.
scoped_ptr<BgSyncRegHandle> CreatedRegistrationHandle(reg);

// Just like you have now, only mucks with the nonadoptable handles.
// Called by ~BgSyncRegHandle
void ReleaesRegistrationHandle(handle_id)

// Called by ~AdoptableBgSyncRegHandle
void ReleaesAdoptableRegistrationHandle(handle_id)


IDMap<scoped_refptr<const RefCountedRegistration>, IDMapOwnPointer>
    registration_handle_ids_;

IDMap<scoped_refptr<const RefCountedRegistration>, IDMapOwnPointer>
    adpotable_registration_handle_ids_;

SWIdToRegistrationsMap sw_to_registrations_map_;


or something like that?

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

I solved the problem without the need for adoptable handles. I used two handles
and a duplicate method.

 
+  scoped_ptr<BackgroundSyncRegistrationHandle> CreateRegistrationHandle(
+      const RefCountedRegistration& registration);
+
+  // Returns the BackgroundSyncRegistration corresponding to |handle_id|.
+  // Returns nullptr if the registration is not found.
+  const BackgroundSyncRegistration* GetRegistrationForHandle(
+      BackgroundSyncRegistrationHandle::HandleId handle_id) const;
+
+  // The BackgroundSyncManager holds references to registrations that have
+  // active Handles. The handles must call this on destruction.
+  void ReleaseHandleId(BackgroundSyncRegistrationHandle::HandleId handle_id);
+
   // Disable the manager. Already queued operations will abort once they start
   // to run (in their impl methods). Future operations will not queue. Any
   // registrations are cleared from memory and the backend (if it's still
@@ -169,7 +192,7 @@ class CONTENT_EXPORT BackgroundSyncManager
 
   // Returns the existing registration in |existing_registration| if it is not
   // null.
-  BackgroundSyncRegistration* LookupRegistration(
+  RefCountedRegistration* LookupRegistration(
       int64 sw_registration_id,
       const RegistrationKey& registration_key);
 
@@ -184,7 +207,7 @@ class CONTENT_EXPORT BackgroundSyncManager
   void AddRegistrationToMap(
       int64 sw_registration_id,
       const GURL& origin,
-      const BackgroundSyncRegistration& sync_registration);
+      const scoped_refptr<RefCountedRegistration>& sync_registration);
 
   void InitImpl(const base::Closure& callback);
   void InitDidGetDataFromBackend(
@@ -196,18 +219,24 @@ class CONTENT_EXPORT BackgroundSyncManager
   void RegisterImpl(int64 sw_registration_id,
                     const BackgroundSyncRegistrationOptions& options,
                     const StatusAndRegistrationCallback& callback);
-  void RegisterDidStore(int64 sw_registration_id,
-                        const BackgroundSyncRegistration& sync_registration,
-                        const StatusAndRegistrationCallback& callback,
-                        ServiceWorkerStatusCode status);
-
-  // Unregister callbacks
-  void UnregisterImpl(
+  void RegisterDidStore(
       int64 sw_registration_id,
-      const RegistrationKey& registration_key,
-      BackgroundSyncRegistration::RegistrationId sync_registration_id,
-      SyncPeriodicity periodicity,
-      const StatusCallback& callback);
+      const scoped_refptr<RefCountedRegistration>& new_registration_ref,
+      const StatusAndRegistrationCallback& callback,
+      ServiceWorkerStatusCode status);
+
+  // Removes the background sync with periodicity |periodicity| and id
+  // |sync_registration_id|. Calls |callback| with
+  // BACKGROUND_SYNC_STATUS_NOT_FOUND if no match is found. Calls |callback|
+  // with BACKGROUND_SYNC_STATUS_OK on success.
+  void Unregister(int64 sw_registration_id,
+                  SyncPeriodicity periodicity,
+                  BackgroundSyncRegistrationHandle::HandleId handle_id,
+                  const StatusCallback& callback);
+  void UnregisterImpl(int64 sw_registration_id,
+                      SyncPeriodicity periodicity,
+                      BackgroundSyncRegistrationHandle::HandleId handle_id,
+                      const StatusCallback& callback);
   void UnregisterDidStore(int64 sw_registration_id,
                           SyncPeriodicity periodicity,
                           const StatusCallback& callback,
@@ -282,6 +311,14 @@ class CONTENT_EXPORT BackgroundSyncManager
   template <typename CallbackT, typename... Params>
   void CompleteOperationCallback(const CallbackT& callback,
                                  Params... parameters);
+  void CompleteStatusAndRegistrationCallback(
+      StatusAndRegistrationCallback callback,
+      BackgroundSyncStatus status,
+      scoped_ptr<BackgroundSyncRegistrationHandle> result);
+  void CompleteStatusAndRegistrationsCallback(
+      StatusAndRegistrationsCallback callback,
+      BackgroundSyncStatus status,
+      scoped_ptr<ScopedVector<BackgroundSyncRegistrationHandle>> results);
   base::Closure MakeEmptyCompletion();
   base::Closure MakeClosureCompletion(const base::Closure& callback);
   StatusAndRegistrationCallback MakeStatusAndRegistrationCompletion(
@@ -299,6 +336,10 @@ class CONTENT_EXPORT BackgroundSyncManager
   scoped_ptr<BackgroundSyncNetworkObserver> network_observer_;
   scoped_ptr<BackgroundSyncPowerObserver> power_observer_;
 
+  // The registrations that clients have handles to.
+  IDMap<scoped_refptr<const RefCountedRegistration>, IDMapOwnPointer>
+      registration_handle_ids_;
+
   base::WeakPtrFactory<BackgroundSyncManager> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(BackgroundSyncManager);