ServiceWorker: Release a reference when it fails to dispatch ExtendableMessageEvent

content/browser/service_worker/service_worker_dispatcher_host_unittest.cc

Index: content/browser/service_worker/service_worker_dispatcher_host_unittest.cc
diff --git a/content/browser/service_worker/service_worker_dispatcher_host_unittest.cc b/content/browser/service_worker/service_worker_dispatcher_host_unittest.cc
index 9bce9aec5328300bd91f5d067ca8a609fd861709..d8a370af8bf8414b14eba126e477b64f01c59082 100644
--- a/content/browser/service_worker/service_worker_dispatcher_host_unittest.cc
+++ b/content/browser/service_worker/service_worker_dispatcher_host_unittest.cc
@@ -11,11 +11,13 @@
 #include "base/files/file_path.h"
 #include "base/run_loop.h"
 #include "content/browser/browser_thread_impl.h"
+#include "content/browser/message_port_service.h"
 #include "content/browser/service_worker/embedded_worker_instance.h"
 #include "content/browser/service_worker/embedded_worker_registry.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_context_wrapper.h"
+#include "content/browser/service_worker/service_worker_handle.h"
 #include "content/common/service_worker/embedded_worker_messages.h"
 #include "content/common/service_worker/service_worker_messages.h"
 #include "content/public/common/content_switches.h"
@@ -35,8 +37,17 @@ static void SaveStatusCallback(bool* called,
   *out = status;
 }
 
+void SetUpDummyMessagePort(std::vector<TransferredMessagePort>* ports) {
+  int port_id = -1;
+  MessagePortService::GetInstance()->Create(MSG_ROUTING_NONE, nullptr,
+                                            &port_id);
+  TransferredMessagePort dummy_port;
+  dummy_port.id = port_id;
+  ports->push_back(dummy_port);
 }
 
