Multi-tab selection: Renaming TabStripModelObserver::TabSelectedAt to ActiveTabChanged

Multi-tab selection: Renaming TabStripModelObserver::TabSelectedAt to ActiveTabChanged

This is done in preparation for multi-tab selection in Linux (http://codereview.chromium.org/6933037/). There was a TODO for that in tab_strip_model_observer.h:75

BUG=NONE
TEST=Tab behavior should be unaffected on all platforms

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=86502

[sky, 2011-05-19 19:40:12]

http://codereview.chromium.org/7043020/diff/3001/chrome/browser/tabs/tab_stri...
File chrome/browser/tabs/tab_strip_model_observer.h (right):

http://codereview.chromium.org/7043020/diff/3001/chrome/browser/tabs/tab_stri...
chrome/browser/tabs/tab_strip_model_observer.h:79: virtual void
TabActivatedAt(TabContentsWrapper* old_contents,
Thanks for getting around to this. I think we want two methods to cover the
different cases:

TabSelectionChanged() <- invoked any time the selection changes in some way.

ActiveTabChanged() <- invoked only if the active tab changed. I think most folks
that override TabSelectedAt now only care about this.

[dpapad, 2011-05-19 20:10:04]

http://codereview.chromium.org/7043020/diff/3001/chrome/browser/tabs/tab_stri...
File chrome/browser/tabs/tab_strip_model_observer.h (right):

http://codereview.chromium.org/7043020/diff/3001/chrome/browser/tabs/tab_stri...
chrome/browser/tabs/tab_strip_model_observer.h:79: virtual void
TabActivatedAt(TabContentsWrapper* old_contents,
On 2011/05/19 19:40:12, sky wrote:
> Thanks for getting around to this. I think we want two methods to cover the
> different cases:
> 
> TabSelectionChanged() <- invoked any time the selection changes in some way.
> 
> ActiveTabChanged() <- invoked only if the active tab changed. I think most
folks
> that override TabSelectedAt now only care about this.

I have added a TabSelectionChanged() method in the CL linked on the description
of this CL (http://codereview.chromium.org/6933037/). But this one (7043020) has
to land before 6933037. Also, do you think it is necessary to re-rename
TabActivatedAt to ActiveTabChanged? I just followed the name suggested in the
comments.

[sky, 2011-05-19 20:56:15]

http://codereview.chromium.org/7043020/diff/3001/chrome/browser/tabs/tab_stri...
File chrome/browser/tabs/tab_strip_model_observer.h (right):

http://codereview.chromium.org/7043020/diff/3001/chrome/browser/tabs/tab_stri...
chrome/browser/tabs/tab_strip_model_observer.h:79: virtual void
TabActivatedAt(TabContentsWrapper* old_contents,
On 2011/05/19 20:10:04, dpapad wrote:
> On 2011/05/19 19:40:12, sky wrote:
> > Thanks for getting around to this. I think we want two methods to cover the
> > different cases:
> > 
> > TabSelectionChanged() <- invoked any time the selection changes in some way.
> > 
> > ActiveTabChanged() <- invoked only if the active tab changed. I think most
> folks
> > that override TabSelectedAt now only care about this.
> 
> I have added a TabSelectionChanged() method in the CL linked on the
description
> of this CL (http://codereview.chromium.org/6933037/). But this one (7043020)
has
> to land before 6933037. Also, do you think it is necessary to re-rename
> TabActivatedAt to ActiveTabChanged? I just followed the name suggested in the
> comments. 

I think ActiveTabChanged is clearer. It also lines up nicely with
ActiveTabClicked.

[dpapad, 2011-05-24 02:16:35]

http://codereview.chromium.org/7043020/diff/3001/chrome/browser/tabs/tab_stri...
File chrome/browser/tabs/tab_strip_model_observer.h (right):

http://codereview.chromium.org/7043020/diff/3001/chrome/browser/tabs/tab_stri...
chrome/browser/tabs/tab_strip_model_observer.h:79: virtual void
TabActivatedAt(TabContentsWrapper* old_contents,
On 2011/05/19 20:56:15, sky wrote:
> On 2011/05/19 20:10:04, dpapad wrote:
> > On 2011/05/19 19:40:12, sky wrote:
> > > Thanks for getting around to this. I think we want two methods to cover
the
> > > different cases:
> > > 
> > > TabSelectionChanged() <- invoked any time the selection changes in some
way.
> > > 
> > > ActiveTabChanged() <- invoked only if the active tab changed. I think most
> > folks
> > > that override TabSelectedAt now only care about this.
> > 
> > I have added a TabSelectionChanged() method in the CL linked on the
> description
> > of this CL (http://codereview.chromium.org/6933037/). But this one (7043020)
> has
> > to land before 6933037. Also, do you think it is necessary to re-rename
> > TabActivatedAt to ActiveTabChanged? I just followed the name suggested in
the
> > comments. 
> 
> I think ActiveTabChanged is clearer. It also lines up nicely with
> ActiveTabClicked.

Done.

[sky, 2011-05-24 15:59:40]

LGTM

[dpapad, 2011-05-24 16:33:07]

Updating the TODO comment in tab_strip_model_observer.h.