diff options
author | sky@chromium.org <sky@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-03-25 15:38:26 +0000 |
---|---|---|
committer | sky@chromium.org <sky@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-03-25 15:38:26 +0000 |
commit | 84c78e049c5a178fea320b27d5da59674bba8070 (patch) | |
tree | 118a2973daf1075b87dfa253764c20e32aa36665 /chrome/browser/tabs | |
parent | 1ca6199521f472a2baac815884fc935f1cc7c92f (diff) | |
download | chromium_src-84c78e049c5a178fea320b27d5da59674bba8070.zip chromium_src-84c78e049c5a178fea320b27d5da59674bba8070.tar.gz chromium_src-84c78e049c5a178fea320b27d5da59674bba8070.tar.bz2 |
Fixes bug in tabstrip that resulted in not repainting when we
should. This was the result of the tabstrip not notifying observers
when the selection changed. For example, if tabs 1 and 2 were selected
and 1 active, and 2 was removed observers weren't notified.
BUG=none
TEST=none
R=ben@chromium.org
Review URL: http://codereview.chromium.org/6736011
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@79403 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_unittest.cc | 28 |
2 files changed, 40 insertions, 2 deletions
diff --git a/chrome/browser/tabs/tab_strip_model.cc b/chrome/browser/tabs/tab_strip_model.cc index 2925e53..407bb00 100644 --- a/chrome/browser/tabs/tab_strip_model.cc +++ b/chrome/browser/tabs/tab_strip_model.cc @@ -242,11 +242,21 @@ TabContentsWrapper* TabStripModel::DetachTabContentsAt(int index) { void TabStripModel::SelectTabContentsAt(int index, bool user_gesture) { DCHECK(ContainsIndex(index)); - TabContentsWrapper* old = + bool had_multi = selection_model_.selected_indices().size() > 1; + TabContentsWrapper* old_contents = (selected_index() == TabStripSelectionModel::kUnselectedIndex) ? NULL : GetSelectedTabContents(); selection_model_.SetSelectedIndex(index); - NotifyTabSelectedIfChanged(old, index, user_gesture); + TabContentsWrapper* new_contents = GetContentsAt(index); + if (old_contents != new_contents && old_contents) { + FOR_EACH_OBSERVER(TabStripModelObserver, observers_, + TabDeselected(old_contents)); + } + if (old_contents != new_contents || had_multi) { + FOR_EACH_OBSERVER(TabStripModelObserver, observers_, + TabSelectedAt(old_contents, new_contents, + selected_index(), user_gesture)); + } } void TabStripModel::MoveTabContentsAt(int index, diff --git a/chrome/browser/tabs/tab_strip_model_unittest.cc b/chrome/browser/tabs/tab_strip_model_unittest.cc index cf734ab..6f51e02 100644 --- a/chrome/browser/tabs/tab_strip_model_unittest.cc +++ b/chrome/browser/tabs/tab_strip_model_unittest.cc @@ -2173,3 +2173,31 @@ TEST_F(TabStripModelTest, CloseSelectedTabs) { EXPECT_EQ(0, strip.selected_index()); strip.CloseAllTabs(); } + +// Verifies that if we change the selection from a multi selection to a single +// selection, but not in a way that changes the selected_index that +// TabSelectedAt is still invoked. +TEST_F(TabStripModelTest, MultipleToSingle) { + TabStripDummyDelegate delegate(NULL); + TabStripModel strip(&delegate, profile()); + TabContentsWrapper* contents1 = CreateTabContents(); + TabContentsWrapper* contents2 = CreateTabContents(); + strip.AppendTabContents(contents1, false); + strip.AppendTabContents(contents2, false); + strip.ToggleSelectionAt(0); + strip.ToggleSelectionAt(1); + + MockTabStripModelObserver observer; + strip.AddObserver(&observer); + // This changes the selection (0 is no longer selected) but the selected_index + // still remains at 1. + strip.SelectTabContentsAt(1, true); + ASSERT_EQ(1, observer.GetStateCount()); + MockTabStripModelObserver::State s( + contents2, 1, MockTabStripModelObserver::SELECT); + s.src_contents = contents2; + s.user_gesture = true; + EXPECT_TRUE(observer.StateEquals(0, s)); + strip.RemoveObserver(&observer); + strip.CloseAllTabs(); +} |