Manage ServiceWorkerDispatcherHost in ServiceWorkerContextCore

content/browser/service_worker/service_worker_provider_host_unittest.cc

Index: content/browser/service_worker/service_worker_provider_host_unittest.cc
diff --git a/content/browser/service_worker/service_worker_provider_host_unittest.cc b/content/browser/service_worker/service_worker_provider_host_unittest.cc
index 36381affadde29b0d594146725220f44fa1125e6..a07fe0a873dce3a20a90026b9cd3e8b40e5fdd90 100644
--- a/content/browser/service_worker/service_worker_provider_host_unittest.cc
+++ b/content/browser/service_worker/service_worker_provider_host_unittest.cc
@@ -55,26 +55,6 @@ class ServiceWorkerProviderHostTest : public testing::Test {
         GURL("https://www.example.com/example"), 2L, context_->AsWeakPtr());
     registration3_ = new ServiceWorkerRegistration(
         GURL("https://other.example.com/"), 3L, context_->AsWeakPtr());
-
-    // Prepare provider hosts (for the same process).
-    std::unique_ptr<ServiceWorkerProviderHost> host1(
-        new ServiceWorkerProviderHost(
-            helper_->mock_render_process_id(), MSG_ROUTING_NONE,
-            1 /* provider_id */, SERVICE_WORKER_PROVIDER_FOR_WINDOW,
-            ServiceWorkerProviderHost::FrameSecurityLevel::SECURE,
-            context_->AsWeakPtr(), NULL));
-    host1->SetDocumentUrl(GURL("https://www.example.com/example1.html"));
-    std::unique_ptr<ServiceWorkerProviderHost> host2(
-        new ServiceWorkerProviderHost(
-            helper_->mock_render_process_id(), MSG_ROUTING_NONE,
-            2 /* provider_id */, SERVICE_WORKER_PROVIDER_FOR_WINDOW,
-            ServiceWorkerProviderHost::FrameSecurityLevel::SECURE,
-            context_->AsWeakPtr(), NULL));
-    host2->SetDocumentUrl(GURL("https://www.example.com/example2.html"));
-    provider_host1_ = host1->AsWeakPtr();
-    provider_host2_ = host2->AsWeakPtr();
-    context_->AddProviderHost(base::WrapUnique(host1.release()));
-    context_->AddProviderHost(base::WrapUnique(host2.release()));
   }
 
   void TearDown() override {
@@ -90,14 +70,26 @@ class ServiceWorkerProviderHostTest : public testing::Test {
     return context_->process_manager()->PatternHasProcessToRun(pattern);
   }
 
+  ServiceWorkerProviderHost* CreateProviderHost(
+      const GURL& document_url,
+      bool is_parent_frame_secure = true) {

[falken, 2017/02/08 06:53:51]

Let's avoid the default param and add a
CreateProviderHostWithInsecureParentFrame instead.

[shimazu, 2017/02/13 03:25:56]

Done.

+    static int provider_id = 1;
+    std::unique_ptr<ServiceWorkerProviderHost> host =
+        CreateProviderHostForWindow(helper_->mock_render_process_id(),
+                                    provider_id++, is_parent_frame_secure,
+                                    helper_->context()->AsWeakPtr());
+    ServiceWorkerProviderHost* host_raw = host.get();
+    host->SetDocumentUrl(document_url);
+    context_->AddProviderHost(std::move(host));
+    return host_raw;
+  }
+
   TestBrowserThreadBundle thread_bundle_;
   std::unique_ptr<EmbeddedWorkerTestHelper> helper_;
   ServiceWorkerContextCore* context_;
   scoped_refptr<ServiceWorkerRegistration> registration1_;
   scoped_refptr<ServiceWorkerRegistration> registration2_;
   scoped_refptr<ServiceWorkerRegistration> registration3_;
-  base::WeakPtr<ServiceWorkerProviderHost> provider_host1_;
-  base::WeakPtr<ServiceWorkerProviderHost> provider_host2_;
   GURL script_url_;
   ServiceWorkerTestContentClient test_content_client_;
   TestContentBrowserClient test_content_browser_client_;
@@ -108,6 +100,11 @@ class ServiceWorkerProviderHostTest : public testing::Test {
 };
 
 TEST_F(ServiceWorkerProviderHostTest, PotentialRegistration_ProcessStatus) {
+  ServiceWorkerProviderHost* provider_host1 =
+      CreateProviderHost(GURL("https://www.example.com/example1.html"));
+  ServiceWorkerProviderHost* provider_host2 =
+      CreateProviderHost(GURL("https://www.example.com/example2.html"));
+
   // Matching registrations have already been set by SetDocumentUrl.
   ASSERT_TRUE(PatternHasProcessToRun(registration1_->pattern()));
 
@@ -115,93 +112,100 @@ TEST_F(ServiceWorkerProviderHostTest, PotentialRegistration_ProcessStatus) {
   ASSERT_TRUE(PatternHasProcessToRun(registration2_->pattern()));
 
   // Adding the same registration twice has no effect.
-  provider_host1_->AddMatchingRegistration(registration1_.get());
+  provider_host1->AddMatchingRegistration(registration1_.get());
   ASSERT_TRUE(PatternHasProcessToRun(registration1_->pattern()));
 
   // Removing a matching registration will decrease the process refs for its
   // pattern.
-  provider_host1_->RemoveMatchingRegistration(registration1_.get());
+  provider_host1->RemoveMatchingRegistration(registration1_.get());
   ASSERT_TRUE(PatternHasProcessToRun(registration1_->pattern()));
-  provider_host2_->RemoveMatchingRegistration(registration1_.get());
+  provider_host2->RemoveMatchingRegistration(registration1_.get());
   ASSERT_FALSE(PatternHasProcessToRun(registration1_->pattern()));
 
   // Matching registration will be removed when moving out of scope
   ASSERT_TRUE(PatternHasProcessToRun(registration2_->pattern()));   // host1,2
   ASSERT_FALSE(PatternHasProcessToRun(registration3_->pattern()));  // no host
-  provider_host1_->SetDocumentUrl(GURL("https://other.example.com/"));
+  provider_host1->SetDocumentUrl(GURL("https://other.example.com/"));
   ASSERT_TRUE(PatternHasProcessToRun(registration2_->pattern()));  // host2
   ASSERT_TRUE(PatternHasProcessToRun(registration3_->pattern()));  // host1
-  provider_host2_->SetDocumentUrl(GURL("https://other.example.com/"));
+  provider_host2->SetDocumentUrl(GURL("https://other.example.com/"));
   ASSERT_FALSE(PatternHasProcessToRun(registration2_->pattern()));  // no host
   ASSERT_TRUE(PatternHasProcessToRun(registration3_->pattern()));   // host1,2
 }
 
 TEST_F(ServiceWorkerProviderHostTest, AssociatedRegistration_ProcessStatus) {
+  ServiceWorkerProviderHost* provider_host1 =
+      CreateProviderHost(GURL("https://www.example.com/example1.html"));
+
   // Associating the registration will also increase the process refs for
   // the registration's pattern.
-  provider_host1_->AssociateRegistration(registration1_.get(),
-                                         false /* notify_controllerchange */);
+  provider_host1->AssociateRegistration(registration1_.get(),
+                                        false /* notify_controllerchange */);
   ASSERT_TRUE(PatternHasProcessToRun(registration1_->pattern()));
 
   // Disassociating the registration shouldn't affect the process refs for
   // the registration's pattern.
-  provider_host1_->DisassociateRegistration();
+  provider_host1->DisassociateRegistration();
   ASSERT_TRUE(PatternHasProcessToRun(registration1_->pattern()));
 }
 
 TEST_F(ServiceWorkerProviderHostTest, MatchRegistration) {
+  ServiceWorkerProviderHost* provider_host1 =
+      CreateProviderHost(GURL("https://www.example.com/example1.html"));
+
   // Match registration should return the longest matching one.
-  ASSERT_EQ(registration2_, provider_host1_->MatchRegistration());
-  provider_host1_->RemoveMatchingRegistration(registration2_.get());
-  ASSERT_EQ(registration1_, provider_host1_->MatchRegistration());
+  ASSERT_EQ(registration2_, provider_host1->MatchRegistration());
+  provider_host1->RemoveMatchingRegistration(registration2_.get());
+  ASSERT_EQ(registration1_, provider_host1->MatchRegistration());
 
   // Should return nullptr after removing all matching registrations.
-  provider_host1_->RemoveMatchingRegistration(registration1_.get());
-  ASSERT_EQ(nullptr, provider_host1_->MatchRegistration());
+  provider_host1->RemoveMatchingRegistration(registration1_.get());
+  ASSERT_EQ(nullptr, provider_host1->MatchRegistration());
 
   // SetDocumentUrl sets all of matching registrations
-  provider_host1_->SetDocumentUrl(GURL("https://www.example.com/example1"));
-  ASSERT_EQ(registration2_, provider_host1_->MatchRegistration());
-  provider_host1_->RemoveMatchingRegistration(registration2_.get());
-  ASSERT_EQ(registration1_, provider_host1_->MatchRegistration());
+  provider_host1->SetDocumentUrl(GURL("https://www.example.com/example1"));
+  ASSERT_EQ(registration2_, provider_host1->MatchRegistration());
+  provider_host1->RemoveMatchingRegistration(registration2_.get());
+  ASSERT_EQ(registration1_, provider_host1->MatchRegistration());
 
   // SetDocumentUrl with another origin also updates matching registrations
-  provider_host1_->SetDocumentUrl(GURL("https://other.example.com/example"));
-  ASSERT_EQ(registration3_, provider_host1_->MatchRegistration());
-  provider_host1_->RemoveMatchingRegistration(registration3_.get());
-  ASSERT_EQ(nullptr, provider_host1_->MatchRegistration());
+  provider_host1->SetDocumentUrl(GURL("https://other.example.com/example"));
+  ASSERT_EQ(registration3_, provider_host1->MatchRegistration());
+  provider_host1->RemoveMatchingRegistration(registration3_.get());
+  ASSERT_EQ(nullptr, provider_host1->MatchRegistration());
 }
 
 TEST_F(ServiceWorkerProviderHostTest, ContextSecurity) {
-  using FrameSecurityLevel = ServiceWorkerProviderHost::FrameSecurityLevel;
+  ServiceWorkerProviderHost* provider_host_secure_parent =
+      CreateProviderHost(GURL("https://www.example.com/example1.html"), true);
+  ServiceWorkerProviderHost* provider_host_insecure_parent =
+      CreateProviderHost(GURL("https://www.example.com/example1.html"), false);
 
   // Insecure document URL.
-  provider_host1_->SetDocumentUrl(GURL("http://host"));
-  provider_host1_->parent_frame_security_level_ = FrameSecurityLevel::SECURE;
-  EXPECT_FALSE(provider_host1_->IsContextSecureForServiceWorker());
+  provider_host_secure_parent->SetDocumentUrl(GURL("http://host"));
+  EXPECT_FALSE(provider_host_secure_parent->IsContextSecureForServiceWorker());
 
   // Insecure parent frame.
-  provider_host1_->SetDocumentUrl(GURL("https://host"));
-  provider_host1_->parent_frame_security_level_ = FrameSecurityLevel::INSECURE;
-  EXPECT_FALSE(provider_host1_->IsContextSecureForServiceWorker());
+  provider_host_insecure_parent->SetDocumentUrl(GURL("https://host"));
+  EXPECT_FALSE(
+      provider_host_insecure_parent->IsContextSecureForServiceWorker());
 
   // Secure URL and parent frame.
-  provider_host1_->SetDocumentUrl(GURL("https://host"));
-  provider_host1_->parent_frame_security_level_ = FrameSecurityLevel::SECURE;
-  EXPECT_TRUE(provider_host1_->IsContextSecureForServiceWorker());
+  provider_host_secure_parent->SetDocumentUrl(GURL("https://host"));
+  EXPECT_TRUE(provider_host_secure_parent->IsContextSecureForServiceWorker());
 
   // Exceptional service worker scheme.
   GURL url(std::string(kServiceWorkerScheme) + "://host");
   EXPECT_TRUE(url.is_valid());
-  provider_host1_->SetDocumentUrl(url);
-  provider_host1_->parent_frame_security_level_ = FrameSecurityLevel::SECURE;
   EXPECT_FALSE(IsOriginSecure(url));
   EXPECT_TRUE(OriginCanAccessServiceWorkers(url));
-  EXPECT_TRUE(provider_host1_->IsContextSecureForServiceWorker());
+  provider_host_secure_parent->SetDocumentUrl(url);
+  EXPECT_TRUE(provider_host_secure_parent->IsContextSecureForServiceWorker());
 
   // Exceptional service worker scheme with insecure parent frame.
-  provider_host1_->parent_frame_security_level_ = FrameSecurityLevel::INSECURE;
-  EXPECT_FALSE(provider_host1_->IsContextSecureForServiceWorker());
+  provider_host_insecure_parent->SetDocumentUrl(url);
+  EXPECT_FALSE(
+      provider_host_insecure_parent->IsContextSecureForServiceWorker());
 }
 
 }  // namespace content