Notify that pending registrations are done when BackgroundSyncManager is disabled.

Registrations that are in the middle of firing when disabling occurs are allowed to complete (so that NotifyWhenDone has the accurate result).

BUG=536875

Review URL: https://codereview.chromium.org/1371233002

Cr-Commit-Position: refs/heads/master@{#352425}

5 files changed, 105 insertions, 52 deletions

diff --git a/content/browser/background_sync/background_sync_manager.cc b/content/browser/background_sync/background_sync_manager.cc
index 477b31f..9461de7 100644
--- a/content/browser/background_sync/background_sync_manager.cc
+++ b/content/browser/background_sync/background_sync_manager.cc
@@ -407,22 +407,7 @@ void BackgroundSyncManager::RegisterImpl(
                 CreateRegistrationHandle(existing_registration_ref).Pass())));
     return;
     } else {
-      DCHECK(!existing_registration_ref->value()->HasCompleted());
-      bool firing = existing_registration_ref->value()->sync_state() ==
-                        BACKGROUND_SYNC_STATE_FIRING ||
-                    existing_registration_ref->value()->sync_state() ==
-                        BACKGROUND_SYNC_STATE_UNREGISTERED_WHILE_FIRING;
-
-      existing_registration_ref->value()->set_sync_state(
-          firing ? BACKGROUND_SYNC_STATE_UNREGISTERED_WHILE_FIRING
-                 : BACKGROUND_SYNC_STATE_UNREGISTERED);
-
-      if (!firing) {
-        // If the registration is currently firing then wait to run
-        // RunDoneCallbacks until after it has finished as it might
-        // change state to SUCCESS first.
-        existing_registration_ref->value()->RunDoneCallbacks();
-      }
+      existing_registration_ref->value()->SetUnregisteredState();
     }
   }
 
@@ -458,6 +443,16 @@ void BackgroundSyncManager::DisableAndClearManager(
   }
 
   disabled_ = true;
+
+  for (auto& sw_id_and_registrations : active_registrations_) {
+    for (auto& key_and_registration :
+         sw_id_and_registrations.second.registration_map) {
+      BackgroundSyncRegistration* registration =
+          key_and_registration.second->value();
+      registration->SetUnregisteredState();
+    }
+  }
+
   active_registrations_.clear();
 
   // Delete all backend entries. The memory representation of registered syncs
@@ -760,23 +755,7 @@ void BackgroundSyncManager::UnregisterImpl(
     return;
   }
 
-  DCHECK(!existing_registration->value()->HasCompleted());
-
-  bool firing = existing_registration->value()->sync_state() ==
-                    BACKGROUND_SYNC_STATE_FIRING ||
-                existing_registration->value()->sync_state() ==
-                    BACKGROUND_SYNC_STATE_UNREGISTERED_WHILE_FIRING;
-
-  existing_registration->value()->set_sync_state(
-      firing ? BACKGROUND_SYNC_STATE_UNREGISTERED_WHILE_FIRING
-             : BACKGROUND_SYNC_STATE_UNREGISTERED);
-
-  if (!firing) {
-    // If the registration is currently firing then wait to run
-    // RunDoneCallbacks until after it has finished as it might
-    // change state to SUCCESS first.
-    existing_registration->value()->RunDoneCallbacks();
-  }
+  existing_registration->value()->SetUnregisteredState();
 
   RemoveActiveRegistration(sw_registration_id, registration_key);
 
@@ -870,13 +849,6 @@ void BackgroundSyncManager::NotifyWhenDoneDidFinish(
     BackgroundSyncState sync_state) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
-  if (disabled_) {
-    base::ThreadTaskRunnerHandle::Get()->PostTask(
-        FROM_HERE, base::Bind(callback, BACKGROUND_SYNC_STATUS_STORAGE_ERROR,
-                              BACKGROUND_SYNC_STATE_FAILED));
-    return;
-  }
-
   callback.Run(BACKGROUND_SYNC_STATUS_OK, sync_state);
 }
 
