Issue metadata
Sign in to add a comment
|
BrowserSideFlingBrowserTest.InertialGSUBubblingStopsWhenParentCannotScroll is flaky |
||||||||||||||||||||||||
Issue descriptionFindit identified the culprit r613979 as introducing flaky test(s) summarized in https://findit-for-me.appspot.com/waterfall/flake/flake-culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyQwsSDEZsYWtlQ3VscHJpdCIxY2hyb21pdW0vNzNhNTllMDVjNTU2ZDliZjNmOWFjMTZkZmM1ZjExODYyZjA4YWI0Ygw Please revert the culprit, or disable the test(s) and find the appropriate owner to fix or delete. 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%20culprit%20r613979&comment=Link%20to%20Culprit%3A%20https://findit-for-me.appspot.com/waterfall/flake/flake-culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyQwsSDEZsYWtlQ3VscHJpdCIxY2hyb21pdW0vNzNhNTllMDVjNTU2ZDliZjNmOWFjMTZkZmM1ZjExODYyZjA4YWI0Ygw Automatically posted by the findit-for-me app (https://goo.gl/Ot9f7N).
,
Dec 5
,
Dec 5
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5f44ca54503e75becb625da8004e933a368c6d58 commit 5f44ca54503e75becb625da8004e933a368c6d58 Author: Roger McFarlane <rogerm@chromium.org> Date: Wed Dec 05 21:37:04 2018 Revert "Set VizDisplayCompositor feature enabled on desktop platforms." This reverts commit 73a59e05c556d9bf3f9ac16dfc5f11862f08ab4b. Reason for revert: crbug.com/912289 Findit identified the culprit r613979 as introducing flaky test(s) summarized in https://findit-for-me.appspot.com/waterfall/flake/flake-culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyQwsSDEZsYWtlQ3VscHJpdCIxY2hyb21pdW0vNzNhNTllMDVjNTU2ZDliZjNmOWFjMTZkZmM1ZjExODYyZjA4YWI0Ygw Original change's description: > Set VizDisplayCompositor feature enabled on desktop platforms. > > In preparation for launching OOP-D on stable switch the > VizDisplayCompositor feature from disabled to enabled by default. This > is following with the finch best practices. > > OOP-D is still disabled by default on Android and Chrome OS. Android > should get switched over shortly after fixing a few failing tests. > Chrome OS isn't ready to launch yet. > > Bug: 730193 > Change-Id: I6f55d40492291321d13925dd685181bdfda9f3d2 > Reviewed-on: https://chromium-review.googlesource.com/c/1361930 > Reviewed-by: Saman Sami <samans@chromium.org> > Commit-Queue: kylechar <kylechar@chromium.org> > Cr-Commit-Position: refs/heads/master@{#613979} TBR=kylechar@chromium.org,samans@chromium.org Change-Id: I55c67764c69656bdbe4dbb58051a8f94e9f9d579 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 730193 Reviewed-on: https://chromium-review.googlesource.com/c/1363808 Reviewed-by: Roger McFarlane <rogerm@chromium.org> Commit-Queue: Roger McFarlane <rogerm@chromium.org> Cr-Commit-Position: refs/heads/master@{#614107} [modify] https://crrev.com/5f44ca54503e75becb625da8004e933a368c6d58/components/viz/common/features.cc
,
Dec 5
,
Dec 6
This flake is on Win 10, in content_browsertests We haven't seen this on the FYI bots. Failure Stack: ./../content/browser/renderer_host/input/fling_browsertest.cc(496): error: Value of: router->forced_last_fling_start_target_to_stop_flinging_for_test() Actual: false Expected: true Stack trace: Backtrace: testing::internal::UnitTestImpl::CurrentOsStackTraceExceptTop [0x00007FF656A9E897+87] testing::internal::AssertHelper::operator= [0x00007FF656A9E41E+78] content::BrowserSideFlingBrowserTest_InertialGSUBubblingStopsWhenParentCannotScroll_Test::RunTestOnMainThread [0x00007FF6560C0144+1044] content::BrowserTestBase::ProxyRunTestOnMainThreadLoop [0x00007FF657D0128D+445] content::ShellBrowserMainParts::PreMainMessageLoopRun [0x00007FF65861A794+68] content::BrowserMainLoop::PreMainMessageLoopRun [0x00007FF656D77C1E+62] content::StartupTaskRunner::RunAllTasksNow [0x00007FF6570B61DB+43] content::BrowserMainLoop::CreateStartupTasks [0x00007FF656D76A47+599] content::BrowserMainRunnerImpl::Initialize [0x00007FF656D79DAB+107] ShellBrowserMain [0x00007FF65A867395+21] content::ShellMainDelegate::RunProcess [0x00007FF65A8658AC+188] content::RunBrowserProcessMain [0x00007FF656CA3D49+89] content::ContentMainRunnerImpl::RunServiceManager [0x00007FF656CA467B+219] content::ContentMainRunnerImpl::Run [0x00007FF656CA456E+238] service_manager::Main [0x00007FF65827925A+554] content::ContentMain [0x00007FF656CA3C8E+62] content::BrowserTestBase::SetUp [0x00007FF657D00F94+1796]
,
Dec 6
,
Dec 11
,
Dec 11
The flake reproduces on Linux too but only if you are't running with xvfb.
$ ./content_browsertests --enable-features=VizDisplayCompositor --gtest_filter=BrowserSideFlingBrowserTest.InertialGSUBubblingStopsWhenParentCannotScroll
I looked at some traces and RenderWidgetHostInputEventRouter::BubbleScrollEvent() isn't getting called when it fails. The passing runs have a call stack that looks basically like this during gesture event dispatch:
SyntheticGestureController::Flush()
InputRouterImpl::SendGestureEventImmediately()
InputRouterImpl::FilterAndSendWebInputEvent()
InputRouterImpl::GestureEventHandled()
RenderWidgetHostInputEventRouter::BubbleScrollEvent()
The failing runs have a call stack that basically looks like this:
SyntheticGestureController::Flush()
InputRouterImpl::SendGestureEventImmediately()
InputRouterImpl::FilterAndSendWebInputEvent()
InputRouterImpl::GestureEventHandled()
The place where passing and failing tests diverge is at [1]. When the test passes filtered_state=NO_CONSUMER_EXISTS on the first try, which seems to then find a different target and send the event to it. When the test fails filtered_state=NOT_CONSUMED and the event gets sent and InputRouterImpl::GestureEventHandled() is called asynchronously. This event dispatch call stack repeats a couple of times with the same behaviour for both passing and failing runs. The failing runs never had RenderWidgetHostInputEventRouter::BubbleScrollEvent() called so they don't set |forced_last_fling_start_target_to_stop_flinging_for_test_| true in RenderWidgetHostInputEventRouter::StopFling().
sahel: Does this sound right to you? Should filtered_state=NO_CONSUMER_EXISTS on the first try for dispatch for the test to pass?
[1] https://cs.chromium.org/chromium/src/content/browser/renderer_host/input/input_router_impl.cc?l=494&rcl=02725453c39524386610ac4eba1e970b22bd9d6f
[2] https://cs.chromium.org/chromium/src/content/browser/renderer_host/input/fling_browsertest.cc?l=495&rcl=30fbb99c379e3aa5cd9db30649fdf370681ae334
,
Dec 11
It looks like the test is failing because the gesture events aren't dispatched to the OOPIF in the first place. If I make the test waiting 500ms before dispatching the event then it passes consistently.
,
Dec 13
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8a1eb21672a188efe4acb847f47072c8717469f4 commit 8a1eb21672a188efe4acb847f47072c8717469f4 Author: kylechar <kylechar@chromium.org> Date: Thu Dec 13 01:17:28 2018 Wait for updated hit test data in test. Add functionality to InertialGSUBubblingStopsWhenParentCannotScroll test so that it waits for updated hit test data to arrive after scrolling the renderer. The test waits for confirmation the scroll happened, however it doesn't wait for confirmation that hit test data is updated which is also required. The test was flaky after enabling OOP-D because hit test data had the OOPIF in the wrong spot and bubble up behaviour wasn't triggered. This happens because hit test data aggregation is moved into the GPU process and can arrive later. Add HitTestTransformChangeObserver to make this possible. The class waits for some change in the root to target transform for a FrameSinkId. Bug: 912289 Change-Id: I9c8b7221e86114b3692f1616a389b6cdf70ed97b Reviewed-on: https://chromium-review.googlesource.com/c/1374109 Commit-Queue: kylechar <kylechar@chromium.org> Reviewed-by: Scott Violet <sky@chromium.org> Reviewed-by: Navid Zolghadr <nzolghadr@chromium.org> Reviewed-by: Jonathan Ross <jonross@chromium.org> Cr-Commit-Position: refs/heads/master@{#616142} [modify] https://crrev.com/8a1eb21672a188efe4acb847f47072c8717469f4/content/browser/renderer_host/input/fling_browsertest.cc [modify] https://crrev.com/8a1eb21672a188efe4acb847f47072c8717469f4/content/public/test/hit_test_region_observer.cc [modify] https://crrev.com/8a1eb21672a188efe4acb847f47072c8717469f4/content/public/test/hit_test_region_observer.h
,
Dec 13
Bots look green |
|||||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||||
Comment 1 by dtapu...@chromium.org
, Dec 5