New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 910586 link

Starred by 1 user

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug
Flaky-Test: SitePerProcessBrowserTouchActionTest.EffectiveTouchActionPropagatesAcrossNestedFrames



Sign in to add a comment

SitePerProcessBrowserTouchActionTest.EffectiveTouchActionPropagatesAcrossNestedFrames is flaky

Project Member Reported by Findit, Nov 30

Issue description


Flaky test: SitePerProcessBrowserTouchActionTest.EffectiveTouchActionPropagatesAcrossNestedFrames
Sample failed build due to flakiness: https://ci.chromium.org/buildbot/chromium.chromiumos/linux-chromeos-rel/16688
Test output log: https://chromium-swarm.appspot.com/task?id=417d6933b8607e10
Culprit (70.0% confidence): r608637
Analysis: https://findit-for-me.appspot.com/waterfall/flake?key=ag9zfmZpbmRpdC1mb3ItbWVy7QELEhdNYXN0ZXJGbGFrZUFuYWx5c2lzUm9vdCK2AWNocm9taXVtLmNocm9taXVtb3MvbGludXgtY2hyb21lb3MtcmVsLzE2Njg4L2NvbnRlbnRfYnJvd3NlcnRlc3RzL1UybDBaVkJsY2xCeWIyTmxjM05DY205M2MyVnlWRzkxWTJoQlkzUnBiMjVVWlhOMExrVm1abVZqZEdsMlpWUnZkV05vUVdOMGFXOXVVSEp2Y0dGbllYUmxjMEZqY205emMwNWxjM1JsWkVaeVlXMWxjdz09DAsSE01hc3RlckZsYWtlQW5hbHlzaXMYAQw

Please revert the culprit, or disable the test and find the appropriate owner.

If the culprit above is wrong, please file a bug using this link:
https://bugs.chromium.org/p/chromium/issues/entry?status=Unconfirmed&labels=Pri-1,Test-Findit-Wrong&components=Tools%3ETest%3EFindit%3EFlakiness&summary=%5BFindit%5D%20Flake%20Analyzer%20-%20Wrong%20result%20for%20SitePerProcessBrowserTouchActionTest.EffectiveTouchActionPropagatesAcrossNestedFrames&comment=Link%20to%20Analysis%3A%20https://findit-for-me.appspot.com/waterfall/flake?key=ag9zfmZpbmRpdC1mb3ItbWVy7QELEhdNYXN0ZXJGbGFrZUFuYWx5c2lzUm9vdCK2AWNocm9taXVtLmNocm9taXVtb3MvbGludXgtY2hyb21lb3MtcmVsLzE2Njg4L2NvbnRlbnRfYnJvd3NlcnRlc3RzL1UybDBaVkJsY2xCeWIyTmxjM05DY205M2MyVnlWRzkxWTJoQlkzUnBiMjVVWlhOMExrVm1abVZqZEdsMlpWUnZkV05vUVdOMGFXOXVVSEp2Y0dGbllYUmxjMEZqY205emMwNWxjM1JsWkVaeVlXMWxjdz09DAsSE01hc3RlckZsYWtlQW5hbHlzaXMYAQw

