By logging the green div has the correct effective touch action and TouchActionFilter::ShouldSuppressManipulation is true when processing the pinch zoom event.

Given that Site Isolation is aiming for M67 launch and this is a blocker, it will be good for someone that knows what the next steps are to take ownership of the bug. Having an unowned blocker 2 weeks before branch point seems problematic.

Also, for repro steps, is the expected behavior that pinch zoom started on top of the green div will disable it completely or that the full page will zoom? I'm observing the latter.

It should disable it completely - as is the case without site isolation.

Another question - is this Android specific or does it apply to other operating systems on touch enabled devices?

Regarding comment #4, the behavior I observe on Clank Canary is identical with Site Isolation enabled or disabled. In both cases, if I put two fingers on top of the green area and move them apart to zoom, the whole page zooms.

I can repro this on a Chromebook Pixel (67.0.3381.0 dev channel).  The pinch zoom works over the green box when Site Isolation is enabled, and not when it is disabled.  Presumably it affects any platform with touchscreens.

sunxd@ or flackr@: Can you help us get this fixed in M67 before Site Isolation gets turned on by default?  Thanks!

The effective touch action of the green div is computed correctly, but looks like the pinch-zoom still bypass the touch action filter.

GesturePinch always goes to the root_view in RenderWidgetHostInputEventRouter.

However, by the time any GesturePinch events are being generated, we should have hit-tested and know which RenderWidgetHostImpl the gestures would normally have gone to (TouchActionFilter lives in InputRouterImpl), so I should think it wouldn't be too difficult to surface the desired action to RWHIER and act accordingly?

From logging, I see that the calls to TouchActionFilter::OnSetTouchAction and TouchActionFilter::FilterGestureEvent are made to different TouchActionFilters. So it looks like the TouchActionFilter corresponding to the child is informed of the touch action, but the TouchActionFilter for the root is not. So when we try to filter the pinch in the root, it lets it through.

Based on #9 and #10, I'm guessing that the pinch event is sent to root surface and is allowed by the root? And the correct behavior should be that event is thrown away as long as it hits a touch-action restricted region?

Yes, so what really needs to happen is that in RWHIER, prior to sending the GesturePinch to the root view, we should check first and give the *hit-tested* target a chance to cancel it (or take whatever other action is appropriate) first. Again, since we know the hit-tested target, we can talk to the TAF for it and see what should be done.

sunxd@: Can you post an update on your progress?  Branch cut is tomorrow-- is a fix on track to land?  Thanks!

Fix is ready writing the test now.

Do we know of a website in the wild that would be affected by this bug?  Maybe Google Maps?

Re c#15: It looks like AMP pages use 'touch-action: pan-y;' on the document element in the OOPIF.

Comment 16: Thanks-- good to know.  That said, AMP is essentially mobile specific and we aren't launching on Android yet.

sunxd@: Any progess on the test for this?  Branch cut was last week, so please try to land a fix ASAP so that it can be merged to M67.

M67 Stable promotion is coming soon. Your bug is labelled as Stable ReleaseBlock, pls make sure to land the fix and request a merge into the release branch ASAP. Thank you.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1767e4163a9955bf94ef6bf1ed055c7899259a4f

commit 1767e4163a9955bf94ef6bf1ed055c7899259a4f
Author: sunxd <sunxd@chromium.org>
Date: Wed Apr 25 22:01:08 2018

Make RWHIER drop pinch gesture event if target does not support it

RenderWidgetHostInputEventRouter sends all pinch gesture event to root
frame, but this might be a problem when the subframe has touch action
restrictions.

To avoid allowing pinch zoom when the target element in the subframe has
touch-action:none property, this CL makes RWHIER checks the target frame
and drop the event if touch action filter rejects the pinch.

