Move handling gesture events from AutofillPopupBaseView to where they actually

happen.

AutofillPopupBaseView and the controllers have absolutely no clue of actual
layout in the UI. Any attempt to work with Points and Rects there is wrong.
The CL is the first attempt to simplify those classes.

Bug: 1028407
Change-Id: I6abaaf44a0d957f5b0e7a476a4d01d9df62c4cad
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2015046
Commit-Queue: Vasilii Sukhanov <vasilii@chromium.org>
Reviewed-by: Friedrich [CET] <fhorschig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#734866}

chrome/browser/ui/android/passwords/password_generation_editing_popup_view_android.cc

@@ -90,12 +90,6 @@ void PasswordGenerationEditingPopupViewAndroid::UpdateBoundsAndRedrawPopup() {
 
 void PasswordGenerationEditingPopupViewAndroid::PasswordSelectionUpdated() {}
 
-bool PasswordGenerationEditingPopupViewAndroid::IsPointInPasswordBounds(
-    const gfx::Point& point) {
-  NOTREACHED();
-  return false;
-}
-
 // static
 PasswordGenerationPopupView* PasswordGenerationPopupView::Create(
     PasswordGenerationPopupController* controller) {

chrome/browser/ui/android/passwords/password_generation_editing_popup_view_android.h

@@ -39,7 +39,6 @@ class PasswordGenerationEditingPopupViewAndroid
   void UpdateState() override;
   void UpdateBoundsAndRedrawPopup() override;
   void PasswordSelectionUpdated() override;
-  bool IsPointInPasswordBounds(const gfx::Point& point) override;
 
   // Weak pointer to the controller.
   PasswordGenerationPopupController* controller_;

chrome/browser/ui/passwords/password_generation_popup_controller.h

@@ -21,6 +21,9 @@ class PasswordGenerationPopupController
   // Called by the view when the password was accepted.
   virtual void PasswordAccepted() = 0;
 
+  // Called by the view when the password was selected.
+  virtual void SetSelected() = 0;
+
   // Accessors
   virtual GenerationUIState state() const = 0;
   virtual bool password_selected() const = 0;

chrome/browser/ui/passwords/password_generation_popup_controller_impl.cc

@@ -296,7 +296,7 @@ void PasswordGenerationPopupControllerImpl::ViewDestroyed() {
 
 void PasswordGenerationPopupControllerImpl::SetSelectionAtPoint(
     const gfx::Point& point) {
-  PasswordSelected(view_->IsPointInPasswordBounds(point));
+  // The view handles mouse events itself.
 }
 
 bool PasswordGenerationPopupControllerImpl::AcceptSelectedLine() {
@@ -315,6 +315,10 @@ bool PasswordGenerationPopupControllerImpl::HasSelection() const {
   return password_selected();
 }
 
+void PasswordGenerationPopupControllerImpl::SetSelected() {
+  PasswordSelected(true);
+}
+
 gfx::NativeView PasswordGenerationPopupControllerImpl::container_view() const {
   return controller_common_.container_view;
 }

chrome/browser/ui/passwords/password_generation_popup_controller_impl.h

@@ -133,6 +133,7 @@ class PasswordGenerationPopupControllerImpl
   bool AcceptSelectedLine() override;
   void SelectionCleared() override;
   bool HasSelection() const override;
+  void SetSelected() override;
   void PasswordAccepted() override;
   gfx::NativeView container_view() const override;
   gfx::Rect popup_bounds() const override;

chrome/browser/ui/passwords/password_generation_popup_view.h

@@ -7,10 +7,6 @@
 
 #include "third_party/skia/include/core/SkColor.h"
 
-namespace gfx {
-class Point;
-}  // namespace gfx
-
 class PasswordGenerationPopupController;
 
 // Interface for creating and controlling a platform dependent view.
@@ -35,8 +31,6 @@ class PasswordGenerationPopupView {
   // Called when the password selection state has changed.
   virtual void PasswordSelectionUpdated() = 0;
 
-  virtual bool IsPointInPasswordBounds(const gfx::Point& point) = 0;
-
   // Note that PasswordGenerationPopupView owns itself, and will only be deleted
   // when Hide() is called.
   static PasswordGenerationPopupView* Create(

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

@@ -253,33 +253,6 @@ void AutofillPopupBaseView::OnMouseReleased(const ui::MouseEvent& event) {
     AcceptSelection(event.location());
 }
 
-void AutofillPopupBaseView::OnGestureEvent(ui::GestureEvent* event) {
-  switch (event->type()) {
-    case ui::ET_GESTURE_TAP_DOWN:
-    case ui::ET_GESTURE_SCROLL_BEGIN:
-    case ui::ET_GESTURE_SCROLL_UPDATE:
-      if (HitTestPoint(event->location()))
-        SetSelection(event->location());
-      else
-        ClearSelection();
-      break;
-    case ui::ET_GESTURE_TAP:
-    case ui::ET_GESTURE_SCROLL_END:
-      if (HitTestPoint(event->location()))
-        AcceptSelection(event->location());
-      else
-        ClearSelection();
-      break;
-    case ui::ET_GESTURE_TAP_CANCEL:
-    case ui::ET_SCROLL_FLING_START:
-      ClearSelection();
-      break;
-    default:
-      return;
-  }
-  event->SetHandled();
-}
-
 void AutofillPopupBaseView::GetAccessibleNodeData(ui::AXNodeData* node_data) {
   // TODO(aleventhal) The correct role spec-wise to use here is kMenu, however
   // as of NVDA 2018.2.1, firing a menu event with kMenu breaks left/right

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

@@ -72,7 +72,6 @@ class AutofillPopupBaseView : public views::WidgetDelegateView,
   void OnMouseMoved(const ui::MouseEvent& event) override;
   bool OnMousePressed(const ui::MouseEvent& event) override;
   void OnMouseReleased(const ui::MouseEvent& event) override;
-  void OnGestureEvent(ui::GestureEvent* event) override;
   void GetAccessibleNodeData(ui::AXNodeData* node_data) override;
 
   // views::WidgetFocusChangeListener implementation.

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

@@ -90,36 +90,6 @@ class AutofillPopupBaseViewTest : public InProcessBrowserTest {
   DISALLOW_COPY_AND_ASSIGN(AutofillPopupBaseViewTest);
 };
 
-IN_PROC_BROWSER_TEST_F(AutofillPopupBaseViewTest, GestureTest) {
-  const int kElementSize = 5;
-  gfx::RectF bounds(0, 0, kElementSize, kElementSize);
-  EXPECT_CALL(mock_delegate_, element_bounds())
-      .WillRepeatedly(ReturnRef(bounds));
-  view_->SetPreferredSize(gfx::Size(2 * kElementSize, 2 * kElementSize));
-  ShowView();
-
-  gfx::Point point = view_->GetLocalBounds().CenterPoint();
-  testing::InSequence dummy;
-
-  // Tap down will select an element.
-  ui::GestureEvent tap_down_event = CreateGestureEvent(ui::ET_GESTURE_TAP_DOWN,
-                                                       point);
-  EXPECT_CALL(mock_delegate_, SetSelectionAtPoint(point));
-  SimulateGesture(&tap_down_event);
-
-  // Tapping will accept the selection.
-  ui::GestureEvent tap_event = CreateGestureEvent(ui::ET_GESTURE_TAP, point);
-  EXPECT_CALL(mock_delegate_, SetSelectionAtPoint(point));
-  EXPECT_CALL(mock_delegate_, AcceptSelectedLine());
-  SimulateGesture(&tap_event);
-
-  // Tapping outside the bounds clears any selection.
-  ui::GestureEvent outside_tap = CreateGestureEvent(ui::ET_GESTURE_TAP,
-                                                    gfx::Point(100, 100));
-  EXPECT_CALL(mock_delegate_, SelectionCleared());
-  SimulateGesture(&outside_tap);
-}
-
 IN_PROC_BROWSER_TEST_F(AutofillPopupBaseViewTest, DoubleClickTest) {
   gfx::RectF bounds(0, 0, 5, 5);
   EXPECT_CALL(mock_delegate_, element_bounds())

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

@@ -135,6 +135,7 @@ class AutofillPopupItemView : public AutofillPopupRowView {
   void OnMouseEntered(const ui::MouseEvent& event) override;
   void OnMouseExited(const ui::MouseEvent& event) override;
   void OnMouseReleased(const ui::MouseEvent& event) override;
+  void OnGestureEvent(ui::GestureEvent* event) override;
 
  protected:
   AutofillPopupItemView(AutofillPopupViewNativeViews* popup_view,
@@ -377,6 +378,26 @@ void AutofillPopupItemView::OnMouseReleased(const ui::MouseEvent& event) {
   }
 }
 
+void AutofillPopupItemView::OnGestureEvent(ui::GestureEvent* event) {
+  AutofillPopupController* controller = popup_view()->controller();
+  if (!controller)
+    return;
+  switch (event->type()) {
+    case ui::ET_GESTURE_TAP_DOWN:
+      controller->SetSelectedLine(line_number());
+      break;
+    case ui::ET_GESTURE_TAP:
+      controller->AcceptSuggestion(line_number());
+      break;
+    case ui::ET_GESTURE_TAP_CANCEL:
+    case ui::ET_GESTURE_END:
+      controller->SelectionCleared();
+      break;
+    default:
+      return;
+  }
+}
+
 void AutofillPopupItemView::CreateContent() {
   AutofillPopupController* controller = popup_view()->controller();
 

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

@@ -208,6 +208,33 @@ TEST_F(AutofillPopupViewNativeViewsTest, AccessibilityTest) {
       node_data_3.GetBoolAttribute(ax::mojom::BoolAttribute::kSelected));
 }
 
+TEST_F(AutofillPopupViewNativeViewsTest, Gestures) {
+  CreateAndShowView({autofill::POPUP_ITEM_ID_PASSWORD_ENTRY,
+                     autofill::POPUP_ITEM_ID_SEPARATOR,
+                     autofill::POPUP_ITEM_ID_ALL_SAVED_PASSWORDS_ENTRY});
+
+  // Tap down will select an element.
+  ui::GestureEvent tap_down_event(
+      /*x=*/0, /*y=*/0, /*flags=*/0, ui::EventTimeForNow(),
+      ui::GestureEventDetails(ui::ET_GESTURE_TAP_DOWN));
+  EXPECT_CALL(autofill_popup_controller_, SetSelectedLine(testing::Eq(0)));
+  view()->GetRowsForTesting()[0]->OnGestureEvent(&tap_down_event);
+
+  // Tapping will accept the selection.
+  ui::GestureEvent tap_event(/*x=*/0, /*y=*/0, /*flags=*/0,
+                             ui::EventTimeForNow(),
+                             ui::GestureEventDetails(ui::ET_GESTURE_TAP));
+  EXPECT_CALL(autofill_popup_controller_, AcceptSuggestion(0));
+  view()->GetRowsForTesting()[0]->OnGestureEvent(&tap_event);
+
+  // Canceling gesture clears any selection.
+  ui::GestureEvent tap_cancel(
+      /*x=*/0, /*y=*/0, /*flags=*/0, ui::EventTimeForNow(),
+      ui::GestureEventDetails(ui::ET_GESTURE_TAP_CANCEL));
+  EXPECT_CALL(autofill_popup_controller_, SelectionCleared());
+  view()->GetRowsForTesting()[2]->OnGestureEvent(&tap_cancel);
+}
+
 TEST_P(AutofillPopupViewNativeViewsForEveryTypeTest, ShowClickTest) {
   const TypeClicks& click = GetParam();
   CreateAndShowView({click.id});

chrome/browser/ui/views/passwords/password_generation_popup_view_views.cc

@@ -4,7 +4,6 @@
 
 #include "chrome/browser/ui/views/passwords/password_generation_popup_view_views.h"
 
-#include "base/macros.h"
 #include "base/strings/string16.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/ui/passwords/password_generation_popup_controller.h"
@@ -41,59 +40,104 @@ class PasswordGenerationPopupViewViews::GeneratedPasswordBox
   GeneratedPasswordBox() = default;
   ~GeneratedPasswordBox() override = default;
 
-  // |password| is the generated password, |suggestion| is the text to the left
-  // of it. |generating_state| means that the generated password is offered.
-  void Init(const base::string16& password,
-            const base::string16& suggestion,
-            PasswordGenerationPopupController::GenerationUIState state) {
+  // Fills the view with strings provided by |controller|.
+  void Init(PasswordGenerationPopupController* controller);
+
+  void UpdatePassword(const base::string16& password) {
+    password_label_->SetText(password);
+  }
+
+  void reset_controller() { controller_ = nullptr; }
+
+  // View:
+  void OnMouseEntered(const ui::MouseEvent& event) override;
+  void OnMouseExited(const ui::MouseEvent& event) override;
+  void OnMouseReleased(const ui::MouseEvent& event) override;
+  void OnGestureEvent(ui::GestureEvent* event) override;
+
+ private:
+  // Construct a ColumnSet with one view on the left and another on the right.
+  static void BuildColumnSet(views::GridLayout* layout);
+
+  views::Label* password_label_ = nullptr;
+  PasswordGenerationPopupController* controller_ = nullptr;
+};
+
+void PasswordGenerationPopupViewViews::GeneratedPasswordBox::Init(
+    PasswordGenerationPopupController* controller) {
+  controller_ = controller;
   views::GridLayout* layout =
       SetLayoutManager(std::make_unique<views::GridLayout>());
   BuildColumnSet(layout);
   layout->StartRow(views::GridLayout::kFixedSize, 0);
 
   layout->AddView(autofill::CreateLabelWithColorReadabilityDisabled(
-        suggestion, ChromeTextContext::CONTEXT_BODY_TEXT_LARGE,
-        state == PasswordGenerationPopupController::kOfferGeneration
+      controller_->SuggestedText(), ChromeTextContext::CONTEXT_BODY_TEXT_LARGE,
+      controller_->state() ==
+              PasswordGenerationPopupController::kOfferGeneration
           ? views::style::STYLE_PRIMARY
           : views::style::STYLE_SECONDARY));
 
   DCHECK(!password_label_);
   password_label_ =
       layout->AddView(autofill::CreateLabelWithColorReadabilityDisabled(
-            password, ChromeTextContext::CONTEXT_BODY_TEXT_LARGE,
+          controller_->password(), ChromeTextContext::CONTEXT_BODY_TEXT_LARGE,
           STYLE_SECONDARY_MONOSPACED));
-  }
+}
 
-  void UpdatePassword(const base::string16& password) {
-    password_label_->SetText(password);
-  }
+void PasswordGenerationPopupViewViews::GeneratedPasswordBox::OnMouseEntered(
+    const ui::MouseEvent& event) {
+  if (controller_)
+    controller_->SetSelected();
+}
+
+void PasswordGenerationPopupViewViews::GeneratedPasswordBox::OnMouseExited(
+    const ui::MouseEvent& event) {
+  if (controller_)
+    controller_->SelectionCleared();
+}
 
-  // views::View:
-  bool CanProcessEventsWithinSubtree() const override {
-    // Send events to the parent view for handling.
-    return false;
+void PasswordGenerationPopupViewViews::GeneratedPasswordBox::OnMouseReleased(
+    const ui::MouseEvent& event) {
+  if (event.IsOnlyLeftMouseButton() && controller_)
+    controller_->PasswordAccepted();
+}
+
+void PasswordGenerationPopupViewViews::GeneratedPasswordBox::OnGestureEvent(
+    ui::GestureEvent* event) {
+  if (!controller_)
+    return;
+  switch (event->type()) {
+    case ui::ET_GESTURE_TAP_DOWN:
+      controller_->SetSelected();
+      break;
+    case ui::ET_GESTURE_TAP:
+      controller_->PasswordAccepted();
+      break;
+    case ui::ET_GESTURE_TAP_CANCEL:
+    case ui::ET_GESTURE_END:
+      controller_->SelectionCleared();
+      break;
+    default:
+      return;
   }
+}
 
- private:
-  // Construct a ColumnSet with one view on the left and another on the right.
-  static void BuildColumnSet(views::GridLayout* layout) {
+// static
+void PasswordGenerationPopupViewViews::GeneratedPasswordBox::BuildColumnSet(
+    views::GridLayout* layout) {
   views::ColumnSet* column_set = layout->AddColumnSet(0);
   column_set->AddColumn(views::GridLayout::LEADING, views::GridLayout::CENTER,
-                          0 /* resize_percent */, views::GridLayout::USE_PREF,
-                          0, 0);
+                        0 /* resize_percent */, views::GridLayout::USE_PREF, 0,
+                        0);
   column_set->AddPaddingColumn(
       0 /* resize_percent */,
       ChromeLayoutProvider::Get()->GetDistanceMetric(
           DISTANCE_BETWEEN_PRIMARY_AND_SECONDARY_LABELS_HORIZONTAL));
-    column_set->AddColumn(views::GridLayout::TRAILING,
-                          views::GridLayout::CENTER, 1.0 /* resize_percent */,
-                          views::GridLayout::USE_PREF, 0, 0);
-  }
-
-  views::Label* password_label_ = nullptr;
-
-  DISALLOW_COPY_AND_ASSIGN(GeneratedPasswordBox);
-};
+  column_set->AddColumn(views::GridLayout::TRAILING, views::GridLayout::CENTER,
+                        1.0 /* resize_percent */, views::GridLayout::USE_PREF,
+                        0, 0);
+}
 
 PasswordGenerationPopupViewViews::PasswordGenerationPopupViewViews(
     PasswordGenerationPopupController* controller,
@@ -112,7 +156,8 @@ void PasswordGenerationPopupViewViews::Show() {
 
 void PasswordGenerationPopupViewViews::Hide() {
   // The controller is no longer valid after it hides us.
-  controller_ = NULL;
+  controller_ = nullptr;
+  password_view_->reset_controller();
 
   DoHide();
 }
@@ -133,9 +178,6 @@ void PasswordGenerationPopupViewViews::UpdateBoundsAndRedrawPopup() {
 }
 
 void PasswordGenerationPopupViewViews::PasswordSelectionUpdated() {
-  if (!password_view_)
-    return;
-
   if (controller_->password_selected())
     NotifyAccessibilityEvent(ax::mojom::Event::kSelection, true);
 
@@ -145,16 +187,6 @@ void PasswordGenerationPopupViewViews::PasswordSelectionUpdated() {
   SchedulePaint();
 }
 
-bool PasswordGenerationPopupViewViews::IsPointInPasswordBounds(
-    const gfx::Point& point) {
-  if (password_view_) {
-    gfx::Point point_password_view = point;
-    ConvertPointToTarget(this, password_view_, &point_password_view);
-    return password_view_->HitTestPoint(point_password_view);
-  }
-  return false;
-}
-
 void PasswordGenerationPopupViewViews::CreateLayoutAndChildren() {
   // Add 1px distance between views for the separator.
   views::BoxLayout* box_layout =
@@ -173,8 +205,7 @@ void PasswordGenerationPopupViewViews::CreateLayoutAndChildren() {
   password_view_->SetBorder(
       views::CreateEmptyBorder(kVerticalPadding, kHorizontalMargin,
                                kVerticalPadding, kHorizontalMargin));
-  password_view_->Init(controller_->password(), controller_->SuggestedText(),
-                       controller_->state());
+  password_view_->Init(controller_);
   AddChildView(password_view_);
   PasswordSelectionUpdated();
 
@@ -200,12 +231,10 @@ void PasswordGenerationPopupViewViews::OnPaint(gfx::Canvas* canvas) {
 
   // Divider line needs to be drawn after OnPaint() otherwise the background
   // will overwrite the divider.
-  if (password_view_) {
   gfx::Rect divider_bounds(0, password_view_->bounds().bottom(),
                            password_view_->width(), 1);
   canvas->FillRect(divider_bounds, GetSeparatorColor());
   }
-}
 
 void PasswordGenerationPopupViewViews::GetAccessibleNodeData(
     ui::AXNodeData* node_data) {

chrome/browser/ui/views/passwords/password_generation_popup_view_views.h

@@ -25,7 +25,6 @@ class PasswordGenerationPopupViewViews : public autofill::AutofillPopupBaseView,
   void UpdatePasswordValue() override;
   void UpdateBoundsAndRedrawPopup() override;
   void PasswordSelectionUpdated() override;
-  bool IsPointInPasswordBounds(const gfx::Point& point) override;
 
  private:
   class GeneratedPasswordBox;