diff options
author | ben@chromium.org <ben@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2009-01-24 07:07:34 +0000 |
---|---|---|
committer | ben@chromium.org <ben@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2009-01-24 07:07:34 +0000 |
commit | 616dab5a62842c3d98b65620ad986fd485e54eec (patch) | |
tree | 9b831f5f45d01502d6f828f54562b7b970501e65 /chrome/browser/tabs | |
parent | 9d80e0794fecdda750b11cf268f811881f0ed0ec (diff) | |
download | chromium_src-616dab5a62842c3d98b65620ad986fd485e54eec.zip chromium_src-616dab5a62842c3d98b65620ad986fd485e54eec.tar.gz chromium_src-616dab5a62842c3d98b65620ad986fd485e54eec.tar.bz2 |
Make sure the TabStripModelDelegate can never be NULL.
TBR=brettw
Review URL: http://codereview.chromium.org/18578
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@8612 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome/browser/tabs')
-rw-r--r-- | chrome/browser/tabs/tab_strip_model.cc | 14 | ||||
-rw-r--r-- | chrome/browser/tabs/tab_strip_model.h | 10 | ||||
-rw-r--r-- | chrome/browser/tabs/tab_strip_model_unittest.cc | 108 |
3 files changed, 68 insertions, 64 deletions
diff --git a/chrome/browser/tabs/tab_strip_model.cc b/chrome/browser/tabs/tab_strip_model.cc index 938bd74..c6b47e3 100644 --- a/chrome/browser/tabs/tab_strip_model.cc +++ b/chrome/browser/tabs/tab_strip_model.cc @@ -22,6 +22,7 @@ TabStripModel::TabStripModel(TabStripModelDelegate* delegate, Profile* profile) selected_index_(kNoTab), closing_all_(false), order_controller_(NULL) { + DCHECK(delegate_); NotificationService::current()->AddObserver(this, NOTIFY_TAB_CONTENTS_DESTROYED, NotificationService::AllSources()); SetOrderController(new TabStripModelOrderController(this)); @@ -299,7 +300,6 @@ bool TabStripModel::ShouldResetGroupOnSelect(TabContents* contents) const { } TabContents* TabStripModel::AddBlankTab(bool foreground) { - DCHECK(delegate_); TabContents* contents = delegate_->CreateTabContentsForURL( delegate_->GetBlankTabURL(), GURL(), profile_, PageTransition::TYPED, false, NULL); @@ -308,7 +308,6 @@ TabContents* TabStripModel::AddBlankTab(bool foreground) { } TabContents* TabStripModel::AddBlankTabAt(int index, bool foreground) { - DCHECK(delegate_); TabContents* contents = delegate_->CreateTabContentsForURL( delegate_->GetBlankTabURL(), GURL(), profile_, PageTransition::LINK, false, NULL); @@ -402,10 +401,7 @@ bool TabStripModel::IsContextMenuCommandEnabled( return next_index != kNoTab; } case CommandDuplicate: - if (delegate_) - return delegate_->CanDuplicateContentsAt(context_index); - else - return false; + return delegate_->CanDuplicateContentsAt(context_index); default: NOTREACHED(); } @@ -425,10 +421,8 @@ void TabStripModel::ExecuteContextMenuCommand( GetContentsAt(context_index)->controller()->Reload(true); break; case CommandDuplicate: - if (delegate_) { - UserMetrics::RecordAction(L"TabContextMenu_Duplicate", profile_); - delegate_->DuplicateContentsAt(context_index); - } + UserMetrics::RecordAction(L"TabContextMenu_Duplicate", profile_); + delegate_->DuplicateContentsAt(context_index); break; case CommandCloseTab: UserMetrics::RecordAction(L"TabContextMenu_CloseTab", profile_); diff --git a/chrome/browser/tabs/tab_strip_model.h b/chrome/browser/tabs/tab_strip_model.h index a9a6578..a6bf00f 100644 --- a/chrome/browser/tabs/tab_strip_model.h +++ b/chrome/browser/tabs/tab_strip_model.h @@ -2,8 +2,8 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#ifndef CHROME_BROWSER_TABS_TAB_STRIP_MODEL_H__ -#define CHROME_BROWSER_TABS_TAB_STRIP_MODEL_H__ +#ifndef CHROME_BROWSER_TABS_TAB_STRIP_MODEL_H_ +#define CHROME_BROWSER_TABS_TAB_STRIP_MODEL_H_ #include <vector> @@ -167,7 +167,7 @@ class TabStripModelDelegate { class TabStripModel : public NotificationObserver { public: // Construct a TabStripModel with a delegate to help it do certain things - // (See TabStripModelDelegate documentation). + // (See TabStripModelDelegate documentation). |delegate| cannot be NULL. TabStripModel(TabStripModelDelegate* delegate, Profile* profile); virtual ~TabStripModel(); @@ -518,7 +518,7 @@ class TabStripModel : public NotificationObserver { typedef ObserverList<TabStripModelObserver> TabStripModelObservers; TabStripModelObservers observers_; - DISALLOW_EVIL_CONSTRUCTORS(TabStripModel); + DISALLOW_COPY_AND_ASSIGN(TabStripModel); }; -#endif // CHROME_BROWSER_TABS_TAB_STRIP_MODEL_H__ +#endif // CHROME_BROWSER_TABS_TAB_STRIP_MODEL_H_ diff --git a/chrome/browser/tabs/tab_strip_model_unittest.cc b/chrome/browser/tabs/tab_strip_model_unittest.cc index 84b3ceb..ef536a0 100644 --- a/chrome/browser/tabs/tab_strip_model_unittest.cc +++ b/chrome/browser/tabs/tab_strip_model_unittest.cc @@ -23,6 +23,45 @@ const TabContentsType kHTTPTabContentsType = const TabContentsType kReplacementContentsType = static_cast<TabContentsType>(kHTTPTabContentsType + 1); +class TabStripDummyDelegate : public TabStripModelDelegate { + public: + explicit TabStripDummyDelegate(TabContents* dummy) + : dummy_contents_(dummy) {} + virtual ~TabStripDummyDelegate() {} + + // Overridden from TabStripModelDelegate: + virtual GURL GetBlankTabURL() const { return NewTabUIURL(); } + virtual void CreateNewStripWithContents(TabContents* contents, + const gfx::Rect& window_bounds, + const DockInfo& dock_info) {} + virtual int GetDragActions() const { return 0; } + virtual TabContents* CreateTabContentsForURL( + const GURL& url, + const GURL& referrer, + Profile* profile, + PageTransition::Type transition, + bool defer_load, + SiteInstance* instance) const { + if (url == NewTabUIURL()) + return dummy_contents_; + return NULL; + } + virtual bool CanDuplicateContentsAt(int index) { return false; } + virtual void DuplicateContentsAt(int index) {} + virtual void CloseFrameAfterDragSession() {} + virtual void CreateHistoricalTab(TabContents* contents) {} + virtual bool RunUnloadListenerBeforeClosing(TabContents* contents) { + return false; + } + + private: + // A dummy TabContents we give to callers that expect us to actually build a + // Destinations tab for them. + TabContents* dummy_contents_; + + DISALLOW_EVIL_CONSTRUCTORS(TabStripDummyDelegate); +}; + // Since you can't just instantiate a TabContents, and some of its methods // are protected, we subclass TabContents with our own testing dummy which // knows how to drive the base class' NavigationController as URLs are @@ -252,7 +291,8 @@ class MockTabStripModelObserver : public TabStripModelObserver { }; TEST_F(TabStripModelTest, TestBasicAPI) { - TabStripModel tabstrip(NULL, profile_); + TabStripDummyDelegate delegate(NULL); + TabStripModel tabstrip(&delegate, profile_); MockTabStripModelObserver observer; tabstrip.AddObserver(&observer); @@ -458,7 +498,8 @@ TEST_F(TabStripModelTest, TestBasicAPI) { } TEST_F(TabStripModelTest, TestBasicOpenerAPI) { - TabStripModel tabstrip(NULL, profile_); + TabStripDummyDelegate delegate(NULL); + TabStripModel tabstrip(&delegate, profile_); EXPECT_TRUE(tabstrip.empty()); // This is a basic test of opener functionality. opener_contents is created @@ -532,7 +573,8 @@ static void InsertTabContentses(TabStripModel* tabstrip, // Tests opening background tabs. TEST_F(TabStripModelTest, TestLTRInsertionOptions) { - TabStripModel tabstrip(NULL, profile_); + TabStripDummyDelegate delegate(NULL); + TabStripModel tabstrip(&delegate, profile_); EXPECT_TRUE(tabstrip.empty()); TabContents* opener_contents = CreateTabContents(); @@ -559,7 +601,8 @@ TEST_F(TabStripModelTest, TestLTRInsertionOptions) { // Finally it tests that a tab opened for some non-link purpose openes at the // end of the strip, not bundled to any existing context. TEST_F(TabStripModelTest, TestInsertionIndexDetermination) { - TabStripModel tabstrip(NULL, profile_); + TabStripDummyDelegate delegate(NULL); + TabStripModel tabstrip(&delegate, profile_); EXPECT_TRUE(tabstrip.empty()); TabContents* opener_contents = CreateTabContents(); @@ -637,7 +680,8 @@ TEST_F(TabStripModelTest, TestInsertionIndexDetermination) { // The opener is selected // TEST_F(TabStripModelTest, TestSelectOnClose) { - TabStripModel tabstrip(NULL, profile_); + TabStripDummyDelegate delegate(NULL); + TabStripModel tabstrip(&delegate, profile_); EXPECT_TRUE(tabstrip.empty()); TabContents* opener_contents = CreateTabContents(); @@ -714,7 +758,8 @@ TEST_F(TabStripModelTest, TestSelectOnClose) { // - Close Tabs To Right // - Close Tabs Opened By TEST_F(TabStripModelTest, TestContextMenuCloseCommands) { - TabStripModel tabstrip(NULL, profile_); + TabStripDummyDelegate delegate(NULL); + TabStripModel tabstrip(&delegate, profile_); EXPECT_TRUE(tabstrip.empty()); TabContents* opener_contents = CreateTabContents(); @@ -771,7 +816,8 @@ TEST_F(TabStripModelTest, TestContextMenuCloseCommands) { // using this "smart" function with a simulated middle click action on a series // of links on the home page. TEST_F(TabStripModelTest, AddTabContents_MiddleClickLinksAndClose) { - TabStripModel tabstrip(NULL, profile_); + TabStripDummyDelegate delegate(NULL); + TabStripModel tabstrip(&delegate, profile_); EXPECT_TRUE(tabstrip.empty()); // Open the Home Page @@ -834,7 +880,8 @@ TEST_F(TabStripModelTest, AddTabContents_MiddleClickLinksAndClose) { // Tests whether or not a TabContents created by a left click on a link that // opens a new tab is inserted correctly adjacent to the tab that spawned it. TEST_F(TabStripModelTest, AddTabContents_LeftClickPopup) { - TabStripModel tabstrip(NULL, profile_); + TabStripDummyDelegate delegate(NULL); + TabStripModel tabstrip(&delegate, profile_); EXPECT_TRUE(tabstrip.empty()); // Open the Home Page @@ -880,7 +927,8 @@ TEST_F(TabStripModelTest, AddTabContents_LeftClickPopup) { // generated urls, also blank tabs) open at the end of the tabstrip instead of // in the middle. TEST_F(TabStripModelTest, AddTabContents_CreateNewBlankTab) { - TabStripModel tabstrip(NULL, profile_); + TabStripDummyDelegate delegate(NULL); + TabStripModel tabstrip(&delegate, profile_); EXPECT_TRUE(tabstrip.empty()); // Open the Home Page @@ -929,7 +977,8 @@ TEST_F(TabStripModelTest, AddTabContents_CreateNewBlankTab) { // Tests whether opener state is correctly forgotten when the user switches // context. TEST_F(TabStripModelTest, AddTabContents_ForgetOpeners) { - TabStripModel tabstrip(NULL, profile_); + TabStripDummyDelegate delegate(NULL); + TabStripModel tabstrip(&delegate, profile_); EXPECT_TRUE(tabstrip.empty()); // Open the Home Page @@ -986,45 +1035,6 @@ TEST_F(TabStripModelTest, AddTabContents_ForgetOpeners) { EXPECT_TRUE(tabstrip.empty()); } -class TabStripDummyDelegate : public TabStripModelDelegate { - public: - explicit TabStripDummyDelegate(TabContents* dummy) - : dummy_contents_(dummy) {} - virtual ~TabStripDummyDelegate() {} - - // Overridden from TabStripModelDelegate: - virtual GURL GetBlankTabURL() const { return NewTabUIURL(); } - virtual void CreateNewStripWithContents(TabContents* contents, - const gfx::Rect& window_bounds, - const DockInfo& dock_info) {} - virtual int GetDragActions() const { return 0; } - virtual TabContents* CreateTabContentsForURL( - const GURL& url, - const GURL& referrer, - Profile* profile, - PageTransition::Type transition, - bool defer_load, - SiteInstance* instance) const { - if (url == NewTabUIURL()) - return dummy_contents_; - return NULL; - } - virtual bool CanDuplicateContentsAt(int index) { return false; } - virtual void DuplicateContentsAt(int index) {} - virtual void CloseFrameAfterDragSession() {} - virtual void CreateHistoricalTab(TabContents* contents) {} - virtual bool RunUnloadListenerBeforeClosing(TabContents* contents) { - return false; - } - - private: - // A dummy TabContents we give to callers that expect us to actually build a - // Destinations tab for them. - TabContents* dummy_contents_; - - DISALLOW_EVIL_CONSTRUCTORS(TabStripDummyDelegate); -}; - // Added for http://b/issue?id=958960 TEST_F(TabStripModelTest, AppendContentsReselectionTest) { TabContents* fake_destinations_tab = CreateTabContents(); |