Make WebUI objects not derive from WebUI. WebUI objects own the controller. This is the ownership model since otherwise we'd have to make the interface class delete it, which is better avoided since it's supposed to be an interface. The controller has a pointer to the WebUI object.

BUG=98716
Review URL: https://chromiumcodereview.appspot.com/9224002

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

9 files changed, 149 insertions, 76 deletions

diff --git a/content/browser/tab_contents/render_view_host_manager.cc b/content/browser/tab_contents/render_view_host_manager.cc
index 63a4f6c..a6ef2b9 100644
--- a/content/browser/tab_contents/render_view_host_manager.cc
+++ b/content/browser/tab_contents/render_view_host_manager.cc
@@ -542,7 +542,7 @@ bool RenderViewHostManager::InitRenderView(RenderViewHost* render_view_host,
   // If the pending navigation is to a WebUI, tell the RenderView about any
   // bindings it will need enabled.
   if (pending_web_ui_.get())
-    render_view_host->AllowBindings(pending_web_ui_->bindings());
+    render_view_host->AllowBindings(pending_web_ui_->GetBindings());
 
   return delegate_->CreateRenderViewForRenderManager(render_view_host);
 }
@@ -557,7 +557,7 @@ void RenderViewHostManager::CommitPending() {
   // Next commit the Web UI, if any.
   web_ui_.swap(pending_web_ui_);
   if (web_ui_.get() && pending_web_ui_.get() && !pending_render_view_host_)
-    web_ui_->controller()->DidBecomeActiveForReusedRenderView();
+    web_ui_->GetController()->DidBecomeActiveForReusedRenderView();
   pending_web_ui_.reset();
 
   // It's possible for the pending_render_view_host_ to be NULL when we aren't
@@ -737,7 +737,7 @@ RenderViewHost* RenderViewHostManager::UpdateRendererStateForNavigate(
     return pending_render_view_host_;
   } else {
     if (pending_web_ui_.get() && render_view_host_->IsRenderViewLive())
-      pending_web_ui_->controller()->RenderViewReused(render_view_host_);
+      pending_web_ui_->GetController()->RenderViewReused(render_view_host_);
 
     // The renderer can exit view source mode when any error or cancellation
     // happen. We must overwrite to recover the mode.
diff --git a/content/browser/tab_contents/render_view_host_manager_unittest.cc b/content/browser/tab_contents/render_view_host_manager_unittest.cc
index a87dddf..a19b4de 100644
--- a/content/browser/tab_contents/render_view_host_manager_unittest.cc
+++ b/content/browser/tab_contents/render_view_host_manager_unittest.cc
@@ -32,6 +32,7 @@ using content::NavigationController;
 using content::NavigationEntry;
 using content::NavigationEntryImpl;
 using content::WebContents;
+using content::WebUIController;
 
 namespace {
 
@@ -50,12 +51,11 @@ class RenderViewHostManagerTestWebUIFactory
   }
 
   // WebUIFactory implementation.
-  virtual WebUI* CreateWebUIForURL(WebContents* source,
-                                   const GURL& url) const OVERRIDE {
+  virtual WebUIController* CreateWebUIForURL(WebUI* web_ui,
+                                             const GURL& url) const OVERRIDE {
     if (!(should_create_webui_ && HasWebUIScheme(url)))
       return NULL;
-    static content::WebUIController temp_controller;
-    return new WebUI(source, &temp_controller);
+    return new WebUIController(web_ui);
   }
 
   virtual bool UseWebUIForURL(content::BrowserContext* browser_context,
diff --git a/content/browser/tab_contents/tab_contents.cc b/content/browser/tab_contents/tab_contents.cc
index 8ea6a1f..8a6b5dd 100644
--- a/content/browser/tab_contents/tab_contents.cc
+++ b/content/browser/tab_contents/tab_contents.cc
@@ -121,6 +121,7 @@ using content::SSLStatus;
 using content::UserMetricsAction;
 using content::WebContents;
 using content::WebContentsObserver;
+using content::WebUIController;
 
 namespace {
 
@@ -431,6 +432,20 @@ TabContentsView* TabContents::GetView() const {
   return view_.get();
 }
 
+WebUI* TabContents::CreateWebUI(const GURL& url) {
+  WebUI* web_ui = new WebUI(this);
+  WebUIController* controller =
+      content::GetContentClient()->browser()->GetWebUIFactory()->
+          CreateWebUIForURL(web_ui, url);
+  if (controller) {
+    web_ui->SetController(controller);
+    return web_ui;
+  }
+
+  delete web_ui;
+  return NULL;
+}
+
 WebUI* TabContents::GetWebUI() const {
   return render_manager_.web_ui() ? render_manager_.web_ui()
       : render_manager_.pending_web_ui();
@@ -457,7 +472,7 @@ const string16& TabContents::GetTitle() const {
     entry = controller_.GetActiveEntry();
     if (!(entry && entry->IsViewSourceMode())) {
       // Give the Web UI the chance to override our title.
-      const string16& title = our_web_ui->overridden_title();
+      const string16& title = our_web_ui->GetOverriddenTitle();
       if (!title.empty())
         return title;
     }
@@ -1121,7 +1136,7 @@ bool TabContents::GotResponseToLockMouseRequest(bool allowed) {
 bool TabContents::FocusLocationBarByDefault() {
   WebUI* web_ui = GetWebUIForCurrentState();
   if (web_ui)
-    return web_ui->focus_location_bar_by_default();
+    return web_ui->ShouldFocusLocationBarByDefault();
   NavigationEntry* entry = controller_.GetActiveEntry();
   if (entry && entry->GetURL() == GURL(chrome::kAboutBlankURL))
     return true;
@@ -1465,8 +1480,7 @@ void TabContents::DidNavigateMainFramePostCommit(
     // that opened the window, as long as both renderers have the same
     // privileges.
     if (delegate_ && opener_web_ui_type_ == GetWebUITypeForCurrentState()) {
-      WebUI* web_ui = content::GetContentClient()->browser()->
-          GetWebUIFactory()->CreateWebUIForURL(this, GetURL());
+      WebUI* web_ui = CreateWebUI(GetURL());
       // web_ui might be NULL if the URL refers to a non-existent extension.
       if (web_ui) {
         render_manager_.SetWebUIPostCommit(web_ui);
@@ -1944,11 +1958,11 @@ void TabContents::RequestTransferURL(const GURL& url,
     // chrome: URLs might have search terms or other stuff we don't want to
     // send to the site), so we send no referrer.
     OpenURLParams params(url, content::Referrer(), disposition,
-        render_manager_.web_ui()->link_transition_type(),
+        render_manager_.web_ui()->GetLinkTransitionType(),
         false /* is_renderer_initiated */);
     params.transferred_global_request_id = old_request_id;
     new_contents = OpenURL(params);
-    transition_type = render_manager_.web_ui()->link_transition_type();
+    transition_type = render_manager_.web_ui()->GetLinkTransitionType();
   } else {
     OpenURLParams params(url, referrer, disposition,
         content::PAGE_TRANSITION_LINK, true /* is_renderer_initiated */);
@@ -2178,8 +2192,7 @@ NavigationControllerImpl& TabContents::GetControllerForRenderManager() {
 }
 
 WebUI* TabContents::CreateWebUIForRenderManager(const GURL& url) {
-  return content::GetContentClient()->browser()->GetWebUIFactory()->
-      CreateWebUIForURL(this, url);
+  return CreateWebUI(url);
 }
 
 NavigationEntry*
diff --git a/content/browser/tab_contents/tab_contents.h b/content/browser/tab_contents/tab_contents.h
index 36239bb..b8d6fd81 100644
--- a/content/browser/tab_contents/tab_contents.h
+++ b/content/browser/tab_contents/tab_contents.h
@@ -133,6 +133,7 @@ class CONTENT_EXPORT TabContents
   virtual RenderViewHost* GetRenderViewHost() const OVERRIDE;
   virtual RenderWidgetHostView* GetRenderWidgetHostView() const OVERRIDE;
   virtual TabContentsView* GetView() const OVERRIDE;
+  virtual WebUI* CreateWebUI(const GURL& url) OVERRIDE;
   virtual WebUI* GetWebUI() const OVERRIDE;
   virtual WebUI* GetCommittedWebUI() const OVERRIDE;
   virtual const string16& GetTitle() const OVERRIDE;
diff --git a/content/browser/tab_contents/tab_contents_unittest.cc b/content/browser/tab_contents/tab_contents_unittest.cc
index b37912d..4a591e2 100644
--- a/content/browser/tab_contents/tab_contents_unittest.cc
+++ b/content/browser/tab_contents/tab_contents_unittest.cc
@@ -30,19 +30,19 @@ using content::BrowserThread;
 using content::NavigationEntry;
 using content::NavigationEntryImpl;
 using content::WebContents;
+using content::WebUIController;
 using webkit::forms::PasswordForm;
 
 namespace {
 
 class TabContentsTestWebUIFactory : public content::EmptyWebUIFactory {
  public:
-  virtual WebUI* CreateWebUIForURL(WebContents* source,
-                                   const GURL& url) const OVERRIDE {
+  virtual WebUIController* CreateWebUIForURL(WebUI* web_ui,
+                                             const GURL& url) const OVERRIDE {
    if (!HasWebUIScheme(url))
      return NULL;
 
-   static content::WebUIController temp_controller;
-   return new WebUI(source, &temp_controller);
+   return new WebUIController(web_ui);
   }
 
   virtual bool UseWebUIForURL(content::BrowserContext* browser_context,
@@ -58,7 +58,6 @@ class TabContentsTestWebUIFactory : public content::EmptyWebUIFactory {
       const GURL& url) const {
     return HasWebUIScheme(url);
   }
-
 };
 
 class TabContentsTestBrowserClient : public content::MockContentBrowserClient {
diff --git a/content/browser/webui/empty_web_ui_factory.cc b/content/browser/webui/empty_web_ui_factory.cc
index eeaf3ef..e5ede37 100644
--- a/content/browser/webui/empty_web_ui_factory.cc
+++ b/content/browser/webui/empty_web_ui_factory.cc
@@ -12,8 +12,8 @@ EmptyWebUIFactory::EmptyWebUIFactory() {
 EmptyWebUIFactory::~EmptyWebUIFactory() {
 }
 
-WebUI* EmptyWebUIFactory::CreateWebUIForURL(WebContents* source,
-                                            const GURL& url) const {
+WebUIController* EmptyWebUIFactory::CreateWebUIForURL(WebUI* web_ui,
+                                                      const GURL& url) const {
   return NULL;
 }
 
diff --git a/content/browser/webui/empty_web_ui_factory.h b/content/browser/webui/empty_web_ui_factory.h
index 7d71628..a6d8084 100644
--- a/content/browser/webui/empty_web_ui_factory.h
+++ b/content/browser/webui/empty_web_ui_factory.h
@@ -17,8 +17,8 @@ class CONTENT_EXPORT EmptyWebUIFactory : public content::WebUIFactory {
   // Returns the singleton instance.
   static EmptyWebUIFactory* GetInstance();
 
-  virtual WebUI* CreateWebUIForURL(WebContents* source,
-                                   const GURL& url) const OVERRIDE;
+  virtual WebUIController* CreateWebUIForURL(WebUI* web_ui,
+                                             const GURL& url) const OVERRIDE;
   virtual WebUI::TypeID GetWebUIType(content::BrowserContext* browser_context,
                                      const GURL& url) const OVERRIDE;
   virtual bool UseWebUIForURL(content::BrowserContext* browser_context,
diff --git a/content/browser/webui/web_ui.cc b/content/browser/webui/web_ui.cc
index 35b418f..c0236b4 100644
--- a/content/browser/webui/web_ui.cc
+++ b/content/browser/webui/web_ui.cc
@@ -41,21 +41,21 @@ string16 WebUI::GetJavascriptCall(
       char16('(') + parameters + char16(')') + char16(';');
 }
 
-WebUI::WebUI(WebContents* contents,
-             WebUIController* controller)
-    : hide_favicon_(false),
-      focus_location_bar_by_default_(false),
+WebUI::WebUI(WebContents* contents)
+    : focus_location_bar_by_default_(false),
       should_hide_url_(false),
       link_transition_type_(content::PAGE_TRANSITION_LINK),
       bindings_(content::BINDINGS_POLICY_WEB_UI),
       register_callback_overwrites_(false),
-      web_contents_(contents),
-      controller_(controller) {
+      web_contents_(contents) {
   DCHECK(contents);
   AddMessageHandler(new GenericHandler());
 }
 
 WebUI::~WebUI() {
+  // Delete the controller first, since it may also be keeping a pointer to some
+  // of the handlers and can call them at destruction.
+  controller_.reset();
   STLDeleteContainerPointers(handlers_.begin(), handlers_.end());
 }
 
@@ -115,6 +115,70 @@ void WebUI::RenderViewCreated(RenderViewHost* render_view_host) {
     render_view_host->SetWebUIProperty("touchOptimized", "true");
 }
 
+WebContents* WebUI::GetWebContents() const {
+  return web_contents_;
+}
+
+bool WebUI::ShouldHideFavicon() const {
+  return hide_favicon_;
+}
+
+void WebUI::HideFavicon() {
+  hide_favicon_ = true;
+}
+
+bool WebUI::ShouldFocusLocationBarByDefault() const {
+  return focus_location_bar_by_default_;
+}
+
+void WebUI::FocusLocationBarByDefault() {
+  focus_location_bar_by_default_ = true;
+}
+
+bool WebUI::ShouldHideURL() const {
+  return should_hide_url_;
+}
+
+void WebUI::HideURL() {
+  should_hide_url_ = true;
+}
+
+const string16& WebUI::GetOverriddenTitle() const {
+  return overridden_title_;
+}
+
+void WebUI::OverrideTitle(const string16& title) {
+  overridden_title_ = title;
+}
+
+content::PageTransition WebUI::GetLinkTransitionType() const {
+  return link_transition_type_;
+}
+
+void WebUI::SetLinkTransitionType(content::PageTransition type) {
+  link_transition_type_ = type;
+}
+
+int WebUI::GetBindings() const {
+  return bindings_;
+}
+
+void WebUI::SetBindings(int bindings) {
+  bindings_ = bindings;
+}
+
+void WebUI::SetFrameXPath(const std::string& xpath) {
+  frame_xpath_ = xpath;
+}
+
+WebUIController* WebUI::GetController() const {
+  return controller_.get();
+}
+
+void WebUI::SetController(WebUIController* controller) {
+  controller_.reset(controller);
+}
+
 void WebUI::CallJavascriptFunction(const std::string& function_name) {
   DCHECK(IsStringASCII(function_name));
   string16 javascript = ASCIIToUTF16(function_name + "();");
diff --git a/content/browser/webui/web_ui.h b/content/browser/webui/web_ui.h
index f804b00..7731262 100644
--- a/content/browser/webui/web_ui.h
+++ b/content/browser/webui/web_ui.h
@@ -38,7 +38,7 @@ class WebUIMessageHandler;
 // ChromeWebUI.
 class CONTENT_EXPORT WebUI : public IPC::Channel::Listener {
  public:
-  WebUI(content::WebContents* contents, content::WebUIController* controller);
+  explicit WebUI(content::WebContents* contents);
   virtual ~WebUI();
 
   // IPC::Channel::Listener implementation:
@@ -54,47 +54,56 @@ class CONTENT_EXPORT WebUI : public IPC::Channel::Listener {
   // reuse RenderView instances.
   void RenderViewCreated(RenderViewHost* render_view_host);
 
-  // Used by WebUIMessageHandlers. If the given message is already registered,
-  // the call has no effect unless |register_callback_overwrites_| is set to
-  // true.
-  typedef base::Callback<void(const base::ListValue*)> MessageCallback;
-  void RegisterMessageCallback(const std::string& message,
-                               const MessageCallback& callback);
+  // TODO(jam): below is what will be the public API of WebUI
+  content::WebContents* GetWebContents() const;
 
   // Returns true if the favicon should be hidden for the current tab.
-  bool hide_favicon() const {
-    return hide_favicon_;
-  }
+  bool ShouldHideFavicon() const;
+  void HideFavicon();
 
   // Returns true if the location bar should be focused by default rather than
   // the page contents. Some pages will want to use this to encourage the user
   // to type in the URL bar.
-  bool focus_location_bar_by_default() const {
-    return focus_location_bar_by_default_;
-  }
+  bool ShouldFocusLocationBarByDefault() const;
+  void FocusLocationBarByDefault();
 
   // Returns true if the page's URL should be hidden. Some Web UI pages
   // like the new tab page will want to hide it.
-  bool should_hide_url() const {
-    return should_hide_url_;
-  }
+  bool ShouldHideURL() const;
+  void HideURL();
 
   // Gets a custom tab title provided by the Web UI. If there is no title
   // override, the string will be empty which should trigger the default title
   // behavior for the tab.
-  const string16& overridden_title() const {
-    return overridden_title_;
-  }
+  const string16& GetOverriddenTitle() const;
+  void OverrideTitle(const string16& title);
 
   // Returns the transition type that should be used for link clicks on this
   // Web UI. This will default to LINK but may be overridden.
-  content::PageTransition link_transition_type() const {
-    return link_transition_type_;
-  }
+  content::PageTransition GetLinkTransitionType() const;
+  void SetLinkTransitionType(content::PageTransition type);
 
-  int bindings() const {
-    return bindings_;
-  }
+  // Allows a controller to override the BindingsPolicy that should be enabled
+  // for this page.
+  int GetBindings() const;
+  void SetBindings(int bindings);
+
+  // Sets the path for the iframe if this WebUI is embedded in a page.
+  void SetFrameXPath(const std::string& xpath);
+
+  // Takes ownership of |handler|, which will be destroyed when the WebUI is.
+  void AddMessageHandler(content::WebUIMessageHandler* handler);
+
+  // Execute a string of raw Javascript on the page.  Overridable for
+  // testing purposes.
+  virtual void ExecuteJavascript(const string16& javascript);
+
+  // Used by WebUIMessageHandlers. If the given message is already registered,
+  // the call has no effect unless |register_callback_overwrites_| is set to
+  // true.
+  typedef base::Callback<void(const base::ListValue*)> MessageCallback;
+  void RegisterMessageCallback(const std::string& message,
+                               const MessageCallback& callback);
 
   // Indicates whether RegisterMessageCallback() will overwrite an existing
   // message callback mapping.  Serves as the hook for test mocks.
@@ -106,11 +115,8 @@ class CONTENT_EXPORT WebUI : public IPC::Channel::Listener {
     register_callback_overwrites_ = value;
   }
 
-  void set_frame_xpath(const std::string& xpath) {
-    frame_xpath_ = xpath;
-  }
-
-  content::WebUIController* controller() const { return controller_; }
+  content::WebUIController* GetController() const;
+  void SetController(content::WebUIController* controller);
 
   // Call a Javascript function by sending its name and arguments down to
   // the renderer.  This is asynchronous; there's no way to get the result
@@ -152,13 +158,10 @@ class CONTENT_EXPORT WebUI : public IPC::Channel::Listener {
       const std::string& function_name,
       const std::vector<const base::Value*>& arg_list);
 
- protected:
-  // Takes ownership of |handler|, which will be destroyed when the WebUI is.
-  void AddMessageHandler(content::WebUIMessageHandler* handler);
-
-  // Execute a string of raw Javascript on the page.  Overridable for
-  // testing purposes.
-  virtual void ExecuteJavascript(const string16& javascript);
+ private:
+  // A map of message name -> message handling callback.
+  typedef std::map<std::string, MessageCallback> MessageCallbackMap;
+  MessageCallbackMap message_callbacks_;
 
   // Options that may be overridden by individual Web UI implementations. The
   // bool options default to false. See the public getters for more information.
@@ -179,19 +182,12 @@ class CONTENT_EXPORT WebUI : public IPC::Channel::Listener {
   // Non-owning pointer to the WebContents this WebUI is associated with.
   content::WebContents* web_contents_;
 
-  // TODO(jam): once WebUI objects aren't also WebUIController, make one own the
-  // other.
-  content::WebUIController* controller_;
-
- private:
-  // A map of message name -> message handling callback.
-  typedef std::map<std::string, MessageCallback> MessageCallbackMap;
-  MessageCallbackMap message_callbacks_;
-
   // The path for the iframe this WebUI is embedded in (empty if not in an
   // iframe).
   std::string frame_xpath_;
 
+  scoped_ptr<content::WebUIController> controller_;
+
   DISALLOW_COPY_AND_ASSIGN(WebUI);
 };