[DevTools] Remove some usages of route_id for shared workers

This is a preparation step to migrate DevTools for shared workers to Mojo.
Drive-by: rewrite the test to use new DevToolsAgentHostObserver.

Bug: 776009
Change-Id: Ia1f3f4952e8d8ac834a36f94c680cb81e7c50e0e
Reviewed-on: https://chromium-review.googlesource.com/777740
Commit-Queue: Dmitry Gozman <dgozman@chromium.org>
Reviewed-by: Alexei Filippov <alph@chromium.org>
Reviewed-by: Tsuyoshi Horo <horo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#517832}

chrome/browser/devtools/devtools_sanity_browsertest.cc

@@ -96,6 +96,7 @@ using app_modal::JavaScriptAppModalDialog;
 using app_modal::NativeAppModalDialog;
 using content::BrowserThread;
 using content::DevToolsAgentHost;
+using content::DevToolsAgentHostObserver;
 using content::NavigationController;
 using content::RenderFrameHost;
 using content::RenderViewHost;
@@ -659,116 +660,53 @@ class WorkerDevToolsSanityTest : public InProcessBrowserTest {
   WorkerDevToolsSanityTest() : window_(NULL) {}
 
  protected:
-  class WorkerData : public base::RefCountedThreadSafe<WorkerData> {
+  class WorkerCreationObserver : public DevToolsAgentHostObserver {
    public:
-    WorkerData() : worker_process_id(0), worker_route_id(0) {}
-    int worker_process_id;
-    int worker_route_id;
-
-   private:
-    friend class base::RefCountedThreadSafe<WorkerData>;
-    ~WorkerData() {}
-  };
-
-  class WorkerCreationObserver : public WorkerServiceObserver {
-   public:
-    explicit WorkerCreationObserver(const std::string& path,
-                                    WorkerData* worker_data)
-        : path_(path), worker_data_(worker_data) {}
+    WorkerCreationObserver(const std::string& path,
+                           scoped_refptr<DevToolsAgentHost>* out_host,
+                           base::Closure quit)
+        : path_(path), out_host_(out_host), quit_(quit) {
+      DevToolsAgentHost::AddObserver(this);
+    }
 
    private:
-    ~WorkerCreationObserver() override {}
-
-    void WorkerCreated(const GURL& url,
-                       const std::string& name,
-                       int process_id,
-                       int route_id) override {
-      if (url.path().rfind(path_) == std::string::npos)
-        return;
-      worker_data_->worker_process_id = process_id;
-      worker_data_->worker_route_id = route_id;
-      WorkerService::GetInstance()->RemoveObserver(this);
-      BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
-                              base::MessageLoop::QuitWhenIdleClosure());
-      delete this;
+    ~WorkerCreationObserver() override {
+      DevToolsAgentHost::RemoveObserver(this);
     }
-    std::string path_;
-    scoped_refptr<WorkerData> worker_data_;
-  };
 
-  class WorkerTerminationObserver : public WorkerServiceObserver {
-   public:
-    explicit WorkerTerminationObserver(WorkerData* worker_data)
-        : worker_data_(worker_data) {
+    void DevToolsAgentHostCreated(DevToolsAgentHost* host) override {
+      if (host->GetType() == DevToolsAgentHost::kTypeSharedWorker &&
+          host->GetURL().path().rfind(path_) != std::string::npos) {
+        *out_host_ = host;
+        BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, quit_);
+        delete this;
+      }
     }
 
-   private:
-    ~WorkerTerminationObserver() override {}
-
-    void WorkerDestroyed(int process_id, int route_id) override {
-      ASSERT_EQ(worker_data_->worker_process_id, process_id);
-      ASSERT_EQ(worker_data_->worker_route_id, route_id);
-      WorkerService::GetInstance()->RemoveObserver(this);
-      BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
-                              base::MessageLoop::QuitWhenIdleClosure());
-      delete this;
-    }
-    scoped_refptr<WorkerData> worker_data_;
+    std::string path_;
+    scoped_refptr<DevToolsAgentHost>* out_host_;
+    base::Closure quit_;
   };
 
-  void RunTest(const char* test_name,
-               const char* test_page,
-               const char* worker_path) {
-    ASSERT_TRUE(spawned_test_server()->Start());
-    GURL url = spawned_test_server()->GetURL(test_page);
-    ui_test_utils::NavigateToURL(browser(), url);
-
-    scoped_refptr<WorkerData> worker_data =
-        WaitForFirstSharedWorker(worker_path);
-    OpenDevToolsWindowForSharedWorker(worker_data.get());
-    RunTestFunction(window_, test_name);
-    CloseDevToolsWindow();
-  }
-
-  static void TerminateWorker(scoped_refptr<WorkerData> worker_data) {
-    if (!WorkerService::GetInstance()->TerminateWorker(
-        worker_data->worker_process_id, worker_data->worker_route_id))
-      FAIL() << "Failed to terminate worker.\n";
-
-    WorkerService::GetInstance()->AddObserver(
-        new WorkerTerminationObserver(worker_data.get()));
-
-    content::RunMessageLoop();
-  }
-
-  static scoped_refptr<WorkerData> WaitForFirstSharedWorker(const char* path) {
-    scoped_refptr<WorkerData> worker_data(new WorkerData());
-
-    std::vector<WorkerService::WorkerInfo> worker_info =
-        WorkerService::GetInstance()->GetWorkers();
-    for (size_t i = 0; i < worker_info.size(); i++) {
-      if (worker_info[i].url.path().rfind(path) == std::string::npos)
-        continue;
-      worker_data->worker_process_id = worker_info[0].process_id;
-      worker_data->worker_route_id = worker_info[0].route_id;
-      return worker_data;
+  static scoped_refptr<DevToolsAgentHost> WaitForFirstSharedWorker(
+      const char* path) {
+    for (auto& host : DevToolsAgentHost::GetOrCreateAll()) {
+      if (host->GetType() == DevToolsAgentHost::kTypeSharedWorker &&
+          host->GetURL().path().rfind(path) != std::string::npos) {
+        return host;
+      }
     }
-
-    WorkerService::GetInstance()->AddObserver(
-        new WorkerCreationObserver(path, worker_data.get()));
-
-    content::RunMessageLoop();
-    return worker_data;
+    scoped_refptr<DevToolsAgentHost> host;
+    base::RunLoop run_loop;
+    new WorkerCreationObserver(path, &host, run_loop.QuitWhenIdleClosure());
+    content::RunThisRunLoop(&run_loop);
+    return host;
   }
 
-  void OpenDevToolsWindowForSharedWorker(WorkerData* worker_data) {
+  void OpenDevToolsWindow(scoped_refptr<DevToolsAgentHost> agent_host) {
     Profile* profile = browser()->profile();
-    scoped_refptr<DevToolsAgentHost> agent_host(
-        DevToolsAgentHost::GetForWorker(
-            worker_data->worker_process_id,
-            worker_data->worker_route_id));
-    window_ = DevToolsWindowTesting::OpenDevToolsWindowForWorkerSync(
-        profile, agent_host.get());
+    window_ =
+        DevToolsWindowTesting::OpenDevToolsWindowSync(profile, agent_host);
   }
 
   void CloseDevToolsWindow() {
@@ -1893,7 +1831,15 @@ IN_PROC_BROWSER_TEST_F(DevToolsSanityTest, SecondTabAfterDevTools) {
 }
 
 IN_PROC_BROWSER_TEST_F(WorkerDevToolsSanityTest, InspectSharedWorker) {
-  RunTest("testSharedWorker", kSharedWorkerTestPage, kSharedWorkerTestWorker);
+  ASSERT_TRUE(spawned_test_server()->Start());
+  GURL url = spawned_test_server()->GetURL(kSharedWorkerTestPage);
+  ui_test_utils::NavigateToURL(browser(), url);
+
+  scoped_refptr<DevToolsAgentHost> host =
+      WaitForFirstSharedWorker(kSharedWorkerTestWorker);
+  OpenDevToolsWindow(host);
+  RunTestFunction(window_, "testSharedWorker");
+  CloseDevToolsWindow();
 }
 
 // Flaky on multiple platforms. See http://crbug.com/432444
@@ -1903,15 +1849,15 @@ IN_PROC_BROWSER_TEST_F(WorkerDevToolsSanityTest,
   GURL url = spawned_test_server()->GetURL(kReloadSharedWorkerTestPage);
   ui_test_utils::NavigateToURL(browser(), url);
 
-  scoped_refptr<WorkerData> worker_data =
+  scoped_refptr<DevToolsAgentHost> host =
       WaitForFirstSharedWorker(kReloadSharedWorkerTestWorker);
-  OpenDevToolsWindowForSharedWorker(worker_data.get());
+  OpenDevToolsWindow(host);
 
   // We should make sure that the worker inspector has loaded before
   // terminating worker.
   RunTestFunction(window_, "testPauseInSharedWorkerInitialization1");
 
-  TerminateWorker(worker_data);
+  host->Close();
 
   // Reload page to restart the worker.
   ui_test_utils::NavigateToURL(browser(), url);

chrome/browser/devtools/devtools_window.h

@@ -102,11 +102,6 @@ class DevToolsWindow : public DevToolsUIBindings::Delegate,
   // Node frontend is always undocked.
   static void OpenNodeFrontendWindow(Profile* profile);
 
-  // Worker frontend is always undocked.
-  static void OpenDevToolsWindowForWorker(
-      Profile* profile,
-      const scoped_refptr<content::DevToolsAgentHost>& worker_agent);
-
   static void InspectElement(content::RenderFrameHost* inspected_frame_host,
                              int x,
                              int y);
@@ -214,6 +209,9 @@ class DevToolsWindow : public DevToolsUIBindings::Delegate,
   static void OpenDevToolsWindowForFrame(
       Profile* profile,
       const scoped_refptr<content::DevToolsAgentHost>& agent_host);
+  static void OpenDevToolsWindowForWorker(
+      Profile* profile,
+      const scoped_refptr<content::DevToolsAgentHost>& worker_agent);
 
   // DevTools lifecycle typically follows this way:
   // - Toggle/Open: client call;

chrome/browser/devtools/devtools_window_testing.cc

@@ -132,11 +132,11 @@ DevToolsWindow* DevToolsWindowTesting::OpenDevToolsWindowSync(
 }
 
 // static
-DevToolsWindow* DevToolsWindowTesting::OpenDevToolsWindowForWorkerSync(
-    Profile* profile, content::DevToolsAgentHost* worker_agent) {
-  DevToolsWindow::OpenDevToolsWindowForWorker(
-      profile, worker_agent);
-  DevToolsWindow* window = DevToolsWindow::FindDevToolsWindow(worker_agent);
+DevToolsWindow* DevToolsWindowTesting::OpenDevToolsWindowSync(
+    Profile* profile,
+    scoped_refptr<content::DevToolsAgentHost> agent_host) {
+  DevToolsWindow::OpenDevToolsWindow(agent_host, profile);
+  DevToolsWindow* window = DevToolsWindow::FindDevToolsWindow(agent_host.get());
   WaitForDevToolsWindowLoad(window);
   return window;
 }

chrome/browser/devtools/devtools_window_testing.h

@@ -29,8 +29,9 @@ class DevToolsWindowTesting {
       bool is_docked);
   static DevToolsWindow* OpenDevToolsWindowSync(
       Browser* browser, bool is_docked);
-  static DevToolsWindow* OpenDevToolsWindowForWorkerSync(
-      Profile* profile, content::DevToolsAgentHost* worker_agent);
+  static DevToolsWindow* OpenDevToolsWindowSync(
+      Profile* profile,
+      scoped_refptr<content::DevToolsAgentHost> agent_host);
 
   // Closes the window like it was user-initiated.
   static void CloseDevToolsWindow(DevToolsWindow* window);

content/browser/devtools/devtools_agent_host_impl.cc

@@ -86,21 +86,6 @@ DevToolsAgentHost::List DevToolsAgentHost::GetOrCreateAll() {
   return result;
 }
 
-// Called on the UI thread.
-// static
-scoped_refptr<DevToolsAgentHost> DevToolsAgentHost::GetForWorker(
-    int worker_process_id,
-    int worker_route_id) {
-  if (scoped_refptr<DevToolsAgentHost> host =
-      SharedWorkerDevToolsManager::GetInstance()
-          ->GetDevToolsAgentHostForWorker(worker_process_id,
-                                          worker_route_id)) {
-    return host;
-  }
-  return ServiceWorkerDevToolsManager::GetInstance()
-      ->GetDevToolsAgentHostForWorker(worker_process_id, worker_route_id);
-}
-
 DevToolsAgentHostImpl::DevToolsAgentHostImpl(const std::string& id) : id_(id) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 }

content/browser/devtools/protocol/service_worker_handler.cc

@@ -12,6 +12,7 @@
 #include "content/browser/background_sync/background_sync_manager.h"
 #include "content/browser/devtools/service_worker_devtools_agent_host.h"
 #include "content/browser/devtools/service_worker_devtools_manager.h"
+#include "content/browser/devtools/shared_worker_devtools_manager.h"
 #include "content/browser/frame_host/frame_tree.h"
 #include "content/browser/frame_host/frame_tree_node.h"
 #include "content/browser/service_worker/embedded_worker_status.h"
@@ -399,8 +400,9 @@ void ServiceWorkerHandler::OnWorkerVersionUpdated(
       } else if (client.second.type ==
                  SERVICE_WORKER_PROVIDER_FOR_SHARED_WORKER) {
         scoped_refptr<DevToolsAgentHost> agent_host(
-            DevToolsAgentHost::GetForWorker(client.second.process_id,
-                                            client.second.route_id));
+            SharedWorkerDevToolsManager::GetInstance()
+                ->GetDevToolsAgentHostForWorker(client.second.process_id,
+                                                client.second.route_id));
         if (agent_host)
           client_set.insert(agent_host->GetId());
       }

content/browser/devtools/shared_worker_devtools_agent_host.cc

@@ -11,16 +11,6 @@
 
 namespace content {
 
-namespace {
-
-void TerminateSharedWorkerOnIO(
-    WorkerDevToolsAgentHost::WorkerId worker_id) {
-  SharedWorkerServiceImpl::GetInstance()->TerminateWorker(
-      worker_id.first, worker_id.second);
-}
-
-}  // namespace
-
 SharedWorkerDevToolsAgentHost::SharedWorkerDevToolsAgentHost(
     WorkerId worker_id,
     const SharedWorkerInstance& shared_worker)
@@ -49,9 +39,8 @@ void SharedWorkerDevToolsAgentHost::Reload() {
 }
 
 bool SharedWorkerDevToolsAgentHost::Close() {
-  BrowserThread::PostTask(
-      BrowserThread::IO, FROM_HERE,
-      base::BindOnce(&TerminateSharedWorkerOnIO, worker_id()));
+  SharedWorkerServiceImpl::GetInstance()->TerminateWorker(worker_id().first,
+                                                          worker_id().second);
   return true;
 }
 

content/browser/shared_worker/shared_worker_service_impl.h

@@ -41,8 +41,10 @@ class CONTENT_EXPORT SharedWorkerServiceImpl : public WorkerService {
   // Returns the SharedWorkerServiceImpl singleton.
   static SharedWorkerServiceImpl* GetInstance();
 
+  // Terminates the given worker. Returns true if the process was found.
+  bool TerminateWorker(int process_id, int route_id);
+
   // WorkerService implementation:
-  bool TerminateWorker(int process_id, int route_id) override;
   void TerminateAllWorkersForTesting(base::OnceClosure callback) override;
   std::vector<WorkerInfo> GetWorkers() override;
   void AddObserver(WorkerServiceObserver* observer) override;

content/public/browser/devtools_agent_host.h

@@ -63,11 +63,6 @@ class CONTENT_EXPORT DevToolsAgentHost
   // does exist.
   static bool HasFor(WebContents* web_contents);
 
-  // Returns DevToolsAgentHost that can be used for inspecting shared worker
-  // with given worker process host id and routing id.
-  static scoped_refptr<DevToolsAgentHost> GetForWorker(int worker_process_id,
-                                                       int worker_route_id);
-
   // Creates DevToolsAgentHost that communicates to the target by means of
   // provided |delegate|. |delegate| ownership is passed to the created agent
   // host.

content/public/browser/worker_service.h

@@ -26,9 +26,6 @@ class CONTENT_EXPORT WorkerService {
   // Returns the WorkerService singleton.
   static WorkerService* GetInstance();
 
-  // Terminates the given worker. Returns true if the process was found.
-  virtual bool TerminateWorker(int process_id, int route_id) = 0;
-
   // Terminates all workers and notifies when complete. This is used for
   // testing when it is important to make sure that all shared worker activity
   // has stopped.