Bug:  827182 
Change-Id: Ifa566446e8c695d934aad4bcaaef49af0cde76e2
Reviewed-on: https://chromium-review.googlesource.com/1007892
Reviewed-by: James MacLean <wjmaclean@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Commit-Queue: Xianda Sun <sunxd@chromium.org>
Cr-Commit-Position: refs/heads/master@{#553778}
[modify] https://crrev.com/1767e4163a9955bf94ef6bf1ed055c7899259a4f/content/browser/renderer_host/render_widget_host_input_event_router.cc
[modify] https://crrev.com/1767e4163a9955bf94ef6bf1ed055c7899259a4f/content/browser/site_per_process_hit_test_browsertest.cc
[add] https://crrev.com/1767e4163a9955bf94ef6bf1ed055c7899259a4f/content/test/data/div_with_touch_action_none.html
[modify] https://crrev.com/1767e4163a9955bf94ef6bf1ed055c7899259a4f/testing/buildbot/filters/viz.content_browsertests.filter

I think we still need to merge this CL into M67.

This bug requires manual review: M67 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Before we approve merge to M67, please answer followings:

* Is the change well baked/verified in Canary, having enough automation tests coverage and safe to merge to M67?

Please note M67 is already in Beta, so merge bar is very high. Thank you.

I would wait for a few days to have it baked in Canary. It was landed yesterday but does have test coverage. And we need this change to unblock site isolation.

Pls update the bug with canary result on Monday (04/30). If change looks good in canary, I will approve merge to M67 and we can take it in for next week M67 Beta. Thank you.

The NextAction date has arrived: 2018-04-30

sunxd@: Can you verify the change still looks good and request merge to M67?  The deadline for getting the merge in is today at 4 pm Pacific.  Thanks!

Yeah the bug is fixed on TOT and no further bugs received, request to merge into M67.

Approving merge to M67 branch 3396 based on comment #28. Please merge ASAP. Thank you.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/75207565afdfbbc0400176c13f969f421eb59964

commit 75207565afdfbbc0400176c13f969f421eb59964
Author: sunxd <sunxd@chromium.org>
Date: Mon Apr 30 18:15:09 2018

Make RWHIER drop pinch gesture event if target does not support it

RenderWidgetHostInputEventRouter sends all pinch gesture event to root
frame, but this might be a problem when the subframe has touch action
restrictions.

To avoid allowing pinch zoom when the target element in the subframe has
touch-action:none property, this CL makes RWHIER checks the target frame
and drop the event if touch action filter rejects the pinch.

Bug:  827182 
Change-Id: Ifa566446e8c695d934aad4bcaaef49af0cde76e2
Reviewed-on: https://chromium-review.googlesource.com/1007892
Reviewed-by: James MacLean <wjmaclean@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Commit-Queue: Xianda Sun <sunxd@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#553778}(cherry picked from commit 1767e4163a9955bf94ef6bf1ed055c7899259a4f)
Reviewed-on: https://chromium-review.googlesource.com/1035370
Reviewed-by: Xianda Sun <sunxd@chromium.org>
Cr-Commit-Position: refs/branch-heads/3396@{#384}
Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428}
[modify] https://crrev.com/75207565afdfbbc0400176c13f969f421eb59964/content/browser/renderer_host/render_widget_host_input_event_router.cc
[modify] https://crrev.com/75207565afdfbbc0400176c13f969f421eb59964/content/browser/site_per_process_hit_test_browsertest.cc
[add] https://crrev.com/75207565afdfbbc0400176c13f969f421eb59964/content/test/data/div_with_touch_action_none.html
[modify] https://crrev.com/75207565afdfbbc0400176c13f969f421eb59964/testing/buildbot/filters/viz.content_browsertests.filter

Thanks for the merge!  I'll mark this as fixed, but let me know if there's anything else needed in this bug.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f4abba7f807f17f8883e6f641f37dc1ba0948652

commit f4abba7f807f17f8883e6f641f37dc1ba0948652
Author: Kevin McNee <mcnee@chromium.org>
Date: Thu May 03 15:30:57 2018

Prevent touchscreen pinch events from being sent to child renderers

If we pinch an element with a touch-action that prevents pinch in an
OOPIF, we route the pinch events to the child so that the child's
TouchActionFilter filters them. Due to  crbug.com/771330 , the
TouchActionFilter could let the pinch events through. We'll
filter such events in RenderWidgetHostViewChildFrame::FilterInputEvent
to prevent them from being sent to the child renderer.

Bug:  827182 
Change-Id: I3782a2d135a008df271a73d79ecade25dcc9f1a5
Reviewed-on: https://chromium-review.googlesource.com/1040354
Reviewed-by: James MacLean <wjmaclean@chromium.org>
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Commit-Queue: Kevin McNee <mcnee@chromium.org>
Cr-Commit-Position: refs/heads/master@{#555743}
[modify] https://crrev.com/f4abba7f807f17f8883e6f641f37dc1ba0948652/content/browser/renderer_host/render_widget_host_view_child_frame.cc
[modify] https://crrev.com/f4abba7f807f17f8883e6f641f37dc1ba0948652/content/browser/site_per_process_hit_test_browsertest.cc

Is CL listed at #32 need a merge to M67?

Thanks for checking!  Kevin mentioned offline that there's no change to user visible behavior for r555743, so no merge should be required.