ServiceWorker: OnSimpleEventFinished could be called after timed out

content/browser/service_worker/embedded_worker_test_helper.cc

 // Copyright 2014 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/embedded_worker_test_helper.h"
 
 #include <map>
 #include <string>
 #include <utility>
 
@@ skipping 43 matching lines @@
   ~MockMessagePortMessageFilter() override {}
   base::AtomicSequenceNumber next_routing_id_;
   ScopedVector<IPC::Message> message_queue_;
 };
 
 }  // namespace
 
 class EmbeddedWorkerTestHelper::MockEmbeddedWorkerSetup
     : public mojom::EmbeddedWorkerSetup {
  public:
-  explicit MockEmbeddedWorkerSetup(
-      const base::WeakPtr<EmbeddedWorkerTestHelper>& helper)
-      : helper_(helper) {}
-
   static void Create(const base::WeakPtr<EmbeddedWorkerTestHelper>& helper,
                      mojom::EmbeddedWorkerSetupRequest request) {
-    mojo::MakeStrongBinding(base::MakeUnique<MockEmbeddedWorkerSetup>(helper),
-                            std::move(request));
+    // TODO(shimazu): Remove this mock after EmbeddedWorkerSetup is removed.
+    NOTREACHED();
   }
 
   void AttachServiceWorkerEventDispatcher(
       int32_t thread_id,
       mojom::ServiceWorkerEventDispatcherRequest request) override {
-    if (!helper_)
-      return;
-    helper_->OnSetupMojo(thread_id, std::move(request));
+    NOTREACHED();
   }
-
- private:
-  base::WeakPtr<EmbeddedWorkerTestHelper> helper_;
 };
 
 EmbeddedWorkerTestHelper::MockEmbeddedWorkerInstanceClient::
     MockEmbeddedWorkerInstanceClient(
         base::WeakPtr<EmbeddedWorkerTestHelper> helper)
     : helper_(helper), binding_(this) {}
 
 EmbeddedWorkerTestHelper::MockEmbeddedWorkerInstanceClient::
     ~MockEmbeddedWorkerInstanceClient() {}
 
 void EmbeddedWorkerTestHelper::MockEmbeddedWorkerInstanceClient::StartWorker(
     const EmbeddedWorkerStartParams& params,
     mojom::ServiceWorkerEventDispatcherRequest dispatcher_request) {
   if (!helper_)
     return;
 
   embedded_worker_id_ = params.embedded_worker_id;
 
   EmbeddedWorkerInstance* worker =
       helper_->registry()->GetWorker(params.embedded_worker_id);
   ASSERT_TRUE(worker != NULL);
   EXPECT_EQ(EmbeddedWorkerStatus::STARTING, worker->status());
 
-  base::ThreadTaskRunnerHandle::Get()->PostTask(
-      FROM_HERE,
-      base::Bind(&EmbeddedWorkerTestHelper::OnStartWorker, helper_->AsWeakPtr(),
-                 params.embedded_worker_id, params.service_worker_version_id,
-                 params.scope, params.script_url, params.pause_after_download));
-  base::ThreadTaskRunnerHandle::Get()->PostTask(
-      FROM_HERE,
-      base::Bind(&EmbeddedWorkerTestHelper::OnSetupMojo, helper_->AsWeakPtr(),
-                 worker->thread_id(), base::Passed(&dispatcher_request)));
+  helper_->OnStartWorkerStub(params, std::move(dispatcher_request));
 }
 
 void EmbeddedWorkerTestHelper::MockEmbeddedWorkerInstanceClient::StopWorker(
     const StopWorkerCallback& callback) {
   if (!helper_)
     return;
 
   ASSERT_TRUE(embedded_worker_id_);
   EmbeddedWorkerInstance* worker =
       helper_->registry()->GetWorker(embedded_worker_id_.value());
   // |worker| is possible to be null when corresponding EmbeddedWorkerInstance
   // is removed right after sending StopWorker.
   if (worker)
     EXPECT_EQ(EmbeddedWorkerStatus::STOPPING, worker->status());
-  callback.Run();
+  helper_->OnStopWorkerStub(callback);
 }
 
 // static
 void EmbeddedWorkerTestHelper::MockEmbeddedWorkerInstanceClient::Bind(
     const base::WeakPtr<EmbeddedWorkerTestHelper>& helper,
     mojom::EmbeddedWorkerInstanceClientRequest request) {
   std::vector<std::unique_ptr<MockEmbeddedWorkerInstanceClient>>* clients =
       helper->mock_instance_clients();
   size_t next_client_index = helper->mock_instance_clients_next_index_;
 
@@ skipping 108 matching lines @@
 
 bool EmbeddedWorkerTestHelper::Send(IPC::Message* message) {
   OnMessageReceived(*message);
   delete message;
   return true;
 }
 
 bool EmbeddedWorkerTestHelper::OnMessageReceived(const IPC::Message& message) {
   bool handled = true;
   IPC_BEGIN_MESSAGE_MAP(EmbeddedWorkerTestHelper, message)
-    IPC_MESSAGE_HANDLER(EmbeddedWorkerMsg_StartWorker, OnStartWorkerStub)
     IPC_MESSAGE_HANDLER(EmbeddedWorkerMsg_ResumeAfterDownload,
                         OnResumeAfterDownloadStub)
-    IPC_MESSAGE_HANDLER(EmbeddedWorkerMsg_StopWorker, OnStopWorkerStub)
     IPC_MESSAGE_HANDLER(EmbeddedWorkerContextMsg_MessageToWorker,
                         OnMessageToWorkerStub)
     IPC_MESSAGE_UNHANDLED(handled = false)
   IPC_END_MESSAGE_MAP()
 
   // IPC::TestSink only records messages that are not handled by filters,
   // so we just forward all messages to the separate sink.
   sink_.OnMessageReceived(message);
 
   return handled;
@@ skipping 18 matching lines @@
   net::HttpResponseInfo info;
   const char data[] =
       "HTTP/1.1 200 OK\0"
       "Content-Type: application/javascript\0"
       "\0";
   info.headers =
       new net::HttpResponseHeaders(std::string(data, arraysize(data)));
   return info;
 }
 
-void EmbeddedWorkerTestHelper::OnStartWorker(int embedded_worker_id,
-                                             int64_t service_worker_version_id,
-                                             const GURL& scope,
-                                             const GURL& script_url,
-                                             bool pause_after_download) {
+void EmbeddedWorkerTestHelper::OnStartWorker(
+    int embedded_worker_id,
+    int64_t service_worker_version_id,
+    const GURL& scope,
+    const GURL& script_url,
+    bool pause_after_download,
+    mojom::ServiceWorkerEventDispatcherRequest request) {
+  EmbeddedWorkerInstance* worker = registry()->GetWorker(embedded_worker_id);
+  ASSERT_TRUE(worker != nullptr);

[falken, 2017/01/11 06:31:55]

nit: Can drop the != nullptr

[shimazu, 2017/01/12 01:45:24]

Done.

+  MockServiceWorkerEventDispatcher::Create(AsWeakPtr(), worker->thread_id(),
+                                           std::move(request));
+
   embedded_worker_id_service_worker_version_id_map_[embedded_worker_id] =
       service_worker_version_id;
   SimulateWorkerReadyForInspection(embedded_worker_id);
   SimulateWorkerScriptCached(embedded_worker_id);
   SimulateWorkerScriptLoaded(embedded_worker_id);
   if (!pause_after_download)
     OnResumeAfterDownload(embedded_worker_id);
 }
 
 void EmbeddedWorkerTestHelper::OnResumeAfterDownload(int embedded_worker_id) {
   SimulateWorkerThreadStarted(GetNextThreadId(), embedded_worker_id);
   SimulateWorkerScriptEvaluated(embedded_worker_id, true /* success */);
   SimulateWorkerStarted(embedded_worker_id);
 }
 
-void EmbeddedWorkerTestHelper::OnStopWorker(int embedded_worker_id) {
-  // By default just notify the sender that the worker is stopped.
-  SimulateWorkerStopped(embedded_worker_id);
+void EmbeddedWorkerTestHelper::OnStopWorker(
+    const mojom::EmbeddedWorkerInstanceClient::StopWorkerCallback& callback) {

[falken, 2017/01/11 06:31:55]

Can you comment why this does nothing but call the callback?

[shimazu, 2017/01/12 01:45:24]

Calling the callback means sending the response back.

[falken, 2017/01/12 02:29:05]

OK I see now the old SimulateWorkerStopped just sent the response back. I guess
we should have a similar comment.

[shimazu, 2017/01/12 03:48:26]

Done.

+  callback.Run();
 }
 
 bool EmbeddedWorkerTestHelper::OnMessageToWorker(int thread_id,
                                                  int embedded_worker_id,
                                                  const IPC::Message& message) {
   bool handled = true;
   current_embedded_worker_id_ = embedded_worker_id;
   IPC_BEGIN_MESSAGE_MAP(EmbeddedWorkerTestHelper, message)
     IPC_MESSAGE_HANDLER(ServiceWorkerMsg_ActivateEvent, OnActivateEventStub)
     IPC_MESSAGE_HANDLER(ServiceWorkerMsg_InstallEvent, OnInstallEventStub)
     IPC_MESSAGE_HANDLER(ServiceWorkerMsg_PushEvent, OnPushEventStub)
     IPC_MESSAGE_UNHANDLED(handled = false)
   IPC_END_MESSAGE_MAP()
   // Record all messages directed to inner script context.
   inner_sink_.OnMessageReceived(message);
   return handled;
 }
 
-void EmbeddedWorkerTestHelper::OnSetupMojo(
-    int thread_id,
-    mojom::ServiceWorkerEventDispatcherRequest dispatcher_request) {
-  MockServiceWorkerEventDispatcher::Create(AsWeakPtr(), thread_id,
-                                           std::move(dispatcher_request));
-}
-
 void EmbeddedWorkerTestHelper::OnActivateEvent(int embedded_worker_id,
                                                int request_id) {
   SimulateSend(new ServiceWorkerHostMsg_ActivateEventFinished(
       embedded_worker_id, request_id,
       blink::WebServiceWorkerEventResultCompleted, base::Time::Now()));
 }
 
 void EmbeddedWorkerTestHelper::OnExtendableMessageEvent(
     mojom::ExtendableMessageEventPtr event,
     const mojom::ServiceWorkerEventDispatcher::
@@ skipping 93 matching lines @@
 }
 
 void EmbeddedWorkerTestHelper::SimulateWorkerStarted(int embedded_worker_id) {
   EmbeddedWorkerInstance* worker = registry()->GetWorker(embedded_worker_id);
   ASSERT_TRUE(worker != NULL);
   registry()->OnWorkerStarted(worker->process_id(), embedded_worker_id);
 }
 
 void EmbeddedWorkerTestHelper::SimulateWorkerStopped(int embedded_worker_id) {
   EmbeddedWorkerInstance* worker = registry()->GetWorker(embedded_worker_id);
-  if (worker != NULL)
-    registry()->OnWorkerStopped(worker->process_id(), embedded_worker_id);
+  ASSERT_TRUE(worker != NULL);

[falken, 2017/01/11 06:31:55]

ASSERT_TRUE(worker) or else change to nullptr

[shimazu, 2017/01/12 01:45:24]

Oops, that's not my intended modification. Restored.

+  registry()->OnWorkerStopped(worker->process_id(), embedded_worker_id);
 }
 
 void EmbeddedWorkerTestHelper::SimulateSend(IPC::Message* message) {
   registry()->OnMessageReceived(*message, mock_render_process_id_);
   delete message;
 }
 
 void EmbeddedWorkerTestHelper::OnStartWorkerStub(
-    const EmbeddedWorkerStartParams& params) {
+    const EmbeddedWorkerStartParams& params,
+    mojom::ServiceWorkerEventDispatcherRequest request) {
   EmbeddedWorkerInstance* worker =
       registry()->GetWorker(params.embedded_worker_id);
   ASSERT_TRUE(worker != NULL);
   EXPECT_EQ(EmbeddedWorkerStatus::STARTING, worker->status());
   base::ThreadTaskRunnerHandle::Get()->PostTask(
       FROM_HERE,
       base::Bind(&EmbeddedWorkerTestHelper::OnStartWorker, AsWeakPtr(),
                  params.embedded_worker_id, params.service_worker_version_id,
-                 params.scope, params.script_url, params.pause_after_download));
+                 params.scope, params.script_url, params.pause_after_download,
+                 base::Passed(&request)));
 }
 
 void EmbeddedWorkerTestHelper::OnResumeAfterDownloadStub(
     int embedded_worker_id) {
   EmbeddedWorkerInstance* worker = registry()->GetWorker(embedded_worker_id);
   ASSERT_TRUE(worker);
   base::ThreadTaskRunnerHandle::Get()->PostTask(
       FROM_HERE, base::Bind(&EmbeddedWorkerTestHelper::OnResumeAfterDownload,
                             AsWeakPtr(), embedded_worker_id));
 }
 
-void EmbeddedWorkerTestHelper::OnStopWorkerStub(int embedded_worker_id) {
-  EmbeddedWorkerInstance* worker = registry()->GetWorker(embedded_worker_id);
-  ASSERT_TRUE(worker != NULL);
+void EmbeddedWorkerTestHelper::OnStopWorkerStub(
+    const mojom::EmbeddedWorkerInstanceClient::StopWorkerCallback& callback) {
   base::ThreadTaskRunnerHandle::Get()->PostTask(
       FROM_HERE, base::Bind(&EmbeddedWorkerTestHelper::OnStopWorker,
-                            AsWeakPtr(), embedded_worker_id));
+                            AsWeakPtr(), callback));
 }
 
 void EmbeddedWorkerTestHelper::OnMessageToWorkerStub(
     int thread_id,
     int embedded_worker_id,
     const IPC::Message& message) {
   EmbeddedWorkerInstance* worker = registry()->GetWorker(embedded_worker_id);
   ASSERT_TRUE(worker != NULL);
   EXPECT_EQ(worker->thread_id(), thread_id);
   base::ThreadTaskRunnerHandle::Get()->PostTask(
@@ skipping 76 matching lines @@
                  service_manager::InterfaceProviderSpec());
 
   std::unique_ptr<service_manager::InterfaceProvider> remote_interfaces(
       new service_manager::InterfaceProvider);
   remote_interfaces->Bind(std::move(interfaces));
   rph->SetRemoteInterfaces(std::move(remote_interfaces));
   return registry;
 }
 
 }  // namespace content