Re-evaluate pinch events at GesturePinchBegin

Right now pinch events are evaluated at GestureScrollBegin, if a gesture
is allowed at GSB, then the entire gesture is allowed. This cause a
problem in the following scenario. We have a div with pan-y, user starts
with scrolling along the y-direction, and then the second finger touches
the screen and generates a pinch zoom gesture. In this case, the pinch
zoom gesture is allowed because the user starts with scrolling along the
y-axis which is allowed.

This CL fixes the problem by re-evaluating the gesture event at
GesturePinchBegin. There are few cases to consider
1. The above described case. We will allow scrolling on the y-axis but
disallow the pinch zoom when the second finger is touched.
2. It should work with certain embedded map case such as the demo here:
http://mustaqahmed.github.io/web/image-panning.html
If a user starts with two finger panning the embedded map, then the
gesture will be evaluated at GestureScrollBegin and will be disallowed,
which means the page will handle the pointer events and manipulate the
map.
3. It should work with double-tap-drag-zoom case. In this case, there
is no GSB, and when GesturePinchBegin arrives, we evaluate that.

Bug: 771330
Change-Id: Ibeaeace11bc017384317f61aabc0f23008ad2b5f
Reviewed-on: https://chromium-review.googlesource.com/c/1297070Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
Reviewed-by: Mustaq Ahmed <mustaq@chromium.org>
Reviewed-by: David Bokan <bokan@chromium.org>
Commit-Queue: Xida Chen <xidachen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#603795}

content/browser/renderer_host/input/touch_action_browsertest.cc

@@ -319,6 +319,42 @@ class TouchActionBrowserTest : public ContentBrowserTest {
                       expected_scroll_position_after_scroll);
   }
 
