summaryrefslogtreecommitdiffstats
path: root/chrome/browser/tabs
diff options
context:
space:
mode:
authorben@chromium.org <ben@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2009-01-24 07:07:34 +0000
committerben@chromium.org <ben@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2009-01-24 07:07:34 +0000
commit616dab5a62842c3d98b65620ad986fd485e54eec (patch)
tree9b831f5f45d01502d6f828f54562b7b970501e65 /chrome/browser/tabs
parent9d80e0794fecdda750b11cf268f811881f0ed0ec (diff)
downloadchromium_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.cc14
-rw-r--r--chrome/browser/tabs/tab_strip_model.h10
-rw-r--r--chrome/browser/tabs/tab_strip_model_unittest.cc108
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();