ServiceWorker: Release a reference when it fails to dispatch ExtendableMessageEvent

content/browser/service_worker/service_worker_dispatcher_host_unittest.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_dispatcher_host.h"
 
 #include <stdint.h>
 #include <utility>
 
 #include "base/command_line.h"
 #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"
 #include "content/public/test/mock_resource_context.h"
 #include "content/public/test/test_browser_thread_bundle.h"
 #include "content/test/test_content_browser_client.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace content {
 
 namespace {
 
 static void SaveStatusCallback(bool* called,
                                ServiceWorkerStatusCode* out,
                                ServiceWorkerStatusCode status) {
   *called = true;
   *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 {
  public:
   TestingServiceWorkerDispatcherHost(
       int process_id,
       ServiceWorkerContextWrapper* context_wrapper,
       ResourceContext* resource_context,
       EmbeddedWorkerTestHelper* helper)
       : ServiceWorkerDispatcherHost(process_id, NULL, resource_context),
         bad_messages_received_count_(0),
         helper_(helper) {
     Init(context_wrapper);
   }
 
   bool Send(IPC::Message* message) override { return helper_->Send(message); }
 
   IPC::TestSink* ipc_sink() { return helper_->ipc_sink(); }
 
   void ShutdownForBadMessage() override { ++bad_messages_received_count_; }
 
   int bad_messages_received_count_;
 
  protected:
   EmbeddedWorkerTestHelper* helper_;
   ~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(); }
 
   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 SendRegister(int64_t provider_id, GURL pattern, GURL worker_url) {
     dispatcher_host_->OnMessageReceived(
         ServiceWorkerHostMsg_RegisterServiceWorker(
             -1, -1, provider_id, pattern, worker_url));
     base::RunLoop().RunUntilIdle();
   }
 
   void Register(int64_t provider_id,
                 GURL pattern,
                 GURL worker_url,
@@ skipping 42 matching lines @@
     base::RunLoop().RunUntilIdle();
   }
 
   void GetRegistrations(int64_t provider_id, uint32_t expected_message) {
     SendGetRegistrations(provider_id);
     EXPECT_TRUE(dispatcher_host_->ipc_sink()->GetUniqueMessageMatching(
         expected_message));
     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,
         SERVICE_WORKER_PROVIDER_FOR_WINDOW, context()->AsWeakPtr(),
         dispatcher_host_.get());
   }
 
-
   TestBrowserThreadBundle browser_thread_bundle_;
   content::MockResourceContext resource_context_;
   scoped_ptr<EmbeddedWorkerTestHelper> helper_;
   scoped_refptr<TestingServiceWorkerDispatcherHost> dispatcher_host_;
 };
 
 class ServiceWorkerTestContentBrowserClient : public TestContentBrowserClient {
  public:
   ServiceWorkerTestContentBrowserClient() {}
   bool AllowServiceWorker(const GURL& scope,
@@ skipping 422 matching lines @@
           process_id, context_wrapper(), &resource_context_, helper_.get()));
 
   // To show the new dispatcher can operate, simulate provider creation. Since
   // 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));
   EXPECT_EQ(0, new_dispatcher_host->bad_messages_received_count_);
 }
 
