service worker: Avoid starting up for activation during shutdown

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/service_worker_context_core.h"
+#include "content/browser/service_worker/service_worker_context_wrapper.h"
 #include "content/browser/service_worker/service_worker_info.h"
 #include "content/browser/service_worker/service_worker_metrics.h"
 #include "content/browser/service_worker/service_worker_register_job.h"
 #include "content/common/service_worker/service_worker_messages.h"
 #include "content/common/service_worker/service_worker_utils.h"
 #include "content/public/browser/browser_thread.h"
 
 namespace content {
 
 namespace {
 
 ServiceWorkerVersionInfo GetVersionInfo(ServiceWorkerVersion* version) {
   if (!version)
     return ServiceWorkerVersionInfo();
   return version->GetInfo();
 }
 
+bool GetShutdownStatusOnUIThread(
+    scoped_refptr<ServiceWorkerContextWrapper> wrapper,
+    ServiceWorkerStatusCode status) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  return wrapper->process_manager()->IsShutdown();
+}
+
 }  // namespace
 
 ServiceWorkerRegistration::ServiceWorkerRegistration(
     const GURL& pattern,
     int64_t registration_id,
     base::WeakPtr<ServiceWorkerContextCore> context)
     : pattern_(pattern),
       registration_id_(registration_id),
       is_deleted_(false),
       is_uninstalling_(false),
@@ skipping 132 matching lines @@
     active_version_ = NULL;
     mask->add(ChangedVersionAttributesMask::ACTIVE_VERSION);
   }
 }
 
 void ServiceWorkerRegistration::ActivateWaitingVersionWhenReady() {
   DCHECK(waiting_version());
   should_activate_when_ready_ = true;
 
   if (!active_version() || !active_version()->HasControllee() ||
-      waiting_version()->skip_waiting())
-    ActivateWaitingVersion();
+      waiting_version()->skip_waiting()) {
+    ActivateWaitingVersion(false);
+  }
 }
 
 void ServiceWorkerRegistration::ClaimClients() {
   DCHECK(context_);
   DCHECK(active_version());
 
   for (std::unique_ptr<ServiceWorkerContextCore::ProviderHostIterator> it =
            context_->GetProviderHostIterator();
        !it->IsAtEnd(); it->Advance()) {
     ServiceWorkerProviderHost* host = it->GetProviderHost();
@@ skipping 45 matching lines @@
                  most_recent_version));
 }
 
 void ServiceWorkerRegistration::OnNoControllees(ServiceWorkerVersion* version) {
   if (!context_)
     return;
   DCHECK_EQ(active_version(), version);
   if (is_uninstalling_)
     Clear();
   else if (should_activate_when_ready_)
-    ActivateWaitingVersion();
+    ActivateWaitingVersion(true);
   is_uninstalling_ = false;
   should_activate_when_ready_ = false;
 }
 
-void ServiceWorkerRegistration::ActivateWaitingVersion() {
+void ServiceWorkerRegistration::ActivateWaitingVersion(bool delay) {
   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();
 
   if (activating_version->is_redundant())
     return;  // Activation is no longer relevant.
 
@@ skipping 14 matching lines @@
   SetActiveVersion(activating_version);
 
   // "8. Run the [[UpdateState]] algorithm passing registration.activeWorker and
   // "activating" as arguments."
   activating_version->SetStatus(ServiceWorkerVersion::ACTIVATING);
   // "9. Fire a simple event named controllerchange..."
   if (activating_version->skip_waiting())
     FOR_EACH_OBSERVER(Listener, listeners_, OnSkippedWaiting(this));
 
   // "10. Queue a task to fire an event named activate..."
+  // The browser could be shutting down. To avoid spurious start worker
+  // failures, wait a bit before continuing.

[nhiroki, 2016/06/01 05:51:47]

Let me add this comment just for future reference...

[Before offline chat w/ falken@]

When delay == true, how about checking shutdown state first and avoiding
activation when in shutdown instead of using a timer?

[After offline chat]

According to falken@, ServiceWorkerContextWrapper::Shutdown() seems to be called
after this point and we may not be able to see the precise shutdown state here.

[falken, 2016/06/01 09:20:05]

Acknowledged.

+  if (delay) {
+    timer_.Start(FROM_HERE, base::TimeDelta::FromSeconds(1),
+                 base::Bind(&ServiceWorkerRegistration::ContinueActivation,
+                            this, activating_version));
+  } else {
+    ContinueActivation(activating_version);
+  }
+}
+
+void ServiceWorkerRegistration::ContinueActivation(
+    const scoped_refptr<ServiceWorkerVersion>& activating_version) {
+  if (!context_)
+    return;
+  if (active_version() != activating_version.get())
+    return;
+  DCHECK_EQ(ServiceWorkerVersion::ACTIVATING, activating_version->status());
   activating_version->RunAfterStartWorker(
       ServiceWorkerMetrics::EventType::ACTIVATE,
       base::Bind(&ServiceWorkerRegistration::DispatchActivateEvent, this,
                  activating_version),
       base::Bind(&ServiceWorkerRegistration::OnActivateEventFinished, this,
                  activating_version));
 }
 
 void ServiceWorkerRegistration::DeleteVersion(
     const scoped_refptr<ServiceWorkerVersion>& version) {
@@ skipping 28 matching lines @@
   }
 }
 
 void ServiceWorkerRegistration::NotifyRegistrationFinished() {
   std::vector<base::Closure> callbacks;
   callbacks.swap(registration_finished_callbacks_);
   for (const auto& callback : callbacks)
     callback.Run();
 }
 
