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

Issue 835058 link

Starred by 3 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug



Sign in to add a comment

SitePerProcessHitTestBrowserTest.RootConsumesScrollDuringOverscrollGesture/1 seems flaky

Project Member Reported by dmu...@chromium.org, Apr 20 2018

Issue description

Comment 1 by dmu...@chromium.org, Apr 20 2018

Labels: -OS-Linux OS-Android

Comment 2 by nasko@chromium.org, Apr 20 2018

Cc: lukasza@chromium.org creis@chromium.org nasko@chromium.org
Owner: wjmaclean@chromium.org
Assigning to wjmaclean@, for triage since this is related to scrolling.
Cc: wjmaclean@chromium.org
Owner: mcnee@chromium.org
mcnee@ - Could you please take a look?

Comment 4 by mcnee@chromium.org, Apr 25 2018

Cc: riajiang@chromium.org
Early in the test we have a sanity check to make sure the point we're using is in the child. In the /1 and /2 versions where we have viz hit testing, we sometimes don't get the child as the target:
../../content/browser/site_per_process_hit_test_browsertest.cc:1047: Failure
Expected equality of these values:
  rwhv_child->GetRenderWidgetHost()
    Which is: 0x7fb0cd7400
  router->GetRenderWidgetHostAtPoint(rwhv_root, point_in_child, &dont_care)
    Which is: 0x7fb0cfb200

Since GetRenderWidgetHostAtPoint is just using the synchronous result, I'm not sure why this would be inconsistent. Perhaps there's another condition we need to wait for before viz hit testing's synchronous result is correct, that wasn't necessary for the non-viz hit testing?

Comment 5 by mcnee@chromium.org, Apr 25 2018

Issue 835481 has been merged into this issue.

Comment 6 by mcnee@chromium.org, Apr 25 2018

Issue 824824 has been merged into this issue.

Comment 7 by mcnee@chromium.org, Apr 25 2018

Labels: -Pri-3 Pri-1
From the duplicates, this looks rather disruptive. I'll go ahead and disable this case.
Cc: fsam...@chromium.org
Any chance this is an issue with the child surface not being ready at the time we do the hit-test? We've had issues with surface hit testing not working on Android prior to this.
Project Member

Comment 9 by bugdroid1@chromium.org, Apr 25 2018

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

commit 239890fc4e6367f7c48ee8719c15ebbf27321ea8
Author: Kevin McNee <mcnee@chromium.org>
Date: Wed Apr 25 23:02:32 2018

Disable RootConsumesScrollDuringOverscrollGesture on android with viz hit testing

This test is flaky on android when viz hit testing is enabled.

Bug: 835058
Change-Id: Ia746468e3595806ca01df494c95fc97ab2927602
Reviewed-on: https://chromium-review.googlesource.com/1028975
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Commit-Queue: Kevin McNee <mcnee@chromium.org>
Cr-Commit-Position: refs/heads/master@{#553802}
[modify] https://crrev.com/239890fc4e6367f7c48ee8719c15ebbf27321ea8/content/browser/site_per_process_hit_test_browsertest.cc

Cc: jonr...@chromium.org mcnee@chromium.org
Owner: riajiang@chromium.org
It is possible that Viz host doesn't have the updated hit-test data yet; WaitForChildFrameSurfaceReady doesn't wait for the hit-test data to arrive at the Viz host end. We are planning to add Viz hit-test testing API to replace  WaitForChildFrameSurfaceReady, will revisit this bug when that's done.

mcnee@, thanks for disabling the test!
Is there a timeline for the replacement for WaitForChildFrameSurfaceReady() ?

This has been a real challenge with OOPIF-relevant tests, and it would be nice to not have a stable solution. :-)
I'm not sure about the timeline in non-viz-hit-test; my understanding is that we want to replace it in both viz-hit-test and non-viz-hit-test tho, jonross@ would know for sure. Viz hit-test is in finch for M67, so I think the earliest to actually replace WaitForChildFrameSurfaceReady would be after M67
Once HitTestQuery (browser process) receives all confirmations of hit tests regions for both classic and Viz, then we'll be able to build an appropriate hit test testing api.

Which will allow all these tests to replace WaitForChildFrameSurfaceReady.

That currently sounds like post M67
Project Member

Comment 14 by bugdroid1@chromium.org, Jul 26

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

commit 8976f940cfd7d12d31012f26c4d416d5b238f587
Author: Ria Jiang <riajiang@chromium.org>
Date: Thu Jul 26 19:23:09 2018

Tentatively enable some SitePerProcessHitTestBrowserTest flaky tests.

RootConsumesScrollDuringOverscrollGesture,
InputEventRouterTouchpadGestureTargetTest and
TouchpadPinchOverOOPIF were flaky tests with Viz hit-testing config.
However, looking at the log, they only happened infrequently on Android
and Windows. I couldn't repro these locally and there have been some
changes with how we wait for hit-test data and setting initial window
bounds, so tentatively re-enable these three tests to see whether they
are still flaky or not.

Bug: 853761, 835058, 838835
Change-Id: I66ae6162ab9065e4a940c5779adb92254a6bf54c
Reviewed-on: https://chromium-review.googlesource.com/1151899
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Commit-Queue: Ria Jiang <riajiang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#578398}
[modify] https://crrev.com/8976f940cfd7d12d31012f26c4d416d5b238f587/content/browser/site_per_process_hit_test_browsertest.cc

Status: Assigned (was: Untriaged)
This bug has an owner, thus, it's been triaged. Changing status to "assigned".

Sign in to add a comment