Fix SysTray focus rect position

ash/common/system/tray/tray_background_view.cc

 // Copyright (c) 2012 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/common/system/tray/tray_background_view.h"
 
 #include <algorithm>
 
 #include "ash/common/ash_constants.h"
 #include "ash/common/material_design/material_design_controller.h"
@@ skipping 566 matching lines @@
 
 gfx::Rect TrayBackgroundView::GetFocusBounds() {
   // The tray itself expands to the right and bottom edge of the screen to make
   // sure clicking on the edges brings up the popup. However, the focus border
   // should be only around the container.
   return GetContentsBounds();
 }
 
 void TrayBackgroundView::OnPaintFocus(gfx::Canvas* canvas) {
   gfx::RectF paint_bounds(GetFocusBounds());
-  paint_bounds.Inset(gfx::Insets(2, -2));
+  if (MaterialDesignController::IsShelfMaterial())
+    paint_bounds.Inset(gfx::Insets(-2, 2, -2, 3));

[Evan Stade, 2016/12/12 23:47:50]

ick at these magic numbers. Where do they come from? Why is GetFocusBounds not
apparently returning the right thing?

I don't think it's necessary to continue adding code to support the non-material
version (existing code for pre-md will be ripped out soon(tm)).

[Greg Levin, 2016/12/13 17:08:07]

Agree with ick.  That said, I'm not sure about the current merits of a deeper
dive vs my bandaid, given that this code still seems to be in flux (see
xiaoyinh@'s comment, and the fact that this inset has been fixed 3 times in the
last month).  Maybe a TODO for when MD changes are finalized?
 
I wasn't sure why we keep coming up with different insets.  I added the non-MD
code on the chance that someone was looking that version, and wanted to avoid
any future change based on the non-MD layout.  That said, I'm not really tuned
in to the MD shelf progress, so will definitely remove it if that makes more
sense.

[tdanderson, 2016/12/13 23:17:09]

Leaving the non-md change in here is fine by me. No need to add a TODO since
this will automatically be addressed when we rip out non-md code.

[Evan Stade, 2016/12/14 00:48:16]

I'm not really ok with these magic numbers though, and I'm still confused why
GetFocusBounds isn't getting the focus bounds.

[Greg Levin, 2016/12/15 17:30:56]

I don't know the details of the code, but the way GetFocusBounds() was
overridden seems wrong for MD shelf (but okay for non-MD). 
CreateInkDropHighlight() was using GetBackgroundBounds() to place the ink drop,
and that seems to work better here.
We still have the magic "1" for non-MD, but that'll be going away soon. 
Otherwise, how does this look?

+  else
+    paint_bounds.Inset(gfx::Insets(1, 1));
   canvas->DrawSolidFocusRect(paint_bounds, kFocusBorderColor,
                              kFocusBorderThickness);
 }
 
 void TrayBackgroundView::OnPaint(gfx::Canvas* canvas) {
   ActionableView::OnPaint(canvas);
   if (!MaterialDesignController::IsShelfMaterial() ||
       shelf()->GetBackgroundType() ==
           ShelfBackgroundType::SHELF_BACKGROUND_DEFAULT ||
       !is_separator_visible_) {
@@ skipping 46 matching lines @@
 }
 
 gfx::Rect TrayBackgroundView::GetBackgroundBounds() const {
   gfx::Insets insets = GetBackgroundInsets();
   gfx::Rect bounds = GetLocalBounds();
   bounds.Inset(insets);
   return bounds;
 }
 
 }  // namespace ash