summaryrefslogtreecommitdiffstats
path: root/chrome/browser
diff options
context:
space:
mode:
authordpapad@chromium.org <dpapad@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2011-06-23 02:08:48 +0000
committerdpapad@chromium.org <dpapad@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2011-06-23 02:08:48 +0000
commit6ccdfd6a972a621805ef9978e611ade6a658a539 (patch)
tree2d48e9f7f7c8f0f8252786276d46f5dd77acf3ff /chrome/browser
parent49ba27e2817e78847960db8c4434f760591fcbd0 (diff)
downloadchromium_src-6ccdfd6a972a621805ef9978e611ade6a658a539.zip
chromium_src-6ccdfd6a972a621805ef9978e611ade6a658a539.tar.gz
chromium_src-6ccdfd6a972a621805ef9978e611ade6a658a539.tar.bz2
Multi-tab: Adding new Notification when tab selection changes (again).
This is mostly a copy of http://codereview.chromium.org/7033048/ which was reverted. Only changes from 7033048 are 1) Fixing linux_touch trivial compile_error. 2) Adding <iterator> header in chrome/browser/ui/views/tabs/tab_strip.cc because Win Builder 2010 failed because of that. BUG=NONE TEST=NONE Review URL: http://codereview.chromium.org/7215003 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@90155 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome/browser')
-rw-r--r--chrome/browser/aeropeek_manager.cc3
-rw-r--r--chrome/browser/extensions/extension_browser_event_router.cc3
-rw-r--r--chrome/browser/tabs/tab_strip_model.cc112
-rw-r--r--chrome/browser/tabs/tab_strip_model.h25
-rw-r--r--chrome/browser/tabs/tab_strip_model_observer.cc4
-rw-r--r--chrome/browser/tabs/tab_strip_model_observer.h25
-rw-r--r--chrome/browser/tabs/tab_strip_model_order_controller.cc3
-rw-r--r--chrome/browser/tabs/tab_strip_model_unittest.cc134
-rw-r--r--chrome/browser/tabs/tab_strip_selection_model.cc6
-rw-r--r--chrome/browser/tabs/tab_strip_selection_model.h3
-rw-r--r--chrome/browser/tabs/tab_strip_selection_model_unittest.cc17
-rw-r--r--chrome/browser/ui/browser.cc3
-rw-r--r--chrome/browser/ui/cocoa/tabs/tab_strip_controller.mm2
-rw-r--r--chrome/browser/ui/gtk/browser_window_gtk.cc3
-rw-r--r--chrome/browser/ui/touch/frame/touch_browser_frame_view.cc3
-rw-r--r--chrome/browser/ui/touch/tabs/touch_tab_strip.cc4
-rw-r--r--chrome/browser/ui/touch/tabs/touch_tab_strip.h4
-rw-r--r--chrome/browser/ui/views/frame/browser_view.cc3
-rw-r--r--chrome/browser/ui/views/tabs/base_tab_strip.h4
-rw-r--r--chrome/browser/ui/views/tabs/browser_tab_strip_controller.cc11
-rw-r--r--chrome/browser/ui/views/tabs/browser_tab_strip_controller.h7
-rw-r--r--chrome/browser/ui/views/tabs/side_tab_strip.cc11
-rw-r--r--chrome/browser/ui/views/tabs/side_tab_strip.h5
-rw-r--r--chrome/browser/ui/views/tabs/tab_strip.cc17
-rw-r--r--chrome/browser/ui/views/tabs/tab_strip.h5
25 files changed, 286 insertions, 131 deletions
diff --git a/chrome/browser/aeropeek_manager.cc b/chrome/browser/aeropeek_manager.cc
index f4a2da6..57df43c 100644
--- a/chrome/browser/aeropeek_manager.cc
+++ b/chrome/browser/aeropeek_manager.cc
@@ -1115,9 +1115,6 @@ void AeroPeekManager::ActiveTabChanged(TabContentsWrapper* old_contents,
TabContentsWrapper* new_contents,
int index,
bool user_gesture) {
- if (old_contents == new_contents)
- return;
-
// Deactivate the old window in the thumbnail list and activate the new one
// to synchronize the thumbnail list with TabStrip.
if (old_contents) {
diff --git a/chrome/browser/extensions/extension_browser_event_router.cc b/chrome/browser/extensions/extension_browser_event_router.cc
index 2db040b..e510b2d 100644
--- a/chrome/browser/extensions/extension_browser_event_router.cc
+++ b/chrome/browser/extensions/extension_browser_event_router.cc
@@ -331,9 +331,6 @@ void ExtensionBrowserEventRouter::ActiveTabChanged(
TabContentsWrapper* new_contents,
int index,
bool user_gesture) {
- if (old_contents == new_contents)
- return;
-
ListValue args;
args.Append(Value::CreateIntegerValue(
ExtensionTabUtil::GetTabId(new_contents->tab_contents())));
diff --git a/chrome/browser/tabs/tab_strip_model.cc b/chrome/browser/tabs/tab_strip_model.cc
index 35aa0d1..13c1b2e 100644
--- a/chrome/browser/tabs/tab_strip_model.cc
+++ b/chrome/browser/tabs/tab_strip_model.cc
@@ -157,10 +157,11 @@ void TabStripModel::InsertTabContentsAt(int index,
FOR_EACH_OBSERVER(TabStripModelObserver, observers_,
TabInsertedAt(contents, index, active));
-
+ TabStripSelectionModel old_model;
+ old_model.Copy(selection_model_);
if (active) {
selection_model_.SetSelectedIndex(index);
- NotifyTabSelectedIfChanged(selected_contents, index, false);
+ NotifyIfActiveOrSelectionChanged(selected_contents, false, old_model);
}
}
@@ -206,6 +207,7 @@ TabContentsWrapper* TabStripModel::DetachTabContentsAt(int index) {
DCHECK(ContainsIndex(index));
TabContentsWrapper* removed_contents = GetContentsAt(index);
+ bool was_selected = IsTabSelected(index);
int next_selected_index = order_controller_->DetermineNewSelectedIndex(index);
delete contents_data_.at(index);
contents_data_.erase(contents_data_.begin() + index);
@@ -215,26 +217,36 @@ TabContentsWrapper* TabStripModel::DetachTabContentsAt(int index) {
FOR_EACH_OBSERVER(TabStripModelObserver, observers_,
TabDetachedAt(removed_contents, index));
if (empty()) {
+ selection_model_.Clear();
// TabDetachedAt() might unregister observers, so send |TabStripEmtpy()| in
// a second pass.
FOR_EACH_OBSERVER(TabStripModelObserver, observers_, TabStripEmpty());
} else {
int old_active = active_index();
selection_model_.DecrementFrom(index);
+ TabStripSelectionModel old_model;
+ old_model.Copy(selection_model_);
if (index == old_active) {
if (!selection_model_.empty()) {
- // A selected tab was removed, but there is still something selected.
+ // The active tab was removed, but there is still something selected.
// Move the active and anchor to the first selected index.
selection_model_.set_active(selection_model_.selected_indices()[0]);
selection_model_.set_anchor(selection_model_.active());
- NotifyTabSelectedIfChanged(removed_contents, active_index(), false);
} else {
// The active tab was removed and nothing is selected. Reset the
// selection and send out notification.
selection_model_.SetSelectedIndex(next_selected_index);
- NotifyTabSelectedIfChanged(removed_contents, next_selected_index,
- false);
}
+ NotifyIfActiveTabChanged(removed_contents, false);
+ }
+
+ // Sending notification in case the detached tab was selected. Using
+ // NotifyIfActiveOrSelectionChanged() here would not guarantee that a
+ // notification is sent even though the tab selection has changed because
+ // |old_model| is stored after calling DecrementFrom().
+ if (was_selected) {
+ FOR_EACH_OBSERVER(TabStripModelObserver, observers_,
+ TabSelectionChanged(old_model));
}
}
return removed_contents;
@@ -242,21 +254,11 @@ TabContentsWrapper* TabStripModel::DetachTabContentsAt(int index) {
void TabStripModel::ActivateTabAt(int index, bool user_gesture) {
DCHECK(ContainsIndex(index));
- bool had_multi = selection_model_.selected_indices().size() > 1;
- TabContentsWrapper* old_contents =
- (active_index() == TabStripSelectionModel::kUnselectedIndex) ?
- NULL : GetActiveTabContents();
+ TabContentsWrapper* old_contents = GetActiveTabContents();
+ TabStripSelectionModel old_model;
+ old_model.Copy(selection_model_);
selection_model_.SetSelectedIndex(index);
- TabContentsWrapper* new_contents = GetContentsAt(index);
- if (old_contents != new_contents && old_contents) {
- FOR_EACH_OBSERVER(TabStripModelObserver, observers_,
- TabDeactivated(old_contents));
- }
- if (old_contents != new_contents || had_multi) {
- FOR_EACH_OBSERVER(TabStripModelObserver, observers_,
- ActiveTabChanged(old_contents, new_contents,
- active_index(), user_gesture));
- }
+ NotifyIfActiveOrSelectionChanged(old_contents, user_gesture, old_model);
}
void TabStripModel::MoveTabContentsAt(int index,
@@ -568,15 +570,18 @@ int TabStripModel::ConstrainInsertionIndex(int index, bool mini_tab) {
void TabStripModel::ExtendSelectionTo(int index) {
DCHECK(ContainsIndex(index));
- int old_active = active_index();
+ TabContentsWrapper* old_contents = GetActiveTabContents();
+ TabStripSelectionModel old_model;
+ old_model.Copy(selection_model());
selection_model_.SetSelectionFromAnchorTo(index);
- // This may not have resulted in a change, but we assume it did.
- NotifyActiveTabChanged(old_active);
+ NotifyIfActiveOrSelectionChanged(old_contents, false, old_model);
}
void TabStripModel::ToggleSelectionAt(int index) {
DCHECK(ContainsIndex(index));
- int old_active = active_index();
+ TabContentsWrapper* old_contents = GetActiveTabContents();
+ TabStripSelectionModel old_model;
+ old_model.Copy(selection_model());
if (selection_model_.IsSelected(index)) {
if (selection_model_.size() == 1) {
// One tab must be selected and this tab is currently selected so we can't
@@ -592,13 +597,15 @@ void TabStripModel::ToggleSelectionAt(int index) {
selection_model_.set_anchor(index);
selection_model_.set_active(index);
}
- NotifyActiveTabChanged(old_active);
+ NotifyIfActiveOrSelectionChanged(old_contents, false, old_model);
}
void TabStripModel::AddSelectionFromAnchorTo(int index) {
- int old_active = active_index();
+ TabContentsWrapper* old_contents = GetActiveTabContents();
+ TabStripSelectionModel old_model;
+ old_model.Copy(selection_model());
selection_model_.AddSelectionFromAnchorTo(index);
- NotifyActiveTabChanged(old_active);
+ NotifyIfActiveOrSelectionChanged(old_contents, false, old_model);
}
bool TabStripModel::IsTabSelected(int index) const {
@@ -609,10 +616,11 @@ bool TabStripModel::IsTabSelected(int index) const {
void TabStripModel::SetSelectionFromModel(
const TabStripSelectionModel& source) {
DCHECK_NE(TabStripSelectionModel::kUnselectedIndex, source.active());
- int old_active_index = active_index();
+ TabContentsWrapper* old_contents = GetActiveTabContents();
+ TabStripSelectionModel old_model;
+ old_model.Copy(selection_model());
selection_model_.Copy(source);
- // This may not have resulted in a change, but we assume it did.
- NotifyActiveTabChanged(old_active_index);
+ NotifyIfActiveOrSelectionChanged(old_contents, false, old_model);
}
void TabStripModel::AddTabContents(TabContentsWrapper* contents,
@@ -1207,33 +1215,31 @@ TabContentsWrapper* TabStripModel::GetContentsAt(int index) const {
return contents_data_.at(index)->contents;
}
-void TabStripModel::NotifyTabSelectedIfChanged(TabContentsWrapper* old_contents,
- int to_index,
- bool user_gesture) {
- TabContentsWrapper* new_contents = GetContentsAt(to_index);
- if (old_contents == new_contents)
- return;
-
- TabContentsWrapper* last_selected_contents = old_contents;
- if (last_selected_contents) {
+void TabStripModel::NotifyIfActiveTabChanged(
+ TabContentsWrapper* old_contents,
+ bool user_gesture) {
+ TabContentsWrapper* new_contents = GetContentsAt(active_index());
+ if (old_contents != new_contents) {
+ if (old_contents) {
+ FOR_EACH_OBSERVER(TabStripModelObserver, observers_,
+ TabDeactivated(old_contents));
+ }
FOR_EACH_OBSERVER(TabStripModelObserver, observers_,
- TabDeactivated(last_selected_contents));
+ ActiveTabChanged(old_contents, new_contents,
+ active_index(), user_gesture));
}
-
- FOR_EACH_OBSERVER(TabStripModelObserver, observers_,
- ActiveTabChanged(last_selected_contents, new_contents,
- active_index(), user_gesture));
}
-void TabStripModel::NotifyActiveTabChanged(int old_active_index) {
- TabContentsWrapper* old_tab =
- old_active_index == TabStripSelectionModel::kUnselectedIndex ?
- NULL : GetTabContentsAt(old_active_index);
- TabContentsWrapper* new_tab =
- active_index() == TabStripSelectionModel::kUnselectedIndex ?
- NULL : GetTabContentsAt(active_index());
- FOR_EACH_OBSERVER(TabStripModelObserver, observers_,
- ActiveTabChanged(old_tab, new_tab, active_index(), true));
+void TabStripModel::NotifyIfActiveOrSelectionChanged(
+ TabContentsWrapper* old_contents,
+ bool user_gesture,
+ const TabStripSelectionModel& old_model) {
+ NotifyIfActiveTabChanged(old_contents, user_gesture);
+
+ if (!selection_model().Equals(old_model)) {
+ FOR_EACH_OBSERVER(TabStripModelObserver, observers_,
+ TabSelectionChanged(old_model));
+ }
}
void TabStripModel::SelectRelativeTab(bool next) {
diff --git a/chrome/browser/tabs/tab_strip_model.h b/chrome/browser/tabs/tab_strip_model.h
index df4d83b..21270cd 100644
--- a/chrome/browser/tabs/tab_strip_model.h
+++ b/chrome/browser/tabs/tab_strip_model.h
@@ -514,15 +514,22 @@ class TabStripModel : public NotificationObserver {
TabContentsWrapper* GetContentsAt(int index) const;
- // If the TabContentsWrapper at |to_index| differs from |old_contents|
- // notifies observers.
- void NotifyTabSelectedIfChanged(TabContentsWrapper* old_contents,
- int to_index,
- bool user_gesture);
-
- // Notifies the observers the active tab changed. |old_active_index| gives
- // the old active index.
- void NotifyActiveTabChanged(int old_active_index);
+ // Notifies the observers if the active tab has changed. If |old_contents| is
+ // non-null a TabDeactivated notification is sent right before sending
+ // ActiveTabChanged notification.
+ void NotifyIfActiveTabChanged(TabContentsWrapper* old_contents,
+ bool user_gesture);
+
+ // Notifies the observers if the active tab or the tab selection has changed.
+ // If |old_contents| is non-null a TabDeactivated notification is sent right
+ // before sending ActiveTabChanged notification. |old_model| is a snapshot of
+ // |selection_model_| before the change.
+ // Note: This function might end up sending 0 to 3 notifications in the
+ // following order: TabDeactivated, ActiveTabChanged, TabSelectionChanged.
+ void NotifyIfActiveOrSelectionChanged(
+ TabContentsWrapper* old_contents,
+ bool user_gesture,
+ const TabStripSelectionModel& old_model);
// Returns the number of New Tab tabs in the TabStripModel.
int GetNewTabCount() const;
diff --git a/chrome/browser/tabs/tab_strip_model_observer.cc b/chrome/browser/tabs/tab_strip_model_observer.cc
index 83b0cf9..cf0d1eb 100644
--- a/chrome/browser/tabs/tab_strip_model_observer.cc
+++ b/chrome/browser/tabs/tab_strip_model_observer.cc
@@ -27,6 +27,10 @@ void TabStripModelObserver::ActiveTabChanged(TabContentsWrapper* old_contents,
bool user_gesture) {
}
+void TabStripModelObserver::TabSelectionChanged(
+ const TabStripSelectionModel& model) {
+}
+
void TabStripModelObserver::TabMoved(TabContentsWrapper* contents,
int from_index,
int to_index) {
diff --git a/chrome/browser/tabs/tab_strip_model_observer.h b/chrome/browser/tabs/tab_strip_model_observer.h
index b275350..5a69eb5 100644
--- a/chrome/browser/tabs/tab_strip_model_observer.h
+++ b/chrome/browser/tabs/tab_strip_model_observer.h
@@ -8,6 +8,7 @@
class TabContentsWrapper;
class TabStripModel;
+class TabStripSelectionModel;
////////////////////////////////////////////////////////////////////////////////
//
@@ -61,24 +62,26 @@ class TabStripModelObserver {
// happens.
virtual void TabDeactivated(TabContentsWrapper* contents);
- // Sent when the selection changes. The previously selected tab is identified
- // by |old_contents| and the newly selected tab by |new_contents|. |index| is
- // the index of |new_contents|. When using multiple selection this may be sent
- // even when the active tab has not changed. For example, if the selection is
- // extended this method is invoked to inform observers the selection has
- // changed, but |old_contents| and |new_contents| are the same. If you only
- // care about when the active tab changes, check for when |old_contents|
- // differs from |new_contents|. |user_gesture| specifies whether or not this
+ // Sent when the active tab changes. The previously active tab is identified
+ // by |old_contents| and the newly active tab by |new_contents|. |index| is
+ // the index of |new_contents|. |user_gesture| specifies whether or not this
// was done by a user input event (e.g. clicking on a tab, keystroke) or as a
// side-effect of some other function.
- //
- // TODO(dpapad): Add TabSelectionChanged method for when the selected tabs
- // change.
+ // Note: It is possible for the selection to change while the active tab
+ // remains unchanged. For example, control-click may not change the active tab
+ // but does change the selection. In this case |ActiveTabChanged| is not sent.
+ // If you care about any changes to the selection, override
+ // TabSelectionChanged.
virtual void ActiveTabChanged(TabContentsWrapper* old_contents,
TabContentsWrapper* new_contents,
int index,
bool user_gesture);
+ // Sent when the selection changes. More precisely when selected tabs, anchor
+ // tab or active tab change. |old_model| is a snapshot of the selection model
+ // before the change. See also ActiveTabChanged for details.
+ virtual void TabSelectionChanged(const TabStripSelectionModel& old_model);
+
// The specified TabContents at |from_index| was moved to |to_index|.
virtual void TabMoved(TabContentsWrapper* contents,
int from_index,
diff --git a/chrome/browser/tabs/tab_strip_model_order_controller.cc b/chrome/browser/tabs/tab_strip_model_order_controller.cc
index 75fc341..c910f12 100644
--- a/chrome/browser/tabs/tab_strip_model_order_controller.cc
+++ b/chrome/browser/tabs/tab_strip_model_order_controller.cc
@@ -111,9 +111,6 @@ void TabStripModelOrderController::ActiveTabChanged(
TabContentsWrapper* new_contents,
int index,
bool user_gesture) {
- if (old_contents == new_contents)
- return;
-
NavigationController* old_opener = NULL;
if (old_contents) {
int index = tabstrip_->GetIndexOfTabContents(old_contents);
diff --git a/chrome/browser/tabs/tab_strip_model_unittest.cc b/chrome/browser/tabs/tab_strip_model_unittest.cc
index 6643bbc..b988941 100644
--- a/chrome/browser/tabs/tab_strip_model_unittest.cc
+++ b/chrome/browser/tabs/tab_strip_model_unittest.cc
@@ -270,7 +270,13 @@ class TabStripModelTest : public RenderViewHostTestHarness {
class MockTabStripModelObserver : public TabStripModelObserver {
public:
- MockTabStripModelObserver() : empty_(true) {}
+ MockTabStripModelObserver() : empty_(true),
+ log_tab_selection_changed_(false),
+ model_(NULL) {}
+ explicit MockTabStripModelObserver(TabStripModel* model)
+ : empty_(true),
+ log_tab_selection_changed_(false),
+ model_(model) {}
~MockTabStripModelObserver() {
STLDeleteContainerPointers(states_.begin(), states_.end());
}
@@ -279,6 +285,7 @@ class MockTabStripModelObserver : public TabStripModelObserver {
INSERT,
CLOSE,
DETACH,
+ ACTIVATE,
SELECT,
MOVE,
CHANGE,
@@ -348,11 +355,22 @@ class MockTabStripModelObserver : public TabStripModelObserver {
TabContentsWrapper* new_contents,
int index,
bool user_gesture) {
- State* s = new State(new_contents, index, SELECT);
+ State* s = new State(new_contents, index, ACTIVATE);
s->src_contents = old_contents;
s->user_gesture = user_gesture;
states_.push_back(s);
}
+ virtual void TabSelectionChanged(const TabStripSelectionModel& old_model) {
+ if (!log_tab_selection_changed())
+ return;
+
+ State* s = new State(model()->GetActiveTabContents(),
+ model()->active_index(),
+ SELECT);
+ s->src_contents = model()->GetTabContentsAt(old_model.active());
+ s->src_index = old_model.active();
+ states_.push_back(s);
+ }
virtual void TabMoved(
TabContentsWrapper* contents, int from_index, int to_index) {
State* s = new State(contents, to_index, MOVE);
@@ -392,12 +410,22 @@ class MockTabStripModelObserver : public TabStripModelObserver {
states_.clear();
}
+ void set_log_tab_selection_changed(bool flag) {
+ log_tab_selection_changed_ = flag;
+ }
+
bool empty() const { return empty_; }
+ bool log_tab_selection_changed() const { return log_tab_selection_changed_; }
+ TabStripModel* model() { return model_; }
private:
std::vector<State*> states_;
bool empty_;
+ // TODO(dpapad): Remove this variable and update TestBasicAPI so that it takes
+ // into account TabSelectionChanged notifications.
+ bool log_tab_selection_changed_;
+ TabStripModel* model_;
DISALLOW_COPY_AND_ASSIGN(MockTabStripModelObserver);
};
@@ -428,7 +456,7 @@ TEST_F(TabStripModelTest, TestBasicAPI) {
State s1(contents1, 0, MockTabStripModelObserver::INSERT);
s1.foreground = true;
EXPECT_TRUE(observer.StateEquals(0, s1));
- State s2(contents1, 0, MockTabStripModelObserver::SELECT);
+ State s2(contents1, 0, MockTabStripModelObserver::ACTIVATE);
s2.src_contents = NULL;
EXPECT_TRUE(observer.StateEquals(1, s2));
observer.ClearStates();
@@ -444,7 +472,7 @@ TEST_F(TabStripModelTest, TestBasicAPI) {
State s1(contents2, 1, MockTabStripModelObserver::INSERT);
s1.foreground = true;
EXPECT_TRUE(observer.StateEquals(0, s1));
- State s2(contents2, 1, MockTabStripModelObserver::SELECT);
+ State s2(contents2, 1, MockTabStripModelObserver::ACTIVATE);
s2.src_contents = contents1;
EXPECT_TRUE(observer.StateEquals(1, s2));
observer.ClearStates();
@@ -467,7 +495,7 @@ TEST_F(TabStripModelTest, TestBasicAPI) {
{
tabstrip.ActivateTabAt(2, true);
EXPECT_EQ(1, observer.GetStateCount());
- State s1(contents3, 2, MockTabStripModelObserver::SELECT);
+ State s1(contents3, 2, MockTabStripModelObserver::ACTIVATE);
s1.src_contents = contents2;
s1.user_gesture = true;
EXPECT_TRUE(observer.StateEquals(0, s1));
@@ -483,14 +511,14 @@ TEST_F(TabStripModelTest, TestBasicAPI) {
EXPECT_EQ(4, observer.GetStateCount());
State s1(detached, 2, MockTabStripModelObserver::DETACH);
EXPECT_TRUE(observer.StateEquals(0, s1));
- State s2(contents2, 1, MockTabStripModelObserver::SELECT);
+ State s2(contents2, 1, MockTabStripModelObserver::ACTIVATE);
s2.src_contents = contents3;
s2.user_gesture = false;
EXPECT_TRUE(observer.StateEquals(1, s2));
State s3(detached, 2, MockTabStripModelObserver::INSERT);
s3.foreground = true;
EXPECT_TRUE(observer.StateEquals(2, s3));
- State s4(detached, 2, MockTabStripModelObserver::SELECT);
+ State s4(detached, 2, MockTabStripModelObserver::ACTIVATE);
s4.src_contents = contents2;
s4.user_gesture = false;
EXPECT_TRUE(observer.StateEquals(3, s4));
@@ -515,7 +543,7 @@ TEST_F(TabStripModelTest, TestBasicAPI) {
EXPECT_TRUE(observer.StateEquals(0, s1));
State s2(contents3, 2, MockTabStripModelObserver::DETACH);
EXPECT_TRUE(observer.StateEquals(1, s2));
- State s3(contents2, 1, MockTabStripModelObserver::SELECT);
+ State s3(contents2, 1, MockTabStripModelObserver::ACTIVATE);
s3.src_contents = contents3;
s3.user_gesture = false;
EXPECT_TRUE(observer.StateEquals(2, s3));
@@ -2062,7 +2090,7 @@ TEST_F(TabStripModelTest, ReplaceSendsSelected) {
EXPECT_TRUE(tabstrip_observer.StateEquals(0, state));
// And the second for selected.
- state = State(new_contents, 0, MockTabStripModelObserver::SELECT);
+ state = State(new_contents, 0, MockTabStripModelObserver::ACTIVATE);
state.src_contents = first_contents;
EXPECT_TRUE(tabstrip_observer.StateEquals(1, state));
@@ -2181,9 +2209,89 @@ TEST_F(TabStripModelTest, CloseSelectedTabs) {
strip.CloseAllTabs();
}
+TEST_F(TabStripModelTest, MultipleSelection) {
+ TabStripDummyDelegate delegate(NULL);
+ TabStripModel strip(&delegate, profile());
+ MockTabStripModelObserver observer(&strip);
+ TabContentsWrapper* contents0 = CreateTabContents();
+ TabContentsWrapper* contents1 = CreateTabContents();
+ TabContentsWrapper* contents2 = CreateTabContents();
+ TabContentsWrapper* contents3 = CreateTabContents();
+ strip.AppendTabContents(contents0, false);
+ strip.AppendTabContents(contents1, false);
+ strip.AppendTabContents(contents2, false);
+ strip.AppendTabContents(contents3, false);
+ observer.set_log_tab_selection_changed(true);
+ strip.AddObserver(&observer);
+
+ // Selection and active tab change.
+ strip.ActivateTabAt(3, true);
+ ASSERT_EQ(2, observer.GetStateCount());
+ ASSERT_EQ(observer.GetStateAt(0)->action,
+ MockTabStripModelObserver::ACTIVATE);
+ MockTabStripModelObserver::State s1(contents3, 3,
+ MockTabStripModelObserver::SELECT);
+ EXPECT_TRUE(observer.StateEquals(1, s1));
+ observer.ClearStates();
+
+ // Adding all tabs to selection, active tab is now at 0.
+ strip.ExtendSelectionTo(0);
+ ASSERT_EQ(2, observer.GetStateCount());
+ ASSERT_EQ(observer.GetStateAt(0)->action,
+ MockTabStripModelObserver::ACTIVATE);
+ MockTabStripModelObserver::State s2(contents0, 0,
+ MockTabStripModelObserver::SELECT);
+ s2.src_contents = contents3;
+ s2.src_index = 3;
+ EXPECT_TRUE(observer.StateEquals(1, s2));
+ observer.ClearStates();
+
+ // Closing one of the selected tabs, not the active one.
+ strip.CloseTabContentsAt(1, TabStripModel::CLOSE_NONE);
+ EXPECT_EQ(3, strip.count());
+ ASSERT_EQ(3, observer.GetStateCount());
+ ASSERT_EQ(observer.GetStateAt(0)->action,
+ MockTabStripModelObserver::CLOSE);
+ ASSERT_EQ(observer.GetStateAt(1)->action,
+ MockTabStripModelObserver::DETACH);
+ ASSERT_EQ(observer.GetStateAt(2)->action,
+ MockTabStripModelObserver::SELECT);
+ observer.ClearStates();
+
+ // Closing the active tab, while there are others tabs selected.
+ strip.CloseTabContentsAt(0, TabStripModel::CLOSE_NONE);
+ EXPECT_EQ(2, strip.count());
+ ASSERT_EQ(4, observer.GetStateCount());
+ ASSERT_EQ(observer.GetStateAt(0)->action,
+ MockTabStripModelObserver::CLOSE);
+ ASSERT_EQ(observer.GetStateAt(1)->action,
+ MockTabStripModelObserver::DETACH);
+ ASSERT_EQ(observer.GetStateAt(2)->action,
+ MockTabStripModelObserver::ACTIVATE);
+ ASSERT_EQ(observer.GetStateAt(3)->action,
+ MockTabStripModelObserver::SELECT);
+ observer.ClearStates();
+
+ // Active tab is at 0, deselecting all but the active tab.
+ strip.ToggleSelectionAt(1);
+ ASSERT_EQ(1, observer.GetStateCount());
+ ASSERT_EQ(observer.GetStateAt(0)->action,
+ MockTabStripModelObserver::SELECT);
+ observer.ClearStates();
+
+ // Attempting to deselect the only selected and therefore active tab,
+ // it is ignored (no notifications being sent) and tab at 0 remains selected
+ // and active.
+ strip.ToggleSelectionAt(0);
+ ASSERT_EQ(0, observer.GetStateCount());
+
+ strip.RemoveObserver(&observer);
+ 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
-// ActiveTabChanged is still invoked.
+// TabSelectionChanged is invoked.
TEST_F(TabStripModelTest, MultipleToSingle) {
TabStripDummyDelegate delegate(NULL);
TabStripModel strip(&delegate, profile());
@@ -2194,7 +2302,8 @@ TEST_F(TabStripModelTest, MultipleToSingle) {
strip.ToggleSelectionAt(0);
strip.ToggleSelectionAt(1);
- MockTabStripModelObserver observer;
+ MockTabStripModelObserver observer(&strip);
+ observer.set_log_tab_selection_changed(true);
strip.AddObserver(&observer);
// This changes the selection (0 is no longer selected) but the selected_index
// still remains at 1.
@@ -2203,7 +2312,8 @@ TEST_F(TabStripModelTest, MultipleToSingle) {
MockTabStripModelObserver::State s(
contents2, 1, MockTabStripModelObserver::SELECT);
s.src_contents = contents2;
- s.user_gesture = true;
+ s.src_index = 1;
+ s.user_gesture = false;
EXPECT_TRUE(observer.StateEquals(0, s));
strip.RemoveObserver(&observer);
strip.CloseAllTabs();
diff --git a/chrome/browser/tabs/tab_strip_selection_model.cc b/chrome/browser/tabs/tab_strip_selection_model.cc
index 43757c3..f2921dc 100644
--- a/chrome/browser/tabs/tab_strip_selection_model.cc
+++ b/chrome/browser/tabs/tab_strip_selection_model.cc
@@ -141,3 +141,9 @@ void TabStripSelectionModel::Copy(const TabStripSelectionModel& source) {
active_ = source.active_;
anchor_ = source.anchor_;
}
+
+bool TabStripSelectionModel::Equals(const TabStripSelectionModel& rhs) const {
+ return active_ == rhs.active() &&
+ anchor_ == rhs.anchor() &&
+ selected_indices() == rhs.selected_indices();
+}
diff --git a/chrome/browser/tabs/tab_strip_selection_model.h b/chrome/browser/tabs/tab_strip_selection_model.h
index 9ce7618..15a0afd 100644
--- a/chrome/browser/tabs/tab_strip_selection_model.h
+++ b/chrome/browser/tabs/tab_strip_selection_model.h
@@ -95,6 +95,9 @@ class TabStripSelectionModel {
// Copies the selection from |source| to this.
void Copy(const TabStripSelectionModel& source);
+ // Compares this selection with |rhs|.
+ bool Equals(const TabStripSelectionModel& rhs) const;
+
private:
SelectedIndices selected_indices_;
diff --git a/chrome/browser/tabs/tab_strip_selection_model_unittest.cc b/chrome/browser/tabs/tab_strip_selection_model_unittest.cc
index 121ef65..ec18932 100644
--- a/chrome/browser/tabs/tab_strip_selection_model_unittest.cc
+++ b/chrome/browser/tabs/tab_strip_selection_model_unittest.cc
@@ -173,3 +173,20 @@ TEST_F(TabStripSelectionModelTest, AddSelectionFromAnchorTo) {
model.AddSelectionFromAnchorTo(0);
EXPECT_EQ("active=0 anchor=2 selection=0 1 2 3 4", StateAsString(model));
}
+
+TEST_F(TabStripSelectionModelTest, Equals) {
+ TabStripSelectionModel model1;
+ model1.SetSelectedIndex(0);
+ model1.AddSelectionFromAnchorTo(4);
+
+ TabStripSelectionModel model2;
+ model2.SetSelectedIndex(0);
+ model2.AddSelectionFromAnchorTo(4);
+
+ EXPECT_TRUE(model1.Equals(model2));
+ EXPECT_TRUE(model2.Equals(model1));
+
+ model2.SetSelectedIndex(0);
+ EXPECT_FALSE(model1.Equals(model2));
+ EXPECT_FALSE(model2.Equals(model1));
+}
diff --git a/chrome/browser/ui/browser.cc b/chrome/browser/ui/browser.cc
index 06ebd10..8658e4f 100644
--- a/chrome/browser/ui/browser.cc
+++ b/chrome/browser/ui/browser.cc
@@ -2909,9 +2909,6 @@ void Browser::ActiveTabChanged(TabContentsWrapper* old_contents,
TabContentsWrapper* new_contents,
int index,
bool user_gesture) {
- if (old_contents == new_contents)
- return;
-
// On some platforms we want to automatically reload tabs that are
// killed when the user selects them.
if (user_gesture && new_contents->tab_contents()->crashed_status() ==
diff --git a/chrome/browser/ui/cocoa/tabs/tab_strip_controller.mm b/chrome/browser/ui/cocoa/tabs/tab_strip_controller.mm
index 22eb4af..ff83b98 100644
--- a/chrome/browser/ui/cocoa/tabs/tab_strip_controller.mm
+++ b/chrome/browser/ui/cocoa/tabs/tab_strip_controller.mm
@@ -1215,7 +1215,7 @@ class NotificationBridge : public NotificationObserver {
// Take closing tabs into account.
NSInteger activeIndex = [self indexFromModelIndex:modelIndex];
- if (oldContents && oldContents != newContents) {
+ if (oldContents) {
int oldModelIndex =
browser_->GetIndexOfController(&(oldContents->controller()));
if (oldModelIndex != -1) { // When closing a tab, the old tab may be gone.
diff --git a/chrome/browser/ui/gtk/browser_window_gtk.cc b/chrome/browser/ui/gtk/browser_window_gtk.cc
index e0bfb04..6b5fab4 100644
--- a/chrome/browser/ui/gtk/browser_window_gtk.cc
+++ b/chrome/browser/ui/gtk/browser_window_gtk.cc
@@ -1174,9 +1174,6 @@ void BrowserWindowGtk::ActiveTabChanged(TabContentsWrapper* old_contents,
TabContentsWrapper* new_contents,
int index,
bool user_gesture) {
- if (old_contents == new_contents)
- return;
-
if (old_contents && !old_contents->tab_contents()->is_being_destroyed())
old_contents->view()->StoreFocus();
diff --git a/chrome/browser/ui/touch/frame/touch_browser_frame_view.cc b/chrome/browser/ui/touch/frame/touch_browser_frame_view.cc
index 50d3367..29aed09 100644
--- a/chrome/browser/ui/touch/frame/touch_browser_frame_view.cc
+++ b/chrome/browser/ui/touch/frame/touch_browser_frame_view.cc
@@ -241,9 +241,6 @@ void TouchBrowserFrameView::ActiveTabChanged(TabContentsWrapper* old_contents,
TabContentsWrapper* new_contents,
int index,
bool user_gesture) {
- if (new_contents == old_contents)
- return;
-
TabContents* contents = new_contents->tab_contents();
if (!TabContentsHasFocus(contents))
return;
diff --git a/chrome/browser/ui/touch/tabs/touch_tab_strip.cc b/chrome/browser/ui/touch/tabs/touch_tab_strip.cc
index 46bfd71..6d7bf25 100644
--- a/chrome/browser/ui/touch/tabs/touch_tab_strip.cc
+++ b/chrome/browser/ui/touch/tabs/touch_tab_strip.cc
@@ -86,7 +86,9 @@ void TouchTabStrip::RemoveTabAt(int model_index) {
StartRemoveTabAnimation(model_index);
}
-void TouchTabStrip::SelectTabAt(int old_model_index, int new_model_index) {
+void TouchTabStrip::SetSelection(
+ const TabStripSelectionModel& old_selection,
+ const TabStripSelectionModel& new_selection) {
SchedulePaint();
}
diff --git a/chrome/browser/ui/touch/tabs/touch_tab_strip.h b/chrome/browser/ui/touch/tabs/touch_tab_strip.h
index f6dadb3..cf4c686 100644
--- a/chrome/browser/ui/touch/tabs/touch_tab_strip.h
+++ b/chrome/browser/ui/touch/tabs/touch_tab_strip.h
@@ -12,6 +12,7 @@ namespace ui {
enum TouchStatus;
}
+class TabStripSelectionModel;
class TouchTab;
///////////////////////////////////////////////////////////////////////////////
@@ -40,7 +41,8 @@ class TouchTabStrip : public BaseTabStrip {
virtual void StopAllHighlighting();
virtual BaseTab* CreateTabForDragging();
virtual void RemoveTabAt(int model_index);
- virtual void SelectTabAt(int old_model_index, int new_model_index);
+ virtual void SetSelection(const TabStripSelectionModel& old_selection,
+ const TabStripSelectionModel& new_selection);
virtual void TabTitleChangedNotLoading(int model_index);
virtual BaseTab* CreateTab();
virtual void StartInsertTabAnimation(int model_index);
diff --git a/chrome/browser/ui/views/frame/browser_view.cc b/chrome/browser/ui/views/frame/browser_view.cc
index 3f57cd6..78a91cd 100644
--- a/chrome/browser/ui/views/frame/browser_view.cc
+++ b/chrome/browser/ui/views/frame/browser_view.cc
@@ -1419,9 +1419,6 @@ void BrowserView::ActiveTabChanged(TabContentsWrapper* old_contents,
TabContentsWrapper* new_contents,
int index,
bool user_gesture) {
- if (old_contents == new_contents)
- return;
-
ProcessTabSelected(new_contents, true);
}
diff --git a/chrome/browser/ui/views/tabs/base_tab_strip.h b/chrome/browser/ui/views/tabs/base_tab_strip.h
index 0d5f1a4..691ed07 100644
--- a/chrome/browser/ui/views/tabs/base_tab_strip.h
+++ b/chrome/browser/ui/views/tabs/base_tab_strip.h
@@ -18,6 +18,7 @@
class BaseTab;
class DraggedTabController;
class TabStripController;
+class TabStripSelectionModel;
// Base class for the view tab strip implementations.
class BaseTabStrip : public AbstractTabStripView,
@@ -62,7 +63,8 @@ class BaseTabStrip : public AbstractTabStripView,
// Selects a tab at the specified index. |old_model_index| is the selected
// index prior to the selection change.
- virtual void SelectTabAt(int old_model_index, int new_model_index) = 0;
+ virtual void SetSelection(const TabStripSelectionModel& old_selection,
+ const TabStripSelectionModel& new_selection) = 0;
// Moves a tab.
virtual void MoveTab(int from_model_index, int to_model_index);
diff --git a/chrome/browser/ui/views/tabs/browser_tab_strip_controller.cc b/chrome/browser/ui/views/tabs/browser_tab_strip_controller.cc
index 9d4f3fa..3979c35 100644
--- a/chrome/browser/ui/views/tabs/browser_tab_strip_controller.cc
+++ b/chrome/browser/ui/views/tabs/browser_tab_strip_controller.cc
@@ -10,6 +10,7 @@
#include "chrome/browser/favicon/favicon_tab_helper.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/tabs/tab_strip_model.h"
+#include "chrome/browser/tabs/tab_strip_selection_model.h"
#include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/tab_contents/tab_contents_wrapper.h"
#include "chrome/browser/ui/tabs/tab_menu_model.h"
@@ -342,13 +343,9 @@ void BrowserTabStripController::TabDetachedAt(TabContentsWrapper* contents,
tabstrip_->RemoveTabAt(model_index);
}
-void BrowserTabStripController::ActiveTabChanged(
- TabContentsWrapper* old_contents,
- TabContentsWrapper* contents,
- int model_index,
- bool user_gesture) {
- tabstrip_->SelectTabAt(model_->GetIndexOfTabContents(old_contents),
- model_index);
+void BrowserTabStripController::TabSelectionChanged(
+ const TabStripSelectionModel& old_model) {
+ tabstrip_->SetSelection(old_model, model_->selection_model());
}
void BrowserTabStripController::TabMoved(TabContentsWrapper* contents,
diff --git a/chrome/browser/ui/views/tabs/browser_tab_strip_controller.h b/chrome/browser/ui/views/tabs/browser_tab_strip_controller.h
index 71ca58a..8d5622f 100644
--- a/chrome/browser/ui/views/tabs/browser_tab_strip_controller.h
+++ b/chrome/browser/ui/views/tabs/browser_tab_strip_controller.h
@@ -16,6 +16,7 @@
class BaseTab;
class BaseTabStrip;
class Browser;
+class TabStripSelectionModel;
struct TabRendererData;
@@ -71,10 +72,8 @@ class BrowserTabStripController : public TabStripController,
bool active) OVERRIDE;
virtual void TabDetachedAt(TabContentsWrapper* contents,
int model_index) OVERRIDE;
- virtual void ActiveTabChanged(TabContentsWrapper* old_contents,
- TabContentsWrapper* contents,
- int model_index,
- bool user_gesture) OVERRIDE;
+ virtual void TabSelectionChanged(
+ const TabStripSelectionModel& old_model) OVERRIDE;
virtual void TabMoved(TabContentsWrapper* contents,
int from_model_index,
int to_model_index) OVERRIDE;
diff --git a/chrome/browser/ui/views/tabs/side_tab_strip.cc b/chrome/browser/ui/views/tabs/side_tab_strip.cc
index 86cc393..5e099e3 100644
--- a/chrome/browser/ui/views/tabs/side_tab_strip.cc
+++ b/chrome/browser/ui/views/tabs/side_tab_strip.cc
@@ -4,6 +4,7 @@
#include "chrome/browser/ui/views/tabs/side_tab_strip.h"
+#include "chrome/browser/tabs/tab_strip_selection_model.h"
#include "chrome/browser/ui/view_ids.h"
#include "chrome/browser/ui/views/tabs/side_tab.h"
#include "chrome/browser/ui/views/tabs/tab_strip_controller.h"
@@ -175,11 +176,11 @@ void SideTabStrip::RemoveTabAt(int model_index) {
StartRemoveTabAnimation(model_index);
}
-void SideTabStrip::SelectTabAt(int old_model_index, int new_model_index) {
- GetBaseTabAtModelIndex(new_model_index)->SchedulePaint();
-
- if (controller()->IsActiveTab(new_model_index))
- MakeTabVisible(ModelIndexToTabIndex(new_model_index));
+void SideTabStrip::SetSelection(const TabStripSelectionModel& old_selection,
+ const TabStripSelectionModel& new_selection) {
+ GetBaseTabAtModelIndex(new_selection.active())->SchedulePaint();
+ if (old_selection.active() != new_selection.active())
+ MakeTabVisible(ModelIndexToTabIndex(new_selection.active()));
}
void SideTabStrip::TabTitleChangedNotLoading(int model_index) {
diff --git a/chrome/browser/ui/views/tabs/side_tab_strip.h b/chrome/browser/ui/views/tabs/side_tab_strip.h
index 7dd9805..6a5ef7e 100644
--- a/chrome/browser/ui/views/tabs/side_tab_strip.h
+++ b/chrome/browser/ui/views/tabs/side_tab_strip.h
@@ -10,6 +10,7 @@
#include "views/controls/button/button.h"
struct TabRendererData;
+class TabStripSelectionModel;
class SideTabStrip : public BaseTabStrip, public views::ButtonListener {
public:
@@ -28,7 +29,9 @@ class SideTabStrip : public BaseTabStrip, public views::ButtonListener {
virtual void StopAllHighlighting() OVERRIDE;
virtual BaseTab* CreateTabForDragging() OVERRIDE;
virtual void RemoveTabAt(int model_index) OVERRIDE;
- virtual void SelectTabAt(int old_model_index, int new_model_index) OVERRIDE;
+ virtual void SetSelection(
+ const TabStripSelectionModel& old_selection,
+ const TabStripSelectionModel& new_selection) OVERRIDE;
virtual void TabTitleChangedNotLoading(int model_index) OVERRIDE;
// views::View overrides:
diff --git a/chrome/browser/ui/views/tabs/tab_strip.cc b/chrome/browser/ui/views/tabs/tab_strip.cc
index e35af1d..16720f4 100644
--- a/chrome/browser/ui/views/tabs/tab_strip.cc
+++ b/chrome/browser/ui/views/tabs/tab_strip.cc
@@ -5,12 +5,14 @@
#include "chrome/browser/ui/views/tabs/tab_strip.h"
#include <algorithm>
+#include <iterator>
#include <vector>
#include "base/compiler_specific.h"
#include "base/stl_util-inl.h"
#include "base/utf_string_conversions.h"
#include "chrome/browser/defaults.h"
+#include "chrome/browser/tabs/tab_strip_selection_model.h"
#include "chrome/browser/themes/theme_service.h"
#include "chrome/browser/ui/view_ids.h"
#include "chrome/browser/ui/views/tabs/tab.h"
@@ -236,7 +238,8 @@ void TabStrip::RemoveTabAt(int model_index) {
StartRemoveTabAnimation(model_index);
}
-void TabStrip::SelectTabAt(int old_model_index, int new_model_index) {
+void TabStrip::SetSelection(const TabStripSelectionModel& old_selection,
+ const TabStripSelectionModel& new_selection) {
// We have "tiny tabs" if the tabs are so tiny that the unselected ones are
// a different size to the selected ones.
bool tiny_tabs = current_unselected_width_ != current_selected_width_;
@@ -246,8 +249,16 @@ void TabStrip::SelectTabAt(int old_model_index, int new_model_index) {
SchedulePaint();
}
- if (old_model_index >= 0) {
- GetTabAtTabDataIndex(ModelIndexToTabIndex(old_model_index))->
+ TabStripSelectionModel::SelectedIndices no_longer_selected;
+ std::insert_iterator<TabStripSelectionModel::SelectedIndices>
+ it(no_longer_selected, no_longer_selected.begin());
+ std::set_difference(old_selection.selected_indices().begin(),
+ old_selection.selected_indices().end(),
+ new_selection.selected_indices().begin(),
+ new_selection.selected_indices().end(),
+ it);
+ for (size_t i = 0; i < no_longer_selected.size(); ++i) {
+ GetTabAtTabDataIndex(ModelIndexToTabIndex(no_longer_selected[i]))->
StopMiniTabTitleAnimation();
}
}
diff --git a/chrome/browser/ui/views/tabs/tab_strip.h b/chrome/browser/ui/views/tabs/tab_strip.h
index 679f17a..4d6dbf9 100644
--- a/chrome/browser/ui/views/tabs/tab_strip.h
+++ b/chrome/browser/ui/views/tabs/tab_strip.h
@@ -16,6 +16,7 @@
#include "views/mouse_watcher.h"
class Tab;
+class TabStripSelectionModel;
namespace views {
class ImageView;
@@ -57,7 +58,9 @@ class TabStrip : public BaseTabStrip,
// BaseTabStrip implementation:
virtual void PrepareForCloseAt(int model_index) OVERRIDE;
virtual void RemoveTabAt(int model_index) OVERRIDE;
- virtual void SelectTabAt(int old_model_index, int new_model_index) OVERRIDE;
+ virtual void SetSelection(
+ const TabStripSelectionModel& old_selection,
+ const TabStripSelectionModel& new_selection) OVERRIDE;
virtual void TabTitleChangedNotLoading(int model_index) OVERRIDE;
virtual void StartHighlight(int model_index) OVERRIDE;
virtual void StopAllHighlighting() OVERRIDE;