Remove Browser dependency in ExtensionFunctionDispatcher

Part 2/4 for chrome.tabs support for non browser windows.

BUG=115532
TEST=All browser tests pass.

Review URL: http://codereview.chromium.org/10021071

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@135562 0039d316-1c4b-4281-b951-d872f2087c98

14 files changed, 108 insertions, 79 deletions

diff --git a/chrome/browser/extensions/browser_extension_window_controller.cc b/chrome/browser/extensions/browser_extension_window_controller.cc
index e0d6a0b..ed647bd 100644
--- a/chrome/browser/extensions/browser_extension_window_controller.cc
+++ b/chrome/browser/extensions/browser_extension_window_controller.cc
@@ -48,12 +48,11 @@ BrowserExtensionWindowController::CreateWindowValueWithTabs() const {
   return result;
 }
 
-bool BrowserExtensionWindowController::CanClose(
-    ExtensionWindowController::Reason* reason) const {
+bool BrowserExtensionWindowController::CanClose(Reason* reason) const {
   // Don't let an extension remove the window if the user is dragging tabs
   // in that window.
   if (!browser_->IsTabStripEditable()) {
-    *reason = ExtensionWindowController::REASON_TAB_STRIP_NOT_EDITABLE;
+    *reason = ExtensionWindowController::REASON_NOT_EDITABLE;
     return false;
   }
   return true;
@@ -65,3 +64,7 @@ void BrowserExtensionWindowController::SetFullscreenMode(
   if (browser_->window()->IsFullscreen() != is_fullscreen)
     browser_->ToggleFullscreenModeWithExtension(extension_url);
 }
+
+Browser* BrowserExtensionWindowController::GetBrowser() const {
+  return browser_;
+}
diff --git a/chrome/browser/extensions/browser_extension_window_controller.h b/chrome/browser/extensions/browser_extension_window_controller.h
index 2ef1e99..543d54f 100644
--- a/chrome/browser/extensions/browser_extension_window_controller.h
+++ b/chrome/browser/extensions/browser_extension_window_controller.h
@@ -22,6 +22,7 @@ class BrowserExtensionWindowController : public ExtensionWindowController {
   virtual bool CanClose(Reason* reason) const OVERRIDE;
   virtual void SetFullscreenMode(bool is_fullscreen,
                                  const GURL& extension_url) const OVERRIDE;
+  virtual Browser* GetBrowser() const OVERRIDE;
 
  private:
   Browser* browser_;
diff --git a/chrome/browser/extensions/extension_function.cc b/chrome/browser/extensions/extension_function.cc
index ef09e34..6f77fa8 100644
--- a/chrome/browser/extensions/extension_function.cc
+++ b/chrome/browser/extensions/extension_function.cc
@@ -8,8 +8,11 @@
 #include "base/logging.h"
 #include "chrome/browser/extensions/extension_function_dispatcher.h"
 #include "chrome/browser/extensions/extension_service.h"
+#include "chrome/browser/extensions/extension_window_controller.h"
+#include "chrome/browser/extensions/extension_window_list.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/renderer_host/chrome_render_message_filter.h"
+#include "chrome/browser/ui/browser_list.h"
 #include "chrome/common/extensions/extension_messages.h"
 #include "content/public/browser/notification_source.h"
 #include "content/public/browser/notification_types.h"
@@ -178,8 +181,49 @@ void UIThreadExtensionFunction::SetRenderViewHost(
       new RenderViewHostTracker(this, render_view_host) : NULL);
 }
 
+// TODO(stevenjb): Replace this with GetExtensionWindowController().
 Browser* UIThreadExtensionFunction::GetCurrentBrowser() {
-  return dispatcher()->GetCurrentBrowser(render_view_host_, include_incognito_);
+  // If the delegate has an associated browser, return it.
+  ExtensionWindowController* window_controller =
+      dispatcher()->delegate()->GetExtensionWindowController();
+  if (window_controller) {
+    Browser* browser = window_controller->GetBrowser();
+    if (browser)
+      return browser;
+  }
+
+  // Otherwise, try to default to a reasonable browser. If |include_incognito_|
+  // is true, we will also search browsers in the incognito version of this
+  // profile. Note that the profile may already be incognito, in which case
+  // we will search the incognito version only, regardless of the value of
+  // |include_incognito|.
+  Profile* profile = Profile::FromBrowserContext(
+      render_view_host_->GetProcess()->GetBrowserContext());
+  Browser* browser = BrowserList::FindAnyBrowser(profile, include_incognito_);
+
+  // NOTE(rafaelw): This can return NULL in some circumstances. In particular,
+  // a background_page onload chrome.tabs api call can make it into here
+  // before the browser is sufficiently initialized to return here.
+  // A similar situation may arise during shutdown.
+  // TODO(rafaelw): Delay creation of background_page until the browser
+  // is available. http://code.google.com/p/chromium/issues/detail?id=13284
+  return browser;
+}
+
+ExtensionWindowController*
+UIThreadExtensionFunction::GetExtensionWindowController() {
+  // If the delegate has an associated window controller, return it.
+  ExtensionWindowController* window_controller =
+      dispatcher()->delegate()->GetExtensionWindowController();
+  if (window_controller)
+    return window_controller;
+
+  Profile* profile = Profile::FromBrowserContext(
+      render_view_host_->GetProcess()->GetBrowserContext());
+  ExtensionWindowList::ProfileMatchType match_type = include_incognito_
+      ? ExtensionWindowController::MATCH_INCOGNITO
+      : ExtensionWindowController::MATCH_NORMAL_ONLY;
+  return ExtensionWindowList::GetInstance()->CurrentWindow(profile, match_type);
 }
 
 void UIThreadExtensionFunction::SendResponse(bool success) {
diff --git a/chrome/browser/extensions/extension_function.h b/chrome/browser/extensions/extension_function.h
index 0f9a6fa..d943a31b 100644
--- a/chrome/browser/extensions/extension_function.h
+++ b/chrome/browser/extensions/extension_function.h
@@ -27,6 +27,7 @@ class Browser;
 class ChromeRenderMessageFilter;
 class ExtensionFunction;
 class ExtensionFunctionDispatcher;
+class ExtensionWindowController;
 class UIThreadExtensionFunction;
 class IOThreadExtensionFunction;
 class Profile;
@@ -277,8 +278,13 @@ class UIThreadExtensionFunction : public ExtensionFunction {
   // This method can return NULL if there is no matching browser, which can
   // happen if only incognito windows are open, or early in startup or shutdown
   // shutdown when there are no active windows.
+  //
+  // TODO(stevenjb): Replace this with GetExtensionWindowController().
   Browser* GetCurrentBrowser();
 
+  // Same as above but uses ExtensionWindowList instead of BrowserList.
+  ExtensionWindowController* GetExtensionWindowController();
+
  protected:
   friend struct content::BrowserThread::DeleteOnThread<
       content::BrowserThread::UI>;
diff --git a/chrome/browser/extensions/extension_function_dispatcher.cc b/chrome/browser/extensions/extension_function_dispatcher.cc
index 29f0129..f86d997 100644
--- a/chrome/browser/extensions/extension_function_dispatcher.cc
+++ b/chrome/browser/extensions/extension_function_dispatcher.cc
@@ -22,8 +22,6 @@
 #include "chrome/browser/external_protocol/external_protocol_handler.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/renderer_host/chrome_render_message_filter.h"
-#include "chrome/browser/ui/browser_list.h"
-#include "chrome/browser/ui/browser_window.h"
 #include "chrome/common/extensions/api/extension_api.h"
 #include "chrome/common/extensions/extension_messages.h"
 #include "chrome/common/extensions/extension_set.h"
@@ -165,32 +163,6 @@ ExtensionFunctionDispatcher::ExtensionFunctionDispatcher(Profile* profile,
 ExtensionFunctionDispatcher::~ExtensionFunctionDispatcher() {
 }
 
-Browser* ExtensionFunctionDispatcher::GetCurrentBrowser(
-    RenderViewHost* render_view_host, bool include_incognito) {
-  Browser* browser = delegate_->GetBrowser();
-
-  // If the delegate has an associated browser, that is always the right answer.
-  if (browser)
-    return browser;
-
-  // Otherwise, try to default to a reasonable browser. If |include_incognito|
-  // is true, we will also search browsers in the incognito version of this
-  // profile. Note that the profile may already be incognito, in which case
-  // we will search the incognito version only, regardless of the value of
-  // |include_incognito|.
-  Profile* profile = Profile::FromBrowserContext(
-      render_view_host->GetProcess()->GetBrowserContext());
-  browser = BrowserList::FindAnyBrowser(profile, include_incognito);
-
-  // NOTE(rafaelw): This can return NULL in some circumstances. In particular,
-  // a background_page onload chrome.tabs api call can make it into here
-  // before the browser is sufficiently initialized to return here.
-  // A similar situation may arise during shutdown.
-  // TODO(rafaelw): Delay creation of background_page until the browser
-  // is available. http://code.google.com/p/chromium/issues/detail?id=13284
-  return browser;
-}
-
 void ExtensionFunctionDispatcher::Dispatch(
     const ExtensionHostMsg_Request_Params& params,
     RenderViewHost* render_view_host) {
diff --git a/chrome/browser/extensions/extension_function_dispatcher.h b/chrome/browser/extensions/extension_function_dispatcher.h
index 9829107..4a731d9 100644
--- a/chrome/browser/extensions/extension_function_dispatcher.h
+++ b/chrome/browser/extensions/extension_function_dispatcher.h
@@ -14,10 +14,10 @@
 #include "ipc/ipc_message.h"
 #include "googleurl/src/gurl.h"
 
-class Browser;
 class ChromeRenderMessageFilter;
 class Extension;
 class ExtensionFunction;
+class ExtensionWindowController;
 class ExtensionInfoMap;
 class Profile;
 struct ExtensionHostMsg_Request_Params;
@@ -53,9 +53,9 @@ class ExtensionFunctionDispatcher
  public:
   class Delegate {
    public:
-    // Returns the browser that this delegate is associated with, if any.
-    // Returns NULL otherwise.
-    virtual Browser* GetBrowser() = 0;
+    // Returns the ExtensionWindowController associated with this delegate,
+    // or NULL if no window is associated with the delegate.
+    virtual ExtensionWindowController* GetExtensionWindowController() const = 0;
 
     // Asks the delegate for any relevant WebContents associated with this
     // context. For example, the WebbContents in which an infobar or
@@ -106,15 +106,6 @@ class ExtensionFunctionDispatcher
   // a response (if any) to the extension.
   void OnExtensionFunctionCompleted(const Extension* extension);
 
-  // Returns the current browser. Callers should generally prefer
-  // ExtensionFunction::GetCurrentBrowser() over this method, as that one
-  // provides the correct value for |include_incognito|.
-  //
-  // See the comments for ExtensionFunction::GetCurrentBrowser() for more
-  // details.
-  Browser* GetCurrentBrowser(content::RenderViewHost* render_view_host,
-                             bool include_incognito);
-
   // The profile that this dispatcher is associated with.
   Profile* profile() { return profile_; }
 
diff --git a/chrome/browser/extensions/extension_function_test_utils.cc b/chrome/browser/extensions/extension_function_test_utils.cc
index 40c0c9a..8b33ed1 100644
--- a/chrome/browser/extensions/extension_function_test_utils.cc
+++ b/chrome/browser/extensions/extension_function_test_utils.cc
@@ -28,8 +28,9 @@ class TestFunctionDispatcherDelegate
   virtual ~TestFunctionDispatcherDelegate() {}
 
  private:
-  virtual Browser* GetBrowser() OVERRIDE {
-    return browser_;
+  virtual ExtensionWindowController* GetExtensionWindowController()
+      const OVERRIDE {
+    return browser_->extension_window_controller();
   }
 
   virtual WebContents* GetAssociatedWebContents() const OVERRIDE {
diff --git a/chrome/browser/extensions/extension_host.cc b/chrome/browser/extensions/extension_host.cc
index f1368ab..71f3281 100644
--- a/chrome/browser/extensions/extension_host.cc
+++ b/chrome/browser/extensions/extension_host.cc
@@ -19,6 +19,7 @@
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/extension_system.h"
 #include "chrome/browser/extensions/extension_tab_util.h"
+#include "chrome/browser/extensions/extension_window_controller.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/app_modal_dialogs/message_box_handler.h"
 #include "chrome/browser/ui/browser.h"
@@ -231,12 +232,9 @@ void ExtensionHost::CreateRenderViewNow() {
   }
 }
 
-const Browser* ExtensionHost::GetBrowser() const {
-  return view() ? view()->browser() : NULL;
-}
-
-Browser* ExtensionHost::GetBrowser() {
-  return view() ? view()->browser() : NULL;
+ExtensionWindowController* ExtensionHost::GetExtensionWindowController() const {
+  return view() && view()->browser() ?
+      view()->browser()->extension_window_controller() : NULL;
 }
 
 const GURL& ExtensionHost::GetURL() const {
@@ -440,7 +438,7 @@ WebContents* ExtensionHost::OpenURLFromTab(WebContents* source,
     case OFF_THE_RECORD: {
       // Only allow these from hosts that are bound to a browser (e.g. popups).
       // Otherwise they are not driven by a user gesture.
-      Browser* browser = GetBrowser();
+      Browser* browser = view() ? view()->browser() : NULL;
       return browser ? browser->OpenURL(params) : NULL;
     }
     default:
@@ -515,14 +513,12 @@ void ExtensionHost::RenderViewCreated(RenderViewHost* render_view_host) {
   if (view_.get())
     view_->RenderViewCreated();
 
-  // If the host is bound to a browser, then extract its window id.
-  // Extensions hosted in ExternalTabContainer objects may not have
-  // an associated browser.
-  const Browser* browser = GetBrowser();
-  if (browser) {
+  // If the host is bound to a window, then extract its id. Extensions hosted
+  // in ExternalTabContainer objects may not have an associated window.
+  ExtensionWindowController* window = GetExtensionWindowController();
+  if (window) {
     render_view_host->Send(new ExtensionMsg_UpdateBrowserWindowId(
-        render_view_host->GetRoutingID(),
-        ExtensionTabUtil::GetWindowId(browser)));
+        render_view_host->GetRoutingID(), window->GetWindowId()));
   }
 }
 
diff --git a/chrome/browser/extensions/extension_host.h b/chrome/browser/extensions/extension_host.h
index be0208a..764f05b 100644
--- a/chrome/browser/extensions/extension_host.h
+++ b/chrome/browser/extensions/extension_host.h
@@ -32,6 +32,7 @@
 
 class Browser;
 class Extension;
+class ExtensionWindowController;
 class PrefsTabHelper;
 
 namespace content {
@@ -180,11 +181,9 @@ class ExtensionHost : public content::WebContentsDelegate,
   // Closes this host (results in deletion).
   void Close();
 
-  // Const version of below function.
-  const Browser* GetBrowser() const;
-
   // ExtensionFunctionDispatcher::Delegate
-  virtual Browser* GetBrowser() OVERRIDE;
+  virtual ExtensionWindowController* GetExtensionWindowController()
+      const OVERRIDE;
 
   // Message handlers.
   void OnRequest(const ExtensionHostMsg_Request_Params& params);
diff --git a/chrome/browser/extensions/extension_tab_helper.cc b/chrome/browser/extensions/extension_tab_helper.cc
index ab65db3..ce76ddc 100644
--- a/chrome/browser/extensions/extension_tab_helper.cc
+++ b/chrome/browser/extensions/extension_tab_helper.cc
@@ -320,12 +320,13 @@ void ExtensionTabHelper::OnImageLoaded(const gfx::Image& image,
   }
 }
 
-Browser* ExtensionTabHelper::GetBrowser() {
+ExtensionWindowController*
+ExtensionTabHelper::GetExtensionWindowController() const  {
   content::WebContents* contents = web_contents();
   TabContentsIterator tab_iterator;
   for (; !tab_iterator.done(); ++tab_iterator) {
     if (contents == (*tab_iterator)->web_contents())
-      return tab_iterator.browser();
+      return tab_iterator.browser()->extension_window_controller();
   }
 
   return NULL;
diff --git a/chrome/browser/extensions/extension_tab_helper.h b/chrome/browser/extensions/extension_tab_helper.h
index e0bbd26..16b4f5f 100644
--- a/chrome/browser/extensions/extension_tab_helper.h
+++ b/chrome/browser/extensions/extension_tab_helper.h
@@ -102,7 +102,8 @@ class ExtensionTabHelper
   virtual bool OnMessageReceived(const IPC::Message& message) OVERRIDE;
 
   // ExtensionFunctionDispatcher::Delegate overrides.
-  virtual Browser* GetBrowser() OVERRIDE;
+  virtual ExtensionWindowController* GetExtensionWindowController()
+      const OVERRIDE;
   virtual content::WebContents* GetAssociatedWebContents() const OVERRIDE;
 
   // Message handlers.
diff --git a/chrome/browser/extensions/extension_tabs_module.cc b/chrome/browser/extensions/extension_tabs_module.cc
index 07af114..995bd14 100644
--- a/chrome/browser/extensions/extension_tabs_module.cc
+++ b/chrome/browser/extensions/extension_tabs_module.cc
@@ -148,10 +148,11 @@ bool GetWindowFromWindowID(UIThreadExtensionFunction* function,
                            int window_id,
                            ExtensionWindowController** controller) {
   if (window_id == extension_misc::kCurrentWindowId) {
-    Browser* browser = function->dispatcher()->delegate()->GetBrowser();
-    // If there is a windowed browser associated with this extension, use that.
-    if (browser && browser->extension_window_controller()) {
-      *controller = browser->extension_window_controller();
+    ExtensionWindowController* extension_window_controller =
+        function->dispatcher()->delegate()->GetExtensionWindowController();
+    // If there is a window controller associated with this extension, use that.
+    if (extension_window_controller) {
+      *controller = extension_window_controller;
     } else {
       // Otherwise get the focused or most recently added window.
       *controller = ExtensionWindowList::GetInstance()->CurrentWindow(
@@ -796,7 +797,7 @@ bool RemoveWindowFunction::RunImpl() {
 
   ExtensionWindowController::Reason reason;
   if (!controller->CanClose(&reason)) {
-    if (reason == ExtensionWindowController::REASON_TAB_STRIP_NOT_EDITABLE)
+    if (reason == ExtensionWindowController::REASON_NOT_EDITABLE)
       error_ = keys::kTabStripNotEditableError;
     return false;
   }
diff --git a/chrome/browser/extensions/extension_window_controller.cc b/chrome/browser/extensions/extension_window_controller.cc
index eda4db3..90ad89f 100644
--- a/chrome/browser/extensions/extension_window_controller.cc
+++ b/chrome/browser/extensions/extension_window_controller.cc
@@ -34,6 +34,10 @@ bool ExtensionWindowController::MatchesProfile(
             match_profile->GetOffTheRecordProfile() == profile_)));
 }
 
+Browser* ExtensionWindowController::GetBrowser() const {
+  return NULL;
+}
+
 namespace keys = extension_tabs_module_constants;
 
 base::DictionaryValue* ExtensionWindowController::CreateWindowValue() const {
diff --git a/chrome/browser/extensions/extension_window_controller.h b/chrome/browser/extensions/extension_window_controller.h
index c479207..06db3c5 100644
--- a/chrome/browser/extensions/extension_window_controller.h
+++ b/chrome/browser/extensions/extension_window_controller.h
@@ -12,6 +12,7 @@
 #include "base/compiler_specific.h"
 
 class BaseWindow;
+class Browser;  // TODO(stevenjb) eliminate this dependency.
 class GURL;
 class Profile;
 class SessionID;
@@ -30,31 +31,35 @@ class ExtensionWindowController {
  public:
   enum Reason {
     REASON_NONE,
-    REASON_TAB_STRIP_NOT_EDITABLE,
+    REASON_NOT_EDITABLE,
   };
+
   enum ProfileMatchType {
     MATCH_NORMAL_ONLY,
     MATCH_INCOGNITO
   };
+
   ExtensionWindowController(BaseWindow* window, Profile* profile);
   virtual ~ExtensionWindowController();
 
   BaseWindow* window() const { return window_; }
 
+  Profile* profile() const { return profile_; }
+
   // Returns true if the window matches the profile.
   bool MatchesProfile(Profile* profile, ProfileMatchType match_type) const;
 
-  // Populates a dictionary for the Window object. Override this to set
-  // implementation specific properties (call the base implementation first to
-  // set common properties).
-  virtual base::DictionaryValue* CreateWindowValue() const;
-
   // Return an id uniquely identifying the window.
   virtual int GetWindowId() const = 0;
 
   // Return the type name for the window.
   virtual std::string GetWindowTypeText() const = 0;
 
+  // Populates a dictionary for the Window object. Override this to set
+  // implementation specific properties (call the base implementation first to
+  // set common properties).
+  virtual base::DictionaryValue* CreateWindowValue() const;
+
   // Populates a dictionary for the Window object, including a list of tabs.
   virtual base::DictionaryValue* CreateWindowValueWithTabs() const = 0;
 
@@ -67,6 +72,10 @@ class ExtensionWindowController {
   virtual void SetFullscreenMode(bool is_fullscreen,
                                  const GURL& extension_url) const = 0;
 
+  // Returns a Browser if available. Defaults to returning NULL.
+  // TODO(stevenjb): Temporary workaround. Eliminate this.
+  virtual Browser* GetBrowser() const;
+
  private:
   BaseWindow* window_;
   Profile* profile_;