service worker: Refactor OnNoControllees

content/browser/service_worker/service_worker_registration.cc

 // Copyright 2013 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/service_worker/service_worker_registration.h"
 
 #include <vector>
 
 #include "content/browser/service_worker/embedded_worker_status.h"
 #include "content/browser/service_worker/service_worker_context_core.h"
@@ skipping 237 matching lines @@
 }
 
 void ServiceWorkerRegistration::OnNoControllees(ServiceWorkerVersion* version) {
   if (!context_)
     return;
   DCHECK_EQ(active_version(), version);
   if (is_uninstalling_)
     Clear();
   else if (should_activate_when_ready_)
     ActivateWaitingVersion(true);
-  is_uninstalling_ = false;
-  should_activate_when_ready_ = false;
 }
 
 void ServiceWorkerRegistration::ActivateWaitingVersion(bool delay) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   DCHECK(context_);
   DCHECK(waiting_version());
   DCHECK(should_activate_when_ready_);
   should_activate_when_ready_ = false;
   scoped_refptr<ServiceWorkerVersion> activating_version = waiting_version();
   scoped_refptr<ServiceWorkerVersion> exiting_version = active_version();
@@ skipping 152 matching lines @@
   context_->storage()->UpdateToActiveState(
       this, base::Bind(&ServiceWorkerUtils::NoOpStatusCallback));
 }
 
 void ServiceWorkerRegistration::OnDeleteFinished(
     ServiceWorkerStatusCode status) {
   // Intentionally empty completion callback, used to prevent
   // |this| from being deleted until the storage method completes.
 }
 
 void ServiceWorkerRegistration::Clear() {

[nhiroki, 2016/07/04 07:53:38]

How about checking pre-conditions before flipping |is_uninstalling_| and
|is_uninstalled_|?

DCHECK(is_uninstalling_);
DCHECK(!is_uninstalled_);

[falken, 2016/07/04 08:14:37]

Unfortunately I'm not 100% sure those hold. Since this patch doesn't change when
Clear() is called, I'll leave out the dchecks in this patch.

In the future, these uninstalling/uninstalled/deleting flags should be merged
into a single Status enum.

[nhiroki, 2016/07/04 08:39:05]

Acknowledged.
It's great to merge them! Are you going to try it? :)
(I tried it before and gave up because of complex state transitions...)

   is_uninstalling_ = false;
   is_uninstalled_ = true;
+  should_activate_when_ready_ = false;
   if (context_)
     context_->storage()->NotifyDoneUninstallingRegistration(this);
 
   std::vector<scoped_refptr<ServiceWorkerVersion>> versions_to_doom;
   ChangedVersionAttributesMask mask;
   if (installing_version_.get()) {
     versions_to_doom.push_back(installing_version_);
     installing_version_ = nullptr;
     mask.add(ChangedVersionAttributesMask::INSTALLING_VERSION);
   }
@@ skipping 30 matching lines @@
   if (!context_) {
     callback.Run(SERVICE_WORKER_ERROR_ABORT);
     return;
   }
   context_->storage()->NotifyDoneInstallingRegistration(
       this, version.get(), status);
   callback.Run(status);
 }
 
 }  // namespace content