BackgroundSyncManager tracks client registrations

content/browser/background_sync/background_sync_service_impl.cc

Index: content/browser/background_sync/background_sync_service_impl.cc
diff --git a/content/browser/background_sync/background_sync_service_impl.cc b/content/browser/background_sync/background_sync_service_impl.cc
index 7c9507f685a8ee3188ec7d2262d93949c20e2ed8..fa2b8e3df01ecb87fdf45613f0c8dc4958d9e714 100644
--- a/content/browser/background_sync/background_sync_service_impl.cc
+++ b/content/browser/background_sync/background_sync_service_impl.cc
@@ -5,8 +5,9 @@
 #include "content/browser/background_sync/background_sync_service_impl.h"
 
 #include "base/memory/weak_ptr.h"
+#include "base/stl_util.h"
 #include "content/browser/background_sync/background_sync_context_impl.h"
-#include "content/browser/background_sync/background_sync_registration.h"
+#include "content/browser/background_sync/client_background_sync_registration.h"
 #include "content/public/browser/browser_thread.h"
 
 namespace content {
@@ -88,6 +89,9 @@ COMPILE_ASSERT_MATCHING_ENUM(BACKGROUND_SYNC_PERIODICITY_MAX,
 
 BackgroundSyncServiceImpl::~BackgroundSyncServiceImpl() {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  BackgroundSyncManager* background_sync_manager =
+      background_sync_context_->background_sync_manager();
+  DCHECK(background_sync_manager);

[iclelland, 2015/08/14 14:44:19]

What's the purpose of this check on destruction?

[jkarlin, 2015/08/17 17:14:49]

Just to verify that services delete before the manager.

 }
 
 BackgroundSyncServiceImpl::BackgroundSyncServiceImpl(
@@ -129,15 +133,22 @@ void BackgroundSyncServiceImpl::Register(content::SyncRegistrationPtr options,
 void BackgroundSyncServiceImpl::Unregister(
     BackgroundSyncPeriodicity periodicity,
     int64_t id,
-    const mojo::String& tag,
     int64_t sw_registration_id,
     const UnregisterCallback& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   BackgroundSyncManager* background_sync_manager =
       background_sync_context_->background_sync_manager();
   DCHECK(background_sync_manager);
-  background_sync_manager->Unregister(
-      sw_registration_id, tag, content::SYNC_ONE_SHOT, id,
+
+  ClientBackgroundSyncRegistration* registration =
+      hosted_registrations_.Lookup(id);
+  if (!registration) {
+    callback.Run(BACKGROUND_SYNC_ERROR_NOT_ALLOWED);
+    return;
+  }
+
+  registration->Unregister(
+      sw_registration_id,
       base::Bind(&BackgroundSyncServiceImpl::OnUnregisterResult,
                  weak_ptr_factory_.GetWeakPtr(), callback));
 }
@@ -183,12 +194,57 @@ void BackgroundSyncServiceImpl::GetPermissionStatus(
   callback.Run(BACKGROUND_SYNC_ERROR_NONE, PERMISSION_STATUS_GRANTED);
 }
 
+void BackgroundSyncServiceImpl::TrackRegistration(
+    SyncRegistrationPtr sync_registration) {
+  // Registrations that the client acquires without the
+  // BackgroundSyncServiceImpl's knowing are registered here.
+  BackgroundSyncManager* background_sync_manager =
+      background_sync_context_->background_sync_manager();
+  DCHECK(background_sync_manager);
+
+  BackgroundSyncRegistrationOptions options =
+      ToBackgroundSyncRegistrationOptions(sync_registration);
+
+  scoped_ptr<ClientBackgroundSyncRegistration> registration(
+      new ClientBackgroundSyncRegistration(background_sync_manager));
+  *registration->options() = options;
+  registration->set_id(sync_registration->id);
+
+  if (hosted_registrations_.Lookup(sync_registration->id)) {
+    // TODO(jkarlin): Abort client.
+    LOG(WARNING) << "Client attempted to track existing registration";
+    return;
+  }
+
+  hosted_registrations_.AddWithID(registration.release(),
+                                  sync_registration->id);
+}
+
+void BackgroundSyncServiceImpl::ReleaseRegistration(int64_t sync_id) {
+  if (!hosted_registrations_.Lookup(sync_id)) {
+    // TODO(jkarlin): Abort client.

[iclelland, 2015/08/14 14:44:18]

As in BackgroundSyncManager -- what happens if we don't abort? Is anything
inconsistent?

[jkarlin, 2015/08/17 17:14:49]

We should abort long term :) In the meanwhile, a client has a registration that
can't be used.

[iclelland, 2015/08/17 19:22:02]

As long as it's not a browser-destablising situation, then I'm okay with it :)

+    LOG(WARNING) << "Client attempted to release non-existing registration";
+    return;
+  }
+
+  hosted_registrations_.Remove(sync_id);
+}
+
 void BackgroundSyncServiceImpl::OnRegisterResult(
     const RegisterCallback& callback,
     BackgroundSyncStatus status,
-    const BackgroundSyncRegistration& result) {
+    scoped_ptr<ClientBackgroundSyncRegistration> result) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  SyncRegistrationPtr mojoResult = ToMojoRegistration(result);
+  ClientBackgroundSyncRegistration* result_ptr = result.get();
+
+  if (status != BACKGROUND_SYNC_STATUS_OK) {
+    callback.Run(static_cast<content::BackgroundSyncError>(status),
+                 SyncRegistrationPtr(content::SyncRegistration::New()));
+    return;
+  }
+
+  hosted_registrations_.AddWithID(result.release(), result_ptr->id());
+  SyncRegistrationPtr mojoResult = ToMojoRegistration(*result_ptr);
   callback.Run(static_cast<content::BackgroundSyncError>(status),
                mojoResult.Pass());
 }
@@ -203,11 +259,17 @@ void BackgroundSyncServiceImpl::OnUnregisterResult(
 void BackgroundSyncServiceImpl::OnGetRegistrationsResult(
     const GetRegistrationsCallback& callback,
     BackgroundSyncStatus status,
-    const std::vector<BackgroundSyncRegistration>& result_registrations) {
+    scoped_ptr<ScopedVector<ClientBackgroundSyncRegistration>>
+        result_registrations) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   mojo::Array<content::SyncRegistrationPtr> mojo_registrations(0);
-  for (const auto& registration : result_registrations)
-    mojo_registrations.push_back(ToMojoRegistration(registration));
+  for (ClientBackgroundSyncRegistration* registration : *result_registrations) {
+    hosted_registrations_.AddWithID(registration, registration->id());
+    mojo_registrations.push_back(ToMojoRegistration(*registration));
+  }
+
+  result_registrations->weak_clear();
+
   callback.Run(static_cast<content::BackgroundSyncError>(status),
                mojo_registrations.Pass());
 }