Revert "Move SeparatorView into its own file"

This reverts commit 172df5f3.

Reason for revert: <INSERT REASONING HERE>

Original change's description:
> Move SeparatorView into its own file
> 
> SeparatorView paints a separator for IconLabelBubbleView in certain
> cases. This moves it into its own file and loosens the coupling
> between the two, while trying not to change any behavior (yet).
> 
> This is in preparation for moving the separator out of
> IconLabelBubbleView entirely.
> 
> Bug: 946293
> Change-Id: Ia3b7c9c51c781dc75a7bcb33064d8cf0dc8c44ea
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1540682
> Commit-Queue: Collin Baker <collinbaker@chromium.org>
> Auto-Submit: Collin Baker <collinbaker@chromium.org>
> Reviewed-by: Bret Sepulveda <bsep@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#645150}

TBR=bsep@chromium.org,collinbaker@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 946293
Change-Id: I874381204cd63d11b63c740a84556b3656d9f3b0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1553934Reviewed-by: Collin Baker <collinbaker@chromium.org>
Commit-Queue: Collin Baker <collinbaker@chromium.org>
Cr-Commit-Position: refs/heads/master@{#648301}

chrome/browser/ui/BUILD.gn

@@ -2633,8 +2633,6 @@ jumbo_split_static_library("ui") {
       "views/location_bar/location_bar_bubble_delegate_view.h",
       "views/location_bar/location_bar_layout.cc",
       "views/location_bar/location_bar_layout.h",
-      "views/location_bar/location_bar_separator_view.cc",
-      "views/location_bar/location_bar_separator_view.h",
       "views/location_bar/location_bar_view.cc",
       "views/location_bar/location_bar_view.h",
       "views/location_bar/location_icon_view.cc",

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

@@ -8,10 +8,8 @@
 #include <memory>
 #include <utility>
 
-#include "base/time/time.h"
 #include "chrome/browser/ui/layout_constants.h"
 #include "chrome/browser/ui/omnibox/omnibox_theme.h"
-#include "chrome/browser/ui/views/location_bar/location_bar_separator_view.h"
 #include "ui/accessibility/ax_node_data.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/material_design/material_design_controller.h"
@@ -46,6 +44,10 @@ constexpr int kIconLabelBubbleSeparatorWidth = 1;
 // or label.
 constexpr int kIconLabelBubbleSpaceBesideSeparator = 8;
 
+// The length of the separator's fade animation. These values are empirical.
+constexpr int kIconLabelBubbleFadeInDurationMs = 250;
+constexpr int kIconLabelBubbleFadeOutDurationMs = 175;
+
 // The type of tweening for the animation.
 const gfx::Tween::Type kIconLabelBubbleTweenType = gfx::Tween::EASE_IN_OUT;
 
@@ -56,14 +58,77 @@ constexpr int kIconLabelBubbleAnimationDurationMs = 3000;
 const double kIconLabelBubbleOpenTimeFraction = 0.2;
 }  // namespace
 
+//////////////////////////////////////////////////////////////////
+// SeparatorView class
+
+IconLabelBubbleView::SeparatorView::SeparatorView(IconLabelBubbleView* owner) {
+  DCHECK(owner);
+  owner_ = owner;
+
+  SetPaintToLayer();
+  layer()->SetFillsBoundsOpaquely(false);
+}
+
+void IconLabelBubbleView::SeparatorView::OnPaint(gfx::Canvas* canvas) {
+  const SkColor background_color = owner_->GetParentBackgroundColor();
+  const SkColor separator_color =
+      SkColorSetA(color_utils::GetColorWithMaxContrast(background_color), 0x69);
+  const float x = GetLocalBounds().right() -
+                  owner_->GetEndPaddingWithSeparator() -
+                  1.0f / canvas->image_scale();
+  canvas->DrawLine(gfx::PointF(x, GetLocalBounds().y()),
+                   gfx::PointF(x, GetLocalBounds().bottom()), separator_color);
+}
+
+void IconLabelBubbleView::SeparatorView::UpdateOpacity() {
+  if (!visible())
+    return;
+
+  // When using focus rings are visible we should hide the separator instantly
+  // when the IconLabelBubbleView is focused. Otherwise we should follow the
+  // inkdrop.
+  if (owner_->focus_ring() && owner_->HasFocus()) {
+    layer()->SetOpacity(0.0f);
+    return;
+  }
+
+  views::InkDrop* ink_drop = owner_->GetInkDrop();
+  DCHECK(ink_drop);
+
+  // If an inkdrop highlight or ripple is animating in or visible, the
+  // separator should fade out.
+  views::InkDropState state = ink_drop->GetTargetInkDropState();
+  float opacity = 0.0f;
+  float duration = kIconLabelBubbleFadeOutDurationMs;
+  if (!ink_drop->IsHighlightFadingInOrVisible() &&
+      (state == views::InkDropState::HIDDEN ||
+       state == views::InkDropState::ACTION_TRIGGERED ||
+       state == views::InkDropState::DEACTIVATED)) {
+    opacity = 1.0f;
+    duration = kIconLabelBubbleFadeInDurationMs;
+  }
+
+  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);
+    layer()->SetOpacity(opacity);
+  }
+}
+
+//////////////////////////////////////////////////////////////////
+// IconLabelBubbleView class
+
 IconLabelBubbleView::IconLabelBubbleView(const gfx::FontList& font_list)
     : LabelButton(nullptr, base::string16()),
