[Views] Fix omnibox separator issue

Currently, the separator in an omnibox bubble view
disappears when the ink drop animates in and then
reappears when it animates out. However, it's
possible for the ink drop to be removed without
animation. This causes the separator to go missing
since its opacity has not be updated.

This CL fix this issue by updating the separator's
opacity when the ink drop layer is removed.

Bug: 772832
Change-Id: I812d8dcacd79851bf23b0dce2d7fd00330887278
Reviewed-on: https://chromium-review.googlesource.com/713398Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Sarah Chan <spqchan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509218}

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

@@ -81,11 +81,16 @@ void IconLabelBubbleView::SeparatorView::UpdateOpacity() {
     SetPaintToLayer();
   layer()->SetFillsBoundsOpaquely(false);
 
-  ui::ScopedLayerAnimationSettings animation(layer()->GetAnimator());
-  animation.SetTransitionDuration(base::TimeDelta::FromMilliseconds(duration));
-  animation.SetTweenType(gfx::Tween::Type::EASE_IN);
-  animation.AddObserver(this);
-  layer()->SetOpacity(opacity);
+  if (disable_animation_for_test_) {
+    layer()->SetOpacity(opacity);
+  } else {
+    ui::ScopedLayerAnimationSettings animation(layer()->GetAnimator());
+    animation.SetTransitionDuration(
+        base::TimeDelta::FromMilliseconds(duration));
+    animation.SetTweenType(gfx::Tween::Type::EASE_IN);
+    animation.AddObserver(this);
+    layer()->SetOpacity(opacity);
+  }
 }
 
 //////////////////////////////////////////////////////////////////
@@ -249,6 +254,7 @@ void IconLabelBubbleView::AddInkDropLayer(ui::Layer* ink_drop_layer) {
 
 void IconLabelBubbleView::RemoveInkDropLayer(ui::Layer* ink_drop_layer) {
   ink_drop_container_->RemoveInkDropLayer(ink_drop_layer);
+  separator_view_->UpdateOpacity();
 }
 
 std::unique_ptr<views::InkDrop> IconLabelBubbleView::CreateInkDrop() {

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

@@ -37,6 +37,34 @@ class IconLabelBubbleView : public views::InkDropObserver,
  public:
   static constexpr int kTrailingPaddingPreMd = 2;
 
+  // A view that draws the separator.
+  class SeparatorView : public views::View,
+                        public ui::ImplicitAnimationObserver {
+   public:
+    explicit SeparatorView(IconLabelBubbleView* owner);
+
+    // views::View:
+    void OnPaint(gfx::Canvas* canvas) override;
+
+    // ui::ImplicitAnimationObserver:
+    void OnImplicitAnimationsCompleted() override;
+
+    // Updates the opacity based on the ink drop's state.
+    void UpdateOpacity();
+
+    void set_disable_animation_for_test(bool disable_animation_for_test) {
+      disable_animation_for_test_ = disable_animation_for_test;
+    }
+
+   private:
+    // Weak.
+    IconLabelBubbleView* owner_;
+
+    bool disable_animation_for_test_;
+
+    DISALLOW_COPY_AND_ASSIGN(SeparatorView);
+  };
+
   IconLabelBubbleView(const gfx::FontList& font_list, bool elide_in_middle);
   ~IconLabelBubbleView() override;
 
@@ -50,6 +78,9 @@ class IconLabelBubbleView : public views::InkDropObserver,
   const views::ImageView* GetImageView() const { return image_; }
   views::ImageView* GetImageView() { return image_; }
 
+  // Exposed for testing.
+  SeparatorView* separator_view() const { return separator_view_; }
+
  protected:
   static constexpr int kOpenTimeMS = 150;
 
@@ -121,28 +152,6 @@ class IconLabelBubbleView : public views::InkDropObserver,
   gfx::Size GetMaxSizeForLabelWidth(int label_width) const;
 
  private:
-  // A view that draws the separator.
-  class SeparatorView : public views::View,
-                        public ui::ImplicitAnimationObserver {
-   public:
-    explicit SeparatorView(IconLabelBubbleView* owner);
-
-    // views::View:
-    void OnPaint(gfx::Canvas* canvas) override;
-
-    // ui::ImplicitAnimationObserver:
-    void OnImplicitAnimationsCompleted() override;
-
-    // Updates the opacity based on the ink drop's state.
-    void UpdateOpacity();
-
-   private:
-    // Weak.
-    IconLabelBubbleView* owner_;
-
-    DISALLOW_COPY_AND_ASSIGN(SeparatorView);
-  };
-
   // 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.

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

@@ -46,6 +46,7 @@ class TestIconLabelBubbleView : public IconLabelBubbleView {
         is_bubble_showing_(false) {
     GetImageView()->SetImageSize(gfx::Size(kImageSize, kImageSize));
     SetLabel(base::ASCIIToUTF16("Label"));
+    separator_view()->set_disable_animation_for_test(true);
   }
 
   void SetCurrentAnimationValue(int value) {
@@ -327,6 +328,32 @@ TEST_F(IconLabelBubbleViewTest, MouseInkDropState) {
             ink_drop()->GetTargetInkDropState());
 }
 
+// Tests the separator opacity. The separator should disappear when there's
+// an ink drop. Otherwise, it should be visible.
+TEST_F(IconLabelBubbleViewTest, SeparatorOpacity) {
+  views::View* separator_view = view()->separator_view();
+  separator_view->SetPaintToLayer();
+  view()->SetLabel(base::ASCIIToUTF16("x"));
+  EXPECT_EQ(1.0f, separator_view->layer()->opacity());
+
+  AttachInkDrop();
+  generator()->PressLeftButton();
+  view()->InkDropAnimationStarted();
+  EXPECT_EQ(views::InkDropState::ACTION_PENDING,
+            ink_drop()->GetTargetInkDropState());
+  EXPECT_EQ(0.0f, separator_view->layer()->opacity());
+
+  generator()->ReleaseLeftButton();
+  EXPECT_EQ(views::InkDropState::ACTIVATED,
+            ink_drop()->GetTargetInkDropState());
+  EXPECT_EQ(0.0f, separator_view->layer()->opacity());
+
+  view()->HideBubble();
+  view()->InkDropAnimationStarted();
+  EXPECT_EQ(views::InkDropState::HIDDEN, ink_drop()->GetTargetInkDropState());
+  EXPECT_EQ(1.0f, separator_view->layer()->opacity());
+}
+
 #if !defined(OS_MACOSX)
 TEST_F(IconLabelBubbleViewTest, GestureInkDropState) {
   AttachInkDrop();