VR: Revert gesture detector to use events instead of state

Using the state produced some E2E tests to fail, because the button down
and button up events happen in the same frame.

Reason for TBR: fix failing tests on CI.

TBR: asimjour
Bug: 867016
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:linux_vr;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Change-Id: Icbc657838d80bf28d60ab2114625936aa4826883
Reviewed-on: https://chromium-review.googlesource.com/1148964
Commit-Queue: Aldo Culquicondor <acondor@chromium.org>
Reviewed-by: Brian Sheedy <bsheedy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577721}

chrome/browser/android/vr/vr_controller.cc

@@ -136,7 +136,7 @@ device::mojom::XRInputSourceStatePtr VrController::GetInputSourceState() {
   // Set the primary button state.
   state->primary_input_pressed = ButtonState(GVR_CONTROLLER_BUTTON_CLICK);
 
-  if (ButtonUpHappened(GVR_CONTROLLER_BUTTON_CLICK))
+  if (ButtonUpHappened(PlatformController::kButtonSelect))
     state->primary_input_clicked = true;
 
   state->description = device::mojom::XRInputSourceDescription::New();
@@ -174,7 +174,7 @@ device::mojom::XRInputSourceStatePtr VrController::GetInputSourceState() {
   return state;
 }
 
-bool VrController::IsButtonDown(PlatformController::ButtonType type) const {
+bool VrController::IsButtonDown(ButtonType type) const {
   return controller_state_->GetButtonState(PlatformToGvrButton(type));
 }
 
@@ -275,18 +275,20 @@ bool VrController::TouchUpHappened() {
   return controller_state_->GetTouchUp();
 }
 
-bool VrController::ButtonDownHappened(gvr::ControllerButton button) {
+bool VrController::ButtonDownHappened(ButtonType button) const {
   // Workaround for GVR sometimes not reporting GetButtonDown when it should.
-  bool detected_down =
-      !previous_button_states_[static_cast<int>(button)] && ButtonState(button);
-  return controller_state_->GetButtonDown(button) || detected_down;
+  auto gvr_button = PlatformToGvrButton(button);
+  bool detected_down = !previous_button_states_[static_cast<int>(button)] &&
+                       ButtonState(gvr_button);
+  return controller_state_->GetButtonDown(gvr_button) || detected_down;
 }
 
-bool VrController::ButtonUpHappened(gvr::ControllerButton button) {
+bool VrController::ButtonUpHappened(ButtonType button) const {
   // Workaround for GVR sometimes not reporting GetButtonUp when it should.
-  bool detected_up =
-      previous_button_states_[static_cast<int>(button)] && !ButtonState(button);
-  return controller_state_->GetButtonUp(button) || detected_up;
+  auto gvr_button = PlatformToGvrButton(button);
+  bool detected_up = previous_button_states_[static_cast<int>(button)] &&
+                     !ButtonState(gvr_button);
+  return controller_state_->GetButtonUp(gvr_button) || detected_up;
 }
 
 bool VrController::ButtonState(gvr::ControllerButton button) const {

chrome/browser/android/vr/vr_controller.h

@@ -67,8 +67,6 @@ class VrController : public PlatformController {
 
   bool TouchUpHappened();
 
-  bool ButtonUpHappened(gvr::ControllerButton button);
-  bool ButtonDownHappened(gvr::ControllerButton button);
   bool ButtonState(gvr::ControllerButton button) const;
 
   bool IsConnected();
@@ -76,6 +74,8 @@ class VrController : public PlatformController {
 
   // PlatformController
   bool IsButtonDown(PlatformController::ButtonType type) const override;
+  bool ButtonUpHappened(PlatformController::ButtonType type) const override;
+  bool ButtonDownHappened(PlatformController::ButtonType type) const override;
   bool IsTouchingTrackpad() const override;
   gfx::PointF GetPositionInTrackpad() const override;
   base::TimeTicks GetLastOrientationTimestamp() const override;

chrome/browser/android/vr/vr_shell_gl.cc

@@ -1275,8 +1275,8 @@ void VrShellGl::HandleControllerInput(const gfx::Point3F& laser_origin,
   controller_->GetTransform(&controller_model.transform);
   auto input_event_list = controller_->DetectGestures();
   controller_model.touchpad_button_state = UiInputManager::ButtonState::UP;
-  DCHECK(!(controller_->ButtonUpHappened(gvr::kControllerButtonClick) &&
-           controller_->ButtonDownHappened(gvr::kControllerButtonClick)))
+  DCHECK(!(controller_->ButtonUpHappened(PlatformController::kButtonSelect) &&
+           controller_->ButtonDownHappened(PlatformController::kButtonSelect)))
       << "Cannot handle a button down and up event within one frame.";
   if (controller_->ButtonState(gvr::kControllerButtonClick)) {
     controller_model.touchpad_button_state = UiInputManager::ButtonState::DOWN;

chrome/browser/vr/gesture_detector.cc

@@ -85,18 +85,14 @@ void GestureDetector::DetectMenuButtonGestures(
     const PlatformController& controller,
     base::TimeTicks current_timestamp) {
   std::unique_ptr<InputEvent> event;
-  if (!menu_button_pressed_ &&
-      controller.IsButtonDown(PlatformController::kButtonMenu)) {
-    menu_button_pressed_ = true;
+  if (controller.ButtonDownHappened(PlatformController::kButtonMenu)) {
     menu_button_down_timestamp_ = current_timestamp;
     menu_button_long_pressed_ = false;
   }
-  if (menu_button_pressed_ &&
-      !controller.IsButtonDown(PlatformController::kButtonMenu)) {
+  if (controller.ButtonUpHappened(PlatformController::kButtonMenu)) {
     event = std::make_unique<InputEvent>(
         menu_button_long_pressed_ ? InputEvent::kMenuButtonLongPressEnd
                                   : InputEvent::kMenuButtonClicked);
-    menu_button_pressed_ = false;
   }
   if (!menu_button_long_pressed_ &&
       controller.IsButtonDown(PlatformController::kButtonMenu) &&

chrome/browser/vr/gesture_detector.h

@@ -99,7 +99,6 @@ class VR_EXPORT GestureDetector {
   bool touch_position_changed_;
 
   base::TimeTicks menu_button_down_timestamp_;
-  bool menu_button_pressed_ = false;
   bool menu_button_long_pressed_ = false;
 
   DISALLOW_COPY_AND_ASSIGN(GestureDetector);

chrome/browser/vr/gesture_detector_unittest.cc

@@ -10,8 +10,11 @@
 
 namespace {
 constexpr float kDelta = 0.001f;
+}
+
+namespace vr {
 
-class MockPlatformController : public vr::PlatformController {
+class MockPlatformController : public PlatformController {
  public:
   MockPlatformController() = default;
   MockPlatformController(bool touching_trackpad,
@@ -19,7 +22,7 @@ class MockPlatformController : public vr::PlatformController {
       : is_touching_trackpad(touching_trackpad),
         last_touch_timestamp(touch_timestamp) {}
 
-  bool IsButtonDown(vr::PlatformController::ButtonType type) const override {
+  bool IsButtonDown(PlatformController::ButtonType type) const override {
     switch (type) {
       case vr::PlatformController::kButtonSelect:
         return is_select_button_down;
@@ -30,6 +33,28 @@ class MockPlatformController : public vr::PlatformController {
     }
   }
 
+  bool ButtonUpHappened(PlatformController::ButtonType type) const override {
+    switch (type) {
+      case vr::PlatformController::kButtonSelect:
+        return was_select_button_down_ && !is_select_button_down;
+      case vr::PlatformController::kButtonMenu:
+        return was_menu_button_down_ && !is_menu_button_down;
+      default:
+        return false;
+    }
+  }
+
+  bool ButtonDownHappened(PlatformController::ButtonType type) const override {
+    switch (type) {
+      case vr::PlatformController::kButtonSelect:
+        return !was_select_button_down_ && is_select_button_down;
+      case vr::PlatformController::kButtonMenu:
+        return !was_menu_button_down_ && is_menu_button_down;
+      default:
+        return false;
+    }
+  }
+
   bool IsTouchingTrackpad() const override { return is_touching_trackpad; }
 
   gfx::PointF GetPositionInTrackpad() const override {
@@ -56,16 +81,22 @@ class MockPlatformController : public vr::PlatformController {
 
   int GetBatteryLevel() const override { return 100; }
 
+  // Call before each frame, if the test needs button up/down events.
+  void Update() {
+    was_select_button_down_ = is_select_button_down;
+    was_menu_button_down_ = is_menu_button_down;
+  }
+
   bool is_touching_trackpad = false;
   bool is_select_button_down = false;
   bool is_menu_button_down = false;
   gfx::PointF position_in_trackpad;
   base::TimeTicks last_touch_timestamp;
-};
 
-}  // namespace
-
-namespace vr {
+ private:
+  bool was_select_button_down_ = false;
+  bool was_menu_button_down_ = false;
+};
 
 TEST(GestureDetector, StartTouchWithoutMoving) {
   GestureDetector detector;
@@ -215,6 +246,7 @@ TEST(GestureDetector, ClickMenuButton) {
   EXPECT_TRUE(gestures.empty());
 
   // Release menu button.
+  controller.Update();
   controller.is_menu_button_down = false;
   timestamp += base::TimeDelta::FromMilliseconds(1);
   gestures = detector.DetectGestures(controller, timestamp);
@@ -233,12 +265,14 @@ TEST(GestureDetector, LongPressMenuButton) {
   EXPECT_TRUE(gestures.empty());
 
   // Keep menu button down.
+  controller.Update();
   timestamp += base::TimeDelta::FromSeconds(1);
   gestures = detector.DetectGestures(controller, timestamp);
   EXPECT_EQ(gestures.size(), 1u);
   EXPECT_EQ(gestures.front()->type(), InputEvent::kMenuButtonLongPressStart);
 
   // Release menu button.
+  controller.Update();
   controller.is_menu_button_down = false;
   timestamp += base::TimeDelta::FromSeconds(1);
   gestures = detector.DetectGestures(controller, timestamp);

chrome/browser/vr/platform_controller.h

@@ -37,6 +37,8 @@ class PlatformController {
   virtual ~PlatformController() {}
 
   virtual bool IsButtonDown(ButtonType type) const = 0;
+  virtual bool ButtonUpHappened(ButtonType type) const = 0;
+  virtual bool ButtonDownHappened(ButtonType type) const = 0;
   virtual bool IsTouchingTrackpad() const = 0;
   virtual gfx::PointF GetPositionInTrackpad() const = 0;
   virtual base::TimeTicks GetLastOrientationTimestamp() const = 0;