+void ServiceWorkerRegistration::SetTimerTaskRunnerForTest(
+    scoped_refptr<base::SingleThreadTaskRunner> task_runner) {
+  timer_.SetTaskRunner(task_runner);

[nhiroki, 2016/06/01 05:51:47]

std::move(task_runner) ?

[falken, 2016/06/01 09:20:05]

Done.

+}
+
 void ServiceWorkerRegistration::RegisterRegistrationFinishedCallback(
     const base::Closure& callback) {
   // This should only be called if the registration is in progress.
   DCHECK(!active_version() && !waiting_version() && !is_uninstalled() &&
          !is_uninstalling());
   registration_finished_callbacks_.push_back(callback);
 }
 
 void ServiceWorkerRegistration::DispatchActivateEvent(
     const scoped_refptr<ServiceWorkerVersion>& activating_version) {
@@ skipping 10 matching lines @@
       base::Bind(&ServiceWorkerRegistration::OnActivateEventFinished, this,
                  activating_version));
   activating_version
       ->DispatchSimpleEvent<ServiceWorkerHostMsg_ActivateEventFinished>(
           request_id, ServiceWorkerMsg_ActivateEvent(request_id));
 }
 
 void ServiceWorkerRegistration::OnActivateEventFinished(
     const scoped_refptr<ServiceWorkerVersion>& activating_version,
     ServiceWorkerStatusCode status) {
-  if (!context_ || activating_version != active_version() ||
-      activating_version->status() != ServiceWorkerVersion::ACTIVATING)
+  ServiceWorkerMetrics::RecordActivateEventStatus(status);
+
+  if (!context_)
     return;
 
-  // |status| is just for UMA. Once we've attempted to dispatch the activate
-  // event to an installed worker, it's committed to becoming active.
-  ServiceWorkerMetrics::RecordActivateEventStatus(status);
+  if (status == SERVICE_WORKER_OK) {
+    CompleteActivation(activating_version, status, false);

[nhiroki, 2016/06/01 05:51:47]

Always recording OK w/ |is_shutdown = false| would be misleading. How about
checking the shutdown state even when an activation succeeds, or skipping
recording OK when not in shutdown?

[falken, 2016/06/01 09:20:05]

OK I went with checking the state always. I had wanted to make the shutdown
stuff be the slow path only taken when needed (e.g., the 1 sec delay is only
when a tab closes, and this UI hop was only on failure). But you're right it's
more accurate UMA to get the shutdown state in all cases, and the UI hop
shouldn't be too costly.

+    return;
+  }
+
+  // Activate is prone to failing due to shutdown, because it's triggered when
+  // tabs close. Try to detect that case.
+  BrowserThread::PostTaskAndReplyWithResult(
+      BrowserThread::UI, FROM_HERE,
+      base::Bind(&GetShutdownStatusOnUIThread,
+                 make_scoped_refptr(context_->wrapper()), status),
+      base::Bind(&ServiceWorkerRegistration::CompleteActivation, this,
+                 activating_version, status));
+}
+
+void ServiceWorkerRegistration::CompleteActivation(
+    const scoped_refptr<ServiceWorkerVersion>& activating_version,
+    ServiceWorkerStatusCode status,
+    bool is_shutdown) {
+  ServiceWorkerMetrics::RecordActivateEventStatusWithShutdownStatus(
+      status, is_shutdown);
+  if (!context_ || activating_version != active_version() ||
+      activating_version->status() != ServiceWorkerVersion::ACTIVATING) {
+    return;
+  }
+
+  if (is_shutdown) {
+    DCHECK_NE(SERVICE_WORKER_OK, status);
+    return;
+  }
 
   // "Run the Update State algorithm passing registration's active worker and
   // 'activated' as the arguments."
   activating_version->SetStatus(ServiceWorkerVersion::ACTIVATED);
   context_->storage()->UpdateToActiveState(
       this, base::Bind(&ServiceWorkerUtils::NoOpStatusCallback));
 }
 
 void ServiceWorkerRegistration::OnDeleteFinished(
     ServiceWorkerStatusCode status) {
@@ skipping 47 matching lines @@
   if (!context_) {
     callback.Run(SERVICE_WORKER_ERROR_ABORT);
     return;
   }
   context_->storage()->NotifyDoneInstallingRegistration(
       this, version.get(), status);
   callback.Run(status);
 }
 
 }  // namespace content