BackgroundSyncManager tracks client registrations

content/browser/background_sync/background_sync_service_impl.h

Index: content/browser/background_sync/background_sync_service_impl.h
diff --git a/content/browser/background_sync/background_sync_service_impl.h b/content/browser/background_sync/background_sync_service_impl.h
index 4f8ab3d4580b2638e8c7d8464d213eb655c5ef34..f890cca4bf085818db4cf6c1ecd045b412e3df11 100644
--- a/content/browser/background_sync/background_sync_service_impl.h
+++ b/content/browser/background_sync/background_sync_service_impl.h
@@ -5,9 +5,9 @@
 #ifndef CONTENT_BROWSER_BACKGROUND_SYNC_BACKGROUND_SYNC_SERVICE_IMPL_H_
 #define CONTENT_BROWSER_BACKGROUND_SYNC_BACKGROUND_SYNC_SERVICE_IMPL_H_
 
-#include <vector>
-
+#include "base/id_map.h"
 #include "base/memory/ref_counted.h"
+#include "base/memory/scoped_vector.h"
 #include "content/browser/background_sync/background_sync_manager.h"
 #include "content/common/background_sync_service.mojom.h"
 #include "third_party/mojo/src/mojo/public/cpp/bindings/binding.h"
@@ -28,14 +28,12 @@ class CONTENT_EXPORT BackgroundSyncServiceImpl
  private:
   friend class BackgroundSyncServiceImplTest;
 
-
   // BackgroundSyncService methods:
   void Register(content::SyncRegistrationPtr options,
                 int64_t sw_registration_id,
                 const RegisterCallback& callback) override;
   void Unregister(BackgroundSyncPeriodicity periodicity,
                   int64_t id,
-                  const mojo::String& tag,
                   int64_t sw_registration_id,
                   const UnregisterCallback& callback) override;
   void GetRegistration(BackgroundSyncPeriodicity periodicity,
@@ -49,16 +47,18 @@ class CONTENT_EXPORT BackgroundSyncServiceImpl
       BackgroundSyncPeriodicity periodicity,
       int64_t sw_registration_id,
       const GetPermissionStatusCallback& callback) override;
+  void TrackRegistration(SyncRegistrationPtr sync_registration) override;
+  void ReleaseRegistration(int64_t sync_id) override;
 
   void OnRegisterResult(const RegisterCallback& callback,
                         BackgroundSyncStatus status,
-                        const BackgroundSyncRegistration& result);
+                        scoped_ptr<BackgroundSyncRegistrationHandle> result);
   void OnUnregisterResult(const UnregisterCallback& callback,
                           BackgroundSyncStatus status);
   void OnGetRegistrationsResult(
       const GetRegistrationsCallback& callback,
       BackgroundSyncStatus status,
-      const std::vector<BackgroundSyncRegistration>& result);
+      scoped_ptr<ScopedVector<BackgroundSyncRegistrationHandle>> result);
 
   // Called when an error is detected on binding_.
   void OnConnectionError();

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

I had a quick chat with rockot about how to manage handle ownership such that
they're never leaked and are reliably present when they should be. 
OnConnectionError() really helps. If a message fails to be delivered to the
remote-side, OnConnectionError() will fire. For example, when a worker thread
terminates and closes its side of the intf. I think that makes it easy to manage
the lifetime of handles attributed to this client-side of this intf ptr
(provided the handles aren't supposed to outlive the intfptr?)

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

Right. If the renderer dies this gets called (which leads to this object's
deletion) and all of the renderer's handles are released. 

@@ -68,6 +68,10 @@ class CONTENT_EXPORT BackgroundSyncServiceImpl
 
   mojo::Binding<BackgroundSyncService> binding_;
 
+  // The registrations that the client might reference.
+  IDMap<BackgroundSyncRegistrationHandle, IDMapOwnPointer>
+      hosted_registrations_;

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

The terms 'host' and 'registration' are both somewhat overloaded in our code
base. Now that you're using the 'handle' term, you can proably rename some
methods and members to better distinguish between where a handle is being used
vs an actual registration.

// Collection of handles owned by this instance. For each entry in the map,
// the handle id is either in transit destined for the client-side, or held in
// an object on the client-side. When this instance dies, these handles are
// freed so we don't leak references to sync registrations.
Map handles_in_use_;

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

Refactored to active_handles_.

Not sure I want to change the mojo function names from registration to handle
however, as the type that's returned is a mojo registration.

+
   base::WeakPtrFactory<BackgroundSyncServiceImpl> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(BackgroundSyncServiceImpl);