+  void DoTwoFingerPan() {
+    DCHECK(URLLoaded());
+
+    const std::string pointer_actions_json = R"HTML(
+        [{"source": "touch", "id": 0,
+              "actions": [
+                { "name": "pointerDown", "x": 10, "y": 125 },
+                { "name": "pointerMove", "x": 10, "y": 155 },
+                { "name": "pointerUp" }]},
+             {"source": "touch", "id": 1,
+              "actions": [
+                { "name": "pointerDown", "x": 15, "y": 125 },
+                { "name": "pointerMove", "x": 15, "y": 155 },
+                { "name": "pointerUp"}]}]
+        )HTML";
+
+    base::JSONReader json_reader;
+    std::unique_ptr<base::Value> params =
+        json_reader.ReadToValue(pointer_actions_json);
+    ASSERT_TRUE(params.get()) << json_reader.GetErrorMessage();
+    ActionsParser actions_parser(params.get());
+
+    ASSERT_TRUE(actions_parser.ParsePointerActionSequence());
+
+    run_loop_ = std::make_unique<base::RunLoop>();
+
+    GetWidgetHost()->QueueSyntheticGesture(
+        SyntheticGesture::Create(actions_parser.gesture_params()),
+        base::BindOnce(&TouchActionBrowserTest::OnSyntheticGestureCompleted,
+                       base::Unretained(this)));
+
+    // Runs until we get the OnSyntheticGestureCompleted callback
+    run_loop_->Run();
+    run_loop_.reset();
+  }
+
   // Generate touch events for a double tap and drag zoom gesture at
   // coordinates (50, 50).
   void DoDoubleTapDragZoom() {
@@ -358,7 +394,6 @@ class TouchActionBrowserTest : public ContentBrowserTest {
     run_loop_.reset();
   }
 
- private:
   void CheckScrollOffset(
       bool wait_until_scrolled,
       const gfx::Vector2d& expected_scroll_position_after_scroll) {
@@ -390,6 +425,7 @@ class TouchActionBrowserTest : public ContentBrowserTest {
     EXPECT_LE(expected_scroll_position_after_scroll.x() / 2, scroll_left);
   }
 
+ private:
   std::unique_ptr<RenderFrameSubmissionObserver> frame_observer_;
   std::unique_ptr<base::RunLoop> run_loop_;
 
@@ -574,6 +610,21 @@ IN_PROC_BROWSER_TEST_F(TouchActionBrowserTest,
                 gfx::Vector2d(45, 0), kShortJankTime);
 }
 
+// TODO(crbug.com/899005): Make this test work on Android.
+#if defined(OS_ANDROID)
+#define MAYBE_TwoFingerPanYDisallowed DISABLED_TwoFingerPanYDisallowed
+#else
+#define MAYBE_TwoFingerPanYDisallowed TwoFingerPanYDisallowed
+#endif
+// Test that two finger panning is treated as pinch zoom and is disallowed when
+// touching the pan-y area.
+IN_PROC_BROWSER_TEST_F(TouchActionBrowserTest, MAYBE_TwoFingerPanYDisallowed) {
+  LoadURL(kTouchActionURLWithOverlapArea);
+
+  DoTwoFingerPan();
+  CheckScrollOffset(true, gfx::Vector2d(0, 0));
+}
+
 namespace {
 
 const std::string kDoubleTapZoomDataURL = R"HTML(

content/browser/renderer_host/input/touch_action_filter.h

@@ -75,27 +75,31 @@ class CONTENT_EXPORT TouchActionFilter {
   friend class TouchActionFilterPinchTest;
   friend class SitePerProcessBrowserTouchActionTest;
 
-  bool ShouldSuppressManipulation(const blink::WebGestureEvent&);
-  bool FilterManipulationEventAndResetState();
+  bool ShouldSuppressScrolling(const blink::WebGestureEvent&);
+  FilterGestureEventResult FilterScrollEventAndResetState();
+  FilterGestureEventResult FilterPinchEventAndResetState();
   void ReportTouchAction();
   void ResetTouchAction();
   void SetTouchAction(cc::TouchAction touch_action);
 
-  // Whether scroll and pinch gestures should be discarded due to touch-action.
-  bool suppress_manipulation_events_;
+  // Whether scroll gestures should be discarded due to touch-action.
+  bool drop_scroll_events_ = false;
+
+  // Whether pinch gestures should be discarded due to touch-action.
+  bool drop_pinch_events_ = false;
 
   // Whether a tap ending event in this sequence should be discarded because a
   // previous GestureTapUnconfirmed event was turned into a GestureTap.
-  bool drop_current_tap_ending_event_;
+  bool drop_current_tap_ending_event_ = false;
 
   // True iff the touch action of the last TapUnconfirmed or Tap event was
   // TOUCH_ACTION_AUTO. The double tap event depends on the touch action of the
   // previous tap or tap unconfirmed. Only valid between a TapUnconfirmed or Tap
   // and the next DoubleTap.
-  bool allow_current_double_tap_event_;
+  bool allow_current_double_tap_event_ = true;
 
   // Force enable zoom for Accessibility.
-  bool force_enable_zoom_;
+  bool force_enable_zoom_ = false;
 
   // Indicates whether this page has touch event handler or not. Set by
   // InputRouterImpl::OnHasTouchEventHandlers. Default to false because one
@@ -107,20 +111,17 @@ class CONTENT_EXPORT TouchActionFilter {
   // before GSE.
   bool gesture_sequence_in_progress_ = false;
 
-  // True if we're between a GSB and a GSE.
-  bool gesture_scroll_in_progress_ = false;
-
   // Increment at receiving ACK for touch start and decrement at touch end.
   int num_of_active_touches_ = 0;
 
   // What touch actions are currently permitted.
   base::Optional<cc::TouchAction> allowed_touch_action_;
 
-  // The touch action that is used for the current scrolling gesture sequence.
-  // At the touch sequence end, the |allowed_touch_action| is reset while this
-  // remains set as the effective touch action, for the still in progress scroll
+  // The touch action that is used for the current gesture sequence. At the
+  // touch sequence end, the |allowed_touch_action_| is reset while this remains
+  // set as the effective touch action, for the still in progress gesture
   // sequence due to fling.
-  base::Optional<cc::TouchAction> scrolling_touch_action_;
+  base::Optional<cc::TouchAction> active_touch_action_;
 
   // Whitelisted touch action received from the compositor.
   base::Optional<cc::TouchAction> white_listed_touch_action_;

content/browser/renderer_host/input/touch_action_filter_unittest.cc

@@ -27,7 +27,7 @@ class TouchActionFilterTest : public testing::Test {
 
  protected:
   base::Optional<cc::TouchAction> ScrollingTouchAction() const {
-    return filter_.scrolling_touch_action_;
+    return filter_.active_touch_action_;
   }
   void ResetTouchAction() { filter_.ResetTouchAction(); }
   void PanTest(cc::TouchAction action,
@@ -635,18 +635,8 @@ class TouchActionFilterPinchTest : public testing::Test {
               FilterGestureEventResult::kFilterGestureEventAllowed);
     EXPECT_EQ(filter_.FilterGestureEvent(&pinch_end),
               FilterGestureEventResult::kFilterGestureEventAllowed);
-    EXPECT_EQ(filter_.FilterGestureEvent(&scroll_end),
-              FilterGestureEventResult::kFilterGestureEventAllowed);
-    filter_.DecreaseActiveTouches();
-
-    // Pinch state is automatically reset at the end of a scroll.
-    filter_.ResetTouchAction();
-    filter_.OnSetTouchAction(cc::kTouchActionAuto);
-    filter_.IncreaseActiveTouches();
-    EXPECT_EQ(filter_.FilterGestureEvent(&tap_down),
-              FilterGestureEventResult::kFilterGestureEventAllowed);
-    EXPECT_EQ(filter_.FilterGestureEvent(&scroll_begin),
-              FilterGestureEventResult::kFilterGestureEventAllowed);
+    EXPECT_FALSE(filter_.drop_pinch_events_);
+    // The pinch gesture is always re-evaluated on pinch begin.
     EXPECT_EQ(filter_.FilterGestureEvent(&pinch_begin),
               FilterGestureEventResult::kFilterGestureEventAllowed);
     EXPECT_EQ(filter_.FilterGestureEvent(&pinch_update),
@@ -657,7 +647,7 @@ class TouchActionFilterPinchTest : public testing::Test {
               FilterGestureEventResult::kFilterGestureEventAllowed);
     filter_.DecreaseActiveTouches();
 
-    // Pinching is only computed at GestureScrollBegin time.
+    // Pinch state is automatically reset at the end of a scroll.
     filter_.ResetTouchAction();
     filter_.OnSetTouchAction(cc::kTouchActionAuto);
     filter_.IncreaseActiveTouches();
@@ -665,20 +655,6 @@ class TouchActionFilterPinchTest : public testing::Test {
               FilterGestureEventResult::kFilterGestureEventAllowed);
     EXPECT_EQ(filter_.FilterGestureEvent(&scroll_begin),
               FilterGestureEventResult::kFilterGestureEventAllowed);
-    EXPECT_EQ(filter_.FilterGestureEvent(&pinch_begin),
-              FilterGestureEventResult::kFilterGestureEventAllowed);
-    EXPECT_EQ(filter_.FilterGestureEvent(&pinch_update),
-              FilterGestureEventResult::kFilterGestureEventAllowed);
-    EXPECT_EQ(filter_.FilterGestureEvent(&pinch_end),
-              FilterGestureEventResult::kFilterGestureEventAllowed);
-    filter_.OnSetTouchAction(cc::kTouchActionNone);
-    EXPECT_EQ(filter_.FilterGestureEvent(&pinch_begin),
-              FilterGestureEventResult::kFilterGestureEventAllowed);
-    EXPECT_EQ(filter_.FilterGestureEvent(&pinch_update),
-              FilterGestureEventResult::kFilterGestureEventAllowed);
-    EXPECT_EQ(filter_.FilterGestureEvent(&pinch_end),
-              FilterGestureEventResult::kFilterGestureEventAllowed);
-    filter_.OnSetTouchAction(cc::kTouchActionAuto);
     EXPECT_EQ(filter_.FilterGestureEvent(&pinch_begin),
               FilterGestureEventResult::kFilterGestureEventAllowed);
     EXPECT_EQ(filter_.FilterGestureEvent(&pinch_update),
@@ -689,30 +665,15 @@ class TouchActionFilterPinchTest : public testing::Test {
               FilterGestureEventResult::kFilterGestureEventAllowed);
     filter_.DecreaseActiveTouches();
 
-    // Once a pinch has started, any change in state won't affect the pinch
-    // gestures since it is computed in GestureScrollBegin.
     filter_.ResetTouchAction();
     filter_.OnSetTouchAction(cc::kTouchActionAuto);
     filter_.IncreaseActiveTouches();
-    EXPECT_EQ(filter_.FilterGestureEvent(&tap_down),
-              FilterGestureEventResult::kFilterGestureEventAllowed);
-    EXPECT_EQ(filter_.FilterGestureEvent(&scroll_begin),
-              FilterGestureEventResult::kFilterGestureEventAllowed);
-    EXPECT_EQ(filter_.FilterGestureEvent(&pinch_begin),
-              FilterGestureEventResult::kFilterGestureEventAllowed);
-    filter_.OnSetTouchAction(cc::kTouchActionNone);
-    EXPECT_EQ(filter_.FilterGestureEvent(&pinch_update),
-              FilterGestureEventResult::kFilterGestureEventAllowed);
-    EXPECT_EQ(filter_.FilterGestureEvent(&pinch_end),
-              FilterGestureEventResult::kFilterGestureEventAllowed);
     EXPECT_EQ(filter_.FilterGestureEvent(&pinch_begin),
               FilterGestureEventResult::kFilterGestureEventAllowed);
     EXPECT_EQ(filter_.FilterGestureEvent(&pinch_update),
               FilterGestureEventResult::kFilterGestureEventAllowed);
     EXPECT_EQ(filter_.FilterGestureEvent(&pinch_end),
               FilterGestureEventResult::kFilterGestureEventAllowed);
-    EXPECT_EQ(filter_.FilterGestureEvent(&scroll_end),
-              FilterGestureEventResult::kFilterGestureEventAllowed);
     filter_.DecreaseActiveTouches();
 
     // Scrolling is allowed when two fingers are down.
@@ -733,8 +694,9 @@ class TouchActionFilterPinchTest : public testing::Test {
               FilterGestureEventResult::kFilterGestureEventAllowed);
     filter_.DecreaseActiveTouches();
 
-    // A pinch event sequence with only one pointer is equivalent to a scroll
-    // gesture, so disallowed as a pinch gesture.
+    // At double-tap-drag-zoom case, the pointer_count is 1 at GesturePinchBegin
+    // and we need to evaluate whether the gesture is allowed or not at that
+    // time.
     scroll_begin.data.scroll_begin.pointer_count = 1;
     filter_.ResetTouchAction();
     filter_.OnSetTouchAction(cc::kTouchActionPinchZoom);
@@ -744,11 +706,11 @@ class TouchActionFilterPinchTest : public testing::Test {
     EXPECT_EQ(filter_.FilterGestureEvent(&scroll_begin),
               FilterGestureEventResult::kFilterGestureEventFiltered);
     EXPECT_EQ(filter_.FilterGestureEvent(&pinch_begin),
-              FilterGestureEventResult::kFilterGestureEventFiltered);
+              FilterGestureEventResult::kFilterGestureEventAllowed);
     EXPECT_EQ(filter_.FilterGestureEvent(&pinch_update),
-              FilterGestureEventResult::kFilterGestureEventFiltered);
+              FilterGestureEventResult::kFilterGestureEventAllowed);
     EXPECT_EQ(filter_.FilterGestureEvent(&pinch_end),
-              FilterGestureEventResult::kFilterGestureEventFiltered);
+              FilterGestureEventResult::kFilterGestureEventAllowed);
     EXPECT_EQ(filter_.FilterGestureEvent(&scroll_end),
               FilterGestureEventResult::kFilterGestureEventFiltered);
     filter_.DecreaseActiveTouches();
@@ -1235,6 +1197,46 @@ TEST_F(TouchActionFilterTest, TapDownWithZeroNumOfActiveTouches) {
   EXPECT_EQ(ScrollingTouchAction().value(), cc::kTouchActionAuto);
 }
 
+// Regression test for crbug.com/771330. One can start one finger panning y, and
+// add another finger to pinch zooming. The pinch zooming should not be allowed
+// if the allowed touch action doesn't allow it.
+TEST_F(TouchActionFilterTest, PinchZoomStartsWithOneFingerPanDisallowed) {
+  filter_.OnHasTouchEventHandlers(true);
+  filter_.OnSetTouchAction(cc::kTouchActionPanY);
+  WebGestureEvent scroll_begin =
+      SyntheticWebGestureEventBuilder::BuildScrollBegin(0, 3, kSourceDevice);
+  WebGestureEvent scroll_update =
+      SyntheticWebGestureEventBuilder::BuildScrollUpdate(5, 10, 0,
+                                                         kSourceDevice);
+  WebGestureEvent pinch_begin = SyntheticWebGestureEventBuilder::Build(
+      WebInputEvent::kGesturePinchBegin, kSourceDevice);
+  WebGestureEvent pinch_update =
+      SyntheticWebGestureEventBuilder::BuildPinchUpdate(1.2f, 5, 5, 0,
+                                                        kSourceDevice);
+  WebGestureEvent pinch_end = SyntheticWebGestureEventBuilder::Build(
+      WebInputEvent::kGesturePinchEnd, kSourceDevice);
+  WebGestureEvent scroll_end = SyntheticWebGestureEventBuilder::Build(
+      WebInputEvent::kGestureScrollEnd, kSourceDevice);
+  EXPECT_EQ(filter_.FilterGestureEvent(&scroll_begin),
+            FilterGestureEventResult::kFilterGestureEventAllowed);
+  EXPECT_EQ(filter_.FilterGestureEvent(&scroll_update),
+            FilterGestureEventResult::kFilterGestureEventAllowed);
+  EXPECT_EQ(filter_.FilterGestureEvent(&pinch_begin),
+            FilterGestureEventResult::kFilterGestureEventFiltered);
+  EXPECT_EQ(filter_.FilterGestureEvent(&pinch_update),
+            FilterGestureEventResult::kFilterGestureEventFiltered);
+  EXPECT_EQ(filter_.FilterGestureEvent(&pinch_end),
+            FilterGestureEventResult::kFilterGestureEventFiltered);
+  EXPECT_EQ(filter_.FilterGestureEvent(&pinch_begin),
+            FilterGestureEventResult::kFilterGestureEventFiltered);
+  EXPECT_EQ(filter_.FilterGestureEvent(&pinch_update),
+            FilterGestureEventResult::kFilterGestureEventFiltered);
+  EXPECT_EQ(filter_.FilterGestureEvent(&pinch_end),
+            FilterGestureEventResult::kFilterGestureEventFiltered);
+  EXPECT_EQ(filter_.FilterGestureEvent(&scroll_end),
+            FilterGestureEventResult::kFilterGestureEventAllowed);
+}
+
 TEST_F(TouchActionFilterTest, ScrollBeginWithoutTapDownWithKnownTouchAction) {
   filter_.OnHasTouchEventHandlers(true);
   EXPECT_FALSE(ScrollingTouchAction().has_value());