Refactor GeofencingManager to have one instance per StoragePartition.

content/browser/geofencing/geofencing_manager.h

Index: content/browser/geofencing/geofencing_manager.h
diff --git a/content/browser/geofencing/geofencing_manager.h b/content/browser/geofencing/geofencing_manager.h
index cae5edd3be778c975885070211c0fec0ed68e377..e0d3551b550d2dcf54f14e8bfc4d6c2265d365db 100644
--- a/content/browser/geofencing/geofencing_manager.h
+++ b/content/browser/geofencing/geofencing_manager.h
@@ -11,7 +11,9 @@
 
 #include "base/callback_forward.h"
 #include "base/macros.h"
+#include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
+#include "content/browser/geofencing/geofencing_registration_delegate.h"
 #include "content/common/content_export.h"
 #include "content/common/geofencing_status.h"
 
@@ -25,34 +27,30 @@ struct WebCircularGeofencingRegion;
 
 namespace content {
 
-class BrowserContext;
-class GeofencingProvider;
+class GeofencingServiceInterface;
+class ServiceWorkerContextWrapper;
 
-// This is the main API to the geofencing subsystem. The application will hold
-// a single instance of this class.
+// This is the main API to the geofencing subsystem. There is one instance of
+// this class per storage partition.
 // This class is responsible for keeping track of which geofences are currently
 // registered by websites/workers, persisting this list of registrations and
-// registering a subset of these active registrations with the underlying
-// platform specific |GeofencingProvider| instance.
-// This class lives on the IO thread, and all public methods of it should only
-// ever be called from that same thread.
-// FIXME: Does it make more sense for this to live on the UI thread instead of
-//     the IO thread?
+// registering them with the global |GeofencingService|.
+// This class is created on the UI thread, but all its methods should only be
+// called from the IO thread.
 // TODO(mek): Implement some kind of persistence of registrations.
-// TODO(mek): Limit the number of geofences that are registered with the
-//    underlying GeofencingProvider.
-class CONTENT_EXPORT GeofencingManager {
+class CONTENT_EXPORT GeofencingManager
+    : NON_EXPORTED_BASE(public GeofencingRegistrationDelegate),
+      public base::RefCountedThreadSafe<GeofencingManager> {
  public:
   typedef base::Callback<void(GeofencingStatus)> StatusCallback;
-  typedef base::Callback<void(
-      GeofencingStatus,
-      const std::map<std::string, blink::WebCircularGeofencingRegion>&)>
-      RegistrationsCallback;
 
-  // Gets a pointer to the singleton instance of the geofencing manager. This
-  // must only be called on the IO thread so that the GeofencingManager is
-  // always instantiated on the same thread. Ownership is NOT returned.
-  static GeofencingManager* GetInstance();
+  explicit GeofencingManager(
+      const scoped_refptr<ServiceWorkerContextWrapper>& service_worker_context);
+
+  // Init and Shutdown are for use on the UI thread when the storagepartition is
+  // being setup and torn down.
+  void Init();
+  void Shutdown();
 
   // Initiates registration of a new geofence. StatusCallback is called when
   // registration has completed or failed (which could possibly be before
@@ -61,9 +59,7 @@ class CONTENT_EXPORT GeofencingManager {
   // (or in progress of being registered) region will fail.
   // TODO(mek): Behavior when using an already used ID might need to be revised
   //     depending on what the actual spec ends up saying about this.
-  void RegisterRegion(BrowserContext* browser_context,
-                      int64 service_worker_registration_id,
-                      const GURL& service_worker_origin,
+  void RegisterRegion(int64 service_worker_registration_id,
                       const std::string& region_id,
                       const blink::WebCircularGeofencingRegion& region,
                       const StatusCallback& callback);
@@ -74,9 +70,7 @@ class CONTENT_EXPORT GeofencingManager {
   // (RegisterRegion hasn't called its callback yet) will fail.
   // TODO(mek): Maybe better behavior would be to allow unregistering still
   //     in-progress registrations.
-  void UnregisterRegion(BrowserContext* browser_context,
-                        int64 service_worker_registration_id,
-                        const GURL& service_worker_origin,
+  void UnregisterRegion(int64 service_worker_registration_id,
                         const std::string& region_id,
                         const StatusCallback& callback);
 
@@ -87,51 +81,64 @@ class CONTENT_EXPORT GeofencingManager {
   // has been called already (so it doesn't include still in progress
   // registrations).
   GeofencingStatus GetRegisteredRegions(
-      BrowserContext* browser_context,
       int64 service_worker_registration_id,
-      const GURL& service_worker_origin,
-      std::map<std::string, blink::WebCircularGeofencingRegion>* regions);
+      std::map<std::string, blink::WebCircularGeofencingRegion>* result);
 
-  void SetProviderForTests(scoped_ptr<GeofencingProvider> provider);
+  void SetServiceForTests(GeofencingServiceInterface* service) {

[Michael van Ouwerkerk, 2014/10/21 11:46:41]

nit: ForTesting

[Marijn Kruisselbrink, 2014/10/21 23:50:07]

Done.

+    service_ = service;
+  }
 
  protected:
-  friend struct DefaultSingletonTraits<GeofencingManager>;
-  friend class GeofencingManagerTest;
-  GeofencingManager();
+  friend class base::RefCountedThreadSafe<GeofencingManager>;
   virtual ~GeofencingManager();
 
  private:
   // Internal bookkeeping associated with each registered geofence.
-  struct RegistrationKey;
   struct Registration;
-  class RegistrationMatches;
 
-  // Called by GeofencingProvider when the platform specific provider completes
-  // registration of a geofence.
-  void RegisterRegionCompleted(const StatusCallback& callback,
-                               const RegistrationKey& key,
-                               GeofencingStatus status,
-                               int registration_id);
+  void InitOnIO();
+  void ShutdownOnIO();
+
+  // GeofencingRegistrationDelegate implementation.
+  virtual void RegistrationFinished(int64 geofencing_registration_id,
+                                    GeofencingStatus status) override;
 
   // Looks up a particular geofence registration. Returns nullptr if no
   // registration with the given IDs exists.
-  Registration* FindRegistration(const RegistrationKey& key);
+  Registration* FindRegistration(int64 service_worker_registration_id,
+                                 const std::string& region_id);
+
+  // Looks up a particular geofence registration. Returns nullptr if no
+  // registration with the given ID exists.
+  Registration* FindRegistrationById(int64 geofencing_registration_id);
 
   // Registers a new registration, returning a reference to the newly inserted
   // object. Assumes no registration with the same IDs currently exists.
   Registration& AddRegistration(
-      const RegistrationKey& key,
-      const blink::WebCircularGeofencingRegion& region);
+      int64 service_worker_registration_id,
+      const std::string& region_id,
+      const blink::WebCircularGeofencingRegion& region,
+      const StatusCallback& callback,
+      int64 geofencing_registration_id);
 
   // Clears a registration.
-  void ClearRegistration(const RegistrationKey& key);
-
-  // List of all currently registered geofences.
-  // TODO(mek): Better way of storing these that allows more efficient lookup
-  //     and deletion.
-  std::vector<Registration> registrations_;
-
-  scoped_ptr<GeofencingProvider> provider_;
+  void ClearRegistration(Registration* registration);
+
+  // Map of all registered regions for a particular service worker registration.
+  typedef std::map<std::string, Registration> RegionIdRegistrationMap;
+  // Map of service worker registration id to the regions registered by that
+  // service worker.
+  typedef std::map<int64, RegionIdRegistrationMap>
+      ServiceWorkerRegistrationsMap;
+  ServiceWorkerRegistrationsMap registrations_;
+
+  // Map of all registered regions by geofencing_registration_id.
+  typedef std::map<int64, RegionIdRegistrationMap::iterator>
+      RegistrationIdRegistrationMap;
+  RegistrationIdRegistrationMap registrations_by_id_;
+
+  GeofencingServiceInterface* service_;
+  scoped_refptr<ServiceWorkerContextWrapper> service_worker_context_;
 
   DISALLOW_COPY_AND_ASSIGN(GeofencingManager);
 };