+}  // namespace
+
 static const int kRenderFrameId = 1;
 
 class TestingServiceWorkerDispatcherHost : public ServiceWorkerDispatcherHost {
@@ -65,25 +76,81 @@ class TestingServiceWorkerDispatcherHost : public ServiceWorkerDispatcherHost {
   ~TestingServiceWorkerDispatcherHost() override {}
 };
 
+class FailToStartWorkerTestHelper : public EmbeddedWorkerTestHelper {
+ public:
+  FailToStartWorkerTestHelper() : EmbeddedWorkerTestHelper(base::FilePath()) {}
+
+  void OnStartWorker(int embedded_worker_id,
+                     int64_t service_worker_version_id,
+                     const GURL& scope,
+                     const GURL& script_url,
+                     bool pause_after_download) override {
+    EmbeddedWorkerInstance* worker = registry()->GetWorker(embedded_worker_id);
+    registry()->OnWorkerStopped(worker->process_id(), embedded_worker_id);
+  }
+};
+
 class ServiceWorkerDispatcherHostTest : public testing::Test {
  protected:
   ServiceWorkerDispatcherHostTest()
       : browser_thread_bundle_(TestBrowserThreadBundle::IO_MAINLOOP) {}
 
   void SetUp() override {
-    helper_.reset(new EmbeddedWorkerTestHelper(base::FilePath()));
-    dispatcher_host_ = new TestingServiceWorkerDispatcherHost(
-        helper_->mock_render_process_id(), context_wrapper(),
-        &resource_context_, helper_.get());
+    Initialize(make_scoped_ptr(new EmbeddedWorkerTestHelper(base::FilePath())));
   }
 
-  void TearDown() override { helper_.reset(); }
+  void TearDown() override {
+    version_ = nullptr;
+    registration_ = nullptr;
+    helper_.reset();
+  }
 
   ServiceWorkerContextCore* context() { return helper_->context(); }
   ServiceWorkerContextWrapper* context_wrapper() {
     return helper_->context_wrapper();
   }
 
+  void Initialize(scoped_ptr<EmbeddedWorkerTestHelper> helper) {
+    helper_.reset(helper.release());
+    dispatcher_host_ = new TestingServiceWorkerDispatcherHost(
+        helper_->mock_render_process_id(), context_wrapper(),
+        &resource_context_, helper_.get());
+  }
+
+  void SetUpRegistration(const GURL& scope, const GURL& script_url) {
+    registration_ = new ServiceWorkerRegistration(
+        scope, 1L, helper_->context()->AsWeakPtr());
+    version_ = new ServiceWorkerVersion(registration_.get(), script_url, 1L,
+                                        helper_->context()->AsWeakPtr());
+    std::vector<ServiceWorkerDatabase::ResourceRecord> records;
+    records.push_back(
+        ServiceWorkerDatabase::ResourceRecord(10, version_->script_url(), 100));
+    version_->script_cache_map()->SetResources(records);
+
+    // Make the registration findable via storage functions.
+    helper_->context()->storage()->LazyInitialize(base::Bind(&base::DoNothing));
+    base::RunLoop().RunUntilIdle();
+    bool called = false;
+    ServiceWorkerStatusCode status = SERVICE_WORKER_ERROR_MAX_VALUE;
+    helper_->context()->storage()->StoreRegistration(
+        registration_.get(), version_.get(),
+        base::Bind(&SaveStatusCallback, &called, &status));
+    base::RunLoop().RunUntilIdle();
+    EXPECT_TRUE(called);
+    EXPECT_EQ(SERVICE_WORKER_OK, status);
+  }
+
+  void SendProviderCreated(ServiceWorkerProviderType type,
+                           const GURL& pattern) {
+    const int64_t kProviderId = 99;
+    dispatcher_host_->OnMessageReceived(ServiceWorkerHostMsg_ProviderCreated(
+        kProviderId, MSG_ROUTING_NONE, type));
+    helper_->SimulateAddProcessToPattern(pattern,
+                                         helper_->mock_render_process_id());
+    provider_host_ = context()->GetProviderHost(
+        helper_->mock_render_process_id(), kProviderId);
+  }
+
   void SendRegister(int64_t provider_id, GURL pattern, GURL worker_url) {
     dispatcher_host_->OnMessageReceived(
         ServiceWorkerHostMsg_RegisterServiceWorker(
@@ -146,6 +213,18 @@ class ServiceWorkerDispatcherHostTest : public testing::Test {
     dispatcher_host_->ipc_sink()->ClearMessages();
   }
 
+  void DispatchExtendableMessageEvent(
+      scoped_refptr<ServiceWorkerVersion> worker,
+      const base::string16& message,
+      const url::Origin& source_origin,
+      const std::vector<TransferredMessagePort>& sent_message_ports,
+      ServiceWorkerProviderHost* sender_provider_host,
+      const ServiceWorkerDispatcherHost::StatusCallback& callback) {
+    dispatcher_host_->DispatchExtendableMessageEvent(
+        std::move(worker), message, source_origin, sent_message_ports,
+        sender_provider_host, callback);
+  }
+
   ServiceWorkerProviderHost* CreateServiceWorkerProviderHost(int provider_id) {
     return new ServiceWorkerProviderHost(
         helper_->mock_render_process_id(), kRenderFrameId, provider_id,
@@ -153,11 +232,13 @@ class ServiceWorkerDispatcherHostTest : public testing::Test {
         dispatcher_host_.get());
   }
 
-
   TestBrowserThreadBundle browser_thread_bundle_;
   content::MockResourceContext resource_context_;
   scoped_ptr<EmbeddedWorkerTestHelper> helper_;
   scoped_refptr<TestingServiceWorkerDispatcherHost> dispatcher_host_;
+  scoped_refptr<ServiceWorkerRegistration> registration_;
+  scoped_refptr<ServiceWorkerVersion> version_;
+  ServiceWorkerProviderHost* provider_host_;
 };
 
 class ServiceWorkerTestContentBrowserClient : public TestContentBrowserClient {
@@ -526,61 +607,33 @@ TEST_F(ServiceWorkerDispatcherHostTest, GetRegistrations_EarlyContextDeletion) {
 }
 
 TEST_F(ServiceWorkerDispatcherHostTest, CleanupOnRendererCrash) {
+  GURL pattern = GURL("http://www.example.com/");
+  GURL script_url = GURL("http://www.example.com/service_worker.js");
   int process_id = helper_->mock_render_process_id();
 
-  // Add a provider and worker.
-  const int64_t kProviderId = 99;  // Dummy value
-  dispatcher_host_->OnMessageReceived(ServiceWorkerHostMsg_ProviderCreated(
-      kProviderId, MSG_ROUTING_NONE, SERVICE_WORKER_PROVIDER_FOR_WINDOW));
+  SendProviderCreated(SERVICE_WORKER_PROVIDER_FOR_WINDOW, pattern);
+  SetUpRegistration(pattern, script_url);
+  int64_t provider_id = provider_host_->provider_id();
 
-  GURL pattern = GURL("http://www.example.com/");
-  scoped_refptr<ServiceWorkerRegistration> registration(
-      new ServiceWorkerRegistration(pattern,
-                                    1L,
-                                    helper_->context()->AsWeakPtr()));
-  scoped_refptr<ServiceWorkerVersion> version(
-      new ServiceWorkerVersion(registration.get(),
-                               GURL("http://www.example.com/service_worker.js"),
-                               1L,
-                               helper_->context()->AsWeakPtr()));
-  std::vector<ServiceWorkerDatabase::ResourceRecord> records;
-  records.push_back(
-      ServiceWorkerDatabase::ResourceRecord(10, version->script_url(), 100));
-  version->script_cache_map()->SetResources(records);
-
-  // Make the registration findable via storage functions.
-  helper_->context()->storage()->LazyInitialize(base::Bind(&base::DoNothing));
-  base::RunLoop().RunUntilIdle();
+  // Start up the worker.
   bool called = false;
   ServiceWorkerStatusCode status = SERVICE_WORKER_ERROR_ABORT;
-  helper_->context()->storage()->StoreRegistration(
-      registration.get(),
-      version.get(),
-      base::Bind(&SaveStatusCallback, &called, &status));
-  base::RunLoop().RunUntilIdle();
-  EXPECT_TRUE(called);
-  ASSERT_EQ(SERVICE_WORKER_OK, status);
-
-  helper_->SimulateAddProcessToPattern(pattern, process_id);
-
-  // Start up the worker.
-  status = SERVICE_WORKER_ERROR_ABORT;
-  version->StartWorker(ServiceWorkerMetrics::EventType::UNKNOWN,
-                       base::Bind(&SaveStatusCallback, &called, &status));
+  version_->StartWorker(ServiceWorkerMetrics::EventType::UNKNOWN,
+                        base::Bind(&SaveStatusCallback, &called, &status));
   base::RunLoop().RunUntilIdle();
 
   EXPECT_TRUE(called);
   EXPECT_EQ(SERVICE_WORKER_OK, status);
 
-  EXPECT_TRUE(context()->GetProviderHost(process_id, kProviderId));
-  EXPECT_EQ(ServiceWorkerVersion::RUNNING, version->running_status());
+  EXPECT_TRUE(context()->GetProviderHost(process_id, provider_id));
+  EXPECT_EQ(ServiceWorkerVersion::RUNNING, version_->running_status());
 
   // Simulate the render process crashing.
   dispatcher_host_->OnFilterRemoved();
 
   // The dispatcher host should clean up the state from the process.
-  EXPECT_FALSE(context()->GetProviderHost(process_id, kProviderId));
-  EXPECT_EQ(ServiceWorkerVersion::STOPPED, version->running_status());
+  EXPECT_FALSE(context()->GetProviderHost(process_id, provider_id));
+  EXPECT_EQ(ServiceWorkerVersion::STOPPED, version_->running_status());
 
   // We should be able to hook up a new dispatcher host although the old object
   // is not yet destroyed. This is what the browser does when reusing a crashed
@@ -593,8 +646,89 @@ TEST_F(ServiceWorkerDispatcherHostTest, CleanupOnRendererCrash) {
   // the old dispatcher cleaned up the old provider host, the new one won't
   // complain.
   new_dispatcher_host->OnMessageReceived(ServiceWorkerHostMsg_ProviderCreated(
-      kProviderId, MSG_ROUTING_NONE, SERVICE_WORKER_PROVIDER_FOR_WINDOW));
+      provider_id, MSG_ROUTING_NONE, SERVICE_WORKER_PROVIDER_FOR_WINDOW));
   EXPECT_EQ(0, new_dispatcher_host->bad_messages_received_count_);
 }
 
+TEST_F(ServiceWorkerDispatcherHostTest, DispatchExtendableMessageEvent) {
+  GURL pattern = GURL("http://www.example.com/");
+  GURL script_url = GURL("http://www.example.com/service_worker.js");
+
+  SendProviderCreated(SERVICE_WORKER_PROVIDER_FOR_CONTROLLER, pattern);
+  SetUpRegistration(pattern, script_url);
+
+  // Set the running hosted version so that we can retrieve a valid service
+  // worker object information for the source attribute of the message event.
+  provider_host_->running_hosted_version_ = version_;
+
+  // Set aside the initial refcount of the worker handle.
+  provider_host_->GetOrCreateServiceWorkerHandle(version_.get());
+  ServiceWorkerHandle* sender_worker_handle =
+      dispatcher_host_->FindServiceWorkerHandle(provider_host_->provider_id(),
+                                                version_->version_id());
+  const int ref_count = sender_worker_handle->ref_count();
+
+  // Dispatch ExtendableMessageEvent.
+  std::vector<TransferredMessagePort> ports;
+  SetUpDummyMessagePort(&ports);
+  bool called = false;
+  ServiceWorkerStatusCode status = SERVICE_WORKER_ERROR_MAX_VALUE;
+  DispatchExtendableMessageEvent(
+      version_, base::string16(), url::Origin(version_->scope().GetOrigin()),
+      ports, provider_host_, base::Bind(&SaveStatusCallback, &called, &status));
+  for (TransferredMessagePort port : ports)
+    EXPECT_TRUE(MessagePortService::GetInstance()->AreMessagesHeld(port.id));
+  EXPECT_EQ(ref_count + 1, sender_worker_handle->ref_count());
+  base::RunLoop().RunUntilIdle();
+  EXPECT_TRUE(called);
+  EXPECT_EQ(SERVICE_WORKER_OK, status);
+
+  // Messages should be held until ports are created at the destination.
+  for (TransferredMessagePort port : ports)
+    EXPECT_TRUE(MessagePortService::GetInstance()->AreMessagesHeld(port.id));
+
+  EXPECT_EQ(ref_count + 1, sender_worker_handle->ref_count());
+}
+
+TEST_F(ServiceWorkerDispatcherHostTest, DispatchExtendableMessageEvent_Fail) {
+  GURL pattern = GURL("http://www.example.com/");
+  GURL script_url = GURL("http://www.example.com/service_worker.js");
+
+  Initialize(make_scoped_ptr(new FailToStartWorkerTestHelper));
+  SendProviderCreated(SERVICE_WORKER_PROVIDER_FOR_CONTROLLER, pattern);
+  SetUpRegistration(pattern, script_url);
+
+  // Set the running hosted version so that we can retrieve a valid service
+  // worker object information for the source attribute of the message event.
+  provider_host_->running_hosted_version_ = version_;
+
+  // Set aside the initial refcount of the worker handle.
+  provider_host_->GetOrCreateServiceWorkerHandle(version_.get());
+  ServiceWorkerHandle* sender_worker_handle =
+      dispatcher_host_->FindServiceWorkerHandle(provider_host_->provider_id(),
+                                                version_->version_id());
+  const int ref_count = sender_worker_handle->ref_count();
+
+  // Try to dispatch ExtendableMessageEvent. This should fail to start the
+  // worker and to dispatch the event.
+  std::vector<TransferredMessagePort> ports;
+  SetUpDummyMessagePort(&ports);
+  bool called = false;
+  ServiceWorkerStatusCode status = SERVICE_WORKER_ERROR_MAX_VALUE;
+  DispatchExtendableMessageEvent(
+      version_, base::string16(), url::Origin(version_->scope().GetOrigin()),
+      ports, provider_host_, base::Bind(&SaveStatusCallback, &called, &status));
+  for (TransferredMessagePort port : ports)
+    EXPECT_TRUE(MessagePortService::GetInstance()->AreMessagesHeld(port.id));
+  EXPECT_EQ(ref_count + 1, sender_worker_handle->ref_count());
+  base::RunLoop().RunUntilIdle();
+  EXPECT_TRUE(called);
+  EXPECT_EQ(SERVICE_WORKER_ERROR_START_WORKER_FAILED, status);
+
+  // The error callback should clean up the ports and handle.
+  for (TransferredMessagePort port : ports)
+    EXPECT_FALSE(MessagePortService::GetInstance()->AreMessagesHeld(port.id));
+  EXPECT_EQ(ref_count, sender_worker_handle->ref_count());
+}
+
 }  // namespace content