New issue
Advanced search Search tips

Issue 892036 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 5
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug
Flaky-Test: telemetry.internal.actions.scroll_unittest.ScrollActionTest.testScrollDistanceSlowWheel

Blocking:
issue 783034



Sign in to add a comment

telemetry.internal.actions.scroll_unittest.ScrollActionTest.testScrollDistanceSlowWheel is flaky

Project Member Reported by Findit, Oct 4

Issue description

Components: Blink>Scroll
Cc: sadrul@chromium.org hu...@vewd.com
Owner: sadrul@chromium.org
Status: Assigned (was: Untriaged)
Suspects:

https://chromium-review.googlesource.com/c/chromium/src/+/1255152
https://chromium-review.googlesource.com/c/chromium/src/+/1257832

Sadrul, Hugo, can you PTAL?
My change is completely unrelated. I'd suspect Sadrul's change...
This is failing pretty consistently on Builders Linux Tests and linux-xenial-rel.

Example failure:
Traceback (most recent call last):
      File "/b/s/w/ir/third_party/catapult/telemetry/telemetry/testing/browser_test_case.py", line 39, in WrappedMethod
        method(self)
      File "/b/s/w/ir/third_party/catapult/telemetry/telemetry/internal/actions/scroll_unittest.py", line 115, in testScrollDistanceSlowWheel
        1000, 300, page_action.GESTURE_SOURCE_MOUSE, 100)
      File "/b/s/w/ir/third_party/catapult/telemetry/telemetry/internal/actions/scroll_unittest.py", line 79, in _RunScrollDistanceTest
        self.assertAlmostEqual(distance, actual, delta=maxError + urlBarError)
    AssertionError: 1000 != 739 within 250 delta

I would also suspect the high-frequency synthetic gesture dispatch CL as the culprit.
Status: Started (was: Assigned)
Yep, I believe it's mine. I think the test is flaky ... the expectation seems suspect. I will bump up the error in the test.
What does "bump up the error" mean? Any objections to just reverting https://chromium-review.googlesource.com/c/chromium/src/+/1255152 (that was deemed the cause of these errors, right?) until you figure out what the problem is?
Cc: vmi...@chromium.org crouleau@chromium.org
 Issue 892235  has been merged into this issue.
Project Member

Comment 8 by bugdroid1@chromium.org, Oct 4

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

commit ae26a2e41edaa334d60048b48c496062a4bb99c1
Author: Marijn Kruisselbrink <mek@chromium.org>
Date: Thu Oct 04 17:55:42 2018

Revert "synthetic gesture: Allow high-frequency dispatch."

This reverts commit 581a728b2665c85e466408c2d53371abcb7f763e.

Reason for revert: Causing significant test flakiness in telemetry.internal.actions.scroll_unittest.ScrollActionTest.testScrollDistanceSlowWheel.

Original change's description:
> synthetic gesture: Allow high-frequency dispatch.
> 
> For synthesized scroll/drag/pinch gestures, dispatch the events
> at a high frequency (120Hz) so that there's at least one event in
> each frame. Dispatching events at a lower frequency (60Hz) means
> the timer can go out of sync with begin-frame, and so there can
> be frames where there was to event dispatch. Our telemetry code
> reports this as jank, which is incorrect. Dispatching at a higher
> frequency for the scroll/drag/pinch gestures resolves this issue.
> 
> BUG= 783034 
> 
> Change-Id: I1102f76ed743231cbb10fdb872e8c66f895fa223
> Reviewed-on: https://chromium-review.googlesource.com/c/1255152
> Reviewed-by: Victor Miura <vmiura@chromium.org>
> Reviewed-by: Navid Zolghadr <nzolghadr@chromium.org>
> Reviewed-by: Lan Wei <lanwei@chromium.org>
> Commit-Queue: Sadrul Chowdhury <sadrul@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#596507}

TBR=sadrul@chromium.org,vmiura@chromium.org,lanwei@chromium.org,nzolghadr@chromium.org