Automatically posted by the findit-for-me app (https://goo.gl/Ot9f7N).
 
Components: Blink>Input
Cc: riajiang@chromium.org
Owner: hirosh...@chromium.org
Status: Started (was: Untriaged)
Started investigation.

The culprit looks like somehow related -- but not sure.
Couldn't found any other culprits. I'll revert the culprit.
According riajiang@: all tests have been running with VizHitTestDrawQuad turned on since May https://chromium-review.googlesource.com/c/chromium/src/+/1053907

Canceled the revert. Still no idea for other culprits though.
Error message:

../../content/browser/site_per_process_browsertest.cc:12769: Failure
Expected equality of these values:
  expected_touch_action
    Which is: 15
  effective_touch_action.has_value() ? effective_touch_action.value() : cc::kTouchActionAuto
    Which is: 63

Cc: holte@chromium.org
It seems fieldtrial_testing_config.json is not applied to content_shell-based tests, including content_browsertests.
So the CL mentioned in Comment #5 didn't have effects on the test.

So resuming the revert and will see whether it fixes the flakiness.
Project Member

Comment 9 by bugdroid1@chromium.org, Nov 30

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

commit 7f3ec0c539215eb28a0d2e602a6a1aa619117d17
Author: Hiroshige Hayashizaki <hiroshige@chromium.org>
Date: Fri Nov 30 21:12:42 2018

Revert "Enable VizHitTestDrawQuad feature by default on TOT for cros."

This reverts commit b5edc37e917603f09aa5d5dc0672c74c3e7308d2.

Reason for revert: suspected to cause flaky failures in
SitePerProcessBrowserTouchActionTest.EffectiveTouchActionPropagatesAcrossNestedFrames
While https://chromium-review.googlesource.com/1053907 enabled
the VizHitTestDrawQuad feature in fieldtrial_testing_config.json,
it doesn't seem to be applied to content_browsertests,
so this CL actually enabled that feature in this test and
possibly caused the flakiness. See crbug/910586.

Original change's description:
> Enable VizHitTestDrawQuad feature by default on TOT for cros.
>
> The OOPIF bug on cros was fixed
> https://bugs.chromium.org/p/chromium/issues/detail?id=877762,
> re-enabling VizHitTestDrawQuad by default for cros platforms.
>
> Bug: 804888
> Change-Id: I93cd237bc0631b6e1f77ae59232937697ce45ef2
> Reviewed-on: https://chromium-review.googlesource.com/c/1337644
> Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
> Commit-Queue: Ria Jiang <riajiang@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#608637}

TBR=sadrul@chromium.org,riajiang@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 804888, 910586
Change-Id: Ic42c16a2b0547878e1e794069f2057f18a74b337
Reviewed-on: https://chromium-review.googlesource.com/c/1356887
Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
Reviewed-by: Hiroshige Hayashizaki <hiroshige@chromium.org>
Cr-Commit-Position: refs/heads/master@{#612784}
[modify] https://crrev.com/7f3ec0c539215eb28a0d2e602a6a1aa619117d17/components/viz/common/features.cc

Status: Fixed (was: Started)
Looks recovered (Analysis result on r612810 shows 100% success). Closing as fixed.
Labels: Merge-Request-72
Requesting merge to M-72 as the original CL landed on M-72 and probably the revert lands on M-73.
Pls apply appropriate OSs label. Thank you.
Labels: OS-Chrome
That CL only enables the feature on ChromeOS tho, so shouldn't affect other platforms (as mentioned in c#6)
Project Member

Comment 15 by sheriffbot@chromium.org, Dec 1

Labels: -Merge-Request-72 Merge-Review-72 Hotlist-Merge-Review
This bug requires manual review: Reverts referenced in bugdroid comments after merge request.
Please contact the milestone owner if you have questions.
Owners: govind@(Android), kariahda@(iOS), djmm@(ChromeOS), abdulsyed@(Desktop)

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

Agree.
Possibly the test has been flaky on all platforms other than ChromeOS since before r608637 because the feature has been ON (and r608637 made ChromeOS flaky as well by switching the feature ON), but I'm not sure.

I reverted the CL and requested the merge to M-72 in a semi-mechanical fashion, to determine the CL caused the flakiness and to fix the alert on the sheriff dashboard.
But it's also fine for example not to merge the revert, deflake the test and reland r608637 instead if you decide to do so and the test flakiness is not so critical (on M-72) that the merge is required.

Feel free to take over this issue, as I expect you are more familiar with the code and test and can do the right decision.
Project Member

Comment 17 by bugdroid1@chromium.org, Dec 3

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

commit ac2fc0c4395278924528e0fa0bdf64e45749c1c8
Author: Hiroshige Hayashizaki <hiroshige@chromium.org>
Date: Mon Dec 03 19:20:50 2018

README: fieldtrial_testing_config.json is not applied to content_browsertests

Source: go/fieldtrial-testing-config#introduction (Google internal)

Bug: 910586
Change-Id: I9bdd750a5712437b952a1220ce5b175dfe4bb21c
Reviewed-on: https://chromium-review.googlesource.com/c/1357541
Reviewed-by: Steven Holte <holte@chromium.org>
Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
Cr-Commit-Position: refs/heads/master@{#613188}
[modify] https://crrev.com/ac2fc0c4395278924528e0fa0bdf64e45749c1c8/testing/variations/README.md

Labels: M-72
Labels: -Hotlist-Merge-Review -Merge-Review-72 Merge-Approved-72
Project Member

Comment 20 by sheriffbot@chromium.org, Dec 10

Cc: djmm@google.com
This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

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

Comment 21 by sheriffbot@chromium.org, Dec 14

This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Cc: -riajiang@chromium.org hirosh...@chromium.org
Labels: -Sheriff-Chromium -Merge-Approved-72
Owner: riajiang@chromium.org
Status: Assigned (was: Fixed)
As per comment #16 and chat with riajiang@, we've decided not to merge the CL to M-72, and reopen this issue and assign to riajian@ to fix the existing flakiness (probably since before the reverted CL).

I removed Sheriff-Chromium flag, as currently the test shouldn't be flaky on ChromeOS, as the original CL was reverted and not relanded on ToT.
FYI The test becomes flaky again recently.
(according to today's Sheriff's comment in
https://chromium-review.googlesource.com/c/chromium/src/+/1356887)
I'm not sure how it was handled though.
Project Member

Comment 24 by bugdroid1@chromium.org, Jan 2

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

commit 7c160fbfde6b590d67e94bf3164067b53d48b48d
Author: Ria Jiang <riajiang@chromium.org>
Date: Wed Jan 02 16:53:20 2019

Fix a flaky SitePerProcessBrowserTouchActionTest test.

SitePerProcessBrowserTouchActionTest.
EffectiveTouchActionPropagatesAcrossNestedFrames is flaky on all
platforms. This is a tentative fix to that test. Ran locally 1k times.

Bug: 910586
Change-Id: Ie3da974058b56de5243cea5d66771b760ce206d6
Reviewed-on: https://chromium-review.googlesource.com/c/1379404
Commit-Queue: Ria Jiang <riajiang@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Reviewed-by: Xianda Sun <sunxd@chromium.org>
Cr-Commit-Position: refs/heads/master@{#619420}
[modify] https://crrev.com/7c160fbfde6b590d67e94bf3164067b53d48b48d/content/browser/site_per_process_browsertest.cc

Sign in to add a comment