Highlight tabs while scrubbing

Highlight tabs while scrubbing
This has been split out from https://codereview.chromium.org/11881042

BUG=124830
TEST=New tests

[sky, 2013-01-28 19:25:29]

https://codereview.chromium.org/12096014/diff/2001/chrome/browser/ui/views/as...
File chrome/browser/ui/views/ash/tab_scrubber.cc (right):

https://codereview.chromium.org/12096014/diff/2001/chrome/browser/ui/views/as...
chrome/browser/ui/views/ash/tab_scrubber.cc:164: if (highlighted_tab_ !=
new_index) {
Merge with if on 152 (since they're the same).

https://codereview.chromium.org/12096014/diff/2001/chrome/browser/ui/views/as...
chrome/browser/ui/views/ash/tab_scrubber.cc:203: if
(activate_timer_.IsRunning())
Its safe to always invoke Stop.

https://codereview.chromium.org/12096014/diff/2001/chrome/browser/ui/views/as...
File chrome/browser/ui/views/ash/tab_scrubber_browsertest.cc (right):

https://codereview.chromium.org/12096014/diff/2001/chrome/browser/ui/views/as...
chrome/browser/ui/views/ash/tab_scrubber_browsertest.cc:10: #include
"chrome/browser/ui/views/ash/tab_scrubber.h"
nit: this should be your first include (same order as tab_scrubber.cc).

https://codereview.chromium.org/12096014/diff/2001/chrome/browser/ui/views/as...
chrome/browser/ui/views/ash/tab_scrubber_browsertest.cc:43:
browser()->tab_strip_model()->AddObserver(this);
You never remove this.

https://codereview.chromium.org/12096014/diff/2001/chrome/browser/ui/views/as...
chrome/browser/ui/views/ash/tab_scrubber_browsertest.cc:64: DCHECK(index -
active_index != 0);
Don't use DCHECKs in tests, use EXPECT/ASSERT

https://codereview.chromium.org/12096014/diff/2001/chrome/browser/ui/views/as...
chrome/browser/ui/views/ash/tab_scrubber_browsertest.cc:149: std::vector<int>
activation_order_;
Would be nice to have a description for some of these.

https://codereview.chromium.org/12096014/diff/2001/chrome/browser/ui/views/as...
chrome/browser/ui/views/ash/tab_scrubber_browsertest.cc:150:
aura::test::EventGenerator* event_generator_;
scoped_ptr?

https://codereview.chromium.org/12096014/diff/2001/chrome/browser/ui/views/as...
chrome/browser/ui/views/ash/tab_scrubber_browsertest.cc:155: };
DISALLOW_...

[DaveMoore, 2013-01-29 01:11:41]

https://codereview.chromium.org/12096014/diff/2001/chrome/browser/ui/views/as...
File chrome/browser/ui/views/ash/tab_scrubber.cc (right):

https://codereview.chromium.org/12096014/diff/2001/chrome/browser/ui/views/as...
chrome/browser/ui/views/ash/tab_scrubber.cc:164: if (highlighted_tab_ !=
new_index) {
On 2013/01/28 19:25:29, sky wrote:
> Merge with if on 152 (since they're the same).

Done.

https://codereview.chromium.org/12096014/diff/2001/chrome/browser/ui/views/as...
chrome/browser/ui/views/ash/tab_scrubber.cc:203: if
(activate_timer_.IsRunning())
On 2013/01/28 19:25:29, sky wrote:
> Its safe to always invoke Stop.

Done.

https://codereview.chromium.org/12096014/diff/2001/chrome/browser/ui/views/as...
File chrome/browser/ui/views/ash/tab_scrubber_browsertest.cc (right):

https://codereview.chromium.org/12096014/diff/2001/chrome/browser/ui/views/as...
chrome/browser/ui/views/ash/tab_scrubber_browsertest.cc:10: #include
"chrome/browser/ui/views/ash/tab_scrubber.h"
On 2013/01/28 19:25:29, sky wrote:
> nit: this should be your first include (same order as tab_scrubber.cc).

Done.

https://codereview.chromium.org/12096014/diff/2001/chrome/browser/ui/views/as...
chrome/browser/ui/views/ash/tab_scrubber_browsertest.cc:43:
browser()->tab_strip_model()->AddObserver(this);
On 2013/01/28 19:25:29, sky wrote:
> You never remove this.

Done.

https://codereview.chromium.org/12096014/diff/2001/chrome/browser/ui/views/as...
chrome/browser/ui/views/ash/tab_scrubber_browsertest.cc:64: DCHECK(index -
active_index != 0);
On 2013/01/28 19:25:29, sky wrote:
> Don't use DCHECKs in tests, use EXPECT/ASSERT

Done.

https://codereview.chromium.org/12096014/diff/2001/chrome/browser/ui/views/as...
chrome/browser/ui/views/ash/tab_scrubber_browsertest.cc:149: std::vector<int>
activation_order_;
On 2013/01/28 19:25:29, sky wrote:
> Would be nice to have a description for some of these.

Done.

https://codereview.chromium.org/12096014/diff/2001/chrome/browser/ui/views/as...
chrome/browser/ui/views/ash/tab_scrubber_browsertest.cc:150:
aura::test::EventGenerator* event_generator_;
On 2013/01/28 19:25:29, sky wrote:
> scoped_ptr?

Done.

https://codereview.chromium.org/12096014/diff/2001/chrome/browser/ui/views/as...
chrome/browser/ui/views/ash/tab_scrubber_browsertest.cc:155: };
On 2013/01/28 19:25:29, sky wrote:
> DISALLOW_...

Done.

[sky, 2013-01-29 15:58:15]

https://codereview.chromium.org/12096014/diff/1009/chrome/browser/ui/views/as...
File chrome/browser/ui/views/ash/tab_scrubber.cc (right):

https://codereview.chromium.org/12096014/diff/1009/chrome/browser/ui/views/as...
chrome/browser/ui/views/ash/tab_scrubber.cc:170:
new_tab->hover_controller()->Show(views::GlowHoverController::PRONOUNCED);
If the tabs move out from under you doesn't it mean you never properly Hide?

[sky, 2013-01-29 15:58:47]

https://codereview.chromium.org/12096014/diff/1009/chrome/browser/ui/views/as...
File chrome/browser/ui/views/ash/tab_scrubber.cc (right):

https://codereview.chromium.org/12096014/diff/1009/chrome/browser/ui/views/as...
chrome/browser/ui/views/ash/tab_scrubber.cc:170:
new_tab->hover_controller()->Show(views::GlowHoverController::PRONOUNCED);
On 2013/01/29 15:58:16, sky wrote:
> If the tabs move out from under you doesn't it mean you never properly Hide?

By shifting I mean say a tab a new tab is added/removed before this index.