Change-Id: I1e4d64dfc3b98a547c38f8f82a2dcd45141fd66b
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  783034 ,  892036 
Reviewed-on: https://chromium-review.googlesource.com/c/1261972
Reviewed-by: Marijn Kruisselbrink <mek@chromium.org>
Commit-Queue: Marijn Kruisselbrink <mek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#596748}
[modify] https://crrev.com/ae26a2e41edaa334d60048b48c496062a4bb99c1/content/browser/renderer_host/input/synthetic_gesture.cc
[modify] https://crrev.com/ae26a2e41edaa334d60048b48c496062a4bb99c1/content/browser/renderer_host/input/synthetic_gesture.h
[modify] https://crrev.com/ae26a2e41edaa334d60048b48c496062a4bb99c1/content/browser/renderer_host/input/synthetic_gesture_controller.cc
[modify] https://crrev.com/ae26a2e41edaa334d60048b48c496062a4bb99c1/content/browser/renderer_host/input/synthetic_gesture_controller.h
[modify] https://crrev.com/ae26a2e41edaa334d60048b48c496062a4bb99c1/content/browser/renderer_host/input/synthetic_pointer_action.cc
[modify] https://crrev.com/ae26a2e41edaa334d60048b48c496062a4bb99c1/content/browser/renderer_host/input/synthetic_pointer_action.h
[modify] https://crrev.com/ae26a2e41edaa334d60048b48c496062a4bb99c1/content/browser/renderer_host/input/synthetic_tap_gesture.cc
[modify] https://crrev.com/ae26a2e41edaa334d60048b48c496062a4bb99c1/content/browser/renderer_host/input/synthetic_tap_gesture.h

Revering the CL
Oh, thank you mek@ for taking care of the revert.
Labels: -Sheriff-Chromium
Project Member

Comment 12 by bugdroid1@chromium.org, Oct 5

The following revision refers to this bug:
  https://chromium.googlesource.com/catapult/+/de52d9ad295ecc6401de6d7cc3b468a0e8292308

commit de52d9ad295ecc6401de6d7cc3b468a0e8292308
Author: Sadrul Habib Chowdhury <sadrul@chromium.org>
Date: Fri Oct 05 13:16:25 2018

scroll: Allow for more errors.

Changing the frequency of scroll-event dispatch mean there are more
events being generated, and thus being queued/coalesced. So it is
possible for the scroll to not have reached the goal by the time the
test terminates. So allow for a little more distance from the target
for the slow-wheel unit-test.

BUG= chromium:892036 

Change-Id: Ifc7a6f79c51a15800fcd702d076237a4ef764049
Reviewed-on: https://chromium-review.googlesource.com/c/1264175
Reviewed-by: Ned Nguyen <nednguyen@google.com>
Commit-Queue: Sadrul Chowdhury <sadrul@chromium.org>

[modify] https://crrev.com/de52d9ad295ecc6401de6d7cc3b468a0e8292308/telemetry/telemetry/internal/actions/scroll_unittest.py

Project Member

Comment 13 by bugdroid1@chromium.org, Oct 5

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

commit a4ac489299429d98c31e8b63ec13d9a248c5866b
Author: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com>
Date: Fri Oct 05 14:19:13 2018

Roll src/third_party/catapult 56216d7832a4..de52d9ad295e (1 commits)

https://chromium.googlesource.com/catapult.git/+log/56216d7832a4..de52d9ad295e


git log 56216d7832a4..de52d9ad295e --date=short --no-merges --format='%ad %ae %s'
2018-10-05 sadrul@chromium.org scroll: Allow for more errors.


Created with:
  gclient setdep -r src/third_party/catapult@de52d9ad295e

The AutoRoll server is located here: https://autoroll.skia.org/r/catapult-autoroll

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.

CQ_INCLUDE_TRYBOTS=luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel

BUG= chromium:892036 
TBR=sullivan@chromium.org

Change-Id: Ic8478361b73397bcad961ca65dc35681cf52910f
Reviewed-on: https://chromium-review.googlesource.com/c/1264625
Reviewed-by: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com>
Commit-Queue: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#597104}
[modify] https://crrev.com/a4ac489299429d98c31e8b63ec13d9a248c5866b/DEPS

Status: Fixed (was: Started)
Blocking: 783034

Sign in to add a comment