ash:Shelf - Update position and hit box of status area.

Hit bounds of status area increased to fill shelf.
Unit tests added for above.
Position of status area and launcher items (App list/Overflow) updated to spec.
Removed duplicate variable for positioning (App list, Overflow Button and status area now share an inset constant).

BUG=288294, 288296, 288399
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=224015

[Harry McCleave, 2013-09-17 22:19:03]

ptal

[Mr4D (OOO till 08-26), 2013-09-18 00:07:49]

Only a few nits. lgtm.

https://chromiumcodereview.appspot.com/23928004/diff/17001/ash/shelf/alternat...
File ash/shelf/alternate_app_list_button.cc (right):

https://chromiumcodereview.appspot.com/23928004/diff/17001/ash/shelf/alternat...
ash/shelf/alternate_app_list_button.cc:110: ShelfLayoutManager::kShelfItemInset
- background_image->width());
Apparently the old value was 5 and this is 3 - I guess that is meant to be?

https://chromiumcodereview.appspot.com/23928004/diff/17001/ash/system/tray/tr...
File ash/system/tray/tray_background_view.cc (right):

https://chromiumcodereview.appspot.com/23928004/diff/17001/ash/system/tray/tr...
ash/system/tray/tray_background_view.cc:471: }
Looking at this function I wonder if it is not better readable when you define
variables for the 4 members and then set the border in line 498 for all 8 cases.

https://chromiumcodereview.appspot.com/23928004/diff/17001/ash/system/tray/tr...
ash/system/tray/tray_background_view.cc:540:
0);//kAlternateLayoutBubblePaddingHorizontalBottom);
I think you can remove the comment here?

[Harry McCleave, 2013-09-18 01:34:55]

https://chromiumcodereview.appspot.com/23928004/diff/17001/ash/shelf/alternat...
File ash/shelf/alternate_app_list_button.cc (right):

https://chromiumcodereview.appspot.com/23928004/diff/17001/ash/shelf/alternat...
ash/shelf/alternate_app_list_button.cc:110: ShelfLayoutManager::kShelfItemInset
- background_image->width());
On 2013/09/18 00:07:49, Mr4D wrote:
> Apparently the old value was 5 and this is 3 - I guess that is meant to be?

Yes, this is intended to keep all items aligned.

https://chromiumcodereview.appspot.com/23928004/diff/17001/ash/system/tray/tr...
File ash/system/tray/tray_background_view.cc (right):

https://chromiumcodereview.appspot.com/23928004/diff/17001/ash/system/tray/tr...
ash/system/tray/tray_background_view.cc:471: }
On 2013/09/18 00:07:49, Mr4D wrote:
> Looking at this function I wonder if it is not better readable when you define
> variables for the 4 members and then set the border in line 498 for all 8
cases.
> 

Done.

https://chromiumcodereview.appspot.com/23928004/diff/17001/ash/system/tray/tr...
ash/system/tray/tray_background_view.cc:540:
0);//kAlternateLayoutBubblePaddingHorizontalBottom);
On 2013/09/18 00:07:49, Mr4D wrote:
> I think you can remove the comment here?

Done.

https://chromiumcodereview.appspot.com/23928004/diff/17001/ash/system/tray/tr...
ash/system/tray/tray_background_view.cc:540:
0);//kAlternateLayoutBubblePaddingHorizontalBottom);
On 2013/09/18 00:07:49, Mr4D wrote:
> I think you can remove the comment here?

Done.

[James Cook, 2013-09-18 01:36:06]

LGTM with nits

https://chromiumcodereview.appspot.com/23928004/diff/23001/ash/shelf/shelf_la...
File ash/shelf/shelf_layout_manager.cc (right):

https://chromiumcodereview.appspot.com/23928004/diff/23001/ash/shelf/shelf_la...
ash/shelf/shelf_layout_manager.cc:729: void
ShelfLayoutManager::CalculateTargetBounds(
Someday, not in this CL, this function needs to be broken into pieces so it can
be less than 100 lines long. :-)

https://chromiumcodereview.appspot.com/23928004/diff/23001/ash/shelf/shelf_la...
File ash/shelf/shelf_layout_manager_unittest.cc (right):

https://chromiumcodereview.appspot.com/23928004/diff/23001/ash/shelf/shelf_la...
ash/shelf/shelf_layout_manager_unittest.cc:1863: // Test bottom left pixel for
left alignment.
Nice, I didn't think of that case.  Good test.

https://chromiumcodereview.appspot.com/23928004/diff/23001/ash/system/tray/sy...
File ash/system/tray/system_tray_unittest.cc (right):

https://chromiumcodereview.appspot.com/23928004/diff/23001/ash/system/tray/sy...
ash/system/tray/system_tray_unittest.cc:31: const int
kStatusTrayOffsetFromScreenEdgeForTest = 4;
Is this needed any more?

https://chromiumcodereview.appspot.com/23928004/diff/23001/ash/system/tray/tr...
File ash/system/tray/tray_background_view.cc (right):

https://chromiumcodereview.appspot.com/23928004/diff/23001/ash/system/tray/tr...
ash/system/tray/tray_background_view.cc:447: int on_edge = (this ==
parent->child_at(0));
int -> bool?

https://chromiumcodereview.appspot.com/23928004/diff/23001/ash/system/tray/tr...
File ash/system/tray/tray_constants.cc (right):

https://chromiumcodereview.appspot.com/23928004/diff/23001/ash/system/tray/tr...
ash/system/tray/tray_constants.cc:18: const int
kAlternateLayoutBubblePaddingHorizontalBottom = 6;
Maybe a quick comment about padding between what and what?

[commit-bot: I haz the power, 2013-09-18 02:40:55]

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

[commit-bot: I haz the power, 2013-09-18 04:42:53]

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

[commit-bot: I haz the power, 2013-09-18 20:46:31]

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

[commit-bot: I haz the power, 2013-09-19 01:21:27]

Change committed as 224015