Give Ash material design tray items the correct size and layout

ash/shelf/overflow_button.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "ash/shelf/overflow_button.h"
 
 #include "ash/common/ash_constants.h"
 #include "ash/common/ash_switches.h"
 #include "ash/common/material_design/material_design_controller.h"
 #include "ash/common/shelf/shelf_constants.h"
@@ skipping 63 matching lines @@
     ShelfWidget* shelf_widget = shelf_->shelf_widget();
     if (shelf_widget &&
         shelf_widget->GetBackgroundType() ==
             ShelfBackgroundType::SHELF_BACKGROUND_DEFAULT) {
       background_color = SkColorSetA(kShelfBaseColor,
                                      GetShelfConstant(SHELF_BACKGROUND_ALPHA));
     }
 
     // TODO(bruthig|tdanderson): The background should be changed using a
     // fade in/out animation.
-    const int kCornerRadius = 2;
-
     SkPaint background_paint;
     background_paint.setFlags(SkPaint::kAntiAlias_Flag);
     background_paint.setColor(background_color);
-    canvas->DrawRoundRect(bounds, kCornerRadius, background_paint);
+    canvas->DrawRoundRect(bounds, kTrayRoundedBorderRadius, background_paint);
 
     if (shelf_->IsShowingOverflowBubble()) {
       SkPaint highlight_paint;
       highlight_paint.setFlags(SkPaint::kAntiAlias_Flag);
       highlight_paint.setColor(kShelfButtonActivatedHighlightColor);
-      canvas->DrawRoundRect(bounds, kCornerRadius, highlight_paint);
+      canvas->DrawRoundRect(bounds, kTrayRoundedBorderRadius, highlight_paint);
     }
   } else {
     ResourceBundle& rb = ResourceBundle::GetSharedInstance();
     const gfx::ImageSkia* background =
         rb.GetImageNamed(NonMaterialBackgroundImageId()).ToImageSkia();
     canvas->DrawImageInt(*background, bounds.x(), bounds.y());
   }
 }
 
 void OverflowButton::PaintForeground(gfx::Canvas* canvas,
@@ skipping 32 matching lines @@
     return IDR_AURA_NOTIFICATION_BACKGROUND_ON_BLACK;
   return IDR_AURA_NOTIFICATION_BACKGROUND_NORMAL;
 }
 
 gfx::Rect OverflowButton::CalculateButtonBounds() {
   ShelfAlignment alignment = shelf_->alignment();
   gfx::Rect bounds(GetContentsBounds());
   ResourceBundle& rb = ResourceBundle::GetSharedInstance();
   if (MaterialDesignController::IsShelfMaterial()) {
     if (shelf_->IsHorizontalAlignment()) {
-      bounds = gfx::Rect((bounds.x() + (bounds.width() - kShelfItemSizeMD) / 2),
-                         bounds.y() + (bounds.height() - kShelfItemSizeMD) / 2,
-                         kShelfItemSizeMD, kShelfItemSizeMD);
+      bounds = gfx::Rect((bounds.x() + (bounds.width() - kTrayItemSize) / 2),

[James Cook, 2016/06/27 18:04:12]

It's odd that you're using a constant named "tray" in code that is sizing a
shelf item that isn't related to the system tray. Maybe continue to use separate
constants? Or come up with a name that implies both?

[tdanderson, 2016/06/27 21:29:42]

This has been in the back of my mind for a while now. I realize "the shelf" and
"the tray [area]" are two different surfaces, but I definitely want to share the
same constant to reinforce the fact that 32dp is a standard size used in MD.
Unfortunately I can't think of any (good) names that don't contain either
"shelf" or "tray". 

Then there is the question of where the constant should live; there is a handful
of constants common to both (button size, icon color, button background when
shelf is transparent, etc) that may benefit from being moved to a new constants
file.

WDYT?

[James Cook, 2016/06/27 22:53:21]

Yeah, I can't think of a good shared name either.

Visually the "shelf" is a superset of shelf+tray, so if there are shared
constants I would name them "shelf" and stick them somewhere shelf-y.

[tdanderson, 2016/06/28 16:32:44]

As discussed, in this CL I will un-blur the lines between the shelf using tray
constants and vice versa. However the longer term plan is to refactor all common
constants to a separate place. Filed crbug.com/623987 to track that work.

+                         bounds.y() + (bounds.height() - kTrayItemSize) / 2,
+                         kTrayItemSize, kTrayItemSize);
     } else {
-      bounds =
-          gfx::Rect(bounds.x() + ((bounds.height() - kShelfItemSizeMD) / 2),
-                    bounds.y() + (bounds.width() - kShelfItemSizeMD) / 2,
-                    kShelfItemSizeMD, kShelfItemSizeMD);
+      bounds = gfx::Rect(bounds.x() + ((bounds.height() - kTrayItemSize) / 2),

[yiyix, 2016/06/27 18:39:43]

May create a constant for (bounds.height() - kTrayItemSize) / 2 and
(bounds.width() - kTrayItemSize) / 2)?

[tdanderson, 2016/06/27 21:29:42]

Done.

+                         bounds.y() + (bounds.width() - kTrayItemSize) / 2,
+                         kTrayItemSize, kTrayItemSize);
     }
   } else {
     const gfx::ImageSkia* background =
         rb.GetImageNamed(NonMaterialBackgroundImageId()).ToImageSkia();
     if (alignment == SHELF_ALIGNMENT_LEFT) {
       bounds =
           gfx::Rect(bounds.right() - background->width() - kShelfItemInset,
                     bounds.y() + (bounds.height() - background->height()) / 2,
                     background->width(), background->height());
     } else if (alignment == SHELF_ALIGNMENT_RIGHT) {
       bounds =
           gfx::Rect(bounds.x() + kShelfItemInset,
                     bounds.y() + (bounds.height() - background->height()) / 2,
                     background->width(), background->height());
     } else {
       bounds =
           gfx::Rect(bounds.x() + (bounds.width() - background->width()) / 2,
                     bounds.y() + kShelfItemInset, background->width(),
                     background->height());
     }
   }
   return bounds;
 }
 
 }  // namespace ash