+TEST_F(ServiceWorkerDispatcherHostTest, DispatchExtendableMessageEvent) {
+  GURL pattern = GURL("http://www.example.com/");
+  GURL scope = GURL("http://www.example.com/service_worker.js");

[falken, 2016/03/17 06:44:36]

script_url

[nhiroki, 2016/03/17 07:58:17]

Good eye. I wrote this line while I was thinking about whether I should replace
'pattern' with 'scope'. That's the reason why I made a mistake :p

+  int process_id = helper_->mock_render_process_id();
+
+  // Add a provider for a service worker to send a message.
+  const int64_t kProviderId = 99;
+  dispatcher_host_->OnMessageReceived(ServiceWorkerHostMsg_ProviderCreated(
+      kProviderId, MSG_ROUTING_NONE, SERVICE_WORKER_PROVIDER_FOR_CONTROLLER));
+  helper_->SimulateAddProcessToPattern(pattern, process_id);
+  ServiceWorkerProviderHost* sender_provider_host =
+      context()->GetProviderHost(process_id, kProviderId);
+
+  // Set up a registration and service worker to receive a message.
+  scoped_refptr<ServiceWorkerRegistration> registration(
+      new ServiceWorkerRegistration(pattern, 1L,
+                                    helper_->context()->AsWeakPtr()));
+  scoped_refptr<ServiceWorkerVersion> version(new ServiceWorkerVersion(
+      registration.get(), scope, 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);
+  ASSERT_EQ(SERVICE_WORKER_OK, status);

[falken, 2016/03/17 06:44:36]

Maybe it's time to refactor lines 640-673 and the similar ones in
CleanupOnRendererCrash and DispatchExtendableMessageEvent_Fail into a helper
function.

[nhiroki, 2016/03/17 07:58:17]

Done.

+
+  // Set the running hosted version so that we can retrieve a valid service
+  // worker object information for the source attribute of the message event.
+  sender_provider_host->running_hosted_version_ = version;
+
+  // Set aside the initial refcount of the worker handle.
+  sender_provider_host->GetOrCreateServiceWorkerHandle(version.get());
+  ServiceWorkerHandle* sender_worker_handle =
+      dispatcher_host_->FindServiceWorkerHandle(
+          sender_provider_host->provider_id(), version->version_id());
+  const int ref_count = sender_worker_handle->ref_count();
+
+  // Dispatch ExtendableMessageEvent.
+  std::vector<TransferredMessagePort> ports;
+  SetUpDummyMessagePort(&ports);
+  called = false;
+  status = SERVICE_WORKER_ERROR_MAX_VALUE;
+  DispatchExtendableMessageEvent(
+      version, base::string16(), url::Origin(version->scope().GetOrigin()),
+      ports, sender_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) {
+  Initialize(make_scoped_ptr(new FailToStartWorkerTestHelper));
+
+  GURL pattern = GURL("http://www.example.com/");
+  GURL scope = GURL("http://www.example.com/service_worker.js");

[falken, 2016/03/17 06:44:36]

script_url

[nhiroki, 2016/03/17 07:58:17]

Done.

+  int process_id = helper_->mock_render_process_id();
+
+  // Add a provider for a service worker to send a message.
+  const int64_t kProviderId = 99;
+  dispatcher_host_->OnMessageReceived(ServiceWorkerHostMsg_ProviderCreated(
+      kProviderId, MSG_ROUTING_NONE, SERVICE_WORKER_PROVIDER_FOR_CONTROLLER));
+  helper_->SimulateAddProcessToPattern(pattern, process_id);
+  ServiceWorkerProviderHost* sender_provider_host =
+      context()->GetProviderHost(process_id, kProviderId);
+
+  // Set up a registration and service worker to receive a message.
+  scoped_refptr<ServiceWorkerRegistration> registration(
+      new ServiceWorkerRegistration(pattern, 1L,
+                                    helper_->context()->AsWeakPtr()));
+  scoped_refptr<ServiceWorkerVersion> version(new ServiceWorkerVersion(
+      registration.get(), scope, 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);
+  ASSERT_EQ(SERVICE_WORKER_OK, status);
+
+  // Set the running hosted version so that we can retrieve a valid service
+  // worker object information for the source attribute of the message event.
+  sender_provider_host->running_hosted_version_ = version;
+
+  // Set aside the initial refcount of the worker handle.
+  sender_provider_host->GetOrCreateServiceWorkerHandle(version.get());
+  ServiceWorkerHandle* sender_worker_handle =
+      dispatcher_host_->FindServiceWorkerHandle(
+          sender_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);
+  called = false;
+  status = SERVICE_WORKER_ERROR_MAX_VALUE;
+  DispatchExtendableMessageEvent(
+      version, base::string16(), url::Origin(version->scope().GetOrigin()),
+      ports, sender_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