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 [email protected] (cherry picked from commit b167f41) Bug: 801583, 829574 Change-Id: I6498ff15d3b1be6a03cab92e9aae8971b54c358e Reviewed-on: https://chromium-review.googlesource.com/1002716 Commit-Queue: Patti <[email protected]> Reviewed-by: Michael Wasserman <[email protected]> Reviewed-by: Peter Kasting <[email protected]> Cr-Original-Commit-Position: refs/heads/master@{#550935} Reviewed-on: https://chromium-review.googlesource.com/1015456 Reviewed-by: Patti <[email protected]> Cr-Commit-Position: refs/branch-heads/3396@{#73} Cr-Branched-From: 9ef2aa8-refs/heads/master@{#550428}
nwjs · Apr 18, 2018 · 1e01ecc · 1e01ecc
1 parent d8acff3
commit 1e01ecc
Show file tree

Hide file tree

Showing 16 changed files with 191 additions and 100 deletions.
diff --git a/chrome/browser/ui/layout_constants.cc b/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

diff --git a/chrome/browser/ui/omnibox/omnibox_theme.cc b/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,
@@ -229,6 +231,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 (6%)
       // and select (8%), which can overlap (for 14%). Pre-blend these with the

diff --git a/chrome/browser/ui/omnibox/omnibox_theme.h b/chrome/browser/ui/omnibox/omnibox_theme.h
@@ -17,6 +17,7 @@ enum class OmniboxPart {
   LOCATION_BAR_SELECTED_KEYWORD,
   LOCATION_BAR_TEXT_DEFAULT,
   LOCATION_BAR_TEXT_DIMMED,
+  LOCATION_BAR_FOCUS_RING,
 
   RESULTS_BACKGROUND,  // Background of the results dropdown.
   RESULTS_ICON,

diff --git a/chrome/browser/ui/views/find_bar_host.cc b/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();

diff --git a/chrome/browser/ui/views/harmony/chrome_layout_provider.cc b/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(

diff --git a/chrome/browser/ui/views/location_bar/background_with_1_px_border.cc b/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,

diff --git a/chrome/browser/ui/views/location_bar/background_with_1_px_border.h b/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.

diff --git a/chrome/browser/ui/views/location_bar/bubble_icon_view.cc b/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);

diff --git a/chrome/browser/ui/views/location_bar/icon_label_bubble_view.cc b/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(),

diff --git a/chrome/browser/ui/views/location_bar/keyword_hint_view.cc b/chrome/browser/ui/views/location_bar/keyword_hint_view.cc
@@ -43,7 +43,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(
@@ -132,7 +132,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));
 
@@ -175,7 +175,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();
@@ -197,18 +197,19 @@ gfx::Size KeywordHintView::CalculatePreferredSize() const {
 }
 
 void KeywordHintView::OnBoundsChanged(const gfx::Rect& previous_bounds) {
-  if (!BackgroundWith1PxBorder::IsRounded())
-    return;
-  const int chip_corner_radius = GetCornerRadius();
-  chip_label_->SetBorder(views::CreateEmptyBorder(
-      gfx::Insets(GetInsets().top(), chip_corner_radius, GetInsets().bottom(),
-                  chip_corner_radius)));
+  if (LocationBarView::IsRounded()) {
+    const int chip_corner_radius = GetCornerRadius();
+    chip_label_->SetBorder(views::CreateEmptyBorder(
+        gfx::Insets(GetInsets().top(), chip_corner_radius, GetInsets().bottom(),
+                    chip_corner_radius)));
+  }
+  views::Button::OnBoundsChanged(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);
@@ -218,7 +219,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;
 }