-      separator_view_(new LocationBarSeparatorView()) {
+      separator_view_(new SeparatorView(this)) {
   SetFontList(font_list);
   SetHorizontalAlignment(gfx::ALIGN_LEFT);
 
-  separator_view_->SetBackgroundColor(GetParentBackgroundColor());
-  UpdateSeparator();
+  separator_view_->SetVisible(ShouldShowSeparator());
   AddChildView(separator_view_);
 
   set_ink_drop_visible_opacity(
@@ -84,7 +149,7 @@ IconLabelBubbleView::IconLabelBubbleView(const gfx::FontList& font_list)
 IconLabelBubbleView::~IconLabelBubbleView() {}
 
 void IconLabelBubbleView::InkDropAnimationStarted() {
-  UpdateSeparator();
+  separator_view_->UpdateOpacity();
 }
 
 void IconLabelBubbleView::InkDropRippleAnimationEnded(
@@ -99,7 +164,8 @@ bool IconLabelBubbleView::ShouldShowLabel() const {
 void IconLabelBubbleView::SetLabel(const base::string16& label_text) {
   SetAccessibleName(label_text);
   label()->SetText(label_text);
-  UpdateSeparator();
+  separator_view_->SetVisible(ShouldShowSeparator());
+  separator_view_->UpdateOpacity();
 }
 
 void IconLabelBubbleView::SetImage(const gfx::ImageSkia& image_skia) {
@@ -188,17 +254,17 @@ void IconLabelBubbleView::Layout() {
                                     GetWidthBetweenIconAndSeparator());
   label()->SetBounds(label_x, 0, label_width, height());
 
-  const int icon_end = label()->text().empty() ? image()->bounds().right()
-                                               : label()->bounds().right();
-
   // The separator should be the same height as the icons.
-  const gfx::Size separator_size(kIconLabelBubbleSeparatorWidth,
-                                 GetLayoutConstant(LOCATION_BAR_ICON_SIZE));
-  const int separator_x = icon_end + GetWidthBetweenIconAndSeparator();
-  const int separator_y = (height() - separator_size.height()) / 2;
-  const gfx::Rect separator_bounds(gfx::Point(separator_x, separator_y),
-                                   separator_size);
-  separator_view_->SetBoundsRect(separator_bounds);
+  const int separator_height = GetLayoutConstant(LOCATION_BAR_ICON_SIZE);
+  gfx::Rect separator_bounds(label()->bounds());
+  separator_bounds.Inset(0, (separator_bounds.height() - separator_height) / 2);
+
+  float separator_width =
+      GetWidthBetweenIconAndSeparator() + GetEndPaddingWithSeparator();
+  int separator_x = label()->text().empty() ? image()->bounds().right()
+                                            : label()->bounds().right();
+  separator_view_->SetBounds(separator_x, separator_bounds.y(), separator_width,
+                             separator_height);
 
   UpdateHighlightPath();
 }
@@ -213,7 +279,6 @@ void IconLabelBubbleView::OnNativeThemeChanged(
   LabelButton::OnNativeThemeChanged(native_theme);
   SetEnabledTextColors(GetTextColor());
   label()->SetBackgroundColor(GetParentBackgroundColor());
-  separator_view_->SetBackgroundColor(GetParentBackgroundColor());
   SchedulePaint();
 }
 
@@ -244,12 +309,12 @@ void IconLabelBubbleView::NotifyClick(const ui::Event& event) {
 }
 
 void IconLabelBubbleView::OnFocus() {
-  UpdateSeparator();
+  separator_view_->UpdateOpacity();
   LabelButton::OnFocus();
 }
 
 void IconLabelBubbleView::OnBlur() {
-  UpdateSeparator();
+  separator_view_->UpdateOpacity();
   LabelButton::OnBlur();
 }
 
@@ -445,34 +510,3 @@ void IconLabelBubbleView::UpdateHighlightPath() {
     focus_ring()->SchedulePaint();
   }
 }
-
-void IconLabelBubbleView::UpdateSeparator() {
-  if (!ShouldShowSeparator()) {
-    separator_view_->SetVisible(false);
-    return;
-  }
-
-  separator_view_->SetVisible(true);
-
-  // When using focus rings, we should hide the separator instantly upon
-  // focus. Otherwise we should follow the inkdrop.
-  if (focus_ring() && HasFocus()) {
-    separator_view_->Hide();
-    return;
-  }
-
-  views::InkDrop* const ink_drop = GetInkDrop();
-  DCHECK(ink_drop);
-
-  // If an inkdrop highlight or ripple is animating in or visible, the
-  // separator should fade out.
-  const views::InkDropState state = ink_drop->GetTargetInkDropState();
-  if (!ink_drop->IsHighlightFadingInOrVisible() &&
-      (state == views::InkDropState::HIDDEN ||
-       state == views::InkDropState::ACTION_TRIGGERED ||
-       state == views::InkDropState::DEACTIVATED)) {
-    separator_view_->FadeIn();
-  } else {
-    separator_view_->FadeOut();
-  }
-}

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

@@ -35,8 +35,6 @@ namespace views {
 class ImageView;
 }
 
-class LocationBarSeparatorView;
-
 // View used to draw a bubble, containing an icon and a label. We use this as a
 // base for the classes that handle the location icon (including the EV bubble),
 // tab-to-search UI, and content settings.
@@ -46,6 +44,30 @@ class IconLabelBubbleView : public views::InkDropObserver,
  public:
   static constexpr int kTrailingPaddingPreMd = 2;
 
+  // A view that draws the separator.
+  class SeparatorView : public views::View {
+   public:
+    explicit SeparatorView(IconLabelBubbleView* owner);
+
+    // views::View:
+    void OnPaint(gfx::Canvas* canvas) 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_ = false;
+
+    DISALLOW_COPY_AND_ASSIGN(SeparatorView);
+  };
+
   explicit IconLabelBubbleView(const gfx::FontList& font_list);
   ~IconLabelBubbleView() override;
 
@@ -67,7 +89,7 @@ class IconLabelBubbleView : public views::InkDropObserver,
   SkColor GetParentBackgroundColor() const;
 
   // Exposed for testing.
-  LocationBarSeparatorView* separator_view() const { return separator_view_; }
+  SeparatorView* separator_view() const { return separator_view_; }
 
   // Exposed for testing.
   bool is_animating_label() const { return slide_animation_.is_animating(); }
@@ -199,10 +221,8 @@ class IconLabelBubbleView : public views::InkDropObserver,
   // bounds and the separator visibility.
   void UpdateHighlightPath();
 
-  void UpdateSeparator();
-
   // The contents of the bubble.
-  LocationBarSeparatorView* separator_view_;
+  SeparatorView* separator_view_;
 
   // The padding of the element that will be displayed after |this|. This value
   // is relevant for calculating the amount of space to reserve after the

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

@@ -7,7 +7,6 @@
 #include "base/optional.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/ui/layout_constants.h"
-#include "chrome/browser/ui/views/location_bar/location_bar_separator_view.h"
 #include "chrome/browser/ui/views/location_bar/location_bar_view.h"
 #include "chrome/test/views/chrome_views_test_base.h"
 #include "components/strings/grit/components_strings.h"

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

-// Copyright 2019 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "chrome/browser/ui/views/location_bar/location_bar_separator_view.h"
-
-#include "ui/compositor/layer.h"
-#include "ui/compositor/layer_animator.h"
-#include "ui/compositor/scoped_layer_animation_settings.h"
-#include "ui/gfx/animation/tween.h"
-#include "ui/gfx/canvas.h"
-#include "ui/gfx/color_utils.h"
-#include "ui/gfx/geometry/point_f.h"
-
-namespace {
-
-// These values are empirical.
-constexpr base::TimeDelta kLocationBarSeparatorFadeInDuration =
-    base::TimeDelta::FromMilliseconds(250);
-constexpr base::TimeDelta kLocationBarSeparatorFadeOutDuration =
-    base::TimeDelta::FromMilliseconds(175);
-
-}  //  namespace
-
-LocationBarSeparatorView::LocationBarSeparatorView() {
-  SetPaintToLayer();
-  layer()->SetFillsBoundsOpaquely(false);
-}
-
-void LocationBarSeparatorView::OnPaint(gfx::Canvas* canvas) {
-  DCHECK_NE(gfx::kPlaceholderColor, background_color_);
-  const SkColor separator_color = SkColorSetA(
-      color_utils::GetColorWithMaxContrast(background_color_), 0x69);
-  const float x = 1.f - 1.f / canvas->image_scale();
-  canvas->DrawLine(gfx::PointF(x, GetLocalBounds().y()),
-                   gfx::PointF(x, GetLocalBounds().bottom()), separator_color);
-}
-
-void LocationBarSeparatorView::Show() {
-  layer()->SetOpacity(0.0f);
-}
-
-void LocationBarSeparatorView::Hide() {
-  layer()->SetOpacity(1.0f);
-}
-
-void LocationBarSeparatorView::FadeIn() {
-  FadeTo(1.0f, kLocationBarSeparatorFadeInDuration);
-}
-
-void LocationBarSeparatorView::FadeOut() {
-  FadeTo(0.0f, kLocationBarSeparatorFadeOutDuration);
-}
-
-void LocationBarSeparatorView::FadeTo(float opacity, base::TimeDelta duration) {
-  if (disable_animation_for_test_) {
-    layer()->SetOpacity(opacity);
-  } else {
-    ui::ScopedLayerAnimationSettings animation(layer()->GetAnimator());
-    animation.SetTransitionDuration(duration);
-    animation.SetTweenType(gfx::Tween::Type::EASE_IN);
-    layer()->SetOpacity(opacity);
-  }
-}

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

-// Copyright 2019 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#ifndef CHROME_BROWSER_UI_VIEWS_LOCATION_BAR_LOCATION_BAR_SEPARATOR_VIEW_H_
-#define CHROME_BROWSER_UI_VIEWS_LOCATION_BAR_LOCATION_BAR_SEPARATOR_VIEW_H_
-
-#include "base/time/time.h"
-#include "third_party/skia/include/core/SkColor.h"
-#include "ui/gfx/color_palette.h"
-#include "ui/views/view.h"
-
-namespace gfx {
-class Canvas;
-}
-
-class LocationBarSeparatorView : public views::View {
- public:
-  LocationBarSeparatorView();
-
-  void SetBackgroundColor(SkColor color) { background_color_ = color; }
-
-  // Show or hide immediately.
-  void Show();
-  void Hide();
-
-  // Animate in or out.
-  void FadeIn();
-  void FadeOut();
-
-  // views::View:
-  void OnPaint(gfx::Canvas* canvas) override;
-
-  void set_disable_animation_for_test(bool disable_animation_for_test) {
-    disable_animation_for_test_ = disable_animation_for_test;
-  }
-
- private:
-  void FadeTo(float opacity, base::TimeDelta duration);
-
-  SkColor background_color_ = gfx::kPlaceholderColor;
-  bool disable_animation_for_test_ = false;
-
-  DISALLOW_COPY_AND_ASSIGN(LocationBarSeparatorView);
-};
-
-#endif  // CHROME_BROWSER_UI_VIEWS_LOCATION_BAR_LOCATION_BAR_SEPARATOR_VIEW_H_