New issue
Advanced search Search tips

Issue 873211 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Aug 15
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Set touch action Auto in MaybeSendSyntheticTapGesture

Project Member Reported by xidac...@chromium.org, Aug 10

Issue description

The MaybeSendSyntheticTapGesture can be called in PreProcessMouseEvent and PreprocessTouchEvent,
in which case the TouchActionFilter::allowed_touch_action_ has no value
and will result in a crash.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Aug 15

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

commit ec552f22953973871cea1e625cf6b366c8c36345
Author: Xida Chen <xidachen@chromium.org>
Date: Wed Aug 15 20:03:21 2018

Set touch action Auto in MaybeSendSyntheticTapGesture

The MaybeSendSyntheticTapGesture can be called in PreProcessMouseEvent,
in which case the TouchActionFilter::allowed_touch_action_ has no value
and will result in a crash.

This CL set the allowed_touch_action_ to auto to fix the issue. A
browser test is added.

Bug:  873211 
Change-Id: If776a93fe2c8f7a311a10bc0fec42cda15d0ae9a
Reviewed-on: https://chromium-review.googlesource.com/1169585
Commit-Queue: Xida Chen <xidachen@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Reviewed-by: Timothy Dresser <tdresser@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583369}
[modify] https://crrev.com/ec552f22953973871cea1e625cf6b366c8c36345/chrome/browser/chrome_site_per_process_browsertest.cc
[modify] https://crrev.com/ec552f22953973871cea1e625cf6b366c8c36345/content/browser/frame_host/render_widget_host_view_guest.cc
[modify] https://crrev.com/ec552f22953973871cea1e625cf6b366c8c36345/content/browser/frame_host/render_widget_host_view_guest.h
[modify] https://crrev.com/ec552f22953973871cea1e625cf6b366c8c36345/content/public/test/browser_test_utils.cc
[modify] https://crrev.com/ec552f22953973871cea1e625cf6b366c8c36345/content/public/test/browser_test_utils.h

Status: Fixed (was: Assigned)
Project Member

Comment 3 by bugdroid1@chromium.org, Aug 15

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

commit d8376c83f55c9dffb695a6f8f2b49cc4c563a0f9
Author: Findit <findit-for-me@appspot.gserviceaccount.com>
Date: Wed Aug 15 23:19:53 2018

Revert "Set touch action Auto in MaybeSendSyntheticTapGesture"

This reverts commit ec552f22953973871cea1e625cf6b366c8c36345.

Reason for revert:

Findit (https://goo.gl/kROfz5) identified CL at revision 583369 as the
culprit for flakes in the build cycles as shown on:
https://findit-for-me.appspot.com/waterfall/flake/flake-culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyQwsSDEZsYWtlQ3VscHJpdCIxY2hyb21pdW0vZWM1NTJmMjI5NTM5NzM4NzFjZWExZTYyNWNmNmIzNjZjOGMzNjM0NQw

Sample Failed Build: https://ci.chromium.org/buildbot/tryserver.chromium.chromiumos/linux-chromeos-rel/71344

Sample Failed Step: mash_browser_tests (with patch)

Sample Flaky Test: ChromeSitePerProcessPDFTest.SendSyntheticTapGestureOOPIF

Original change's description:
> Set touch action Auto in MaybeSendSyntheticTapGesture
> 
> The MaybeSendSyntheticTapGesture can be called in PreProcessMouseEvent,
> in which case the TouchActionFilter::allowed_touch_action_ has no value
> and will result in a crash.
> 
> This CL set the allowed_touch_action_ to auto to fix the issue. A
> browser test is added.
> 
> Bug:  873211 
> Change-Id: If776a93fe2c8f7a311a10bc0fec42cda15d0ae9a
> Reviewed-on: https://chromium-review.googlesource.com/1169585
> Commit-Queue: Xida Chen <xidachen@chromium.org>
> Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
> Reviewed-by: Timothy Dresser <tdresser@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#583369}

Change-Id: I2b4c4fb6b76a7a904603e7bf1086fe4f42d9f1e3
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  873211 ,  874667 
Reviewed-on: https://chromium-review.googlesource.com/1176503
Cr-Commit-Position: refs/heads/master@{#583438}
[modify] https://crrev.com/d8376c83f55c9dffb695a6f8f2b49cc4c563a0f9/chrome/browser/chrome_site_per_process_browsertest.cc
[modify] https://crrev.com/d8376c83f55c9dffb695a6f8f2b49cc4c563a0f9/content/browser/frame_host/render_widget_host_view_guest.cc
[modify] https://crrev.com/d8376c83f55c9dffb695a6f8f2b49cc4c563a0f9/content/browser/frame_host/render_widget_host_view_guest.h
[modify] https://crrev.com/d8376c83f55c9dffb695a6f8f2b49cc4c563a0f9/content/public/test/browser_test_utils.cc
[modify] https://crrev.com/d8376c83f55c9dffb695a6f8f2b49cc4c563a0f9/content/public/test/browser_test_utils.h

Project Member

Comment 4 by bugdroid1@chromium.org, Aug 17

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

commit 9658adb66240d3575b3c885f6e675da0f36ce25a
Author: Xida Chen <xidachen@chromium.org>
Date: Fri Aug 17 18:38:30 2018

Reland: Set touch action Auto in MaybeSendSyntheticTapGesture

Previously the CL was reverted because it hits a DCHECK in the test.
The reason is that in the test, the guest web contents is ready, but it
is not yet attached to the render widget host view.

This CL fixes the problem by waiting for at most of 200ms for the guest
web contents to be attached. I have ran this locally for more than
20 times and there is no failure.

Patchset 1 is the same as the CL that was reverted, so that it is
easier to review

Bug:  873211 
Change-Id: I5c36da5be5d812bbd5f1964605fc6697166a2539
Reviewed-on: https://chromium-review.googlesource.com/1177884
Commit-Queue: Xida Chen <xidachen@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584136}
[modify] https://crrev.com/9658adb66240d3575b3c885f6e675da0f36ce25a/chrome/browser/chrome_site_per_process_browsertest.cc
[modify] https://crrev.com/9658adb66240d3575b3c885f6e675da0f36ce25a/content/browser/frame_host/render_widget_host_view_guest.cc
[modify] https://crrev.com/9658adb66240d3575b3c885f6e675da0f36ce25a/content/browser/frame_host/render_widget_host_view_guest.h
[modify] https://crrev.com/9658adb66240d3575b3c885f6e675da0f36ce25a/content/public/test/browser_test_utils.cc
[modify] https://crrev.com/9658adb66240d3575b3c885f6e675da0f36ce25a/content/public/test/browser_test_utils.h

Sign in to add a comment