Update omnibox dropdown selection/hover background drawing for MD.

chrome/browser/ui/views/omnibox/omnibox_result_view.cc

Index: chrome/browser/ui/views/omnibox/omnibox_result_view.cc
diff --git a/chrome/browser/ui/views/omnibox/omnibox_result_view.cc b/chrome/browser/ui/views/omnibox/omnibox_result_view.cc
index b81569068cb18c9c2d21c90c3fa62efeb9a80eb8..db32db044b1df1eb2b072d9b94c93fdb98a5f809 100644
--- a/chrome/browser/ui/views/omnibox/omnibox_result_view.cc
+++ b/chrome/browser/ui/views/omnibox/omnibox_result_view.cc
@@ -21,6 +21,8 @@
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "chrome/browser/ui/layout_constants.h"
+#include "chrome/browser/ui/views/location_bar/background_with_1_px_border.h"
+#include "chrome/browser/ui/views/location_bar/icon_label_bubble_view.h"
 #include "chrome/browser/ui/views/location_bar/location_bar_view.h"
 #include "chrome/browser/ui/views/omnibox/omnibox_popup_contents_view.h"
 #include "chrome/grit/generated_resources.h"
@@ -40,6 +42,7 @@
 #include "ui/gfx/paint_vector_icon.h"
 #include "ui/gfx/range/range.h"
 #include "ui/gfx/render_text.h"
+#include "ui/gfx/scoped_canvas.h"
 #include "ui/gfx/text_utils.h"
 #include "ui/gfx/vector_icons_public.h"
 #include "ui/native_theme/native_theme.h"
@@ -638,10 +641,16 @@ void OmniboxResultView::InitContentsRenderTextIfNecessary() const {
 }
 
 void OmniboxResultView::Layout() {
-  const int horizontal_padding =
+  int horizontal_padding =
       GetLayoutConstant(LOCATION_BAR_HORIZONTAL_PADDING);
-  const int start_x = StartMargin() + horizontal_padding;
-  const int end_x = width() - EndMargin() - horizontal_padding;
+  // In non-material, the horizontal bounds we're given are indented inside the
+  // omnibox border.  In material, we're given the outside bounds, so we can
+  // match the omnibox border outline shape exactly in OnPaint().  So we have to
+  // inset here to keep the icons lined up.
+  const int border_padding = ui::MaterialDesignController::IsModeMaterial() ?
+    GetLayoutConstant(LOCATION_BAR_BORDER_THICKNESS) : 0;
+  const int start_x = border_padding + horizontal_padding;
+  const int end_x = width() - border_padding - horizontal_padding;
 
   const gfx::ImageSkia icon = GetIcon();
   // Pre-MD, normal icons are 19 px wide, while extension icons are 16 px wide.
@@ -677,13 +686,34 @@ void OmniboxResultView::Layout() {
 }
 
 void OmniboxResultView::OnBoundsChanged(const gfx::Rect& previous_bounds) {
-  animation_->SetSlideDuration((width() - StartMargin() - EndMargin()) / 4);
+  animation_->SetSlideDuration(width() / 4);
 }
 
 void OmniboxResultView::OnPaint(gfx::Canvas* canvas) {
   const ResultViewState state = GetState();
-  if (state != NORMAL)
-    canvas->DrawColor(GetColor(state, BACKGROUND));
+  if (state != NORMAL) {
+    const SkColor background_color = GetColor(state, BACKGROUND);
+    if (ui::MaterialDesignController::IsModeMaterial()) {
+      SkPaint paint;
+      paint.setAntiAlias(true);
+      paint.setColor(background_color);
+
+      // This is similar to the code in BackgroundWith1PxBorder, except that
+      // instead of stroking along a path offset by 0.5 px, we're filling to
+      // where the edge of that stroke would be (i.e. on a whole-pixel
+      // boundary).
+      gfx::RectF rect(GetLocalBounds());
+      gfx::ScopedCanvas scoped_canvas(canvas);
+      const float scale = canvas->UndoDeviceScaleFactor();
+      rect.Scale(scale);
+      rect.Inset(GetLayoutConstant(LOCATION_BAR_BORDER_THICKNESS) * scale - 1,
+                 0);
+      canvas->DrawRoundRect(
+          rect, BackgroundWith1PxBorder::kCornerRadius * scale + 1, paint);

[Evan Stade, 2016/06/16 16:11:32]

Would it be possible to set a bgwith1pxborder on |this|, making the border and
bg the same color?

[Peter Kasting, 2016/06/16 17:57:13]

Yes, although doing it right took more effort than I expected.  I decided not to
expose a color-setting method on the background, choosing instead to recreate it
when the line state changes -- which corresponds almost exactly to when
Invalidate() gets called.  (Our caller takes care not to call us when nothing
has changed, e.g. the mouse moves around within the same line.)

But Invalidate() is only called for changes after initial creation; to get the
initial state right, we also need to hook SetMatch().  Turns out it wanted to do
a lot of the same things as Invalidate(), so I just made one call the other.

I also had to touch the popup model to have it update the hovered line before
invalidating.  It already did this with the selected line, so this seemed like a
good consistency change anyway.  I used the same name/comment style as in the
selected line code.

+    } else {
+      canvas->DrawColor(background_color);
+    }
+  }
 
   // NOTE: While animating the keyword match, both matches may be visible.
 
@@ -829,13 +859,3 @@ void OmniboxResultView::AppendAnswerTextHelper(gfx::RenderText* destination,
       GetNativeTheme()->GetSystemColor(text_style.colors[GetState()]), range);
   destination->ApplyBaselineStyle(text_style.baseline, range);
 }
-
-int OmniboxResultView::StartMargin() const {
-  return ui::MaterialDesignController::IsModeMaterial() ?
-      model_->start_margin() : 0;
-}
-
-int OmniboxResultView::EndMargin() const {
-  return ui::MaterialDesignController::IsModeMaterial() ?
-      model_->end_margin() : 0;
-}