shelf: Fix bug with swipes triggering the back button.

It happens because swipes may send out TAP_DOWN events which create a
key down back event. But the key down back event is the one that
triggers the browser to go back, so change to only simulate a key down
event on a tap or mouse release event.

Test: manual
Bug: 943154
Change-Id: Id0e8f575b3646d14e1eb39a9b5c98f64dfd874cc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1529543
Commit-Queue: Sammie Quon <sammiequon@chromium.org>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#642066}

ash/shelf/back_button.cc

@@ -38,21 +38,13 @@ BackButton::~BackButton() = default;
 
 void BackButton::OnGestureEvent(ui::GestureEvent* event) {
   ShelfButton::OnGestureEvent(event);
-  if (event->type() == ui::ET_GESTURE_TAP ||
-      event->type() == ui::ET_GESTURE_TAP_DOWN) {
-    GenerateAndSendBackEvent(event->type());
-  }
-}
-
-bool BackButton::OnMousePressed(const ui::MouseEvent& event) {
-  ShelfButton::OnMousePressed(event);
-  GenerateAndSendBackEvent(event.type());
-  return true;
+  if (event->type() == ui::ET_GESTURE_TAP)
+    GenerateAndSendBackEvent();
 }
 
 void BackButton::OnMouseReleased(const ui::MouseEvent& event) {
   ShelfButton::OnMouseReleased(event);
-  GenerateAndSendBackEvent(event.type());
+  GenerateAndSendBackEvent();
 }
 
 void BackButton::PaintButtonContents(gfx::Canvas* canvas) {
@@ -67,28 +59,15 @@ const char* BackButton::GetClassName() const {
   return kViewClassName;
 }
 
-void BackButton::GenerateAndSendBackEvent(
-    const ui::EventType& original_event_type) {
-  ui::EventType new_event_type;
-  switch (original_event_type) {
-    case ui::ET_MOUSE_PRESSED:
-    case ui::ET_GESTURE_TAP_DOWN:
-      new_event_type = ui::ET_KEY_PRESSED;
-      break;
-    case ui::ET_MOUSE_RELEASED:
-    case ui::ET_GESTURE_TAP:
-      new_event_type = ui::ET_KEY_RELEASED;
-      base::RecordAction(base::UserMetricsAction("Tablet_BackButton"));
-      break;
-    default:
-      return;
-  }
+void BackButton::GenerateAndSendBackEvent() {
+  base::RecordAction(base::UserMetricsAction("Tablet_BackButton"));
 
   // Send the back event to the root window of the back button's widget.
   const views::Widget* widget = GetWidget();
   if (widget && widget->GetNativeWindow()) {
     aura::Window* root_window = widget->GetNativeWindow()->GetRootWindow();
-    ui::KeyEvent key_event(new_event_type, ui::VKEY_BROWSER_BACK, ui::EF_NONE);
+    ui::KeyEvent key_event(ui::ET_KEY_PRESSED, ui::VKEY_BROWSER_BACK,
+                           ui::EF_NONE);
     ignore_result(
         root_window->GetHost()->event_sink()->OnEventFromSource(&key_event));
   }

ash/shelf/back_button.h

@@ -26,15 +26,16 @@ class ASH_EXPORT BackButton : public ShelfControlButton {
  protected:
   // views::Button:
   void OnGestureEvent(ui::GestureEvent* event) override;
-  bool OnMousePressed(const ui::MouseEvent& event) override;
   void OnMouseReleased(const ui::MouseEvent& event) override;
   void PaintButtonContents(gfx::Canvas* canvas) override;
   const char* GetClassName() const override;
 
  private:
-  // Generate and send a VKEY_BROWSER_BACK key event when the back button
-  // is pressed.
-  void GenerateAndSendBackEvent(const ui::EventType& original_event_type);
+  // Generate and send a VKEY_BROWSER_BACK key event sequence when the back
+  // button is pressed. This should on be called on a tap down or mouse release
+  // event, and will only send a key down event since that is the one which
+  // triggers the back event.
+  void GenerateAndSendBackEvent();
 
   DISALLOW_COPY_AND_ASSIGN(BackButton);
 };

ash/shelf/back_button_unittest.cc

@@ -107,13 +107,17 @@ TEST_F(BackButtonTest, BackKeySequenceGenerated) {
   ui::TestAcceleratorTarget target_back_release;
   controller->Register({accelerator_back_release}, &target_back_release);
 
-  // Verify that by clicking the back button, a back key sequence will be
-  // generated.
+  // Verify that by pressing the back button no event is generated on the press,
+  // but there is one generated on the release.
   ui::test::EventGenerator* generator = GetEventGenerator();
   generator->MoveMouseTo(back_button()->GetBoundsInScreen().CenterPoint());
-  generator->ClickLeftButton();
+  generator->PressLeftButton();
+  EXPECT_EQ(0, target_back_press.accelerator_count());
+  EXPECT_EQ(0, target_back_release.accelerator_count());
+
+  generator->ReleaseLeftButton();
   EXPECT_EQ(1, target_back_press.accelerator_count());
-  EXPECT_EQ(1, target_back_release.accelerator_count());
+  EXPECT_EQ(0, target_back_release.accelerator_count());
 }
 
 }  // namespace ash