VR: Clean-up input handling

Before this CL, target_element in UiInputManager would also be the input
caputring element. In this CL, the target_element is always the element behind
the ray. This makes the code easier to read and simplifies hover events.

The only functional change in this CL is that we send hover events to non-input
caputring elements. For example, if we have element 1 and element 2, clicking on
element 1 and moving to element 2 will send HoverLeave to element 1 and
HoverEnter to element 2.

This CL also removed the unused ButtonState::CLICKED

Bug: 
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I1be4a8f3cf593e6341df0540efcfd0bc5456f217
Reviewed-on: https://chromium-review.googlesource.com/830886Reviewed-by: Ian Vollick <vollick@chromium.org>
Commit-Queue: Yash Malik <ymalik@chromium.org>
Cr-Commit-Position: refs/heads/master@{#524489}

chrome/browser/vr/ui_input_manager.h

@@ -35,8 +35,6 @@ class UiInputManager {
   enum ButtonState {
     UP,       // The button is released.
     DOWN,     // The button is pressed.
-    CLICKED,  // Since the last update the button has been pressed and released.
-              // The button is released now.
   };
 
   // When testing, it can be useful to hit test directly along the laser.
@@ -80,9 +78,9 @@ class UiInputManager {
                        const gfx::PointF& target_point);
   void SendScrollUpdate(GestureList* gesture_list,
                         const gfx::PointF& target_point);
-  void SendHoverLeave(UiElement* target);
-  bool SendHoverEnter(UiElement* target, const gfx::PointF& target_point);
-  void SendHoverMove(const gfx::PointF& target_point);
+
+  void SendHoverEvents(UiElement* target, const gfx::PointF& target_point);
+  void SendMove(UiElement* element, const gfx::PointF& target_point);
   void SendButtonDown(UiElement* target,
                       const gfx::PointF& target_point,
                       ButtonState button_state);
@@ -100,10 +98,10 @@ class UiInputManager {
   // independently and we should only cancel flings on the relevant element
   // when we do cancel flings.
   int fling_target_id_ = 0;
-  int input_locked_element_id_ = 0;
+  int input_capture_element_id_ = 0;
+  int focused_element_id_ = 0;
   bool in_click_ = false;
   bool in_scroll_ = false;
-  int focused_element_id_ = 0;
 
   HitTestStrategy hit_test_strategy_ = HitTestStrategy::PROJECT_TO_WORLD_ORIGIN;
 

chrome/browser/vr/ui_input_manager_unittest.cc

@@ -34,8 +34,6 @@ namespace vr {
 
 constexpr UiInputManager::ButtonState kUp = UiInputManager::ButtonState::UP;
 constexpr UiInputManager::ButtonState kDown = UiInputManager::ButtonState::DOWN;
-constexpr UiInputManager::ButtonState kClick =
-    UiInputManager::ButtonState::CLICKED;
 
 class MockRect : public Rect {
  public:
@@ -257,22 +255,17 @@ TEST_F(UiInputManagerTest, HoverClick) {
   HandleInput(kForwardVector, kUp);
   Mock::VerifyAndClearExpectations(p_element);
 
-  // Perform a click (both press and release) on the element.
-  EXPECT_CALL(*p_element, OnMove(_));
-  EXPECT_CALL(*p_element, OnButtonDown(_));
-  EXPECT_CALL(*p_element, OnButtonUp(_));
-  HandleInput(kForwardVector, kClick);
-  Mock::VerifyAndClearExpectations(p_element);
-
   // Move off of the element.
   EXPECT_CALL(*p_element, OnHoverLeave());
   HandleInput(kBackwardVector, kUp);
   Mock::VerifyAndClearExpectations(p_element);
 
   // Press while not on the element, move over the element, move away, then
-  // release. The element should receive no input.
+  // release. The element should receive hover events.
   HandleInput(kBackwardVector, kDown);
+  EXPECT_CALL(*p_element, OnHoverEnter(_));
   HandleInput(kForwardVector, kDown);
+  EXPECT_CALL(*p_element, OnHoverLeave());
   HandleInput(kBackwardVector, kUp);
   Mock::VerifyAndClearExpectations(p_element);
 
@@ -280,12 +273,10 @@ TEST_F(UiInputManagerTest, HoverClick) {
   EXPECT_CALL(*p_element, OnHoverEnter(_));
   EXPECT_CALL(*p_element, OnButtonDown(_));
   HandleInput(kForwardVector, kDown);
-  EXPECT_CALL(*p_element, OnMove(_));
+  EXPECT_CALL(*p_element, OnHoverLeave());
   HandleInput(kBackwardVector, kDown);
   Mock::VerifyAndClearExpectations(p_element);
-  EXPECT_CALL(*p_element, OnMove(_));
   EXPECT_CALL(*p_element, OnButtonUp(_));
-  EXPECT_CALL(*p_element, OnHoverLeave());
   HandleInput(kBackwardVector, kUp);
   Mock::VerifyAndClearExpectations(p_element);
 }
@@ -297,17 +288,22 @@ TEST_F(UiInputManagerTest, ReleaseButtonOnAnotherElement) {
   StrictMock<MockRect>* p_front_element = CreateAndAddMockElement(-5.f);
   StrictMock<MockRect>* p_back_element = CreateAndAddMockElement(5.f);
 
+  // TODO(ymalik): We should test verify that the functions called on the
+  // element are in the element's local coordinate space, but that would require
+  // writing a matcher for gfx::Point3F.
   // Press on an element, move away, then release.
   EXPECT_CALL(*p_front_element, OnHoverEnter(_));
   EXPECT_CALL(*p_front_element, OnButtonDown(_));
-  EXPECT_CALL(*p_front_element, OnMove(_));
-  EXPECT_CALL(*p_front_element, OnMove(_));
-  EXPECT_CALL(*p_front_element, OnButtonUp(_));
+  HandleInput(kForwardVector, kDown);
   EXPECT_CALL(*p_front_element, OnHoverLeave());
   EXPECT_CALL(*p_back_element, OnHoverEnter(_));
-  HandleInput(kForwardVector, kDown);
   HandleInput(kBackwardVector, kDown);
+  EXPECT_CALL(*p_back_element, OnMove(_));
+  EXPECT_CALL(*p_front_element, OnButtonUp(_));
   HandleInput(kBackwardVector, kUp);
+  EXPECT_CALL(*p_back_element, OnHoverLeave());
+  EXPECT_CALL(*p_front_element, OnHoverEnter(_));
+  HandleInput(kForwardVector, kUp);
 }
 
 // Test that input is tolerant of disappearing elements.