[omnibox] Remove text movement in Refresh when opening suggestions.

This involves several small changes:
- Adjust the layout of IconLabelBubbleView to account for the case
  where we want to show the separator but not the label.
- Move the extra space shown in suggest mode from LocationBarView to
  IconLabelBubbleView so it can use the logic and spacing values there.
- Add LocationIconView::ShouldShowSeparator() to indicate that we want
  to show the separator in the steady-state case.
- Adjust the spacing in the suggestion result view to match the current
  spec.

Also, remove a bit of dead code: GetMaxSizeForLabelWidth() and
GetOuterPadding() in IconLabelBubbleView.

Bug: 853355
Change-Id: I108dbf80de3c060ca56bbe502101dfbb1123ef4e
Reviewed-on: https://chromium-review.googlesource.com/1136493
Commit-Queue: Justin Donnelly <jdonnelly@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576325}

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

@@ -180,6 +180,10 @@ bool IconLabelBubbleView::ShouldShowSeparator() const {
   return ShouldShowLabel();
 }
 
+bool IconLabelBubbleView::ShouldShowExtraSpace() const {
+  return false;
+}
+
 double IconLabelBubbleView::WidthMultiplier() const {
   return 1.0;
 }
@@ -232,8 +236,12 @@ void IconLabelBubbleView::Layout() {
   separator_bounds.Inset(0, (separator_bounds.height() - separator_height) / 2);
 
   float separator_width = GetPrefixedSeparatorWidth() + GetEndPadding();
-  separator_view_->SetBounds(label_->bounds().right(), separator_bounds.y(),
-                             separator_width, separator_height);
+  int separator_x =
+      ui::MaterialDesignController::IsRefreshUi() && label_->text().empty()
+          ? image_->bounds().right()
+          : label_->bounds().right();
+  separator_view_->SetBounds(separator_x, separator_bounds.y(), separator_width,
+                             separator_height);
 
   gfx::Rect ink_drop_bounds = GetLocalBounds();
   if (ShouldShowSeparator()) {
@@ -382,7 +390,11 @@ SkColor IconLabelBubbleView::GetParentBackgroundColor() const {
 }
 
 gfx::Size IconLabelBubbleView::GetSizeForLabelWidth(int label_width) const {
-  gfx::Size size(GetNonLabelSize());
+  gfx::Size size(image_->GetPreferredSize());
+  size.Enlarge(
+      GetInsets().left() + GetPrefixedSeparatorWidth() + GetEndPadding(),
+      GetInsets().height());
+
   const bool shrinking = IsShrinking();
   // Animation continues for the last few pixels even after the label is not
   // visible in order to slide the icon into its final position. Therefore it
@@ -395,8 +407,7 @@ gfx::Size IconLabelBubbleView::GetSizeForLabelWidth(int label_width) const {
     // enough to show the icon. We don't want to shrink all the way back to
     // zero, since this would mean the view would completely disappear and then
     // pop back to an icon after the animation finishes.
-    const int max_width = size.width() + GetInternalSpacing() + label_width +
-                          GetPrefixedSeparatorWidth();
+    const int max_width = size.width() + GetInternalSpacing() + label_width;
     const int current_width = WidthMultiplier() * max_width;
     size.set_width(shrinking ? std::max(current_width, size.width())
                              : current_width);
@@ -404,15 +415,6 @@ gfx::Size IconLabelBubbleView::GetSizeForLabelWidth(int label_width) const {
   return size;
 }
 
-gfx::Size IconLabelBubbleView::GetMaxSizeForLabelWidth(int label_width) const {
-  gfx::Size size(GetNonLabelSize());
-  if (ShouldShowLabel() || IsShrinking()) {
-    size.Enlarge(
-        GetInternalSpacing() + label_width + GetPrefixedSeparatorWidth(), 0);
-  }
-  return size;
-}
-
 int IconLabelBubbleView::GetInternalSpacing() const {
   if (image_->GetPreferredSize().IsEmpty())
     return 0;
@@ -426,7 +428,9 @@ int IconLabelBubbleView::GetInternalSpacing() const {
 }
 
 int IconLabelBubbleView::GetPrefixedSeparatorWidth() const {
-  return ShouldShowSeparator() ? kSeparatorWidth + kSpaceBesideSeparator : 0;
+  return ShouldShowSeparator() || ShouldShowExtraSpace()
+             ? kSeparatorWidth + kSpaceBesideSeparator
+             : 0;
 }
 
 int IconLabelBubbleView::GetEndPadding() const {
@@ -435,12 +439,6 @@ int IconLabelBubbleView::GetEndPadding() const {
   return GetInsets().right();
 }
 
-gfx::Size IconLabelBubbleView::GetNonLabelSize() const {
-  gfx::Size size(image_->GetPreferredSize());
-  size.Enlarge(GetInsets().left() + GetEndPadding(), GetInsets().height());
-  return size;
-}
-
 bool IconLabelBubbleView::OnActivate(const ui::Event& event) {
   if (ShowBubble(event)) {
     AnimateInkDrop(views::InkDropState::ACTIVATED, nullptr);

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

@@ -107,6 +107,12 @@ class IconLabelBubbleView : public views::InkDropObserver,
   // Returns true when the separator should be visible.
   virtual bool ShouldShowSeparator() const;
 
+  // Returns true when additional padding equal to GetPrefixedSeparatorWidth()
+  // should be added to the end of the view. This is useful in the case where
+  // it's required to layout subsequent views in the same position regardless
+  // of whether the separator is shown or not.
+  virtual bool ShouldShowExtraSpace() const;
+
   // Returns a multiplier used to calculate the actual width of the view based
   // on its desired width.  This ranges from 0 for a zero-width view to 1 for a
   // full-width view and can be used to animate the width of the view.
@@ -153,16 +159,7 @@ class IconLabelBubbleView : public views::InkDropObserver,
 
   gfx::Size GetSizeForLabelWidth(int label_width) const;
 
-  // Returns the maximum size for the label width. The value ignores
-  // WidthMultiplier().
-  gfx::Size GetMaxSizeForLabelWidth(int label_width) const;
-
  private:
-  // Amount of padding from the leading edge of the view to the leading edge of
-  // the image, and from the trailing edge of the label (or image, if the label
-  // is invisible) to the trailing edge of the view.
-  int GetOuterPadding() const;
-
   // Spacing between the image and the label.
   int GetInternalSpacing() const;
 
@@ -173,9 +170,6 @@ class IconLabelBubbleView : public views::InkDropObserver,
   // Padding after the separator.
   int GetEndPadding() const;
 
-  // Gets the minimum size to use when the label is not shown.
-  gfx::Size GetNonLabelSize() const;
-
   // The view has been activated by a user gesture such as spacebar.
   // Returns true if some handling was performed.
   bool OnActivate(const ui::Event& event);

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

@@ -469,16 +469,10 @@ void LocationBarView::Layout() {
   keyword_hint_view_->SetVisible(false);
 
   const int item_padding = GetLayoutConstant(LOCATION_BAR_ELEMENT_PADDING);
-
-  constexpr int kTextIndentDp = 12;
-  int leading_edit_item_padding =
-      ui::MaterialDesignController::IsRefreshUi()
-          ? GetOmniboxPopupView()->IsOpen() ? kTextIndentDp : 0
-          : item_padding;
   // We always subtract the left padding of the OmniboxView itself to allow for
   // an extended I-beam click target without affecting actual layout.
-  leading_edit_item_padding -= omnibox_view_->GetInsets().left();
-
+  const int leading_edit_item_padding =
+      item_padding - omnibox_view_->GetInsets().left();
   LocationBarLayout leading_decorations(
       LocationBarLayout::LEFT_EDGE, item_padding, leading_edit_item_padding);
   LocationBarLayout trailing_decorations(LocationBarLayout::RIGHT_EDGE,
@@ -1235,11 +1229,6 @@ void LocationBarView::OnPopupVisibilityChanged() {
   // The focus ring may be hidden or shown when the popup visibility changes.
   if (focus_ring_)
     focus_ring_->SchedulePaint();
-
-  if (ui::MaterialDesignController::IsRefreshUi()) {
-    Layout();
-    SchedulePaint();
-  }
 }
 
 const ToolbarModel* LocationBarView::GetToolbarModel() const {

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

@@ -15,6 +15,7 @@
 #include "content/public/browser/web_contents.h"
 #include "ui/accessibility/ax_node_data.h"
 #include "ui/base/l10n/l10n_util.h"
+#include "ui/base/material_design/material_design_controller.h"
 #include "ui/views/controls/label.h"
 
 using content::WebContents;
@@ -71,6 +72,17 @@ SkColor LocationIconView::GetTextColor() const {
       location_bar_->GetToolbarModel()->GetSecurityLevel(false));
 }
 
+bool LocationIconView::ShouldShowSeparator() const {
+  return ShouldShowLabel() ||
+         (ui::MaterialDesignController::IsRefreshUi() &&
+          !location_bar_->GetOmniboxView()->IsEditingOrEmpty());
+}
+
+bool LocationIconView::ShouldShowExtraSpace() const {
+  return ui::MaterialDesignController::IsRefreshUi() &&
+         location_bar_->GetOmniboxView()->IsEditingOrEmpty();
+}
+
 bool LocationIconView::ShowBubble(const ui::Event& event) {
   auto* contents = location_bar_->GetWebContents();
   if (!contents)

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

@@ -29,6 +29,8 @@ class LocationIconView : public IconLabelBubbleView {
   bool GetTooltipText(const gfx::Point& p,
                       base::string16* tooltip) const override;
   SkColor GetTextColor() const override;
+  bool ShouldShowSeparator() const override;
+  bool ShouldShowExtraSpace() const override;
   bool ShowBubble(const ui::Event& event) override;
   void GetAccessibleNodeData(ui::AXNodeData* node_data) override;
   bool IsBubbleShowing() const override;

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

@@ -36,7 +36,7 @@ static constexpr int kRefreshMarginLeft = 4;
 
 // In the MD refresh or rich suggestions, x-offset of the content and
 // description text.
-static constexpr int kTextIndent = 48;
+static constexpr int kTextIndent = 47;
 
 // TODO(dschuyler): Perhaps this should be based on the font size
 // instead of hardcoded to 2 dp (e.g. by adding a space in an