Make devtools_window.cc multi-desktop aware.

chrome/browser/devtools/devtools_window.cc

 // Copyright (c) 2012 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 <algorithm>
 
 #include "base/command_line.h"
 #include "base/json/json_writer.h"
 #include "base/lazy_instance.h"
 #include "base/string_number_conversions.h"
 #include "base/stringprintf.h"
 #include "base/utf_string_conversions.h"
 #include "base/values.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/devtools/devtools_window.h"
 #include "chrome/browser/extensions/api/debugger/debugger_api.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/extension_system.h"
 #include "chrome/browser/file_select_helper.h"
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/browser/prefs/scoped_user_pref_update.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/sessions/session_tab_helper.h"
 #include "chrome/browser/themes/theme_service.h"
 #include "chrome/browser/themes/theme_service_factory.h"
 #include "chrome/browser/ui/browser.h"
-#include "chrome/browser/ui/browser_list.h"
+#include "chrome/browser/ui/browser_iterator.h"
 #include "chrome/browser/ui/browser_list_impl.h"
 #include "chrome/browser/ui/browser_window.h"
+#include "chrome/browser/ui/host_desktop.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/common/chrome_notification_types.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/extensions/manifest_url_handler.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/render_messages.h"
 #include "chrome/common/url_constants.h"
 #include "content/public/browser/child_process_security_policy.h"
 #include "content/public/browser/devtools_agent_host.h"
 #include "content/public/browser/devtools_client_host.h"
@@ skipping 368 matching lines @@
   browser_->tab_strip_model()->AddWebContents(
       web_contents_, -1, content::PAGE_TRANSITION_AUTO_TOPLEVEL,
       TabStripModel::ADD_ACTIVE);
 }
 
 bool DevToolsWindow::FindInspectedBrowserAndTabIndex(Browser** browser,
                                                      int* tab) {
   if (!inspected_web_contents_)
     return false;
 
-  bool found = FindInspectedBrowserAndTabIndexFromBrowserList(
-      chrome::BrowserListImpl::GetInstance(chrome::HOST_DESKTOP_TYPE_NATIVE),
-      browser,
-      tab);
-  // On Windows 8 we can have the desktop environment and the ASH environment
-  // active concurrently. If we fail to find the inspected web contents in the
-  // native browser list, then we should look in the ASH browser list.
-#if defined(OS_WIN) && defined(USE_AURA)
-  if (!found) {
-    found = FindInspectedBrowserAndTabIndexFromBrowserList(
-        chrome::BrowserListImpl::GetInstance(chrome::HOST_DESKTOP_TYPE_ASH),
-        browser,
-        tab);
-  }
-#endif
-  return found;
-}
-
-bool DevToolsWindow::FindInspectedBrowserAndTabIndexFromBrowserList(
-    chrome::BrowserListImpl* browser_list,
-    Browser** browser,
-    int* tab) {
-  if (!inspected_web_contents_)
-    return false;
-
-  for (chrome::BrowserListImpl::const_iterator it = browser_list->begin();
-       it != browser_list->end(); ++it) {
-    int tab_index = (*it)->tab_strip_model()->GetIndexOfWebContents(
+  for (chrome::BrowserIterator it; !it.done(); it.Next()) {
+    int tab_index = it->tab_strip_model()->GetIndexOfWebContents(
         inspected_web_contents_);
     if (tab_index != TabStripModel::kNoTab) {
       *browser = *it;
       *tab = tab_index;
       return true;
     }
   }
   return false;
 }
 
@@ skipping 369 matching lines @@
 
 void DevToolsWindow::OpenInNewTab(const std::string& url) {
   OpenURLParams params(GURL(url),
                        content::Referrer(),
                        NEW_FOREGROUND_TAB,
                        content::PAGE_TRANSITION_LINK,
                        false /* is_renderer_initiated */);
   if (inspected_web_contents_) {
     inspected_web_contents_->OpenURL(params);
   } else {
-    for (BrowserList::const_iterator it = BrowserList::begin();
-         it != BrowserList::end(); ++it) {
+    chrome::HostDesktopType host_desktop_type;

[pfeldman, 2013/02/09 10:15:53]

I am not sure I follow the code below.

Note that we never want to open given |url| in the browser_ (the one we created
for the devtools front-end).

[gab, 2013/02/09 17:38:05]

Right, this code does the same thing as the previous code (i.e., find a tabbed
browser (thus not browser_ which I believe to not be tabbed)); the only
difference is it's trying to infer context (desktop type) from browser_ in order
to know on which desktop it should look for a tabbed browser (the previous code
would always look on the native desktop; thus if this was to be called on the
ash desktop, a tabbed browser on the native desktop (potentially not even
visible to the user at that point) would open the URL...

This code seems to assume that there is a tabbed browser available though, is
that always true? It also doesn't try to separate browsers per profile... should
that matter? Perhaps one of the methods in browser_finder.h (e.g.,
FindBrowserWithProfile():
https://code.google.com/p/chromium/codesearch#chrome/src/chrome/browser/ui/br...)
is better suited than this iterator lookup?

+    if (browser_) {
+      host_desktop_type = browser_->host_desktop_type();
+    } else {
+      // There should always be a browser when there are no inspected web
+      // contents.
+      NOTREACHED();
+      host_desktop_type = chrome::GetActiveDesktop();
+    }
+
+    const chrome::BrowserListImpl* browser_list =
+        chrome::BrowserListImpl::GetInstance(host_desktop_type);
+    for (chrome::BrowserListImpl::const_iterator it = browser_list->begin();
+         it != browser_list->end(); ++it) {
       if ((*it)->type() == Browser::TYPE_TABBED) {
         (*it)->OpenURL(params);
         break;
       }
     }
   }
 }
 
 void DevToolsWindow::SaveToFile(const std::string& url,
                                 const std::string& content,
@@ skipping 147 matching lines @@
 
 // static
 DevToolsDockSide DevToolsWindow::SideFromString(
     const std::string& dock_side) {
   if (dock_side == kDockSideRight)
     return DEVTOOLS_DOCK_SIDE_RIGHT;
   if (dock_side == kDockSideBottom)
     return DEVTOOLS_DOCK_SIDE_BOTTOM;
   return DEVTOOLS_DOCK_SIDE_UNDOCKED;
 }