[ServiceWorker] Mojofy InstallEvent of Service Worker

content/browser/service_worker/embedded_worker_test_helper.cc

Index: content/browser/service_worker/embedded_worker_test_helper.cc
diff --git a/content/browser/service_worker/embedded_worker_test_helper.cc b/content/browser/service_worker/embedded_worker_test_helper.cc
index 388d0757841baaf5a3731038a62f5dfd6f9d6be0..b03792c1a0cea084ccd09b885eff3e6dff5b55d8 100644
--- a/content/browser/service_worker/embedded_worker_test_helper.cc
+++ b/content/browser/service_worker/embedded_worker_test_helper.cc
@@ -27,6 +27,7 @@
 #include "content/common/background_fetch/background_fetch_types.h"
 #include "content/common/service_worker/embedded_worker_messages.h"
 #include "content/common/service_worker/embedded_worker_start_params.h"
+#include "content/common/service_worker/service_worker_event_dispatcher.mojom.h"
 #include "content/common/service_worker/service_worker_messages.h"
 #include "content/common/service_worker/service_worker_utils.h"
 #include "content/public/common/push_event_payload.h"
@@ -158,6 +159,14 @@ class EmbeddedWorkerTestHelper::MockServiceWorkerEventDispatcher
 
   ~MockServiceWorkerEventDispatcher() override {}
 
+  void DispatchInstallEvent(
+      mojom::ServiceWorkerInstallEventMethodsAssociatedPtrInfo client,
+      const DispatchInstallEventCallback& callback) override {
+    if (!helper_)
+      return;
+    helper_->OnInstallEventStub(callback);
+  }
+
   void DispatchActivateEvent(
       const DispatchActivateEventCallback& callback) override {
     if (!helper_)
@@ -418,11 +427,6 @@ bool EmbeddedWorkerTestHelper::OnMessageToWorker(int thread_id,
                                                  int embedded_worker_id,
                                                  const IPC::Message& message) {
   bool handled = true;

[leonhsl(Using Gerrit), 2017/04/23 03:33:10]

After install event mojofication this function will become trivial, and some of
its related functions will also become trivial.

Hi, Shimazu-san, can we keep this as is in this CL and do thorough cleanup with
a follow-up CL?

[shimazu, 2017/04/24 05:00:13]

It's great work!
Let's remove no-op functions (inner_sink(), for example) and redundant members
like |inner_sink_| in this patch.

[xiaofengzhang, 2017/04/24 05:44:08]

Hi leon@, shimazu-san@, OnMessageToWorker is still used by
TEST_F(ServiceWorkerVersionTest, DispatchEvent) in
service_worker_version_unittest.cc to received TestMsg_TestEvent, etc.
It's seems better to keeps it or mojofy it in another CL?

[shimazu, 2017/04/24 06:22:18]

I feel separating cleanup work is not preferable because it's likely to keep
some garbage left behind.

Now I checked the code and found the last user of SWVersion::DispatchEvent is
InstallEvent. 
I think SWVersion::DispatchEvent method is also no longer needed, and we can
also remove the tests related to that while we should keep a test for
SWVersion::RegisterRequestCallback because it's used for FetchEvent.

In summary, what we should do here is:
- removing SWVersion::DispatchEvent, EWTestHelper::innser_sink() and
EWTestHelper::OnMessageToWorker.
- removing ServiceWorkerVersionTest::DispatchEvent, ::DispatchConcurrentEvent, 
::DispatchEvent_MultipleResponse and DispatchEventToStoppedWorker
- adding a test for SWVersion::RegisterRequestCallback.

[xiaofengzhang, 2017/04/25 06:01:28]

Done. Thanks a lot for you detailed explanation!

-  current_embedded_worker_id_ = embedded_worker_id;
-  IPC_BEGIN_MESSAGE_MAP(EmbeddedWorkerTestHelper, message)
-    IPC_MESSAGE_HANDLER(ServiceWorkerMsg_InstallEvent, OnInstallEventStub)
-    IPC_MESSAGE_UNHANDLED(handled = false)
-  IPC_END_MESSAGE_MAP()
   // Record all messages directed to inner script context.
   inner_sink_.OnMessageReceived(message);
   return handled;
@@ -473,14 +477,12 @@ void EmbeddedWorkerTestHelper::OnExtendableMessageEvent(
   callback.Run(SERVICE_WORKER_OK, base::Time::Now());
 }
 
-void EmbeddedWorkerTestHelper::OnInstallEvent(int embedded_worker_id,
-                                              int request_id) {
-  // The installing worker may have been doomed and terminated.
-  if (!registry()->GetWorker(embedded_worker_id))
-    return;
-  SimulateSend(new ServiceWorkerHostMsg_InstallEventFinished(
-      embedded_worker_id, request_id,
-      blink::kWebServiceWorkerEventResultCompleted, true, base::Time::Now()));
+void EmbeddedWorkerTestHelper::OnInstallEvent(
+    const mojom::ServiceWorkerEventDispatcher::DispatchInstallEventCallback&
+        callback) {
+  dispatched_events()->push_back(Event::Install);
+  callback.Run(SERVICE_WORKER_OK, true /* has_fetch_handler */,
+               base::Time::Now());
 }
 
 void EmbeddedWorkerTestHelper::OnFetchEvent(
@@ -735,11 +737,12 @@ void EmbeddedWorkerTestHelper::OnExtendableMessageEventStub(
                             AsWeakPtr(), base::Passed(&event), callback));
 }
 
-void EmbeddedWorkerTestHelper::OnInstallEventStub(int request_id) {
+void EmbeddedWorkerTestHelper::OnInstallEventStub(
+    const mojom::ServiceWorkerEventDispatcher::DispatchInstallEventCallback&
+        callback) {
   base::ThreadTaskRunnerHandle::Get()->PostTask(
-      FROM_HERE,
-      base::Bind(&EmbeddedWorkerTestHelper::OnInstallEvent, AsWeakPtr(),
-                 current_embedded_worker_id_, request_id));
+      FROM_HERE, base::Bind(&EmbeddedWorkerTestHelper::OnInstallEvent,
+                            AsWeakPtr(), callback));
 }
 
 void EmbeddedWorkerTestHelper::OnFetchEventStub(