Update appearance of omnibox focus for MD.

chrome/browser/ui/views/location_bar/location_bar_view.cc

Index: chrome/browser/ui/views/location_bar/location_bar_view.cc
diff --git a/chrome/browser/ui/views/location_bar/location_bar_view.cc b/chrome/browser/ui/views/location_bar/location_bar_view.cc
index 6b4235d1f1b8bcacf45da01c17582ccdf9de49a4..03b1bd9fd829ed85c232fa1937f3587bf0ca12cc 100644
--- a/chrome/browser/ui/views/location_bar/location_bar_view.cc
+++ b/chrome/browser/ui/views/location_bar/location_bar_view.cc
@@ -1258,16 +1258,27 @@ void LocationBarView::OnBoundsChanged(const gfx::Rect& previous_bounds) {
 }
 
 void LocationBarView::OnFocus() {
-  // Explicitly focus the omnibox so a focus ring will be displayed around it on
-  // Windows.
   omnibox_view_->SetFocus();
 }
 
 void LocationBarView::OnPaint(gfx::Canvas* canvas) {
   View::OnPaint(canvas);
 
-  if (ui::MaterialDesignController::IsModeMaterial() && !is_popup_mode_)
-    return;  // The background and border are painted by our Background.
+  if (ui::MaterialDesignController::IsModeMaterial()) {
+    if (show_focus_rect_ && omnibox_view_->HasFocus()) {
+      SkPaint paint;
+      paint.setAntiAlias(true);
+      paint.setColor(GetNativeTheme()->GetSystemColor(
+          ui::NativeTheme::NativeTheme::kColorId_FocusedBorderColor));
+      paint.setStyle(SkPaint::kStroke_Style);
+      paint.setStrokeWidth(1);

[Peter Kasting, 2016/05/20 20:32:49]

This will scale up on higher scale factors, which the code in
BackgroundWith1PxBorder won't do.  It seems like this should match that
behavior, just in a different color.

This suggests to me that maybe instead of adding this code here,
BackgroundWith1PxBorder should just do what it's doing today except use
kColorId_FocusedBorderColor in this case.  I'd think the easiest way to do that
would be for that class to be able to call a method on this one that either
answers "is this view visibly focused" or else answers "what color should the
border be drawn in".

[Evan Stade, 2016/05/20 20:54:43]

It's my understanding that the focus border is intentionally thicker in the 2x
case (the mock is at 2x and shows a 2px border).

[Peter Kasting, 2016/05/20 20:57:13]

Can we double-check that this intentionally differs from the omnibox border
behavior (unless you already asked)?  I'm OK with it if so (though in that case
we should probably have a comment about why this isn't using the normal border),
but the difference seems so unexpected that I'd think maybe the mock was put
together reasonably quickly or Sebastien forgot about the hairline omnibox
border in the normal case or something.

+      gfx::RectF focus_rect(GetLocalBounds());
+      focus_rect.Inset(gfx::InsetsF(0.5f));
+      canvas->DrawRoundRect(focus_rect, 2.5f, paint);
+    }
+    if (!is_popup_mode_)
+      return;  // The background and border are painted by our Background.
+  }
 
   // Fill the location bar background color behind the border.  Parts of the
   // border images are meant to rest atop the toolbar background and parts atop
@@ -1302,8 +1313,10 @@ void LocationBarView::PaintChildren(const ui::PaintContext& context) {
   // the focus rect to appear on top of children we paint here rather than
   // OnPaint().
   // Note: |Canvas::DrawFocusRect| paints a dashed rect with gray color.
-  if (show_focus_rect_ && HasFocus())
+  if (!ui::MaterialDesignController::IsModeMaterial() && show_focus_rect_ &&
+      HasFocus()) {

[Peter Kasting, 2016/05/20 20:32:49]

Nit: No {} on multiline conditional with single-line body; see e.g. line 717 for
existing file style

[Evan Stade, 2016/05/20 22:07:32]

Done.

     recorder.canvas()->DrawFocusRect(omnibox_view_->bounds());
+  }
 
   if (!ui::MaterialDesignController::IsModeMaterial() && !is_popup_mode_) {
     views::Painter::PaintPainterAt(recorder.canvas(), border_painter_.get(),
@@ -1365,10 +1378,6 @@ void LocationBarView::OnChanged() {
   SchedulePaint();
 }
 
-void LocationBarView::OnSetFocus() {
-  GetFocusManager()->SetFocusedView(this);
-}
-
 const ToolbarModel* LocationBarView::GetToolbarModel() const {
   return delegate_->GetToolbarModel();
 }