Update omnibox dropdown selection/hover background drawing for MD.

chrome/browser/ui/views/omnibox/omnibox_result_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.
 
 // For WinDDK ATL compatibility, these ATL headers must come first.
 #include "build/build_config.h"
 
 #if defined(OS_WIN)
 #include <atlbase.h>  // NOLINT
 #include <atlwin.h>  // NOLINT
 #endif
 
 #include "base/macros.h"
 #include "chrome/browser/ui/views/omnibox/omnibox_result_view.h"
 
 #include <limits.h>
 
 #include <algorithm>  // NOLINT
 
 #include "base/i18n/bidi_line_iterator.h"
 #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"
 #include "components/omnibox/browser/omnibox_popup_model.h"
 #include "grit/components_scaled_resources.h"
 #include "grit/theme_resources.h"
 #include "third_party/skia/include/core/SkColor.h"
 #include "ui/accessibility/ax_view_state.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/material_design/material_design_controller.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/base/theme_provider.h"
 #include "ui/gfx/canvas.h"
 #include "ui/gfx/color_palette.h"
 #include "ui/gfx/color_utils.h"
 #include "ui/gfx/image/image.h"
 #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"
 
 using ui::NativeTheme;
 
 namespace {
 
 // A mapping from OmniboxResultView's ResultViewState/ColorKind types to
 // NativeTheme colors.
@@ skipping 578 matching lines @@
 
 void OmniboxResultView::InitContentsRenderTextIfNecessary() const {
   if (!contents_rendertext_) {
     contents_rendertext_.reset(
         CreateClassifiedRenderText(
             match_.contents, match_.contents_class, false).release());
   }
 }
 
 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.
   // The code in IconLabelBubbleView::Layout() positions these icons in the
   // omnibox using ICON_LABEL_VIEW_TRAILING_PADDING, so we use that here as well
   // so the icons will line up.
   //
   // Technically we don't need the IsModeMaterial() check here, but it will make
   // it easier to see that all this code is dead once we switch to MD.
   int icon_x = start_x;
@@ skipping 15 matching lines @@
     keyword_text_bounds_.SetRect(
         kw_text_x, 0, std::max(end_x - kw_text_x, 0), height());
     keyword_icon_->SetPosition(
         gfx::Point(kw_x, (height() - keyword_icon_->height()) / 2));
   }
 
   text_bounds_.SetRect(text_x, 0, std::max(text_width, 0), height());
 }
 
 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.
 
   if (!ShowOnlyKeywordMatch()) {
     canvas->DrawImageInt(GetIcon(), GetMirroredXForRect(icon_bounds_),
                          icon_bounds_.y());
     int x = GetMirroredXForRect(text_bounds_);
     mirroring_context_->Initialize(x, text_bounds_.width());
     InitContentsRenderTextIfNecessary();
 
@@ skipping 125 matching lines @@
   destination->AppendText(text);
   const TextStyle& text_style = GetTextStyle(text_type);
   // TODO(dschuyler): follow up on the problem of different font sizes within
   // one RenderText.  Maybe with destination->SetFontList(...).
   destination->ApplyWeight(
       is_bold ? gfx::Font::Weight::BOLD : gfx::Font::Weight::NORMAL, range);
   destination->ApplyColor(
       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;
-}