service worker: Wait for inflight requests before activating

content/browser/service_worker/service_worker_registration_unittest.cc

Index: content/browser/service_worker/service_worker_registration_unittest.cc
diff --git a/content/browser/service_worker/service_worker_registration_unittest.cc b/content/browser/service_worker/service_worker_registration_unittest.cc
index 417896b6f419498e17dcd2a351ea72130f6e9fef..7f5bb1ed7782349f68f87c2d9fd9520313b95881 100644
--- a/content/browser/service_worker/service_worker_registration_unittest.cc
+++ b/content/browser/service_worker/service_worker_registration_unittest.cc
@@ -11,15 +11,31 @@
 #include "base/logging.h"
 #include "base/run_loop.h"
 #include "base/threading/thread_task_runner_handle.h"
+#include "content/browser/service_worker/embedded_worker_status.h"
 #include "content/browser/service_worker/embedded_worker_test_helper.h"
 #include "content/browser/service_worker/service_worker_context_core.h"
 #include "content/browser/service_worker/service_worker_registration_handle.h"
+#include "content/browser/service_worker/service_worker_test_utils.h"
+#include "content/common/service_worker/service_worker_utils.h"
 #include "content/public/test/test_browser_thread_bundle.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "url/gurl.h"
 
 namespace content {
 
+namespace {
+
+int CreateInflightRequest(ServiceWorkerVersion* version) {
+  version->StartWorker(ServiceWorkerMetrics::EventType::PUSH,
+                       base::Bind(&ServiceWorkerUtils::NoOpStatusCallback));
+  base::RunLoop().RunUntilIdle();
+  return version->StartRequest(
+      ServiceWorkerMetrics::EventType::PUSH,
+      base::Bind(&ServiceWorkerUtils::NoOpStatusCallback));
+}
+
+}  // namespace
+
 class ServiceWorkerRegistrationTest : public testing::Test {
  public:
   ServiceWorkerRegistrationTest()
@@ -27,6 +43,9 @@ class ServiceWorkerRegistrationTest : public testing::Test {
 
   void SetUp() override {
     helper_.reset(new EmbeddedWorkerTestHelper(base::FilePath()));
+
+    helper_->context()->storage()->LazyInitialize(base::Bind(&base::DoNothing));
+    base::RunLoop().RunUntilIdle();
   }
 
   void TearDown() override {
@@ -35,6 +54,7 @@ class ServiceWorkerRegistrationTest : public testing::Test {
   }
 
   ServiceWorkerContextCore* context() { return helper_->context(); }
+  ServiceWorkerStorage* storage() { return helper_->context()->storage(); }
 
   class RegistrationListener : public ServiceWorkerRegistration::Listener {
    public:
@@ -160,4 +180,151 @@ TEST_F(ServiceWorkerRegistrationTest, FailedRegistrationNoCrash) {
   // Don't crash when handle gets destructed.
 }
 
+// Sets up a registration with a waiting worker, and an active worker
+// with a controllee and an inflight request.
+class ServiceWorkerActivationTest : public ServiceWorkerRegistrationTest {
+ public:
+  ServiceWorkerActivationTest() : ServiceWorkerRegistrationTest() {}
+
+  void SetUp() override {
+    ServiceWorkerRegistrationTest::SetUp();
+
+    const GURL kScope("https://www.example.not/");
+    const GURL kScript("https://www.example.not/service_worker.js");
+
+    registration_ = new ServiceWorkerRegistration(
+        kScope, storage()->NewRegistrationId(), context()->AsWeakPtr());
+
+    // Create an active version.
+    scoped_refptr<ServiceWorkerVersion> version_1 = new ServiceWorkerVersion(
+        registration_.get(), kScript, storage()->NewVersionId(),
+        context()->AsWeakPtr());
+    registration_->SetActiveVersion(version_1);
+    version_1->SetStatus(ServiceWorkerVersion::ACTIVATED);
+
+    // Store the registration.
+    std::vector<ServiceWorkerDatabase::ResourceRecord> records;
+    records.push_back(ServiceWorkerDatabase::ResourceRecord(
+        10, version_1->script_url(), 100));
+    version_1->script_cache_map()->SetResources(records);
+    ServiceWorkerStatusCode status = SERVICE_WORKER_ERROR_MAX_VALUE;
+    context()->storage()->StoreRegistration(
+        registration_.get(), version_1.get(),
+        CreateReceiverOnCurrentThread(&status));
+    base::RunLoop().RunUntilIdle();
+    ASSERT_EQ(SERVICE_WORKER_OK, status);
+
+    // Give the active version a controllee.
+    host_.reset(new ServiceWorkerProviderHost(
+        33 /* dummy render process id */,
+        MSG_ROUTING_NONE /* render_frame_id */, 1 /* dummy provider_id */,
+        SERVICE_WORKER_PROVIDER_FOR_WINDOW,
+        ServiceWorkerProviderHost::FrameSecurityLevel::SECURE,
+        context()->AsWeakPtr(), nullptr));
+    version_1->AddControllee(host_.get());
+
+    // Give the active version an in-flight request.
+    request_id_ = CreateInflightRequest(version_1.get());
+
+    // Create a waiting version.
+    scoped_refptr<ServiceWorkerVersion> version_2 = new ServiceWorkerVersion(
+        registration_.get(), kScript, storage()->NewVersionId(),
+        context()->AsWeakPtr());
+    registration_->SetWaitingVersion(version_2);
+    version_2->SetStatus(ServiceWorkerVersion::INSTALLED);
+
+    // Set it to activate when ready. The original version should still be
+    // active.
+    registration_->ActivateWaitingVersionWhenReady();
+    base::RunLoop().RunUntilIdle();
+    EXPECT_EQ(version_1.get(), registration_->active_version());
+  }
+
+  void TearDown() override {
+    registration_->active_version()->RemoveListener(registration_.get());
+    ServiceWorkerRegistrationTest::TearDown();
+  }
+
+  ServiceWorkerRegistration* registration() { return registration_.get(); }
+  ServiceWorkerProviderHost* controllee() { return host_.get(); }
+  int inflight_request_id() { return request_id_; }
+
+ private:
+  scoped_refptr<ServiceWorkerRegistration> registration_;
+  std::unique_ptr<ServiceWorkerProviderHost> host_;
+  int request_id_;
+};
+
+// Test activation triggered by loss of controllee.
+TEST_F(ServiceWorkerActivationTest, NoControllee) {

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

It would be better to swap line 258-259 with line 276-277, because actually this
tests activation when an inflight request is done.

[falken, 2016/07/08 07:06:45]

Good eye, done.

+  scoped_refptr<ServiceWorkerRegistration> reg = registration();
+  scoped_refptr<ServiceWorkerVersion> version_1 = reg->active_version();
+  scoped_refptr<ServiceWorkerVersion> version_2 = reg->waiting_version();
+
+  // Remove the controllee. Since there is an in-flight request,
+  // activation should not yet happen.
+  version_1->RemoveControllee(controllee());
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(version_1.get(), reg->active_version());
+
+  // Finish the request. Activation should happen.
+  version_1->FinishRequest(inflight_request_id(), true /* was_handled */);
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(version_2.get(), reg->active_version());
+}
+
+// Test activation triggered by finishing all requests.
+TEST_F(ServiceWorkerActivationTest, NoInflightRequest) {

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

ditto.

+  scoped_refptr<ServiceWorkerRegistration> reg = registration();
+  scoped_refptr<ServiceWorkerVersion> version_1 = reg->active_version();
+  scoped_refptr<ServiceWorkerVersion> version_2 = reg->waiting_version();
+
+  // Finish the request. Since there is a controllee, activation should not yet
+  // happen.
+  version_1->FinishRequest(inflight_request_id(), true /* was_handled */);
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(version_1.get(), reg->active_version());
+
+  // Remove the controllee. Activation should happen.
+  version_1->RemoveControllee(controllee());
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(version_2.get(), reg->active_version());
+}
+
+// Test activation triggered by skipWaiting.
+TEST_F(ServiceWorkerActivationTest, SkipWaiting) {
+  scoped_refptr<ServiceWorkerRegistration> reg = registration();
+  scoped_refptr<ServiceWorkerVersion> version_1 = reg->active_version();
+  scoped_refptr<ServiceWorkerVersion> version_2 = reg->waiting_version();
+
+  // Finish the in-flight request. Since there is a controllee,
+  // activation should not happen.
+  version_1->FinishRequest(inflight_request_id(), true /* was_handled */);
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(version_1.get(), reg->active_version());
+
+  // Call skipWaiting. Activation should happen.
+  version_2->OnSkipWaiting(77 /* dummy request_id */);
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(version_2.get(), reg->active_version());
+}
+
+// Test activation triggered by skipWaiting and finishing requests.
+TEST_F(ServiceWorkerActivationTest, SkipWaitingWithInflightRequest) {
+  scoped_refptr<ServiceWorkerRegistration> reg = registration();
+  scoped_refptr<ServiceWorkerVersion> version_1 = reg->active_version();
+  scoped_refptr<ServiceWorkerVersion> version_2 = reg->waiting_version();
+
+  // Set skip waiting flag. Since there is still an in-flight request,
+  // activation should not happen.
+  version_2->OnSkipWaiting(77 /* dummy request_id */);
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(version_1.get(), reg->active_version());
+
+  // Finish the request. Activation should happen.
+  version_1->FinishRequest(inflight_request_id(), true /* was_handled */);
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(version_2.get(), reg->active_version());
+}
+
 }  // namespace content