Refactor GeofencingManager to have one instance per StoragePartition.

content/browser/geofencing/geofencing_service.cc

Index: content/browser/geofencing/geofencing_service.cc
diff --git a/content/browser/geofencing/geofencing_service.cc b/content/browser/geofencing/geofencing_service.cc
new file mode 100644
index 0000000000000000000000000000000000000000..4d66c173dae8190bb27da79e97fde020d82d25b9
--- /dev/null
+++ b/content/browser/geofencing/geofencing_service.cc
@@ -0,0 +1,199 @@
+// Copyright 2014 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/geofencing/geofencing_service.h"
+
+#include "base/memory/singleton.h"
+#include "base/message_loop/message_loop.h"
+#include "content/browser/geofencing/geofencing_provider.h"
+#include "content/browser/geofencing/geofencing_registration_delegate.h"
+#include "content/public/browser/browser_thread.h"
+#include "third_party/WebKit/public/platform/WebCircularGeofencingRegion.h"
+
+namespace content {
+
+namespace {
+
+void RunSoon(const base::Closure& callback) {
+  if (!callback.is_null())
+    base::MessageLoop::current()->PostTask(FROM_HERE, callback);
+}
+
+}  // namespace
+
+struct GeofencingService::Registration {
+  Registration();
+  Registration(const blink::WebCircularGeofencingRegion& region,
+               int64 geofencing_registration_id,
+               GeofencingRegistrationDelegate* delegate);
+
+  blink::WebCircularGeofencingRegion region;
+  int64 geofencing_registration_id;
+  GeofencingRegistrationDelegate* delegate;
+
+  enum RegistrationState {
+    // In progress of being registered with provider.
+    STATE_REGISTERING,
+    // Currently registered with provider.
+    STATE_REGISTERED,
+    // In progress of being registered with provider, but should be unregistered
+    // and deleted.
+    STATE_SHOULD_UNREGISTER_AND_DELETE,
+    // Not currently registered with provider, but still an active registration.
+    STATE_UNREGISTERED
+  };
+  RegistrationState state;
+};
+
+GeofencingService::Registration::Registration()
+    : geofencing_registration_id(-1),
+      delegate(nullptr),
+      state(STATE_UNREGISTERED) {
+}
+
+GeofencingService::Registration::Registration(
+    const blink::WebCircularGeofencingRegion& region,
+    int64 geofencing_registration_id,
+    GeofencingRegistrationDelegate* delegate)
+    : region(region),
+      geofencing_registration_id(geofencing_registration_id),
+      delegate(delegate),
+      state(STATE_REGISTERING) {
+}
+
+GeofencingService::GeofencingService() : next_registration_id_(0) {
+}
+
+GeofencingService::~GeofencingService() {
+}
+
+GeofencingService* GeofencingService::GetInstance() {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  return Singleton<GeofencingService>::get();
+}
+
+bool GeofencingService::IsServiceAvailable() {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  return EnsureProvider();
+}
+
+int64 GeofencingService::RegisterRegion(
+    const blink::WebCircularGeofencingRegion& region,
+    GeofencingRegistrationDelegate* delegate) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+
+  int64 geofencing_registration_id = GetNextId();
+  registrations_[geofencing_registration_id] =
+      Registration(region, geofencing_registration_id, delegate);
+
+  if (!EnsureProvider()) {
+    RunSoon(
+        base::Bind(&GeofencingService::NotifyRegistrationFinished,
+                   base::Unretained(this),
+                   geofencing_registration_id,
+                   GEOFENCING_STATUS_OPERATION_FAILED_SERVICE_NOT_AVAILABLE));
+    return geofencing_registration_id;

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

Minting a new id seems pointless if the operation failed. How about returning a
well known invalid id and not adding a new registration to the map?

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

I'd rather not handle different failure cases differently, requiring two
separate code paths for code calling RegisterRegion to handle failures, etc.
Having said that, I could get rid of this part and just DCHECK or something
instead. GeofencingManager already covers the same error condition this is
trying to check for, and there are unit tests to make sure of that, so that
might be a nice way to get rid of the awkwardness of minting a new ID in this
case.

+  }
+
+  provider_->RegisterRegion(
+      geofencing_registration_id,
+      region,
+      base::Bind(&GeofencingService::NotifyRegistrationFinished,
+                 base::Unretained(this),
+                 geofencing_registration_id));
+  return geofencing_registration_id;
+}
+
+void GeofencingService::UnregisterRegion(int64 geofencing_registration_id) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+
+  RegistrationsMap::iterator registration_iterator =
+      registrations_.find(geofencing_registration_id);
+  DCHECK(registration_iterator != registrations_.end());
+
+  if (!EnsureProvider())
+    return;
+
+  switch (registration_iterator->second.state) {
+    case Registration::STATE_REGISTERED:
+      provider_->UnregisterRegion(geofencing_registration_id);
+    // fallthru
+    case Registration::STATE_UNREGISTERED:
+      registrations_.erase(registration_iterator);
+      break;
+    case Registration::STATE_REGISTERING:
+      // Update state, NotifyRegistrationFinished will take care of actually
+      // unregistering.
+      registration_iterator->second.state =
+          Registration::STATE_SHOULD_UNREGISTER_AND_DELETE;
+      break;
+    case Registration::STATE_SHOULD_UNREGISTER_AND_DELETE:
+      // Should not happen.
+      NOTREACHED();
+      break;
+  }
+}
+
+void GeofencingService::SetProviderForTests(
+    scoped_ptr<GeofencingProvider> provider) {
+  DCHECK(!provider_.get());
+  provider_ = provider.Pass();
+}
+
+int GeofencingService::RegistrationCountForTests() {
+  return registrations_.size();
+}
+
+bool GeofencingService::EnsureProvider() {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+
+  if (!provider_) {
+    // TODO(mek): Create platform specific provider.
+    return false;
+  }
+  return true;
+}
+
+int64 GeofencingService::GetNextId() {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+
+  return next_registration_id_++;
+}
+
+void GeofencingService::NotifyRegistrationFinished(
+    int64 geofencing_registration_id,
+    GeofencingStatus status) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+
+  RegistrationsMap::iterator registration_iterator =
+      registrations_.find(geofencing_registration_id);
+  DCHECK(registration_iterator != registrations_.end());
+  DCHECK(registration_iterator->second.state ==
+             Registration::STATE_REGISTERING ||
+         registration_iterator->second.state ==
+             Registration::STATE_SHOULD_UNREGISTER_AND_DELETE);
+
+  if (registration_iterator->second.state ==
+      Registration::STATE_SHOULD_UNREGISTER_AND_DELETE) {
+    // Don't call delegate, but unregister with provider if registration was
+    // succesfull.
+    if (status == GEOFENCING_STATUS_OK) {
+      provider_->UnregisterRegion(geofencing_registration_id);
+    }
+    registrations_.erase(registration_iterator);
+    return;
+  }
+
+  // Normal case, mark as registered and call delegate.
+  registration_iterator->second.state = Registration::STATE_REGISTERED;
+  registration_iterator->second.delegate->RegistrationFinished(
+      geofencing_registration_id, status);
+
+  if (status != GEOFENCING_STATUS_OK) {
+    // Registration failed, remove from our book-keeping.
+    registrations_.erase(registration_iterator);
+  }
+}
+
+}  // namespace content