From 9155324b8d7e4b465553e42366319a4e1587ac20 Mon Sep 17 00:00:00 2001 From: "nick@chromium.org" Date: Mon, 2 Aug 2010 18:24:53 +0000 Subject: Revert 54560 - Change removing method, GetBrowser from TabContentsDelegate, as this was breaking an abstraction layer. This routine was originally added in CL 434046, which required the Browser* to construct extension popup views from within Chrome-Frame instances. I changed all accesses to Browser instances from usage of the above method, to either iterating the BrowserList using the situation-specific profile as a search key, or modifying the appropriate delegate interfaces to provide the functionality that was previously used directly via the Browser. BUG=None TEST=None Review URL: http://codereview.chromium.org/2941001 TBR=twiz@google.com Review URL: http://codereview.chromium.org/3012042 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@54565 0039d316-1c4b-4281-b951-d872f2087c98 --- .../autofill/autofill_cc_infobar_delegate.cc | 13 +++-- .../autofill/autofill_cc_infobar_delegate.h | 4 ++ chrome/browser/autofill/autofill_manager.h | 3 -- chrome/browser/browser.cc | 56 +++++++++------------- chrome/browser/browser.h | 17 ++----- chrome/browser/browser_browsertest.cc | 20 ++++---- chrome/browser/browser_init.cc | 2 +- .../browser/cocoa/tab_strip_controller_unittest.mm | 2 - chrome/browser/dom_ui/dom_ui.cc | 2 +- chrome/browser/dom_ui/dom_ui.h | 2 +- chrome/browser/dom_ui/history2_ui.cc | 8 +--- chrome/browser/dom_ui/history_ui.cc | 8 +--- chrome/browser/extensions/extension_dom_ui.cc | 9 ++-- chrome/browser/external_tab_container_win.h | 2 + chrome/browser/tab_contents/tab_contents.cc | 8 ++++ .../browser/tab_contents/tab_contents_delegate.cc | 4 ++ .../browser/tab_contents/tab_contents_delegate.h | 4 ++ chrome/browser/tabs/tab_strip_model.h | 3 -- chrome/browser/tabs/tab_strip_model_unittest.cc | 1 - chrome/browser/views/frame/browser_view.cc | 2 +- .../views/tabs/browser_tab_strip_controller.cc | 21 ++++---- .../views/tabs/browser_tab_strip_controller.h | 6 +-- 22 files changed, 91 insertions(+), 106 deletions(-) (limited to 'chrome') diff --git a/chrome/browser/autofill/autofill_cc_infobar_delegate.cc b/chrome/browser/autofill/autofill_cc_infobar_delegate.cc index 7953d86..89c17ba 100644 --- a/chrome/browser/autofill/autofill_cc_infobar_delegate.cc +++ b/chrome/browser/autofill/autofill_cc_infobar_delegate.cc @@ -9,6 +9,7 @@ #include "base/histogram.h" #include "chrome/browser/autofill/autofill_cc_infobar.h" #include "chrome/browser/autofill/autofill_manager.h" +#include "chrome/browser/browser.h" #include "chrome/browser/pref_service.h" #include "chrome/browser/profile.h" #include "chrome/browser/tab_contents/tab_contents.h" @@ -22,9 +23,15 @@ AutoFillCCInfoBarDelegate::AutoFillCCInfoBarDelegate(TabContents* tab_contents, AutoFillManager* host) : ConfirmInfoBarDelegate(tab_contents), + browser_(NULL), host_(host) { - if (tab_contents) + if (tab_contents) { + // This is NULL for TestTabContents. + if (tab_contents->delegate()) + browser_ = tab_contents->delegate()->GetBrowser(); + tab_contents->AddInfoBar(this); + } } AutoFillCCInfoBarDelegate::~AutoFillCCInfoBarDelegate() { @@ -96,8 +103,8 @@ std::wstring AutoFillCCInfoBarDelegate::GetLinkText() { } bool AutoFillCCInfoBarDelegate::LinkClicked(WindowOpenDisposition disposition) { - host_->tab_contents()->OpenURL(GURL(kAutoFillLearnMoreUrl), GURL(), - NEW_FOREGROUND_TAB, PageTransition::TYPED); + browser_->OpenURL(GURL(kAutoFillLearnMoreUrl), GURL(), NEW_FOREGROUND_TAB, + PageTransition::TYPED); return false; } diff --git a/chrome/browser/autofill/autofill_cc_infobar_delegate.h b/chrome/browser/autofill/autofill_cc_infobar_delegate.h index 6a8091e..32a7c82 100644 --- a/chrome/browser/autofill/autofill_cc_infobar_delegate.h +++ b/chrome/browser/autofill/autofill_cc_infobar_delegate.h @@ -11,6 +11,7 @@ #include "chrome/browser/tab_contents/infobar_delegate.h" class AutoFillManager; +class Browser; class SkBitmap; class TabContents; @@ -45,6 +46,9 @@ class AutoFillCCInfoBarDelegate : public ConfirmInfoBarDelegate { } private: + // The browser. + Browser* browser_; + // The AutoFillManager that initiated this InfoBar. AutoFillManager* host_; diff --git a/chrome/browser/autofill/autofill_manager.h b/chrome/browser/autofill/autofill_manager.h index 4858a5e..964b8bc 100644 --- a/chrome/browser/autofill/autofill_manager.h +++ b/chrome/browser/autofill/autofill_manager.h @@ -45,9 +45,6 @@ class AutoFillManager : public RenderViewHostDelegate::AutoFill, // Registers our Enable/Disable AutoFill pref. static void RegisterUserPrefs(PrefService* prefs); - // Returns the TabContents hosting this AutoFillManager. - TabContents* tab_contents() const { return tab_contents_; } - // RenderViewHostDelegate::AutoFill implementation: virtual void FormSubmitted(const webkit_glue::FormData& form); virtual void FormsSeen(const std::vector& forms); diff --git a/chrome/browser/browser.cc b/chrome/browser/browser.cc index d5ffa64..cdd32ae 100644 --- a/chrome/browser/browser.cc +++ b/chrome/browser/browser.cc @@ -531,10 +531,10 @@ TabContents* Browser::OpenApplication(Profile* profile, case Extension::LAUNCH_WINDOW: case Extension::LAUNCH_PANEL: tab = Browser::OpenApplicationWindow(profile, extension, container, - GURL(), &browser); + GURL()); break; case Extension::LAUNCH_TAB: { - tab = Browser::OpenApplicationTab(profile, extension, &browser); + tab = Browser::OpenApplicationTab(profile, extension); break; } default: @@ -542,6 +542,7 @@ TabContents* Browser::OpenApplication(Profile* profile, break; } if (tab) { + Browser* browser = tab->delegate()->GetBrowser(); if (browser && extension && extension->launch_fullscreen()) browser->window()->SetFullscreen(true); } @@ -553,8 +554,7 @@ TabContents* Browser::OpenApplicationWindow( Profile* profile, Extension* extension, Extension::LaunchContainer container, - const GURL& url_input, - Browser** browser) { + const GURL& url_input) { GURL url; if (!url_input.is_empty()) { if (extension) @@ -570,15 +570,15 @@ TabContents* Browser::OpenApplicationWindow( RegisterAppPrefs(app_name); bool as_panel = extension && (container == Extension::LAUNCH_PANEL); - Browser* local_browser = Browser::CreateForApp(app_name, extension, profile, - as_panel); - TabContents* tab_contents = local_browser->AddTabWithURL( + Browser* browser = Browser::CreateForApp(app_name, extension, profile, + as_panel); + TabContents* tab_contents = browser->AddTabWithURL( url, GURL(), PageTransition::START_PAGE, -1, TabStripModel::ADD_SELECTED, NULL, std::string()); tab_contents->GetMutableRendererPrefs()->can_accept_load_drops = false; tab_contents->render_view_host()->SyncRendererPrefs(); - local_browser->window()->Show(); + browser->window()->Show(); // TODO(jcampan): http://crbug.com/8123 we should not need to set the initial // focus explicitly. @@ -591,42 +591,33 @@ TabContents* Browser::OpenApplicationWindow( // OnDidGetApplicationInfo, which calls // web_app::UpdateShortcutForTabContents when it sees UPDATE_SHORTCUT as // pending web app action. - local_browser->pending_web_app_action_ = UPDATE_SHORTCUT; + browser->pending_web_app_action_ = UPDATE_SHORTCUT; } - if (browser) - *browser = local_browser; - return tab_contents; } // static TabContents* Browser::OpenApplicationWindow(Profile* profile, - GURL& url, Browser** browser) { - return OpenApplicationWindow(profile, NULL, Extension::LAUNCH_WINDOW, url, - browser); + GURL& url) { + return OpenApplicationWindow(profile, NULL, Extension::LAUNCH_WINDOW, url); } // static TabContents* Browser::OpenApplicationTab(Profile* profile, - Extension* extension, - Browser** browser) { - Browser* local_browser = BrowserList::GetLastActiveWithProfile(profile); - if (!local_browser || local_browser->type() != Browser::TYPE_NORMAL) + Extension* extension) { + Browser* browser = BrowserList::GetLastActiveWithProfile(profile); + if (!browser || browser->type() != Browser::TYPE_NORMAL) return NULL; // TODO(erikkay): This doesn't seem like the right transition in all cases. PageTransition::Type transition = PageTransition::START_PAGE; GURL url = extension->GetFullLaunchURL(); TabContents* tab_contents = - local_browser->CreateTabContentsForURL(url, GURL(), profile, - transition, false, NULL); + browser->CreateTabContentsForURL(url, GURL(), profile, + transition, false, NULL); tab_contents->SetExtensionApp(extension); - local_browser->AddTab(tab_contents, transition); - - if (browser) - *browser = local_browser; - + browser->AddTab(tab_contents, transition); return tab_contents; } @@ -2383,13 +2374,6 @@ void Browser::ToggleUseVerticalTabs() { UseVerticalTabsChanged(); } -bool Browser::LargeIconsPermitted() const { - // We don't show the big icons in tabs for TYPE_EXTENSION_APP windows because - // for those windows, we already have a big icon in the top-left outside any - // tab. Having big tab icons too looks kinda redonk. - return TYPE_EXTENSION_APP != type(); -} - /////////////////////////////////////////////////////////////////////////////// // Browser, TabStripModelObserver implementation: @@ -2941,6 +2925,10 @@ void Browser::OnDidGetApplicationInfo(TabContents* tab_contents, pending_web_app_action_ = NONE; } +Browser* Browser::GetBrowser() { + return this; +} + void Browser::ContentTypeChanged(TabContents* source) { if (source == GetSelectedTabContents()) UpdateZoomCommandsForTabState(); @@ -3741,7 +3729,7 @@ bool Browser::HandleCrossAppNavigation(TabContents* source, if (destination_extension->launch_container() == Extension::LAUNCH_WINDOW) { Browser::OpenApplicationWindow(profile_, destination_extension, - Extension::LAUNCH_WINDOW, url, NULL); + Extension::LAUNCH_WINDOW, url); return true; } } diff --git a/chrome/browser/browser.h b/chrome/browser/browser.h index c990ee2..87456fb 100644 --- a/chrome/browser/browser.h +++ b/chrome/browser/browser.h @@ -227,29 +227,20 @@ class Browser : public TabStripModelDelegate, // app panel window, otherwise it will be opened as as either // Browser::Type::APP a.k.a. "thin frame" (if |extension| is NULL) or // Browser::Type::EXTENSION_APP (if |extension| is non-NULL). - // Returns the browser hosting for the TabContents via optional parameter, - // |browser|. static TabContents* OpenApplicationWindow( Profile* profile, Extension* extension, Extension::LaunchContainer container, - const GURL& url, - Browser** browser); + const GURL& url); // Open an application for |extension| in a new application window or panel. - // Returns the browser hosting the TabContents via optional parameter, - // |browser|. static TabContents* OpenApplicationWindow(Profile* profile, - GURL& url, - Browser** browser); + GURL& url); // Open an application for |extension| in a new application tab. Returns // NULL if there are no appropriate existing browser windows for |profile|. - // Returns the browser hosting the TabContents via optional parameter, - // |browser|. static TabContents* OpenApplicationTab(Profile* profile, - Extension* extension, - Browser** browser); + Extension* extension); // Opens a new window and opens the bookmark manager. static void OpenBookmarkManagerWindow(Profile* profile); @@ -664,7 +655,6 @@ class Browser : public TabStripModelDelegate, virtual void ToggleUseVerticalTabs(); virtual bool CanRestoreTab(); virtual void RestoreTab(); - virtual bool LargeIconsPermitted() const; // Overridden from TabStripModelObserver: virtual void TabInsertedAt(TabContents* contents, @@ -744,6 +734,7 @@ class Browser : public TabStripModelDelegate, virtual bool ShouldAddNavigationsToHistory() const; virtual void OnDidGetApplicationInfo(TabContents* tab_contents, int32 page_id); + virtual Browser* GetBrowser(); virtual void ContentTypeChanged(TabContents* source); // Overridden from SelectFileDialog::Listener: diff --git a/chrome/browser/browser_browsertest.cc b/chrome/browser/browser_browsertest.cc index 68a29d3..528c9e4 100644 --- a/chrome/browser/browser_browsertest.cc +++ b/chrome/browser/browser_browsertest.cc @@ -707,8 +707,7 @@ IN_PROC_BROWSER_TEST_F(BrowserAppRefocusTest, MAYBE_OpenTab) { ASSERT_EQ(NULL, Browser::FindAppTab(browser(), extension_app_)); // Open a tab with the app. - TabContents* tab = Browser::OpenApplicationTab(profile_, extension_app_, - NULL); + TabContents* tab = Browser::OpenApplicationTab(profile_, extension_app_); ASSERT_TRUE(WaitForTab(tab)); ASSERT_EQ(2, browser()->tab_count()); @@ -765,7 +764,7 @@ IN_PROC_BROWSER_TEST_F(BrowserAppRefocusTest, MAYBE_OpenPanel) { // Open the app in a panel. Browser::OpenApplicationWindow(profile_, extension_app_, - Extension::LAUNCH_PANEL, GURL(), NULL); + Extension::LAUNCH_PANEL, GURL()); Browser* app_panel = BrowserList::GetLastActive(); ASSERT_TRUE(app_panel); ASSERT_NE(app_panel, browser()) << "New browser should have opened."; @@ -798,7 +797,7 @@ IN_PROC_BROWSER_TEST_F(BrowserAppRefocusTest, MAYBE_OpenWindow) { // Open a window with the app. Browser::OpenApplicationWindow(profile_, extension_app_, - Extension::LAUNCH_WINDOW, GURL(), NULL); + Extension::LAUNCH_WINDOW, GURL()); Browser* app_window = BrowserList::GetLastActive(); ASSERT_TRUE(app_window); ASSERT_NE(app_window, browser()) << "New browser should have opened."; @@ -826,7 +825,7 @@ IN_PROC_BROWSER_TEST_F(BrowserAppRefocusTest, MAYBE_WindowBeforeTab) { ASSERT_EQ(1, browser()->tab_count()); // Open a tab with the app. - Browser::OpenApplicationTab(profile_, extension_app_, NULL); + Browser::OpenApplicationTab(profile_, extension_app_); ASSERT_TRUE(ui_test_utils::WaitForNavigationInCurrentTab(browser())); ASSERT_EQ(2, browser()->tab_count()); int app_tab_index = browser()->selected_index(); @@ -834,7 +833,7 @@ IN_PROC_BROWSER_TEST_F(BrowserAppRefocusTest, MAYBE_WindowBeforeTab) { // Open a window with the app. Browser::OpenApplicationWindow(profile_, extension_app_, - Extension::LAUNCH_WINDOW, GURL(), NULL); + Extension::LAUNCH_WINDOW, GURL()); Browser* app_window = BrowserList::GetLastActive(); ASSERT_TRUE(app_window); ASSERT_NE(app_window, browser()) << "New browser should have opened."; @@ -855,8 +854,7 @@ IN_PROC_BROWSER_TEST_F(BrowserAppRefocusTest, MAYBE_PanelBeforeTab) { ASSERT_EQ(1, browser()->tab_count()); // Open a tab with the app. - TabContents* tab = Browser::OpenApplicationTab(profile_, extension_app_, - NULL); + TabContents* tab = Browser::OpenApplicationTab(profile_, extension_app_); ASSERT_TRUE(WaitForTab(tab)); ASSERT_EQ(2, browser()->tab_count()); int app_tab_index = browser()->selected_index(); @@ -864,7 +862,7 @@ IN_PROC_BROWSER_TEST_F(BrowserAppRefocusTest, MAYBE_PanelBeforeTab) { // Open a panel with the app. Browser::OpenApplicationWindow(profile_, extension_app_, - Extension::LAUNCH_PANEL, GURL(), NULL); + Extension::LAUNCH_PANEL, GURL()); Browser* app_panel = BrowserList::GetLastActive(); ASSERT_TRUE(app_panel); ASSERT_NE(app_panel, browser()) << "New browser should have opened."; @@ -884,7 +882,7 @@ IN_PROC_BROWSER_TEST_F(BrowserAppRefocusTest, MAYBE_TabInFocusedWindow) { ASSERT_EQ(1, browser()->tab_count()); - Browser::OpenApplicationTab(profile_, extension_app_, NULL); + Browser::OpenApplicationTab(profile_, extension_app_); ASSERT_TRUE(ui_test_utils::WaitForNavigationInCurrentTab(browser())); ASSERT_EQ(2, browser()->tab_count()); int app_tab_index = browser()->selected_index(); @@ -894,7 +892,7 @@ IN_PROC_BROWSER_TEST_F(BrowserAppRefocusTest, MAYBE_TabInFocusedWindow) { Browser* extra_browser = CreateBrowser(profile_); ASSERT_EQ(extra_browser, BrowserList::GetLastActive()); - Browser::OpenApplicationTab(profile_, extension_app_, NULL); + Browser::OpenApplicationTab(profile_, extension_app_); ASSERT_TRUE(ui_test_utils::WaitForNavigationInCurrentTab(extra_browser)); ASSERT_EQ(2, extra_browser->tab_count()); app_tab_index = extra_browser->selected_index(); diff --git a/chrome/browser/browser_init.cc b/chrome/browser/browser_init.cc index cf06a3c..58615eb 100644 --- a/chrome/browser/browser_init.cc +++ b/chrome/browser/browser_init.cc @@ -632,7 +632,7 @@ bool BrowserInit::LaunchWithProfile::OpenApplicationWindow(Profile* profile) { ChildProcessSecurityPolicy::GetInstance(); if (policy->IsWebSafeScheme(url.scheme()) || url.SchemeIs(chrome::kFileScheme)) { - Browser::OpenApplicationWindow(profile, url, NULL); + Browser::OpenApplicationWindow(profile, url); return true; } } diff --git a/chrome/browser/cocoa/tab_strip_controller_unittest.mm b/chrome/browser/cocoa/tab_strip_controller_unittest.mm index d41a9f0..2a161ab 100644 --- a/chrome/browser/cocoa/tab_strip_controller_unittest.mm +++ b/chrome/browser/cocoa/tab_strip_controller_unittest.mm @@ -67,8 +67,6 @@ class TestTabStripDelegate : public TabStripModelDelegate { virtual bool UseVerticalTabs() const { return false; } virtual void ToggleUseVerticalTabs() {} - - virtual bool LargeIconsPermitted() const { return true; } }; class TabStripControllerTest : public CocoaTest { diff --git a/chrome/browser/dom_ui/dom_ui.cc b/chrome/browser/dom_ui/dom_ui.cc index debe141..82dcb4b 100644 --- a/chrome/browser/dom_ui/dom_ui.cc +++ b/chrome/browser/dom_ui/dom_ui.cc @@ -86,7 +86,7 @@ void DOMUI::RegisterMessageCallback(const std::string &message, message_callbacks_.insert(std::make_pair(message, callback)); } -Profile* DOMUI::GetProfile() const { +Profile* DOMUI::GetProfile() { return tab_contents()->profile(); } diff --git a/chrome/browser/dom_ui/dom_ui.h b/chrome/browser/dom_ui/dom_ui.h index d29425d..1b333a8 100644 --- a/chrome/browser/dom_ui/dom_ui.h +++ b/chrome/browser/dom_ui/dom_ui.h @@ -116,7 +116,7 @@ class DOMUI { TabContents* tab_contents() const { return tab_contents_; } - Profile* GetProfile() const; + Profile* GetProfile(); protected: void AddMessageHandler(DOMMessageHandler* handler); diff --git a/chrome/browser/dom_ui/history2_ui.cc b/chrome/browser/dom_ui/history2_ui.cc index 2ac33ea..067fec3 100644 --- a/chrome/browser/dom_ui/history2_ui.cc +++ b/chrome/browser/dom_ui/history2_ui.cc @@ -17,7 +17,6 @@ #include "base/values.h" #include "chrome/browser/bookmarks/bookmark_model.h" #include "chrome/browser/browser.h" -#include "chrome/browser/browser_list.h" #include "chrome/browser/chrome_thread.h" #include "chrome/browser/dom_ui/dom_ui_favicon_source.h" #include "chrome/browser/metrics/user_metrics.h" @@ -236,11 +235,8 @@ void BrowsingHistoryHandler2::HandleRemoveURLsOnOneDay(const Value* value) { } void BrowsingHistoryHandler2::HandleClearBrowsingData(const Value* value) { - // TODO(beng): This is an improper direct dependency on Browser. Route this - // through some sort of delegate. - Browser* browser = BrowserList::FindBrowserWithProfile(dom_ui_->GetProfile()); - if (browser) - browser->OpenClearBrowsingDataDialog(); + dom_ui_->tab_contents()->delegate()->GetBrowser()-> + OpenClearBrowsingDataDialog(); } void BrowsingHistoryHandler2::QueryComplete( diff --git a/chrome/browser/dom_ui/history_ui.cc b/chrome/browser/dom_ui/history_ui.cc index e106c4c..fc4fd63 100644 --- a/chrome/browser/dom_ui/history_ui.cc +++ b/chrome/browser/dom_ui/history_ui.cc @@ -17,7 +17,6 @@ #include "base/values.h" #include "chrome/browser/bookmarks/bookmark_model.h" #include "chrome/browser/browser.h" -#include "chrome/browser/browser_list.h" #include "chrome/browser/chrome_thread.h" #include "chrome/browser/dom_ui/dom_ui_favicon_source.h" #include "chrome/browser/metrics/user_metrics.h" @@ -236,11 +235,8 @@ void BrowsingHistoryHandler::HandleRemoveURLsOnOneDay(const Value* value) { } void BrowsingHistoryHandler::HandleClearBrowsingData(const Value* value) { - // TODO(beng): This is an improper direct dependency on Browser. Route this - // through some sort of delegate. - Browser* browser = BrowserList::FindBrowserWithProfile(dom_ui_->GetProfile()); - if (browser) - browser->OpenClearBrowsingDataDialog(); + dom_ui_->tab_contents()->delegate()->GetBrowser()-> + OpenClearBrowsingDataDialog(); } void BrowsingHistoryHandler::QueryComplete( diff --git a/chrome/browser/extensions/extension_dom_ui.cc b/chrome/browser/extensions/extension_dom_ui.cc index 550f45f..7cbe08f 100644 --- a/chrome/browser/extensions/extension_dom_ui.cc +++ b/chrome/browser/extensions/extension_dom_ui.cc @@ -176,9 +176,12 @@ void ExtensionDOMUI::ProcessDOMUIMessage(const std::string& message, } Browser* ExtensionDOMUI::GetBrowser() const { - // TODO(beng): This is an improper direct dependency on Browser. Route this - // through some sort of delegate. - return BrowserList::FindBrowserWithProfile(DOMUI::GetProfile()); + Browser* browser = NULL; + TabContentsDelegate* tab_contents_delegate = tab_contents()->delegate(); + if (tab_contents_delegate) + browser = tab_contents_delegate->GetBrowser(); + + return browser; } Profile* ExtensionDOMUI::GetProfile() { diff --git a/chrome/browser/external_tab_container_win.h b/chrome/browser/external_tab_container_win.h index 031378e..0db6625 100644 --- a/chrome/browser/external_tab_container_win.h +++ b/chrome/browser/external_tab_container_win.h @@ -148,6 +148,8 @@ class ExternalTabContainer : public TabContentsDelegate, const NavigationEntry::SSLStatus& ssl, bool show_history); + virtual Browser* GetBrowser() { return browser_.get(); } + // Overriden from TabContentsDelegate::AutomationResourceRoutingDelegate virtual void RegisterRenderViewHost(RenderViewHost* render_view_host); virtual void UnregisterRenderViewHost(RenderViewHost* render_view_host); diff --git a/chrome/browser/tab_contents/tab_contents.cc b/chrome/browser/tab_contents/tab_contents.cc index 41e644c..5ab30a2 100644 --- a/chrome/browser/tab_contents/tab_contents.cc +++ b/chrome/browser/tab_contents/tab_contents.cc @@ -23,6 +23,7 @@ #include "chrome/browser/autofill/autofill_manager.h" #include "chrome/browser/blocked_popup_container.h" #include "chrome/browser/bookmarks/bookmark_model.h" +#include "chrome/browser/browser.h" #include "chrome/browser/browser_process.h" #include "chrome/browser/browser_shutdown.h" #include "chrome/browser/cert_store.h" @@ -538,6 +539,13 @@ void TabContents::SetExtensionAppById(const std::string& extension_app_id) { } SkBitmap* TabContents::GetExtensionAppIcon() { + // We don't show the big icons in tabs for TYPE_EXTENSION_APP windows because + // for those windows, we already have a big icon in the top-left outside any + // tab. Having big tab icons too looks kinda redonk. + Browser* browser = delegate_ ? delegate_->GetBrowser() : NULL; + if (browser && browser->type() == Browser::TYPE_EXTENSION_APP) + return NULL; + if (extension_app_icon_.empty()) return NULL; diff --git a/chrome/browser/tab_contents/tab_contents_delegate.cc b/chrome/browser/tab_contents/tab_contents_delegate.cc index 8a03160..9cf9f88 100644 --- a/chrome/browser/tab_contents/tab_contents_delegate.cc +++ b/chrome/browser/tab_contents/tab_contents_delegate.cc @@ -139,6 +139,10 @@ void TabContentsDelegate::OnDidGetApplicationInfo(TabContents* tab_contents, int32 page_id) { } +Browser* TabContentsDelegate::GetBrowser() { + return NULL; +} + gfx::NativeWindow TabContentsDelegate::GetFrameNativeWindow() { return NULL; } diff --git a/chrome/browser/tab_contents/tab_contents_delegate.h b/chrome/browser/tab_contents/tab_contents_delegate.h index c97483c..ff1f49b 100644 --- a/chrome/browser/tab_contents/tab_contents_delegate.h +++ b/chrome/browser/tab_contents/tab_contents_delegate.h @@ -19,6 +19,7 @@ #include "webkit/glue/context_menu.h" #include "webkit/glue/window_open_disposition.h" +class Browser; class DownloadItem; class ExtensionFunctionDispatcher; class GURL; @@ -247,6 +248,9 @@ class TabContentsDelegate : public AutomationResourceRoutingDelegate { virtual void OnDidGetApplicationInfo(TabContents* tab_contents, int32 page_id); + // Returns the browser in which the tab contents is being displayed. + virtual Browser* GetBrowser(); + // Returns the native window framing the view containing the tab contents. virtual gfx::NativeWindow GetFrameNativeWindow(); diff --git a/chrome/browser/tabs/tab_strip_model.h b/chrome/browser/tabs/tab_strip_model.h index 92cecfd..f93bfc5 100644 --- a/chrome/browser/tabs/tab_strip_model.h +++ b/chrome/browser/tabs/tab_strip_model.h @@ -241,9 +241,6 @@ class TabStripModelDelegate { // Toggles the use of the vertical tabstrip. virtual void ToggleUseVerticalTabs() = 0; - // Returns true if the tab strip can use large icons. - virtual bool LargeIconsPermitted() const = 0; - protected: virtual ~TabStripModelDelegate() {} }; diff --git a/chrome/browser/tabs/tab_strip_model_unittest.cc b/chrome/browser/tabs/tab_strip_model_unittest.cc index 6f53630..c885d5f 100644 --- a/chrome/browser/tabs/tab_strip_model_unittest.cc +++ b/chrome/browser/tabs/tab_strip_model_unittest.cc @@ -80,7 +80,6 @@ class TabStripDummyDelegate : public TabStripModelDelegate { virtual void BookmarkAllTabs() {} virtual bool UseVerticalTabs() const { return false; } virtual void ToggleUseVerticalTabs() {} - virtual bool LargeIconsPermitted() const { return true; } private: // A dummy TabContents we give to callers that expect us to actually build a diff --git a/chrome/browser/views/frame/browser_view.cc b/chrome/browser/views/frame/browser_view.cc index 1df704d..0e11eaa 100644 --- a/chrome/browser/views/frame/browser_view.cc +++ b/chrome/browser/views/frame/browser_view.cc @@ -1701,7 +1701,7 @@ void BrowserView::InitTabStrip(TabStripModel* model) { } BrowserTabStripController* tabstrip_controller = - new BrowserTabStripController(browser_.get(), model); + new BrowserTabStripController(model); if (UseVerticalTabs()) tabstrip_ = new SideTabStrip(tabstrip_controller); diff --git a/chrome/browser/views/tabs/browser_tab_strip_controller.cc b/chrome/browser/views/tabs/browser_tab_strip_controller.cc index f0422f1..1221133 100644 --- a/chrome/browser/views/tabs/browser_tab_strip_controller.cc +++ b/chrome/browser/views/tabs/browser_tab_strip_controller.cc @@ -112,11 +112,9 @@ class BrowserTabStripController::TabContextMenuContents //////////////////////////////////////////////////////////////////////////////// // BrowserTabStripController, public: -BrowserTabStripController::BrowserTabStripController(Browser* browser, - TabStripModel* model) +BrowserTabStripController::BrowserTabStripController(TabStripModel* model) : model_(model), - tabstrip_(NULL), - browser_(browser) { + tabstrip_(NULL) { model_->AddObserver(this); notification_registrar_.Add(this, @@ -271,7 +269,12 @@ void BrowserTabStripController::CreateNewTab() { UserMetrics::RecordAction(UserMetricsAction("NewTab_Button"), model_->profile()); - if (browser_ && browser_->OpenAppsPanelAsNewTab()) + TabContents* selected_tab = model_->GetSelectedTabContents(); + if (!selected_tab) + return; + + Browser* browser = selected_tab->delegate()->GetBrowser(); + if (browser->OpenAppsPanelAsNewTab()) return; model_->delegate()->AddBlankTab(true); @@ -365,13 +368,7 @@ void BrowserTabStripController::SetTabRendererDataFromModel( TabContents* contents, int model_index, TabRendererData* data) { - SkBitmap* app_icon = NULL; - - // Extension App icons are slightly larger than favicons, so only allow - // them if permitted by the model. - if (model_->delegate()->LargeIconsPermitted()) - app_icon = contents->GetExtensionAppIcon(); - + SkBitmap* app_icon = contents->GetExtensionAppIcon(); if (app_icon) data->favicon = *app_icon; else diff --git a/chrome/browser/views/tabs/browser_tab_strip_controller.h b/chrome/browser/views/tabs/browser_tab_strip_controller.h index e900413..662a0c1 100644 --- a/chrome/browser/views/tabs/browser_tab_strip_controller.h +++ b/chrome/browser/views/tabs/browser_tab_strip_controller.h @@ -13,7 +13,6 @@ class BaseTab; class BaseTabStrip; -class Browser; struct TabRendererData; @@ -23,7 +22,7 @@ class BrowserTabStripController : public TabStripController, public TabStripModelObserver, public NotificationObserver { public: - BrowserTabStripController(Browser* browser, TabStripModel* model); + explicit BrowserTabStripController(TabStripModel* model); virtual ~BrowserTabStripController(); void InitFromModel(BaseTabStrip* tabstrip); @@ -105,9 +104,6 @@ class BrowserTabStripController : public TabStripController, BaseTabStrip* tabstrip_; - // Non-owning pointer to the browser which is using this controller. - Browser* browser_; - // If non-NULL it means we're showing a menu for the tab. scoped_ptr context_menu_contents_; -- cgit v1.1