[ServiceWorker] Add EmbeddedWorkerInstanceHost Interface.

content/browser/service_worker/service_worker_version_unittest.cc

Index: content/browser/service_worker/service_worker_version_unittest.cc
diff --git a/content/browser/service_worker/service_worker_version_unittest.cc b/content/browser/service_worker/service_worker_version_unittest.cc
index b34a5b03aed94ab57cd1bfcb753f360b795931de..08aa0d18b0ab8d95ad4c059a8efb366fd8220bf6 100644
--- a/content/browser/service_worker/service_worker_version_unittest.cc
+++ b/content/browser/service_worker/service_worker_version_unittest.cc
@@ -292,15 +292,19 @@ class MessageReceiverDisallowStart : public MessageReceiver {
 
   enum class StartMode { STALL, FAIL, SUCCEED };
 
-  void 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) override {
+  void 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,
+                     mojom::EmbeddedWorkerInstanceHostAssociatedPtrInfo
+                         instance_host) override {
     switch (mode_) {
       case StartMode::STALL:
+        // Prepare for OnStopWorker().
+        instance_host_ptr_map_[embedded_worker_id].Bind(
+            std::move(instance_host));

[shimazu, 2017/04/10 05:09:22]

What happen if the |instance_host| isn't kept?
It's a test of disallowing StartWorker, so I assume |instance_host| might be
unbound state and OnStopWorker wouldn't be called.

[leonhsl(Using Gerrit), 2017/04/10 06:10:36]

In the following 2 tests OnStopWorker() will be called:
ServiceWorkerFailToStartTest.RestartStalledWorker
  It firstly sets STALL mode to make the first OnStartWorker() to fail, and then
sets SUCCEED mode and stops the worker, this will make OnStopWorker() here to be
called, and the test wants to verify that the worker can be restarted again
successfully.

ServiceWorkerFailToStartTest.Timeout
  It sets STALL mode to make the OnStartWorker() to fail, and then simulate
triggering the timeout timer function, which will stop the worker, thus
OnStopWorker() here will be called.

So we need to bind |instance_host| here for preparation.

[shimazu, 2017/04/10 09:33:40]

Ah, got it. Thanks!
I feel code is a bit duplicated because MessageReceiverDisallowStart has its own
map though EWTestHelper has it. It's also weird that
ServiceWorkerFailToStartTest.RestartStalledWorker actually doesn't want to fail
StartWorker but want to control the order of replies which are conveyed via
EWHost interface.

I don't have good idea for now but I think we might be able to make the tests
simpler. I'd like to investigate more tomorrow.

[shimazu, 2017/04/11 04:32:40]

Sorry for late!
The reason why I feel a bit weird might be that EWTestHelper originally
simulated IPCs by calling Simulate~~~ with stepping through On~~~ implemented in
EWTestHelper, but it could be decoupled from EWTestHelper if these things are
moved to MockEWInstanceClient. It'll be big change, so I think this test is good
as is.

         break;  // Do nothing.
       case StartMode::FAIL:
         ASSERT_EQ(current_mock_instance_index_ + 1,
@@ -311,17 +315,31 @@ class MessageReceiverDisallowStart : public MessageReceiver {
       case StartMode::SUCCEED:
         MessageReceiver::OnStartWorker(
             embedded_worker_id, service_worker_version_id, scope, script_url,
-            pause_after_download, std::move(request));
+            pause_after_download, std::move(request), std::move(instance_host));
         break;
     }
     current_mock_instance_index_++;
   }
 
+  void OnStopWorker(int embedded_worker_id) override {
+    if (instance_host_ptr_map_[embedded_worker_id]) {
+      instance_host_ptr_map_[embedded_worker_id]->OnStopped();
+      base::RunLoop().RunUntilIdle();
+      return;
+    }
+    EmbeddedWorkerTestHelper::OnStopWorker(embedded_worker_id);
+  }
+
   void set_start_mode(StartMode mode) { mode_ = mode; }
 
  private:
   uint32_t current_mock_instance_index_ = 0;
   StartMode mode_ = StartMode::STALL;
+
+  std::map<
+      int /* embedded_worker_id */,
+      mojom::EmbeddedWorkerInstanceHostAssociatedPtr /* instance_host_ptr */>
+      instance_host_ptr_map_;
   DISALLOW_COPY_AND_ASSIGN(MessageReceiverDisallowStart);
 };
 
@@ -350,19 +368,13 @@ class NoOpStopWorkerEmbeddedWorkerInstanceClient
       base::WeakPtr<EmbeddedWorkerTestHelper> helper)
       : EmbeddedWorkerTestHelper::MockEmbeddedWorkerInstanceClient(helper) {}
   ~NoOpStopWorkerEmbeddedWorkerInstanceClient() override {
-    // Stop callback should be called once.
-    if (pending_stop_callback_)
-      pending_stop_callback_.Run();
   }
 
  protected:
-  void StopWorker(const StopWorkerCallback& callback) override {
-    pending_stop_callback_ = std::move(callback);
+  void StopWorker() override {
     // Do nothing.
   }
 
-  StopWorkerCallback pending_stop_callback_;
-
  private:
   DISALLOW_COPY_AND_ASSIGN(NoOpStopWorkerEmbeddedWorkerInstanceClient);
 };
@@ -375,9 +387,7 @@ class MessageReceiverDisallowStop : public MessageReceiver {
   }
   ~MessageReceiverDisallowStop() override {}
 
-  void OnStopWorker(
-      const mojom::EmbeddedWorkerInstanceClient::StopWorkerCallback& callback)
-      override {
+  void OnStopWorker(int embedded_worker_id) override {
     // Do nothing.
   }
 
@@ -830,11 +840,11 @@ class MessageReceiverControlEvents : public MessageReceiver {
     extendable_message_event_callback_ = callback;
   }
 
-  void OnStopWorker(
-      const mojom::EmbeddedWorkerInstanceClient::StopWorkerCallback& callback)
-      override {
+  void OnStopWorker(int embedded_worker_id) override {
     EXPECT_FALSE(stop_worker_callback_);
-    stop_worker_callback_ = callback;
+    stop_worker_callback_ =
+        base::Bind(&MessageReceiverControlEvents::SimulateWorkerStopped,
+                   base::Unretained(this), embedded_worker_id);
   }
 
   const mojom::ServiceWorkerEventDispatcher::
@@ -843,15 +853,12 @@ class MessageReceiverControlEvents : public MessageReceiver {
     return extendable_message_event_callback_;
   }
 
-  const mojom::EmbeddedWorkerInstanceClient::StopWorkerCallback&
-  stop_worker_callback() {
-    return stop_worker_callback_;
-  }
+  const base::Closure& stop_worker_callback() { return stop_worker_callback_; }
 
  private:
   mojom::ServiceWorkerEventDispatcher::DispatchExtendableMessageEventCallback
       extendable_message_event_callback_;
-  mojom::EmbeddedWorkerInstanceClient::StopWorkerCallback stop_worker_callback_;
+  base::Closure stop_worker_callback_;
 };
 
 class ServiceWorkerRequestTimeoutTest : public ServiceWorkerVersionTest {
@@ -869,8 +876,7 @@ class ServiceWorkerRequestTimeoutTest : public ServiceWorkerVersionTest {
         ->extendable_message_event_callback();
   }
 
-  const mojom::EmbeddedWorkerInstanceClient::StopWorkerCallback&
-  stop_worker_callback() {
+  const base::Closure& stop_worker_callback() {
     return static_cast<MessageReceiverControlEvents*>(helper_.get())
         ->stop_worker_callback();
   }