[omnibox] Don't indent the text if the location icon text is showing.

The point of the text indent is to align with the suggestions in the
dropdown. But if the location icon has text, it won't align anyway so
the indent is not useful and looks odd.

Bug: 859543

Change-Id: I355c81016a2691bcc237b108eec80318dccf1b50
Reviewed-on: https://chromium-review.googlesource.com/1159229Reviewed-by: Tommy Li <tommycli@chromium.org>
Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Commit-Queue: Justin Donnelly <jdonnelly@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580571}

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

@@ -160,6 +160,10 @@ void IconLabelBubbleView::InkDropAnimationStarted() {
 void IconLabelBubbleView::InkDropRippleAnimationEnded(
     views::InkDropState state) {}
 
+bool IconLabelBubbleView::ShouldShowLabel() const {
+  return label_->visible() && !label_->text().empty();
+}
+
 void IconLabelBubbleView::SetLabel(const base::string16& label) {
   label_->SetText(label);
   separator_view_->SetVisible(ShouldShowSeparator());
@@ -173,10 +177,6 @@ void IconLabelBubbleView::OnBubbleCreated(views::Widget* bubble_widget) {
   bubble_widget->AddObserver(this);
 }
 
-bool IconLabelBubbleView::ShouldShowLabel() const {
-  return label_->visible() && !label_->text().empty();
-}
-
 bool IconLabelBubbleView::ShouldShowSeparator() const {
   return ShouldShowLabel();
 }

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

@@ -67,6 +67,9 @@ class IconLabelBubbleView : public views::InkDropObserver,
   void InkDropAnimationStarted() override;
   void InkDropRippleAnimationEnded(views::InkDropState state) override;
 
+  // Returns true when the label should be visible.
+  virtual bool ShouldShowLabel() const;
+
   void SetLabel(const base::string16& label);
   void SetImage(const gfx::ImageSkia& image);
 
@@ -96,9 +99,6 @@ class IconLabelBubbleView : public views::InkDropObserver,
   // Gets the color for displaying text.
   virtual SkColor GetTextColor() const = 0;
 
-  // Returns true when the label should be visible.
-  virtual bool ShouldShowLabel() const;
-
   // Returns true when the separator should be visible.
   virtual bool ShouldShowSeparator() const;
 

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

@@ -475,13 +475,26 @@ void LocationBarView::Layout() {
 
   const int item_padding = GetLayoutConstant(LOCATION_BAR_ELEMENT_PADDING);
 
+  int leading_edit_item_padding = item_padding;
+  if (OmniboxFieldTrial::IsJogTextfieldOnPopupEnabled()) {
+    // With jog enabled, the text should be indented only when the popup is open
+    // and the location icon view does *not* have a label. In most cases, we
+    // only care that the popup is open, in which case we indent to align with
+    // the text in the popup. But if there's text in the location icon view
+    // (which can happen with zero suggest, which continues to show security or
+    // EV cert text at the same time as the popup is open), the text in the
+    // omnibox can't align with the text of the suggestions, so the indent just
+    // moves the text for no apparent reason.
+
+    // TODO(jdonnelly): The better solution may be to remove the location icon
+    // text when zero suggest triggers.
+    const bool should_indent = GetOmniboxPopupView()->IsOpen() &&
+                               !location_icon_view_->ShouldShowLabel();
     // We have an odd indent value because this is what matches the odd text
     // indent value in OmniboxMatchCellView.
     constexpr int kTextJogIndentDp = 11;
-  int leading_edit_item_padding =
-      OmniboxFieldTrial::IsJogTextfieldOnPopupEnabled()
-          ? GetOmniboxPopupView()->IsOpen() ? kTextJogIndentDp : 0
-          : item_padding;
+    leading_edit_item_padding = should_indent ? kTextJogIndentDp : 0;
+  }
 
   // We always subtract the left padding of the OmniboxView itself to allow for
   // an extended I-beam click target without affecting actual layout.