@@ -1118,8 +1090,9 @@ void BackgroundSyncManager::EventComplete(
     const base::Closure& callback,
     ServiceWorkerStatusCode status_code) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  if (disabled_)
-    return;
+
+  // Do not check for disabled as events that were firing when disabled should
+  // be allowed to complete (for NotifyWhenDone).
 
   op_scheduler_.ScheduleOperation(base::Bind(
       &BackgroundSyncManager::EventCompleteImpl, weak_ptr_factory_.GetWeakPtr(),
@@ -1134,15 +1107,10 @@ void BackgroundSyncManager::EventCompleteImpl(
     const base::Closure& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
-  if (disabled_) {
-    base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE,
-                                                  base::Bind(callback));
-    return;
-  }
-
   BackgroundSyncRegistration* registration =
       registration_handle->registration();
   DCHECK(registration);
+  DCHECK(!registration->HasCompleted());
 
   // The event ran to completion, we should count it, no matter what happens
   // from here.
@@ -1175,6 +1143,12 @@ void BackgroundSyncManager::EventCompleteImpl(
     NOTREACHED();
   }
 
+  if (disabled_) {
+    base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE,
+                                                  base::Bind(callback));
+    return;
+  }
+
   StoreRegistrations(
       service_worker_id,
       base::Bind(&BackgroundSyncManager::EventCompleteDidStore,
diff --git a/content/browser/background_sync/background_sync_manager.h b/content/browser/background_sync/background_sync_manager.h
index a845dd3..a066cd6 100644
--- a/content/browser/background_sync/background_sync_manager.h
+++ b/content/browser/background_sync/background_sync_manager.h
@@ -185,10 +185,12 @@ class CONTENT_EXPORT BackgroundSyncManager
       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
-  // functioning). The manager will reenable itself once it receives the
-  // OnStorageWiped message or on browser restart.
+  // to run (in their impl methods). Future operations will not queue. The one
+  // exception is already firing events -- their responses will be processed in
+  // order to notify their final state.
+  // The list of active registrations is cleared and the backend is also cleared
+  // (if it's still functioning). The manager will reenable itself once it
+  // receives the OnStorageWiped message or on browser restart.
   void DisableAndClearManager(const base::Closure& callback);
   void DisableAndClearDidGetRegistrations(
       const base::Closure& callback,
diff --git a/content/browser/background_sync/background_sync_manager_unittest.cc b/content/browser/background_sync/background_sync_manager_unittest.cc
index 72db49f..b7af781 100644
--- a/content/browser/background_sync/background_sync_manager_unittest.cc
+++ b/content/browser/background_sync/background_sync_manager_unittest.cc
@@ -1372,6 +1372,62 @@ TEST_F(BackgroundSyncManagerTest, OverwriteFiringRegistrationWhichFails) {
   EXPECT_EQ(BACKGROUND_SYNC_STATE_FAILED, sync_state);
 }
 
+TEST_F(BackgroundSyncManagerTest, DisableWhilePendingNotifiesDone) {
+  InitSyncEventTest();
+
+  // Register a one-shot that must wait for network connectivity before it
+  // can fire.
+  SetNetwork(net::NetworkChangeNotifier::CONNECTION_NONE);
+  EXPECT_TRUE(Register(sync_options_1_));
+
+  // Listen for notification of completion.
+  bool notify_done_called = false;
+  BackgroundSyncStatus status = BACKGROUND_SYNC_STATUS_OK;
+  BackgroundSyncState sync_state = BACKGROUND_SYNC_STATE_SUCCESS;
+  callback_registration_handle_->NotifyWhenDone(base::Bind(
+      &NotifyWhenDoneCallback, &notify_done_called, &status, &sync_state));
+  base::RunLoop().RunUntilIdle();
+  EXPECT_FALSE(notify_done_called);
+
+  // Corrupting the backend should result in the manager disabling itself on the
+  // next operation. While disabling, it should finalize any pending
+  // registrations.
+  test_background_sync_manager_->set_corrupt_backend(true);
+  EXPECT_FALSE(Register(sync_options_2_));
+  EXPECT_TRUE(notify_done_called);
+  EXPECT_EQ(BACKGROUND_SYNC_STATE_UNREGISTERED, sync_state);
+}
+
+TEST_F(BackgroundSyncManagerTest, DisableWhileFiringNotifiesDone) {
+  InitDelayedSyncEventTest();
+
+  // Register a one-shot that pauses mid-fire.
+  RegisterAndVerifySyncEventDelayed(sync_options_1_);
+
+  // Listen for notification of completion.
+  bool notify_done_called = false;
+  BackgroundSyncStatus status = BACKGROUND_SYNC_STATUS_OK;
+  BackgroundSyncState sync_state = BACKGROUND_SYNC_STATE_SUCCESS;
+  callback_registration_handle_->NotifyWhenDone(base::Bind(
+      &NotifyWhenDoneCallback, &notify_done_called, &status, &sync_state));
+  base::RunLoop().RunUntilIdle();
+  EXPECT_FALSE(notify_done_called);
+
+  // Corrupting the backend should result in the manager disabling itself on the
+  // next operation. Even though the manager is disabled, the firing sync event
+  // should still be able to complete successfully and notify as much.
+  test_background_sync_manager_->set_corrupt_backend(true);
+  EXPECT_FALSE(Register(sync_options_2_));
+  EXPECT_FALSE(notify_done_called);
+  test_background_sync_manager_->set_corrupt_backend(false);
+
+  // Successfully complete the firing event.
+  sync_fired_callback_.Run(SERVICE_WORKER_OK);
+  base::RunLoop().RunUntilIdle();
+  EXPECT_TRUE(notify_done_called);
+  EXPECT_EQ(BACKGROUND_SYNC_STATE_SUCCESS, sync_state);
+}
+
 // TODO(jkarlin): Change this to a periodic test as one-shots can't be power
 // dependent according to spec.
 TEST_F(BackgroundSyncManagerTest, OneShotFiresOnPowerChange) {
diff --git a/content/browser/background_sync/background_sync_registration.cc b/content/browser/background_sync/background_sync_registration.cc
index 30bbf9e..d3da8e8 100644
--- a/content/browser/background_sync/background_sync_registration.cc
+++ b/content/browser/background_sync/background_sync_registration.cc
@@ -62,4 +62,20 @@ bool BackgroundSyncRegistration::HasCompleted() const {
   return false;
 }
 
+void BackgroundSyncRegistration::SetUnregisteredState() {
+  DCHECK(!HasCompleted());
+  bool firing = sync_state_ == BACKGROUND_SYNC_STATE_FIRING ||
+                sync_state_ == BACKGROUND_SYNC_STATE_UNREGISTERED_WHILE_FIRING;
+
+  sync_state_ = firing ? BACKGROUND_SYNC_STATE_UNREGISTERED_WHILE_FIRING
+                       : BACKGROUND_SYNC_STATE_UNREGISTERED;
+
+  if (!firing) {
+    // If the registration is currently firing then wait to run
+    // RunDoneCallbacks until after it has finished as it might
+    // change state to SUCCESS first.
+    RunDoneCallbacks();
+  }
+}
+
 }  // namespace content
diff --git a/content/browser/background_sync/background_sync_registration.h b/content/browser/background_sync/background_sync_registration.h
index 741c888..556aad0 100644
--- a/content/browser/background_sync/background_sync_registration.h
+++ b/content/browser/background_sync/background_sync_registration.h
@@ -34,6 +34,11 @@ class CONTENT_EXPORT BackgroundSyncRegistration {
   void RunDoneCallbacks();
   bool HasCompleted() const;
 
+  // If the registration is currently firing, sets its state to
+  // BACKGROUND_SYNC_STATE_UNREGISTERED_WHILE_FIRING. If it is firing, it sets
+  // the state to BACKGROUND_SYNC_STATE_UNREGISTERED and calls RunDoneCallbacks.
+  void SetUnregisteredState();
+
   const BackgroundSyncRegistrationOptions* options() const { return &options_; }
   BackgroundSyncRegistrationOptions* options() { return &options_; }