ServiceWorker: keep dispatcher hosts for unittests in the test helper

content/browser/service_worker/embedded_worker_test_helper.h

 // 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.
 
 #ifndef CONTENT_BROWSER_SERVICE_WORKER_EMBEDDED_WORKER_TEST_HELPER_H_
 #define CONTENT_BROWSER_SERVICE_WORKER_EMBEDDED_WORKER_TEST_HELPER_H_
 
 #include <stdint.h>
 
 #include <map>
@@ skipping 101 matching lines @@
   bool Send(IPC::Message* message) override;
 
   // IPC::Listener implementation.
   bool OnMessageReceived(const IPC::Message& msg) override;
 
   // Register a mojo endpoint object derived from
   // MockEmbeddedWorkerInstanceClient.
   void RegisterMockInstanceClient(
       std::unique_ptr<MockEmbeddedWorkerInstanceClient> client);
 
+  // Registers the dispatcher host for the process to a map managed by this test
+  // helper. If there is a existing dispatcher host, it'll removed before adding
+  // to the map. This should be called before ServiceWorkerDispatcherHost::Init
+  // because it internally calls ServiceWorkerContextCore::AddDispatcherHost.
+  void RegisterMockDispatcherHost(

[falken, 2017/06/12 06:37:32]

I think we don't need to say Mock since it takes a real
ServiceWorkerDispatcherHost.

Add a comment that if a nullptr is passed, it removes the existing dispatcher
host from the map.

[shimazu, 2017/06/12 06:51:08]

I prefer saying Mock here because unittests should treat the dispatcher host
registered from here as a mock even though the type is SWDispatcherHost. I
thought having "Mock" in the method name clarifies how the dispatcher host
should be handled in the test. 
WDYT? 
That seems great. Added.

[falken, 2017/06/12 07:08:20]

I think it's confusing because all the other occurrences of "Mock" in this file
are about a type with "Mock" in the name. If I saw MockDispatcherHost I would
first think I need to pass a MockServiceWorkerDispatcherHost to it. That seems
to be the convention in most of the codebase.

But maybe I don't understand what you mean by the unittests should treat the
dispatcher host as a mock.

[shimazu, 2017/06/12 08:04:07]

I see, I agree with that. 
I just thought "Mock" might be good to indicate it's different from the usual
path, but now I think it's clear because this is the method for testing helper.
Updated.

+      int process_id,
+      scoped_refptr<ServiceWorkerDispatcherHost> dispatcher_host);
+
   template <typename MockType, typename... Args>
   MockType* CreateAndRegisterMockInstanceClient(Args&&... args);
 
   // IPC sink for EmbeddedWorker messages.
   IPC::TestSink* ipc_sink() { return &sink_; }
 
   std::vector<Event>* dispatched_events() { return &events_; }
 
   std::vector<std::unique_ptr<MockEmbeddedWorkerInstanceClient>>*
   mock_instance_clients() {
@@ skipping 233 matching lines @@
   std::unique_ptr<MockType> mock =
       base::MakeUnique<MockType>(std::forward<Args>(args)...);
   MockType* mock_rawptr = mock.get();
   RegisterMockInstanceClient(std::move(mock));
   return mock_rawptr;
 }
 
 }  // namespace content
 
 #endif  // CONTENT_BROWSER_SERVICE_WORKER_EMBEDDED_WORKER_TEST_HELPER_H_