ash:Shelf Overflow Button

ash:Shelf-Overflow Button

Overflow Button updated to use Rotated image on side shelf.
Overflow Button background changed to match app list background in alternate shelf layout.

BUG=265661, 265662, 163000
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=216531

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=216865

[Harry McCleave, 2013-08-07 03:19:52]

ptal

[James Cook, 2013-08-07 18:23:13]

https://codereview.chromium.org/22527003/diff/3001/ash/launcher/launcher_view.cc
File ash/launcher/launcher_view.cc (right):

https://codereview.chromium.org/22527003/diff/3001/ash/launcher/launcher_view...
ash/launcher/launcher_view.cc:470: for (int i=0; i >= first_visible_index_ && i
<= last_visible_index_; ++i)
spaces around =, and should i start at first_visible_index_?

https://codereview.chromium.org/22527003/diff/3001/ash/launcher/overflow_butt...
File ash/launcher/overflow_button.cc (right):

https://codereview.chromium.org/22527003/diff/3001/ash/launcher/overflow_butt...
ash/launcher/overflow_button.cc:37: const int kImagePaddingFromShelf = 5;
Comment padding from what to what.

https://codereview.chromium.org/22527003/diff/3001/ash/launcher/overflow_butt...
ash/launcher/overflow_button.cc:47: left_image_ =
gfx::ImageSkiaOperations::CreateRotatedImage(*bottom_image_,
Could these be lazy-initialized, like the first time they are used in OnPaint? 
Like check if they are empty, and only then create them?  Then we wouldn't have
to do image creation for the common case of the bottom-aligned shelf.

[Harry McCleave, 2013-08-07 22:50:59]

https://codereview.chromium.org/22527003/diff/3001/ash/launcher/launcher_view.cc
File ash/launcher/launcher_view.cc (right):

https://codereview.chromium.org/22527003/diff/3001/ash/launcher/launcher_view...
ash/launcher/launcher_view.cc:470: for (int i=0; i >= first_visible_index_ && i
<= last_visible_index_; ++i)
On 2013/08/07 18:23:13, James Cook wrote:
> spaces around =, and should i start at first_visible_index_?

Spacing done, first_visible_index and last_visible_index could both be -1 if no
items are available thus starting at 0 to ensure no look-up at index -1, I added
the -1 check as a conditional outside of the loop for readability (and a very
minuscule performance improvement).

https://codereview.chromium.org/22527003/diff/3001/ash/launcher/overflow_butt...
File ash/launcher/overflow_button.cc (right):

https://codereview.chromium.org/22527003/diff/3001/ash/launcher/overflow_butt...
ash/launcher/overflow_button.cc:37: const int kImagePaddingFromShelf = 5;
On 2013/08/07 18:23:13, James Cook wrote:
> Comment padding from what to what.

Done.

https://codereview.chromium.org/22527003/diff/3001/ash/launcher/overflow_butt...
ash/launcher/overflow_button.cc:47: left_image_ =
gfx::ImageSkiaOperations::CreateRotatedImage(*bottom_image_,
On 2013/08/07 18:23:13, James Cook wrote:
> Could these be lazy-initialized, like the first time they are used in OnPaint?

> Like check if they are empty, and only then create them?  Then we wouldn't
have
> to do image creation for the common case of the bottom-aligned shelf.

Done.

[James Cook, 2013-08-07 23:22:05]

LGTM

https://codereview.chromium.org/22527003/diff/3001/ash/launcher/launcher_view.cc
File ash/launcher/launcher_view.cc (right):

https://codereview.chromium.org/22527003/diff/3001/ash/launcher/launcher_view...
ash/launcher/launcher_view.cc:470: for (int i=0; i >= first_visible_index_ && i
<= last_visible_index_; ++i)
On 2013/08/07 22:50:59, Harry McCleave wrote:
> On 2013/08/07 18:23:13, James Cook wrote:
> > spaces around =, and should i start at first_visible_index_?
> 
> Spacing done, first_visible_index and last_visible_index could both be -1 if
no
> items are available thus starting at 0 to ensure no look-up at index -1, I
added
> the -1 check as a conditional outside of the loop for readability (and a very
> minuscule performance improvement).

Much easier to understand, thanks.

[commit-bot: I haz the power, 2013-08-07 23:32:10]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/harrym@chromium.org/22527003/9001

[commit-bot: I haz the power, 2013-08-08 00:41:22]

Retried try job too often on linux_rel for step(s) interactive_ui_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[commit-bot: I haz the power, 2013-08-08 00:45:11]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/harrym@chromium.org/22527003/9001

[commit-bot: I haz the power, 2013-08-09 01:24:53]

Commit queue rejected this change because the description was changed
between the time the change entered the commit queue and the time it
was ready to commit. You can safely check the commit box again.

[commit-bot: I haz the power, 2013-08-09 01:27:50]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/harrym@chromium.org/22527003/9001

[commit-bot: I haz the power, 2013-08-09 01:41:52]

Change committed as 216531

[commit-bot: I haz the power, 2013-08-09 20:15:28]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/harrym@chromium.org/22527003/47001

[commit-bot: I haz the power, 2013-08-09 20:31:43]

Step "update" is always a major failure.
Look at the try server FAQ for more details.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win&number...

[commit-bot: I haz the power, 2013-08-09 20:42:41]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/harrym@chromium.org/22527003/47001

[commit-bot: I haz the power, 2013-08-10 15:19:26]

Change committed as 216865