New issue
Advanced search Search tips

Issue 912289 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Dec 13
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 1
Type: Bug
Flaky-Test: BrowserSideFlingBrowserTest.InertialGSUBubblingStopsWhenParentCannotScroll



Sign in to add a comment

BrowserSideFlingBrowserTest.InertialGSUBubblingStopsWhenParentCannotScroll is flaky

Project Member Reported by Findit, Dec 5

Issue description

Components: Blink>Input
Owner: kylec...@chromium.org
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
Labels: -Sheriff-Chromium
Cc: jonr...@chromium.org
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]

Labels: OS-Windows
Status: Started (was: Untriaged)
Cc: sahel@chromium.org riajiang@chromium.org
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
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.
Project Member

Comment 10 by bugdroid1@chromium.org, 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

Status: Fixed (was: Started)
Bots look green

Sign in to add a comment