Multi-tab selection for Linux.

chrome/browser/tabs/tab_strip_model_observer.h

Index: chrome/browser/tabs/tab_strip_model_observer.h
diff --git a/chrome/browser/tabs/tab_strip_model_observer.h b/chrome/browser/tabs/tab_strip_model_observer.h
index 290d7700c87e506128313b0009e96ecee1c71663..54f4ea6adf3eae4e0909b06eafbaced662007e15 100644
--- a/chrome/browser/tabs/tab_strip_model_observer.h
+++ b/chrome/browser/tabs/tab_strip_model_observer.h
@@ -6,6 +6,8 @@
 #define CHROME_BROWSER_TABS_TAB_STRIP_MODEL_OBSERVER_H_
 #pragma once
 
+#include <vector>
+
 class TabContentsWrapper;
 class TabStripModel;
 
@@ -71,14 +73,17 @@ class TabStripModelObserver {
   // differs from |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.
   virtual void ActiveTabChanged(TabContentsWrapper* old_contents,
                                 TabContentsWrapper* new_contents,
                                 int index,
                                 bool user_gesture);
 
+  // Sent when the selection changes. |previously_selected| and
+  // |currently_selected| contain the indices of the tabs that were previously
+  // and currently selected respectively.
+  virtual void TabSelectionChanged(const std::vector<int>& previously_selected,

[sky, 2011/06/01 20:56:04]

Add test coverage of this.
Also, it seems like you don't always invoke this when the selected indices
change. The case I think you don't send out notification are add/remove. For
example, if you add/remove tabs, the selection in terms of indices changes.

[dpapad, 2011/06/02 00:28:22]

Yes, currently TabSelectionChanged() is called from ActivateTabAt(),
ExtendSelectionTo(), ToggleSelectionAt(), AddSelectionFromAnchorTo() and
SetSelectionFromModel(). It is intentionally not called from
InsertTabContentsAt() and CloseTabContentsAt(), for 2 reasons, 1st) These events
seem to be handled fine by existing callbacks (in terms of repainting) and 2nd)
to avoid sending indices of tabs that no longer exist (as noted in one of your
comments). 
I see two approaches
1) Add some more comments in TabSelectionChanged explaining that it is not
always fired when the selection changes.
2) Call TabSelectionChanged() from InsertTabContentsAt() and
CloseTabContentsAt() too and find out how to deal with differences in model_ and
tab_data_ in TabStripGtk (maybe send the TabContentsWrapper* instead of the
indices?).

[sky, 2011/06/02 16:07:14]

You're right on 1. My intention with the two methods is the following:
. ActiveTabChanged: only sent when the active tab changes. Currently it isn't
and most implementations check before doing anything. At the time I added
multi-selection I went this route, rather than a new method, as we weren't sure
if we were going to keep the feature.
. TabSelectionChanged: sent when the selection changes, but not necessarily the
active tab, and not when tabs are inserted/removed.

Since TabSelectionChanged will be sent when ActiveTabChanged is sent now that
means it should be enough for the TabStrip implementations to only listen to
TabSelectionChanged.

I think it would be best to separate out the model changes into a patch doing
the following:
. Only send ActiveTabChanged when the active tab really changes. You'll then be
able to remove a bunch of checks I added. See
http://src.chromium.org/viewvc/chrome?view=rev&revision=76619 .
. Add TabSelectionChanged and change the places that care about any selection
change to override TabSelectionChanged instead of ActiveTabChanged.
. For TabSelectionChanged, make it take const TabStripSelectionModel& as the
only argument. It'll be the selection before the change. If folks care about the
new selection, they can query the model.
. Add/update tests.

+                                   const std::vector<int>& currently_selected);
+
   // The specified TabContents at |from_index| was moved to |to_index|.
   virtual void TabMoved(TabContentsWrapper* contents,
                         int from_index,