Add a shadow to the password generation drop-down.

The CL moves as much as possible from AutofillPopupViewNativeViews to
AutofillPopupBaseView (parent for the generation drop-down). At the same time
AutofillPopupViewViews implements the stuff specific for it and irrelevant for
the new views prompts. AutofillPopupViewViews is gonna be deleted soon anyway.

Bug: 851021
Change-Id: I4711c29220c841356e5a7d8adca0cd5c222c8cde
Reviewed-on: https://chromium-review.googlesource.com/1136633Reviewed-by: Fabio Tirelo <ftirelo@chromium.org>
Reviewed-by: Tommy Martino <tmartino@chromium.org>
Reviewed-by: Evan Stade <estade@chromium.org>
Commit-Queue: Vasilii Sukhanov <vasilii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575698}

chrome/browser/ui/passwords/password_generation_popup_controller_impl.cc

@@ -169,9 +169,17 @@ int PasswordGenerationPopupControllerImpl::GetMinimumWidth() {
 void PasswordGenerationPopupControllerImpl::CalculateBounds() {
   gfx::Size bounds = view_->GetPreferredSizeOfPasswordView();
 
+  gfx::Rect new_element_bounds = gfx::ToEnclosingRect(element_bounds());
+  // Consider the element is |kElementBorderPadding| pixels larger at the top
+  // and at the bottom in order to reposition the dropdown, so that it doesn't
+  // look too close to the element.
+  constexpr int kElementBorderPadding = 1;
+  new_element_bounds.Inset(/*horizontal=*/0,
+                           /*vertical=*/-kElementBorderPadding);
+
   popup_bounds_ = view_common_.CalculatePopupBounds(
-      bounds.width(), bounds.height(), gfx::ToEnclosingRect(element_bounds()),
-      container_view(), IsRTL());
+      bounds.width(), bounds.height(), new_element_bounds, container_view(),
+      IsRTL());
 }
 
 void PasswordGenerationPopupControllerImpl::Show(GenerationState state) {

chrome/browser/ui/views/autofill/autofill_popup_base_view.cc

@@ -12,28 +12,22 @@
 #include "chrome/browser/ui/autofill/popup_view_common.h"
 #include "chrome/browser/ui/browser_finder.h"
 #include "chrome/browser/ui/browser_window.h"
+#include "chrome/browser/ui/views/harmony/chrome_layout_provider.h"
 #include "chrome/grit/generated_resources.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/ui_features.h"
 #include "ui/native_theme/native_theme.h"
 #include "ui/views/border.h"
-#include "ui/views/controls/scroll_view.h"
+#include "ui/views/bubble/bubble_border.h"
 #include "ui/views/focus/focus_manager.h"
+#include "ui/views/layout/fill_layout.h"
 
 namespace autofill {
 
-namespace {
-
-// The minimum vertical space between the bottom of the autofill popup and the
-// bottom of the Chrome frame.
-// TODO(crbug.com/739978): Investigate if we should compute this distance
-// programmatically. 10dp may not be enough for windows with thick borders.
-const int kPopupBottomMargin = 10;
-
-// The thickness of the border for the autofill popup in dp.
-const int kPopupBorderThicknessDp = 1;
-
-}  // namespace
+int AutofillPopupBaseView::GetCornerRadius() {
+  return ChromeLayoutProvider::Get()->GetCornerRadiusMetric(
+      views::EMPHASIS_MEDIUM);
+}
 
 AutofillPopupBaseView::AutofillPopupBaseView(
     AutofillPopupViewDelegate* delegate,
@@ -141,25 +135,42 @@ void AutofillPopupBaseView::RemoveObserver() {
   views::WidgetFocusManager::GetInstance()->RemoveFocusChangeListener(this);
 }
 
-void AutofillPopupBaseView::AdjustBoundsForBorder(gfx::Rect* bounds) const {
-  DCHECK(bounds);
-  bounds->set_height(bounds->height() + 2 * kPopupBorderThicknessDp);
-  bounds->set_width(bounds->width() + 2 * kPopupBorderThicknessDp);
+// TODO(crbug.com/831603): Inline this function once AutofillPopupViewViews is
+// deleted.
+void AutofillPopupBaseView::AddExtraInitParams(
+    views::Widget::InitParams* params) {
+  // Ensure the bubble border is not painted on an opaque background.
+  params->opacity = views::Widget::InitParams::TRANSLUCENT_WINDOW;
+  params->shadow_type = views::Widget::InitParams::SHADOW_TYPE_NONE;
 }
 
 std::unique_ptr<views::View> AutofillPopupBaseView::CreateWrapperView() {
-  auto wrapper_view = std::make_unique<views::ScrollView>();
-  scroll_view_ = wrapper_view.get();
-  scroll_view_->set_hide_horizontal_scrollbar(true);
-  scroll_view_->SetContents(this);
+  // Create a wrapper view that contains the current view and will receive the
+  // bubble border. This is needed so that a clipping path can be later applied
+  // on the contents only and not affect the border.
+  auto wrapper_view = std::make_unique<views::View>();
+  wrapper_view->SetLayoutManager(std::make_unique<views::FillLayout>());
+  wrapper_view->AddChildView(this);
   return wrapper_view;
 }
 
 std::unique_ptr<views::Border> AutofillPopupBaseView::CreateBorder() {
-  return views::CreateSolidBorder(
-      kPopupBorderThicknessDp,
-      GetNativeTheme()->GetSystemColor(
-          ui::NativeTheme::kColorId_UnfocusedBorderColor));
+  auto border = std::make_unique<views::BubbleBorder>(
+      views::BubbleBorder::NONE, views::BubbleBorder::SMALL_SHADOW,
+      SK_ColorWHITE);
+  border->SetCornerRadius(GetCornerRadius());
+  border->set_md_shadow_elevation(
+      ChromeLayoutProvider::Get()->GetShadowElevationMetric(
+          views::EMPHASIS_MEDIUM));
+  return border;
+}
+
+void AutofillPopupBaseView::SetClipPath() {
+  SkRect local_bounds = gfx::RectToSkRect(GetLocalBounds());
+  SkScalar radius = SkIntToScalar(GetCornerRadius());
+  gfx::Path clip_path;
+  clip_path.addRoundRect(local_bounds, radius, radius);
+  set_clip_path(clip_path);
 }
 
 void AutofillPopupBaseView::DoUpdateBoundsAndRedrawPopup() {
@@ -169,27 +180,27 @@ void AutofillPopupBaseView::DoUpdateBoundsAndRedrawPopup() {
 
   gfx::Rect clipping_bounds = CalculateClippingBounds();
 
-  int available_vertical_space = clipping_bounds.height() -
-                                 (bounds.y() - clipping_bounds.y()) -
-                                 kPopupBottomMargin;
-
-  if (available_vertical_space < bounds.height()) {
-    // The available space is not enough for the full popup so clamp the widget
-    // to what's available. Since the scroll view will show a scroll bar,
-    // increase the width so that the content isn't partially hidden.
-    const int extra_width =
-        scroll_view_ ? scroll_view_->GetScrollBarLayoutWidth() : 0;
-    bounds.set_width(bounds.width() + extra_width);
+  int available_vertical_space =
+      clipping_bounds.height() - (bounds.y() - clipping_bounds.y());
+
+  if (available_vertical_space < bounds.height())
     bounds.set_height(available_vertical_space);
-  }
 
   // Account for the scroll view's border so that the content has enough space.
-  AdjustBoundsForBorder(&bounds);
+  bounds.Inset(-GetWidget()->GetRootView()->border()->GetInsets());
   GetWidget()->SetBounds(bounds);
+  SetClipPath();
 
   SchedulePaint();
 }
 
+gfx::Rect AutofillPopupBaseView::CalculateClippingBounds() const {
+  if (parent_widget_)
+    return parent_widget_->GetClientAreaBoundsInScreen();
+
+  return PopupViewCommon().GetWindowBounds(delegate_->container_view());
+}
+
 void AutofillPopupBaseView::OnNativeFocusChanged(gfx::NativeView focused_now) {
   if (GetWidget() && GetWidget()->GetNativeView() != focused_now)
     HideController();
@@ -314,13 +325,6 @@ void AutofillPopupBaseView::HideController() {
   // timing of that deletion is tricky.
 }
 
-gfx::Rect AutofillPopupBaseView::CalculateClippingBounds() const {
-  if (parent_widget_)
-    return parent_widget_->GetClientAreaBoundsInScreen();
-
-  return PopupViewCommon().GetWindowBounds(delegate_->container_view());
-}
-
 gfx::NativeView AutofillPopupBaseView::container_view() {
   return delegate_->container_view();
 }

chrome/browser/ui/views/autofill/autofill_popup_base_view.h

@@ -30,6 +30,9 @@ namespace autofill {
 class AutofillPopupBaseView : public views::WidgetDelegateView,
                               public views::WidgetFocusChangeListener,
                               public views::WidgetObserver {
+ public:
+  static int GetCornerRadius();
+
  protected:
   explicit AutofillPopupBaseView(AutofillPopupViewDelegate* delegate,
                                  views::Widget* parent_widget);
@@ -41,10 +44,9 @@ class AutofillPopupBaseView : public views::WidgetDelegateView,
   // Hide the widget and delete |this|.
   void DoHide();
 
-  // Grows |bounds| to account for the border of the popup.
-  void AdjustBoundsForBorder(gfx::Rect* bounds) const;
-
-  virtual void AddExtraInitParams(views::Widget::InitParams* params) {}
+  // TODO(crbug.com/831603): make the methods private and non-virtual when
+  // AutofillPopupViewViews is gone.
+  virtual void AddExtraInitParams(views::Widget::InitParams* params);
 
   // Returns the widget's contents view.
   virtual std::unique_ptr<views::View> CreateWrapperView();
@@ -52,9 +54,17 @@ class AutofillPopupBaseView : public views::WidgetDelegateView,
   // Returns the border to be applied to the popup.
   virtual std::unique_ptr<views::Border> CreateBorder();
 
+  // Ensure the child views are not rendered beyond the bubble border
+  // boundaries. Should be overridden together with CreateBorder.
+  virtual void SetClipPath();
+
   // Update size of popup and paint (virtual for testing).
   virtual void DoUpdateBoundsAndRedrawPopup();
 
+  // Compute the space available for the popup. It's the space between its top
+  // and the bottom of its parent view, minus some margin space.
+  gfx::Rect CalculateClippingBounds() const;
+
   const AutofillPopupViewDelegate* delegate() { return delegate_; }
 
  private:
@@ -89,10 +99,6 @@ class AutofillPopupBaseView : public views::WidgetDelegateView,
   // eventually hide this view in the process.
   void HideController();
 
-  // Compute the space available for the popup. It's the space between its top
-  // and the bottom of its parent view, minus some margin space.
-  gfx::Rect CalculateClippingBounds() const;
-
   // Must return the container view for this popup.
   gfx::NativeView container_view();
 
@@ -102,8 +108,6 @@ class AutofillPopupBaseView : public views::WidgetDelegateView,
   // The widget of the window that triggered this popup. Weak reference.
   views::Widget* parent_widget_;
 
-  views::ScrollView* scroll_view_;
-
   // The time when the popup was shown.
   base::Time show_time_;
 

chrome/browser/ui/views/autofill/autofill_popup_base_view_browsertest.cc

@@ -17,6 +17,7 @@
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/events/event_utils.h"
+#include "ui/views/border.h"
 #include "ui/views/test/views_test_base.h"
 #include "ui/views/widget/widget.h"
 
@@ -156,7 +157,10 @@ IN_PROC_BROWSER_TEST_F(AutofillPopupBaseViewTest, CorrectBoundsTest) {
                                  ->GetWidget()
                                  ->GetClientAreaBoundsInScreen()
                                  .origin();
+  // The expected origin is shifted to accomodate the border of the bubble.
   gfx::Point expected_point = bounds.origin();
+  gfx::Insets border = view_->GetWidget()->GetRootView()->border()->GetInsets();
+  expected_point.Offset(-border.left(), -border.top());
   EXPECT_EQ(expected_point, display_point);
 }
 

chrome/browser/ui/views/autofill/autofill_popup_view_native_views.cc

@@ -59,11 +59,6 @@ const SkColor kAutofillPopupWarningColor = gfx::kGoogleRed600;
 // border doesn't look too close to the element.
 constexpr int kElementBorderPadding = 1;
 
-int GetCornerRadius() {
-  return ChromeLayoutProvider::Get()->GetCornerRadiusMetric(
-      views::EMPHASIS_MEDIUM);
-}
-
 int GetContentsVerticalPadding() {
   return ChromeLayoutProvider::Get()->GetDistanceMetric(
       DISTANCE_CONTENT_LIST_VERTICAL_MULTI);
@@ -71,7 +66,7 @@ int GetContentsVerticalPadding() {
 
 int GetHorizontalMargin() {
   return views::MenuConfig::instance().item_horizontal_padding +
-         GetCornerRadius();
+         autofill::AutofillPopupBaseView::GetCornerRadius();
 }
 
 }  // namespace
@@ -389,7 +384,7 @@ int AutofillPopupFooterView::GetPrimaryTextStyle() {
 AutofillPopupFooterView::AutofillPopupFooterView(
     AutofillPopupController* controller,
     int line_number)
-    : AutofillPopupItemView(controller, line_number, GetCornerRadius()) {
+    : AutofillPopupItemView(controller, line_number) {
   SetFocusBehavior(FocusBehavior::ALWAYS);
 }
 
@@ -459,7 +454,7 @@ void AutofillPopupWarningView::GetAccessibleNodeData(
 
 void AutofillPopupWarningView::CreateContent() {
   int horizontal_margin = GetHorizontalMargin();
-  int vertical_margin = GetCornerRadius();
+  int vertical_margin = AutofillPopupBaseView::GetCornerRadius();
 
   SetLayoutManager(std::make_unique<views::FillLayout>());
   SetBorder(views::CreateEmptyBorder(
@@ -692,35 +687,6 @@ int AutofillPopupViewNativeViews::AdjustWidth(int width) const {
   return width;
 }
 
-void AutofillPopupViewNativeViews::AddExtraInitParams(
-    views::Widget::InitParams* params) {
-  // Ensure the bubble border is not painted on an opaque background.
-  params->opacity = views::Widget::InitParams::TRANSLUCENT_WINDOW;
-  params->shadow_type = views::Widget::InitParams::SHADOW_TYPE_NONE;
-}
-
-std::unique_ptr<views::View> AutofillPopupViewNativeViews::CreateWrapperView() {
-  // Create a wrapper view that contains the current view and will receive the
-  // bubble border. This is needed so that a clipping path can be later applied
-  // on the contents only and not affect the border.
-  auto wrapper_view = std::make_unique<views::View>();
-  wrapper_view->SetLayoutManager(std::make_unique<views::FillLayout>());
-  wrapper_view->AddChildView(this);
-  return wrapper_view;
-}
-
-std::unique_ptr<views::Border> AutofillPopupViewNativeViews::CreateBorder() {
-  auto border = std::make_unique<views::BubbleBorder>(
-      views::BubbleBorder::NONE, views::BubbleBorder::SMALL_SHADOW,
-      SK_ColorWHITE);
-  border->SetCornerRadius(GetCornerRadius());
-  border->set_md_shadow_elevation(
-      ChromeLayoutProvider::Get()->GetShadowElevationMetric(
-          views::EMPHASIS_MEDIUM));
-  bubble_border_ = border.get();
-  return border;
-}
-
 void AutofillPopupViewNativeViews::DoUpdateBoundsAndRedrawPopup() {
   gfx::Size size = CalculatePreferredSize();
   gfx::Rect popup_bounds;
@@ -758,17 +724,9 @@ void AutofillPopupViewNativeViews::DoUpdateBoundsAndRedrawPopup() {
 
   SetSize(size);
 
-  popup_bounds.Inset(-bubble_border_->GetInsets());
-
+  popup_bounds.Inset(-GetWidget()->GetRootView()->border()->GetInsets());
   GetWidget()->SetBounds(popup_bounds);
-
-  // Ensure the child views are not rendered beyond the bubble border
-  // boundaries.
-  SkRect local_bounds = gfx::RectToSkRect(GetLocalBounds());
-  SkScalar radius = SkIntToScalar(bubble_border_->GetBorderCornerRadius());
-  gfx::Path clip_path;
-  clip_path.addRoundRect(local_bounds, radius, radius);
-  set_clip_path(clip_path);
+  SetClipPath();
 
   SchedulePaint();
 }

chrome/browser/ui/views/autofill/autofill_popup_view_native_views.h

@@ -12,7 +12,6 @@
 #include "ui/gfx/color_palette.h"
 #include "ui/gfx/font_list.h"
 #include "ui/gfx/path.h"
-#include "ui/views/bubble/bubble_border.h"
 
 #include <memory>
 #include <vector>
@@ -101,15 +100,11 @@ class AutofillPopupViewNativeViews : public AutofillPopupBaseView,
   int AdjustWidth(int width) const;
 
   // AutofillPopupBaseView:
-  void AddExtraInitParams(views::Widget::InitParams* params) override;
-  std::unique_ptr<views::View> CreateWrapperView() override;
-  std::unique_ptr<views::Border> CreateBorder() override;
   void DoUpdateBoundsAndRedrawPopup() override;
 
   // Controller for this view.
   AutofillPopupController* controller_;
   std::vector<AutofillPopupRowView*> rows_;
-  views::BubbleBorder* bubble_border_;
   views::BoxLayout* layout_;
   views::ScrollView* scroll_view_;
 

chrome/browser/ui/views/autofill/autofill_popup_view_views.cc

@@ -28,6 +28,7 @@
 #include "ui/gfx/text_utils.h"
 #include "ui/views/accessibility/view_accessibility.h"
 #include "ui/views/border.h"
+#include "ui/views/controls/scroll_view.h"
 #include "ui/views/view.h"
 #include "ui/views/widget/widget.h"
 
@@ -35,6 +36,15 @@ namespace autofill {
 
 namespace {
 
+// The minimum vertical space between the bottom of the autofill popup and the
+// bottom of the Chrome frame.
+// TODO(crbug.com/739978): Investigate if we should compute this distance
+// programmatically. 10dp may not be enough for windows with thick borders.
+const int kPopupBottomMargin = 10;
+
+// The thickness of the border for the autofill popup in dp.
+const int kPopupBorderThicknessDp = 1;
+
 // Child view only for triggering accessibility events. Rendering is handled
 // by |AutofillPopupViewViews|.
 class AutofillPopupChildView : public views::View {
@@ -154,6 +164,57 @@ void AutofillPopupViewViews::OnPaint(gfx::Canvas* canvas) {
   }
 }
 
+void AutofillPopupViewViews::AddExtraInitParams(
+    views::Widget::InitParams* params) {}
+
+std::unique_ptr<views::View> AutofillPopupViewViews::CreateWrapperView() {
+  auto wrapper_view = std::make_unique<views::ScrollView>();
+  scroll_view_ = wrapper_view.get();
+  scroll_view_->set_hide_horizontal_scrollbar(true);
+  scroll_view_->SetContents(this);
+  return wrapper_view;
+}
+
+std::unique_ptr<views::Border> AutofillPopupViewViews::CreateBorder() {
+  return views::CreateSolidBorder(
+      kPopupBorderThicknessDp,
+      GetNativeTheme()->GetSystemColor(
+          ui::NativeTheme::kColorId_UnfocusedBorderColor));
+}
+
+void AutofillPopupViewViews::SetClipPath() {}
+
+// The method differs from the implementation in AutofillPopupBaseView due to
+// |scroll_view_|. The base class doesn't support scrolling when there is not
+// enough vertical space.
+void AutofillPopupViewViews::DoUpdateBoundsAndRedrawPopup() {
+  gfx::Rect bounds = delegate()->popup_bounds();
+
+  SetSize(bounds.size());
+
+  gfx::Rect clipping_bounds = CalculateClippingBounds();
+
+  int available_vertical_space = clipping_bounds.height() -
+                                 (bounds.y() - clipping_bounds.y()) -
+                                 kPopupBottomMargin;
+
+  if (available_vertical_space < bounds.height()) {
+    // The available space is not enough for the full popup so clamp the widget
+    // to what's available. Since the scroll view will show a scroll bar,
+    // increase the width so that the content isn't partially hidden.
+    const int extra_width =
+        scroll_view_ ? scroll_view_->GetScrollBarLayoutWidth() : 0;
+    bounds.set_width(bounds.width() + extra_width);
+    bounds.set_height(available_vertical_space);
+  }
+
+  // Account for the scroll view's border so that the content has enough space.
+  bounds.Inset(-GetWidget()->GetRootView()->border()->GetInsets());
+  GetWidget()->SetBounds(bounds);
+
+  SchedulePaint();
+}
+
 AutofillPopupChildView* AutofillPopupViewViews::GetChildRow(
     size_t child_index) const {
   DCHECK_LT(child_index, static_cast<size_t>(child_count()));

chrome/browser/ui/views/autofill/autofill_popup_view_views.h

@@ -11,6 +11,7 @@
 #include "base/optional.h"
 #include "chrome/browser/ui/autofill/autofill_popup_view.h"
 #include "chrome/browser/ui/views/autofill/autofill_popup_base_view.h"
+#include "ui/views/controls/scroll_view.h"
 
 namespace autofill {
 
@@ -42,6 +43,13 @@ class AutofillPopupViewViews : public AutofillPopupBaseView,
   // views::Views implementation
   void OnPaint(gfx::Canvas* canvas) override;
 
+  // AutofillPopupBaseView implementation
+  void AddExtraInitParams(views::Widget::InitParams* params) override;
+  std::unique_ptr<views::View> CreateWrapperView() override;
+  std::unique_ptr<views::Border> CreateBorder() override;
+  void SetClipPath() override;
+  void DoUpdateBoundsAndRedrawPopup() override;
+
   // Draw the given autofill entry in |entry_rect|.
   void DrawAutofillEntry(gfx::Canvas* canvas,
                          int index,
@@ -56,6 +64,8 @@ class AutofillPopupViewViews : public AutofillPopupBaseView,
 
   AutofillPopupController* controller_;  // Weak reference.
 
+  views::ScrollView* scroll_view_;
+
   DISALLOW_COPY_AND_ASSIGN(AutofillPopupViewViews);
 };