Omnibox/Views: Remove LocationBarView stroke for touch mode.

This patch removes the 1px outline on LocationBarView and switches to using a
views::FocusRing in touch mode.

See screenshots -
https://drive.google.com/file/d/1g4O_mh4oPb8PJIAb7pjDq0rK32KIUUvP/view?usp=sharing

Bug: 801583, 829574
Change-Id: I6498ff15d3b1be6a03cab92e9aae8971b54c358e
Reviewed-on: https://chromium-review.googlesource.com/1002716
Commit-Queue: Patti <patricialor@chromium.org>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550935}

chrome/browser/ui/layout_constants.cc

@@ -58,13 +58,7 @@ int GetLayoutConstant(LayoutConstant constant) {
     case LOCATION_BAR_ICON_SIZE:
       return touch_optimized_material ? 20 : 16;
     case LOCATION_BAR_ICON_INTERIOR_PADDING:
-      if (touch_optimized_material) {
-        // TODO(crbug.com/801583): This should actually be 8, but until
-        // LocationBarView is updated to remove its stroke, subtract the dips
-        // reserved for the stroke first.
-        return 7;
-      }
-      return 4;
+      return touch_optimized_material ? 8 : 4;
     case TABSTRIP_NEW_TAB_BUTTON_SPACING: {
       // In non-touch optimized UI, we make the new tab button overlap with the
       // last tab in the tabstrip (i.e negative spacing). However, in

chrome/browser/ui/omnibox/omnibox_theme.cc

@@ -65,6 +65,8 @@ ui::NativeTheme::ColorId GetLegacyColorId(ui::NativeTheme* native_theme,
                  : NativeId::kColorId_LinkEnabled;
     case OmniboxPart::LOCATION_BAR_TEXT_DEFAULT:
       return NativeId::kColorId_TextfieldDefaultColor;
+    case OmniboxPart::LOCATION_BAR_FOCUS_RING:
+      return NativeId::kColorId_FocusedBorderColor;
     case OmniboxPart::RESULTS_BACKGROUND:
       return NormalHoveredSelected(
           state, NativeId::kColorId_ResultsTableNormalBackground,
@@ -230,6 +232,8 @@ SkColor GetOmniboxColor(OmniboxPart part,
       return GetSecurityChipColor(tint, state);
     case OmniboxPart::LOCATION_BAR_SELECTED_KEYWORD:
       return dark ? gfx::kGoogleGrey100 : gfx::kGoogleBlue600;
+    case OmniboxPart::LOCATION_BAR_FOCUS_RING:
+      return dark ? gfx::kGoogleBlueDark600 : gfx::kGoogleBlue600;
     case OmniboxPart::RESULTS_BACKGROUND:
       // The spec calls for transparent black (or white) overlays for hover (8%)
       // and select (6%), which can overlap (for 14%). Pre-blend these with the

chrome/browser/ui/omnibox/omnibox_theme.h

@@ -18,6 +18,7 @@ enum class OmniboxPart {
   LOCATION_BAR_TEXT_DEFAULT,
   LOCATION_BAR_TEXT_DIMMED,
   LOCATION_BAR_BUBBLE_OUTLINE,
+  LOCATION_BAR_FOCUS_RING,
 
   RESULTS_BACKGROUND,  // Background of the results dropdown.
   RESULTS_ICON,

chrome/browser/ui/views/find_bar_host.cc

@@ -265,9 +265,8 @@ gfx::Rect FindBarHost::GetDialogPosition(gfx::Rect avoid_overlapping_rect) {
   if (widget_bounds.IsEmpty())
     return gfx::Rect();
 
-  gfx::Insets insets =
-      view()->border()->GetInsets() -
-      gfx::Insets(0, BackgroundWith1PxBorder::kLocationBarBorderThicknessDip);
+  gfx::Insets insets = view()->border()->GetInsets() -
+                       gfx::Insets(0, LocationBarView::GetBorderThicknessDip());
 
   // Ask the view how large an area it needs to draw on.
   gfx::Size prefsize = view()->GetPreferredSize();

chrome/browser/ui/views/harmony/chrome_layout_provider.cc

@@ -122,8 +122,10 @@ bool ChromeLayoutProvider::IsHarmonyMode() const {
 int ChromeLayoutProvider::GetCornerRadiusMetric(
     ChromeEmphasisMetric emphasis_metric,
     const gfx::Rect& bounds) const {
-  // Outside of MD (refresh) mode, just stick to the current fixed value.
-  return emphasis_metric == EMPHASIS_HIGH ? 0 : 4;
+  // Use the current fixed value for non-EMPHASIS_HIGH.
+  return emphasis_metric == EMPHASIS_HIGH
+             ? std::min(bounds.width(), bounds.height()) / 2
+             : 4;
 }
 
 int ChromeLayoutProvider::GetShadowElevationMetric(

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

@@ -5,6 +5,7 @@
 #include "chrome/browser/ui/views/location_bar/background_with_1_px_border.h"
 
 #include "cc/paint/paint_flags.h"
+#include "chrome/browser/ui/views/location_bar/location_bar_view.h"
 #include "third_party/skia/include/core/SkPath.h"
 #include "third_party/skia/include/pathops/SkPathOps.h"
 #include "ui/base/material_design/material_design_controller.h"
@@ -19,11 +20,6 @@ BackgroundWith1PxBorder::BackgroundWith1PxBorder(SkColor background,
   SetNativeControlColor(background);
 }
 
-// static
-bool BackgroundWith1PxBorder::IsRounded() {
-  return ui::MaterialDesignController::IsNewerMaterialUi();
-}
-
 void BackgroundWith1PxBorder::PaintFocusRing(gfx::Canvas* canvas,
                                              ui::NativeTheme* theme,
                                              const gfx::Rect& local_bounds) {
@@ -42,7 +38,7 @@ void BackgroundWith1PxBorder::Paint(gfx::Canvas* canvas,
 }
 
 float BackgroundWith1PxBorder::GetBorderRadius(int height_in_px) const {
-  if (IsRounded()) {
+  if (LocationBarView::IsRounded()) {
     // This method returns the inner radius of the border, so subtract 1 pixel
     // off the final border radius since the border thickness is always 1px.
     return height_in_px / 2.f - 1;
@@ -60,13 +56,12 @@ void BackgroundWith1PxBorder::Paint(gfx::Canvas* canvas,
   gfx::RectF border_rect_f(bounds);
   border_rect_f.Scale(scale);
 
-  // Inset by |kLocationBarBorderThicknessDip|, then draw the border along the
-  // outside edge of the result. Make sure the inset amount is a whole number so
-  // the border will still be aligned to the pixel grid. std::floor is chosen
-  // here to ensure the border will be fully contained within the
-  // |kLocationBarBorderThicknessDip| region.
-  border_rect_f.Inset(
-      gfx::InsetsF(std::floor(kLocationBarBorderThicknessDip * scale)));
+  // Inset by |kBorderThicknessDip|, then draw the border along the outside edge
+  // of the result. Make sure the inset amount is a whole number so the border
+  // will still be aligned to the pixel grid. std::floor is chosen here to
+  // ensure the border will be fully contained within the |kBorderThicknessDip|
+  // region.
+  border_rect_f.Inset(gfx::InsetsF(std::floor(kBorderThicknessDip * scale)));
 
   SkRRect inner_rect(SkRRect::MakeRectXY(gfx::RectFToSkRect(border_rect_f),
                                          inner_border_radius,

chrome/browser/ui/views/location_bar/background_with_1_px_border.h

@@ -21,19 +21,17 @@ class View;
 // BackgroundWith1PxBorder renders a solid background color, with a one pixel
 // border with rounded corners. This accounts for the scaling of the canvas, so
 // that the border is one pixel regardless of display scaling.
+// TODO(patricialor): Delete this & replace with CreateRoundRectWith1PxPainter.
 class BackgroundWith1PxBorder : public views::Background {
  public:
-  // The thickness of the location bar's border in DIP.
-  static constexpr int kLocationBarBorderThicknessDip = 1;
+  // The thickness of the border in DIP.
+  static constexpr int kBorderThicknessDip = 1;
 
   // The legacy (non touch/material) border radius.
   static constexpr int kLegacyBorderRadiusPx = 2;
 
   BackgroundWith1PxBorder(SkColor background, SkColor border);
 
-  // Whether the OmniboxBackgroundBorder is a pill shape.
-  static bool IsRounded();
-
   void set_blend_mode(SkBlendMode blend_mode) { blend_mode_ = blend_mode; }
 
   // Paints a blue focus ring that draws over the top of the existing border.

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

@@ -205,7 +205,7 @@ SkColor BubbleIconView::GetInkDropBaseColor() const {
 }
 
 std::unique_ptr<views::InkDropMask> BubbleIconView::CreateInkDropMask() const {
-  if (!BackgroundWith1PxBorder::IsRounded())
+  if (!LocationBarView::IsRounded())
     return nullptr;
   return std::make_unique<views::RoundRectInkDropMask>(size(), gfx::Insets(),
                                                        height() / 2.f);

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

@@ -307,7 +307,7 @@ SkColor IconLabelBubbleView::GetInkDropBaseColor() const {
 
 std::unique_ptr<views::InkDropMask> IconLabelBubbleView::CreateInkDropMask()
     const {
-  if (!BackgroundWith1PxBorder::IsRounded())
+  if (!LocationBarView::IsRounded())
     return nullptr;
   return std::make_unique<views::RoundRectInkDropMask>(
       ink_drop_container_->size(), gfx::Insets(),

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

@@ -52,7 +52,7 @@ KeywordHintView::KeywordHintView(views::ButtonListener* listener,
   constexpr int kPaddingInsideBorder = 5;
   // Even though the border is 1 px thick visibly, it takes 1 DIP logically for
   // the non-rounded style.
-  const int horizontal_padding = BackgroundWith1PxBorder::IsRounded()
+  const int horizontal_padding = LocationBarView::IsRounded()
                                      ? GetCornerRadius()
                                      : kPaddingInsideBorder + 1;
   chip_label_->SetBorder(
@@ -160,7 +160,7 @@ void KeywordHintView::SetKeyword(const base::string16& keyword,
 }
 
 gfx::Insets KeywordHintView::GetInsets() const {
-  if (!BackgroundWith1PxBorder::IsRounded())
+  if (!LocationBarView::IsRounded())
     return gfx::Insets(0,
                        GetLayoutConstant(LOCATION_BAR_ICON_INTERIOR_PADDING));
 
@@ -203,7 +203,7 @@ void KeywordHintView::Layout() {
   gfx::Size leading_size(leading_label_->GetPreferredSize());
   leading_label_->SetBounds(GetInsets().left(), 0,
                             show_labels ? leading_size.width() : 0, height());
-  const int chip_height = BackgroundWith1PxBorder::IsRounded()
+  const int chip_height = LocationBarView::IsRounded()
                               ? GetLayoutConstant(LOCATION_BAR_ICON_SIZE) +
                                     chip_container_->GetInsets().height()
                               : height();
@@ -225,7 +225,7 @@ gfx::Size KeywordHintView::CalculatePreferredSize() const {
 }
 
 void KeywordHintView::OnBoundsChanged(const gfx::Rect& previous_bounds) {
-  if (BackgroundWith1PxBorder::IsRounded()) {
+  if (LocationBarView::IsRounded()) {
     const int chip_corner_radius = GetCornerRadius();
     chip_label_->SetBorder(views::CreateEmptyBorder(
         gfx::Insets(GetInsets().top(), chip_corner_radius, GetInsets().bottom(),
@@ -237,7 +237,7 @@ void KeywordHintView::OnBoundsChanged(const gfx::Rect& previous_bounds) {
 views::Label* KeywordHintView::CreateLabel(SkColor text_color,
                                            SkColor background_color) {
   views::Label* label =
-      new views::Label(base::string16(), BackgroundWith1PxBorder::IsRounded()
+      new views::Label(base::string16(), LocationBarView::IsRounded()
                                              ? CONTEXT_OMNIBOX_DECORATION
                                              : CONTEXT_OMNIBOX_PRIMARY);
   label->SetEnabledColor(text_color);
@@ -247,7 +247,7 @@ views::Label* KeywordHintView::CreateLabel(SkColor text_color,
 }
 
 int KeywordHintView::GetCornerRadius() const {
-  if (!BackgroundWith1PxBorder::IsRounded())
+  if (!LocationBarView::IsRounded())
     return GetLayoutConstant(LOCATION_BAR_BUBBLE_CORNER_RADIUS);
   return chip_container_->height() / 2;
 }

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

@@ -43,6 +43,7 @@
 #include "chrome/browser/ui/views/autofill/save_card_icon_view.h"
 #include "chrome/browser/ui/views/chrome_platform_style.h"
 #include "chrome/browser/ui/views/frame/browser_view.h"
+#include "chrome/browser/ui/views/harmony/chrome_layout_provider.h"
 #include "chrome/browser/ui/views/harmony/chrome_typography.h"
 #include "chrome/browser/ui/views/location_bar/background_with_1_px_border.h"
 #include "chrome/browser/ui/views/location_bar/bubble_icon_view.h"
@@ -125,8 +126,7 @@ using views::View;
 namespace {
 
 bool InTouchableMode() {
-  return ui::MaterialDesignController::GetMode() ==
-         ui::MaterialDesignController::MATERIAL_TOUCH_OPTIMIZED;
+  return ui::MaterialDesignController::IsTouchOptimizedUiEnabled();
 }
 
 OmniboxTint GetTintForProfile(Profile* profile) {
@@ -146,6 +146,36 @@ OmniboxTint GetTintForProfile(Profile* profile) {
   return OmniboxTint::LIGHT;
 }
 
+// Returns true when a views::FocusRing should be used.
+bool ShouldUseFocusRingView(bool show_focus_ring) {
+  return (show_focus_ring && LocationBarView::IsRounded()) ||
+         ChromePlatformStyle::ShouldOmniboxUseFocusRing();
+}
+
+// Installs a focus ring on the LocationBarView if it's focused and returns it.
+views::View* InstallFocusRing(LocationBarView* view) {
+  if (!view->HasFocus())
+    return nullptr;
+  return views::FocusRing::Install(
+      view, view->GetColor(OmniboxPart::LOCATION_BAR_FOCUS_RING),
+      view->GetBorderRadius());
+}
+
+views::View* UninstallFocusRing(LocationBarView* view) {
+  // This should be a no-op if the focus ring doesn't exist.
+  views::FocusRing::Uninstall(view);
+  return nullptr;
+}
+
+// Helper function to create a rounded rect background (no stroke).
+std::unique_ptr<views::Background> CreateRoundRectBackground(SkColor bg_color,
+                                                             float radius) {
+  std::unique_ptr<views::Background> background = CreateBackgroundFromPainter(
+      views::Painter::CreateSolidRoundRectPainter(bg_color, radius));
+  background->SetNativeControlColor(bg_color);
+  return background;
+}
+
 }  // namespace
 
 // LocationBarView -----------------------------------------------------------
@@ -163,7 +193,8 @@ LocationBarView::LocationBarView(Browser* browser,
       browser_(browser),
       delegate_(delegate),
       is_popup_mode_(is_popup_mode),
-      tint_(GetTintForProfile(profile)) {
+      tint_(GetTintForProfile(profile)),
+      focus_ring_(nullptr) {
   edit_bookmarks_enabled_.Init(
       bookmarks::prefs::kEditBookmarksEnabled, profile->GetPrefs(),
       base::Bind(&LocationBarView::UpdateWithoutTabRestore,
@@ -294,6 +325,22 @@ SkColor LocationBarView::GetOpaqueBorderColor(bool incognito) const {
                             ThemeProperties::COLOR_TOOLBAR, incognito));
 }
 
+// static
+int LocationBarView::GetBorderThicknessDip() {
+  return IsRounded() ? 0 : BackgroundWith1PxBorder::kBorderThicknessDip;
+}
+
+// static
+bool LocationBarView::IsRounded() {
+  return ui::MaterialDesignController::IsNewerMaterialUi();
+}
+
+float LocationBarView::GetBorderRadius() {
+  return IsRounded() ? ChromeLayoutProvider::Get()->GetCornerRadiusMetric(
+                           EMPHASIS_HIGH, GetLocalBounds())
+                     : GetLayoutConstant(LOCATION_BAR_BUBBLE_CORNER_RADIUS);
+}
+
 SkColor LocationBarView::GetSecurityChipColor(
     security_state::SecurityLevel security_level) const {
   // Only used in ChromeOS.
@@ -345,6 +392,8 @@ void LocationBarView::SetImeInlineAutocompletion(const base::string16& text) {
 
 void LocationBarView::SetShowFocusRect(bool show) {
   show_focus_rect_ = show;
+  focus_ring_ = ShouldUseFocusRingView(show) ? InstallFocusRing(this)
+                                             : UninstallFocusRing(this);
   SchedulePaint();
 }
 
@@ -718,13 +767,12 @@ int LocationBarView::GetAvailableTextHeight() {
 }
 
 void LocationBarView::OnOmniboxFocused() {
-  if (ChromePlatformStyle::ShouldOmniboxUseFocusRing())
-    views::FocusRing::Install(this);
+  if (ShouldUseFocusRingView(show_focus_rect_))
+    focus_ring_ = InstallFocusRing(this);
 }
 
 void LocationBarView::OnOmniboxBlurred() {
-  if (ChromePlatformStyle::ShouldOmniboxUseFocusRing())
-    views::FocusRing::Uninstall(this);
+  focus_ring_ = UninstallFocusRing(this);
 }
 
 // static
@@ -753,7 +801,7 @@ SkColor LocationBarView::GetBorderColor() const {
 gfx::Rect LocationBarView::GetLocalBoundsWithoutEndcaps() const {
   const float device_scale_factor = layer()->device_scale_factor();
   const int border_radius =
-      BackgroundWith1PxBorder::IsRounded()
+      IsRounded()
           ? height() / 2
           : gfx::ToCeiledInt(BackgroundWith1PxBorder::kLegacyBorderRadiusPx /
                              device_scale_factor);
@@ -763,22 +811,29 @@ gfx::Rect LocationBarView::GetLocalBoundsWithoutEndcaps() const {
 }
 
 int LocationBarView::GetHorizontalEdgeThickness() const {
-  return is_popup_mode_
-             ? 0
-             : BackgroundWith1PxBorder::kLocationBarBorderThicknessDip;
+  return is_popup_mode_ ? 0 : GetBorderThicknessDip();
 }
 
 void LocationBarView::RefreshBackground() {
   SkColor background_color = GetColor(OmniboxPart::LOCATION_BAR_BACKGROUND);
   SkColor border_color = GetBorderColor();
 
-  // When the omnibox dropdown is open, match its color.
-  if (InTouchableMode() && GetOmniboxPopupView()->IsOpen()) {
-    background_color = border_color = GetColor(OmniboxPart::RESULTS_BACKGROUND);
+  if (InTouchableMode()) {
+    // When the omnibox dropdown is open, match its color and remove the focus
+    // ring.
+    if (GetOmniboxPopupView()->IsOpen()) {
+      background_color = border_color =
+          GetColor(OmniboxPart::RESULTS_BACKGROUND);
+    }
+    if (focus_ring_)
+      focus_ring_->SetVisible(!GetOmniboxPopupView()->IsOpen());
   }
 
   if (is_popup_mode_) {
     SetBackground(views::CreateSolidBackground(background_color));
+  } else if (GetBorderThicknessDip() == 0) {
+    SetBackground(
+        CreateRoundRectBackground(background_color, GetBorderRadius()));
   } else {
     SetBackground(std::make_unique<BackgroundWith1PxBorder>(background_color,
                                                             border_color));
@@ -1068,6 +1123,7 @@ void LocationBarView::OnBoundsChanged(const gfx::Rect& previous_bounds) {
   OmniboxPopupView* popup = GetOmniboxPopupView();
   if (popup->IsOpen())
     popup->UpdatePopupAppearance();
+  RefreshBackground();
 }
 
 void LocationBarView::OnFocus() {
@@ -1077,8 +1133,7 @@ void LocationBarView::OnFocus() {
 void LocationBarView::OnPaint(gfx::Canvas* canvas) {
   View::OnPaint(canvas);
 
-  if (show_focus_rect_ && omnibox_view_->HasFocus() &&
-      !ChromePlatformStyle::ShouldOmniboxUseFocusRing()) {
+  if (show_focus_rect_ && omnibox_view_->HasFocus() && !focus_ring_) {
     static_cast<BackgroundWith1PxBorder*>(background())
         ->PaintFocusRing(canvas, GetNativeTheme(), GetLocalBounds());
   }
@@ -1174,6 +1229,6 @@ void LocationBarView::SetFocusAndSelection(bool select_all) {
 
 // static
 int LocationBarView::GetTotalVerticalPadding() {
-  return BackgroundWith1PxBorder::kLocationBarBorderThicknessDip +
+  return GetBorderThicknessDip() +
          GetLayoutConstant(LOCATION_BAR_ELEMENT_PADDING);
 }