From e0c7c263ba8720bc8616fc1ed73a1480dc931f6f Mon Sep 17 00:00:00 2001 From: "stoyan@chromium.org" Date: Thu, 23 Apr 2009 23:09:43 +0000 Subject: Move Browser implementation of callback interfaces into the private section. Split TabContentsDelegate and PageNavigator. Review URL: http://codereview.chromium.org/88063 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@14375 0039d316-1c4b-4281-b951-d872f2087c98 --- chrome/browser/bookmarks/bookmark_utils.cc | 3 +- chrome/browser/browser.cc | 32 +++++++++++++++ chrome/browser/browser.h | 48 ++++++++++++++++++---- chrome/browser/browser_focus_uitest.cc | 4 +- chrome/browser/chrome_plugin_host.cc | 2 +- chrome/browser/extensions/extension_tabs_module.cc | 2 +- chrome/browser/extensions/extension_view.cc | 7 +--- .../browser/tab_contents/tab_contents_delegate.h | 21 ++++------ chrome/browser/views/bookmark_bar_view.cc | 4 +- chrome/test/ui_test_utils.cc | 3 +- 10 files changed, 89 insertions(+), 37 deletions(-) diff --git a/chrome/browser/bookmarks/bookmark_utils.cc b/chrome/browser/bookmarks/bookmark_utils.cc index cb36eee..e22911f 100644 --- a/chrome/browser/bookmarks/bookmark_utils.cc +++ b/chrome/browser/bookmarks/bookmark_utils.cc @@ -61,8 +61,7 @@ class NewBrowserPageNavigator : public PageNavigator { // Always open the first tab in the foreground. disposition = NEW_FOREGROUND_TAB; } - browser_->OpenURLFromTab(NULL, url, referrer, NEW_FOREGROUND_TAB, - transition); + browser_->OpenURL(url, referrer, NEW_FOREGROUND_TAB, transition); } private: diff --git a/chrome/browser/browser.cc b/chrome/browser/browser.cc index 4b5234e..36e3be6 100644 --- a/chrome/browser/browser.cc +++ b/chrome/browser/browser.cc @@ -543,6 +543,30 @@ TabContents* Browser::AddTabWithNavigationController( return tc; } +void Browser::AddTabContents(TabContents* new_contents, + WindowOpenDisposition disposition, + const gfx::Rect& initial_pos, + bool user_gesture) { + AddNewContents(NULL, new_contents, disposition, initial_pos, user_gesture); +} + +void Browser::CloseTabContents(TabContents* contents) { + CloseContents(contents); +} + +void Browser::BrowserShowHtmlDialog(HtmlDialogUIDelegate* delegate, + void* parent_window) { + ShowHtmlDialog(delegate, parent_window); +} + +void Browser::BrowserRenderWidgetShowing() { + RenderWidgetShowing(); +} + +void Browser::ToolbarSizeChanged(bool is_animating) { + ToolbarSizeChanged(NULL, is_animating); +} + TabContents* Browser::AddRestoredTab( const std::vector& navigations, int tab_index, @@ -1627,6 +1651,14 @@ void Browser::TabStripEmpty() { } /////////////////////////////////////////////////////////////////////////////// +// Browser, PageNavigator implementation: +void Browser::OpenURL(const GURL& url, const GURL& referrer, + WindowOpenDisposition disposition, + PageTransition::Type transition) { + OpenURLFromTab(NULL, url, referrer, disposition, transition); +} + +/////////////////////////////////////////////////////////////////////////////// // Browser, TabContentsDelegate implementation: void Browser::OpenURLFromTab(TabContents* source, diff --git a/chrome/browser/browser.h b/chrome/browser/browser.h index d0f2d6f..9566d1f 100644 --- a/chrome/browser/browser.h +++ b/chrome/browser/browser.h @@ -40,6 +40,7 @@ class TabNavigation; class Browser : public TabStripModelDelegate, public TabStripModelObserver, public TabContentsDelegate, + public PageNavigator, public CommandUpdater::CommandUpdaterDelegate, public NotificationObserver, public SelectFileDialog::Listener { @@ -234,6 +235,29 @@ class Browser : public TabStripModelDelegate, int tab_index, int selected_navigation, bool select); + // Creates a new tab with the already-created TabContents 'new_contents'. + // The window for the added contents will be reparented correctly when this + // method returns. If |disposition| is NEW_POPUP, |pos| should hold the + // initial position. + void AddTabContents(TabContents* new_contents, + WindowOpenDisposition disposition, + const gfx::Rect& initial_pos, + bool user_gesture); + void CloseTabContents(TabContents* contents); + + // Show a dialog with HTML content. |delegate| contains a pointer to the + // delegate who knows how to display the dialog (which file URL and JSON + // string input to use during initialization). |parent_window| is the window + // that should be parent of the dialog, or NULL for the default. + void BrowserShowHtmlDialog(HtmlDialogUIDelegate* delegate, + void* parent_window); + + // Called when a popup select is about to be displayed. + void BrowserRenderWidgetShowing(); + + // Notification that some of our content has changed size as + // part of an animation. + void ToolbarSizeChanged(bool is_animating); // Replaces the state of the currently selected tab with the session // history restored from the SessionRestore system. @@ -366,14 +390,29 @@ class Browser : public TabStripModelDelegate, static Browser* GetBrowserForController( const NavigationController* controller, int* index); + + // Helper function to create a new popup window. + static void BuildPopupWindowHelper(TabContents* source, + TabContents* new_contents, + const gfx::Rect& initial_pos, + Browser::Type browser_type, + Profile* profile, + bool honor_saved_maximized_state); + // Calls ExecuteCommandWithDisposition with the given disposition. void ExecuteCommandWithDisposition(int id, WindowOpenDisposition); // Interface implementations //////////////////////////////////////////////// + // Overridden from PageNavigator + virtual void OpenURL(const GURL& url, const GURL& referrer, + WindowOpenDisposition disposition, + PageTransition::Type transition); + // Overridden from CommandUpdater::CommandUpdaterDelegate: virtual void ExecuteCommand(int id); + private: // Overridden from TabStripModelDelegate: virtual TabContents* AddBlankTab(bool foreground); virtual TabContents* AddBlankTabAt(int index, bool foreground); @@ -458,15 +497,6 @@ class Browser : public TabStripModelDelegate, const NotificationSource& source, const NotificationDetails& details); - // Helper function to create a new popup window. - static void BuildPopupWindowHelper(TabContents* source, - TabContents* new_contents, - const gfx::Rect& initial_pos, - Browser::Type browser_type, - Profile* profile, - bool honor_saved_maximized_state); - - private: // Command and state updating /////////////////////////////////////////////// // Initialize state for all browser commands. diff --git a/chrome/browser/browser_focus_uitest.cc b/chrome/browser/browser_focus_uitest.cc index a19e072..4d46a59 100644 --- a/chrome/browser/browser_focus_uitest.cc +++ b/chrome/browser/browser_focus_uitest.cc @@ -78,7 +78,7 @@ IN_PROC_BROWSER_TEST_F(BrowserFocusTest, DISABLED_BrowsersRememberFocus) { // Open a new browser window. Browser* browser2 = Browser::Create(browser()->profile()); ASSERT_TRUE(browser2); - browser2->AddBlankTab(true); + browser2->tabstrip_model()->delegate()->AddBlankTab(true); browser2->window()->Show(); ui_test_utils::NavigateToURL(browser2, url); @@ -177,7 +177,7 @@ IN_PROC_BROWSER_TEST_F(BrowserFocusTest, BackgroundBrowserDontStealFocus) { // Open a new browser window. Browser* browser2 = Browser::Create(browser()->profile()); ASSERT_TRUE(browser2); - browser2->AddBlankTab(true); + browser2->tabstrip_model()->delegate()->AddBlankTab(true); browser2->window()->Show(); GURL steal_focus_url = server->TestServerPageW(kStealFocusPage); ui_test_utils::NavigateToURL(browser2, steal_focus_url); diff --git a/chrome/browser/chrome_plugin_host.cc b/chrome/browser/chrome_plugin_host.cc index 72a7309..66c0183 100644 --- a/chrome/browser/chrome_plugin_host.cc +++ b/chrome/browser/chrome_plugin_host.cc @@ -326,7 +326,7 @@ class ModelessHtmlDialogDelegate : public HtmlDialogUIDelegate { void Show() { DCHECK(MessageLoop::current() == main_message_loop_); Browser* browser = BrowserList::GetLastActive(); - browser->ShowHtmlDialog(this, parent_wnd_); + browser->BrowserShowHtmlDialog(this, parent_wnd_); } // Gives the JSON result string back to the plugin. diff --git a/chrome/browser/extensions/extension_tabs_module.cc b/chrome/browser/extensions/extension_tabs_module.cc index b4f773a..278ebd7 100644 --- a/chrome/browser/extensions/extension_tabs_module.cc +++ b/chrome/browser/extensions/extension_tabs_module.cc @@ -265,7 +265,7 @@ bool RemoveTabFunction::RunImpl() { int tab_index; TabStripModel* tab_strip = browser->tabstrip_model(); if (GetIndexOfTabId(tab_strip, tab_id, &tab_index)) { - browser->CloseContents(tab_strip->GetTabContentsAt(tab_index)); + browser->CloseTabContents(tab_strip->GetTabContentsAt(tab_index)); return true; } diff --git a/chrome/browser/extensions/extension_view.cc b/chrome/browser/extensions/extension_view.cc index 960765a..05d310e 100755 --- a/chrome/browser/extensions/extension_view.cc +++ b/chrome/browser/extensions/extension_view.cc @@ -132,9 +132,7 @@ void ExtensionView::ShowCreatedWindow(int route_id, bool user_gesture) { WebContents* contents = delegate_view_helper_.GetCreatedWindow(route_id); if (contents) { - // TODO(erikkay) is it safe to pass in NULL as source? - browser_->AddNewContents(NULL, contents, disposition, initial_pos, - user_gesture); + browser_->AddTabContents(contents, disposition, initial_pos, user_gesture); } } @@ -142,7 +140,7 @@ void ExtensionView::ShowCreatedWidget(int route_id, const gfx::Rect& initial_pos) { RenderWidgetHostView* widget_host_view = delegate_view_helper_.GetCreatedWidget(route_id); - browser_->RenderWidgetShowing(); + browser_->BrowserRenderWidgetShowing(); // TODO(erikkay): These two lines could be refactored with TabContentsView. widget_host_view->InitAsPopup(render_view_host()->view(), initial_pos); @@ -165,4 +163,3 @@ void ExtensionView::TakeFocus(bool reverse) { void ExtensionView::HandleKeyboardEvent(const NativeWebKeyboardEvent& event) { } - diff --git a/chrome/browser/tab_contents/tab_contents_delegate.h b/chrome/browser/tab_contents/tab_contents_delegate.h index 87e228e..f69d623 100644 --- a/chrome/browser/tab_contents/tab_contents_delegate.h +++ b/chrome/browser/tab_contents/tab_contents_delegate.h @@ -7,15 +7,16 @@ #include "base/basictypes.h" #include "base/gfx/rect.h" -#include "chrome/browser/tab_contents/page_navigator.h" -#include "chrome/common/navigation_types.h" +#include "chrome/common/page_transition_types.h" +#include "webkit/glue/window_open_disposition.h" class TabContents; class HtmlDialogUIDelegate; +class GURL; // Objects implement this interface to get notified about changes in the // TabContents and to provide necessary functionality. -class TabContentsDelegate : public PageNavigator { +class TabContentsDelegate { public: // Opens a new URL inside the passed in TabContents (if source is 0 open // in the current front-most tab), unless |disposition| indicates the url @@ -28,16 +29,6 @@ class TabContentsDelegate : public PageNavigator { WindowOpenDisposition disposition, PageTransition::Type transition) = 0; - // Wrapper around OpenURLFromTab when there is no source for the given URL - // (it will use the current onep. - // - // This implements the PageNavigator interface. - virtual void OpenURL(const GURL& url, const GURL& referrer, - WindowOpenDisposition disposition, - PageTransition::Type transition) { - OpenURLFromTab(NULL, url, referrer, disposition, transition); - } - // Called to inform the delegate that the tab content's navigation state // changed. The |changed_flags| indicates the parts of the navigation state // that have been updated, and is any combination of the @@ -164,6 +155,10 @@ class TabContentsDelegate : public PageNavigator { virtual bool TakeFocus(bool reverse) { return false; } + + protected: + ~TabContentsDelegate() {} + }; #endif // CHROME_BROWSER_TAB_CONTENTS_TAB_CONTENTS_DELEGATE_H_ diff --git a/chrome/browser/views/bookmark_bar_view.cc b/chrome/browser/views/bookmark_bar_view.cc index 5acf012..b0be7dd 100644 --- a/chrome/browser/views/bookmark_bar_view.cc +++ b/chrome/browser/views/bookmark_bar_view.cc @@ -897,12 +897,12 @@ int BookmarkBarView::GetToolbarOverlap() { void BookmarkBarView::AnimationProgressed(const Animation* animation) { if (browser_) - browser_->ToolbarSizeChanged(NULL, true); + browser_->ToolbarSizeChanged(true); } void BookmarkBarView::AnimationEnded(const Animation* animation) { if (browser_) - browser_->ToolbarSizeChanged(NULL, false); + browser_->ToolbarSizeChanged(false); SchedulePaint(); } diff --git a/chrome/test/ui_test_utils.cc b/chrome/test/ui_test_utils.cc index 2d91bcc..bd8ff59 100644 --- a/chrome/test/ui_test_utils.cc +++ b/chrome/test/ui_test_utils.cc @@ -94,8 +94,7 @@ void NavigateToURLBlockUntilNavigationsComplete(Browser* browser, int number_of_navigations) { NavigationController* controller = &browser->GetSelectedTabContents()->controller(); - browser->OpenURLFromTab(browser->GetSelectedTabContents(), url, GURL(), - CURRENT_TAB, PageTransition::TYPED); + browser->OpenURL(url, GURL(), CURRENT_TAB, PageTransition::TYPED); WaitForNavigations(controller, number_of_navigations); } -- cgit v1.1