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

Issue metadata

Status: Fixed
Owner:
Closed: Jul 20
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug

Blocking:
issue 785986



Sign in to add a comment

SitePerProcessBrowserTest.ViewBoundsInNestedFrameTest Times out in Viz

Project Member Reported by jonr...@chromium.org, Jun 1

Issue description

OS: Linux
Test Suite: content_browsertests
Test: SitePerProcessBrowserTest.ViewBoundsInNestedFrameTest

Background: WaitForChildFrameSurfaceReady, doesn't work in Viz as the surface embedding information is no longer delivered to the browser process.

I'm working on replacing it with two separate APIs. One can listen to frame submissions (RenderFrameSubmissionObserver) the other is in development, and listens to hit test data arriving in the browser process.

While looking at the tests in question I have ran into issues with SitePerProcessBrowserTest.ViewBoundsInNestedFrameTest


While running in Viz (--enable-features=VizDisplayCompositor) this test times out when it calls:

SynchronizeVisualPropertiesMessageFilter::WaitForRect
 
Reproduced today ahead of triage
Cc: kenrb@chromium.org
Owner: fsam...@chromium.org
Status: Assigned (was: Untriaged)
Passing to Fady.
CC kenrb@ FYI
Labels: -Pri-3 Pri-1
This seems to have been fixed.
Project Member

Comment 5 by bugdroid1@chromium.org, Jul 11

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

commit a57a12587f7ace3b15480262c181ced968346ea4
Author: Jonathan Ross <jonross@chromium.org>
Date: Wed Jul 11 20:15:50 2018

Updating Viz Content BrowserTests Filter

A few tests have been fixed. This updates the filter accordingly to re-enable them.

TEST=SitePerProcessBrowserTest.ViewBoundsInNestedFrameTest
SitePerProcessHitTestBrowserTest.TouchpadPinchOverOOPIF
Bug:  848825 ,  848348 

Change-Id: If05696e57dfee18cc6ae06c21f7d968982c31219
Reviewed-on: https://chromium-review.googlesource.com/1133863
Reviewed-by: Jonathan Ross <jonross@chromium.org>
Cr-Commit-Position: refs/heads/master@{#574302}
[modify] https://crrev.com/a57a12587f7ace3b15480262c181ced968346ea4/testing/buildbot/filters/viz.android.content_browsertests.filter
[modify] https://crrev.com/a57a12587f7ace3b15480262c181ced968346ea4/testing/buildbot/filters/viz.content_browsertests.filter

Status: WontFix (was: Assigned)
Status: Assigned (was: WontFix)
I lied, this still fails on Windows:

https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Mojo%20Windows/16448
Project Member

Comment 8 by bugdroid1@chromium.org, Jul 11

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

commit 99b62af5a66207b0fe84080bdd23492471a9ba94
Author: Jonathan Ross <jonross@chromium.org>
Date: Wed Jul 11 22:24:25 2018

Disable SitePerProcessBrowserTest.ViewBoundsInNestedFrameTest on Viz

Apparently this test can still fail on Windows. So re-disabling it until it can be investigated further.

TBR=fsamuel@chromium.org
TEST=SitePerProcessBrowserTest.ViewBoundsInNestedFrameTest

Bug:  848825 
Change-Id: I00828a10cfd5c8c111b30f61eb5d8acbf57da2ea
Reviewed-on: https://chromium-review.googlesource.com/1133875
Reviewed-by: Jonathan Ross <jonross@chromium.org>
Cr-Commit-Position: refs/heads/master@{#574377}
[modify] https://crrev.com/99b62af5a66207b0fe84080bdd23492471a9ba94/testing/buildbot/filters/viz.android.content_browsertests.filter
[modify] https://crrev.com/99b62af5a66207b0fe84080bdd23492471a9ba94/testing/buildbot/filters/viz.content_browsertests.filter

Project Member

Comment 9 by bugdroid1@chromium.org, Jul 12

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

commit e1aa80280d36b8411d252286601369145224f684
Author: kylechar <kylechar@chromium.org>
Date: Thu Jul 12 00:50:02 2018

Revert "Updating Viz Content BrowserTests Filter"

This reverts commit a57a12587f7ace3b15480262c181ced968346ea4.

Reason for revert: SitePerProcessBrowserTest.ViewBoundsInNestedFrameTest is failing.

Original change's description:
> Updating Viz Content BrowserTests Filter
> 
> A few tests have been fixed. This updates the filter accordingly to re-enable them.
> 
> TEST=SitePerProcessBrowserTest.ViewBoundsInNestedFrameTest
> SitePerProcessHitTestBrowserTest.TouchpadPinchOverOOPIF
> Bug:  848825 ,  848348 
> 
> Change-Id: If05696e57dfee18cc6ae06c21f7d968982c31219
> Reviewed-on: https://chromium-review.googlesource.com/1133863
> Reviewed-by: Jonathan Ross <jonross@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#574302}

TBR=jonross@chromium.org,kylechar@chromium.org

Change-Id: Ic6de880ae04a3535c00bfab1cf39e61a335fe43e
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  848825 ,  848348 
Reviewed-on: https://chromium-review.googlesource.com/1134144
Reviewed-by: kylechar <kylechar@chromium.org>
Commit-Queue: kylechar <kylechar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#574434}
[modify] https://crrev.com/e1aa80280d36b8411d252286601369145224f684/testing/buildbot/filters/viz.android.content_browsertests.filter
[modify] https://crrev.com/e1aa80280d36b8411d252286601369145224f684/testing/buildbot/filters/viz.content_browsertests.filter

Project Member

Comment 10 by bugdroid1@chromium.org, Jul 12

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

commit 9e0d4f50a43ad0ec12342ccbd16bf63f571651a7
Author: kylechar <kylechar@chromium.org>
Date: Thu Jul 12 15:43:27 2018

Update viz_content_browsertests filter.

Undo revert in https://crrev.com/c/1134144 which didn't end up being
needed. Revert was in response to FindIt and failures in
SitePerProcessBrowserTest.ViewBoundsInNestedFrameTest but that problem
had already been addressed.

TBR: jonross@chromium.org
Bug:  848825 ,  848348 
Change-Id: Ie00ce45db8b380847dddf4fb099a120ce7ada987
Reviewed-on: https://chromium-review.googlesource.com/1135146
Reviewed-by: kylechar <kylechar@chromium.org>
Reviewed-by: Jonathan Ross <jonross@chromium.org>
Commit-Queue: kylechar <kylechar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#574572}
[modify] https://crrev.com/9e0d4f50a43ad0ec12342ccbd16bf63f571651a7/testing/buildbot/filters/viz.android.content_browsertests.filter
[modify] https://crrev.com/9e0d4f50a43ad0ec12342ccbd16bf63f571651a7/testing/buildbot/filters/viz.content_browsertests.filter

Cc: riajiang@chromium.org
+riajiang@ as this may be impacted by hit testing.

Did a bit of extra digging into the differences here.

In Viz the SynchronizeVisualPropertiesMessageFilter::WaitForRect fails to complete, because the scroll never occurs.

There seems to be a difference in attempting to target the scroll, vs bubbling it.

In LayerTreeHostImpl::ScrollBeginImpl
  Classic: active_tree_->CurrentlyScrollingNode() != ViewportMainScrollNode() so bubbling isn't set. Instead the scroll event is handled. Later on the frame rect is updated
  Viz: active_tree_->CurrentlyScrollingNode() == ViewportMainScrollNode() and Viewport::CanScroll returns false. So bubbling of the event is set, and scrolling then never happens.


Owner: jonr...@chromium.org
Status: Started (was: Assigned)
This is the same root cause as a series of failures that riajiang@ is fixing in SitePerProcessHitTestBrowserTest. The helper method in use to get the page scale factor is using legacy path. Which doesn't work in Viz.

I'll have a patch shortly.
Project Member

Comment 13 by bugdroid1@chromium.org, Jul 19

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

commit 9e9d680e28af98d88320126067072b33588c47d0
Author: jonross <jonross@chromium.org>
Date: Thu Jul 19 21:56:19 2018

Update SitePerProcessBrowserTest Page Scale Factor Reading

SitePerProcessBrowserTest had a helper method to lookup the page scale factor.
This was then being used to direct input events for testing.

However it was using the legacy path of cached CompositorFrameMetadata which
does not work in Viz. Leading to some tests incorrectly targetting the wrong
views.

This updates them to use the newer RenderFrameMetadata path which works both in
classic and Viz modes.

TEST=SitePerProcessBrowserTest.ViewBoundsInNestedFrameTest,
SitePerProcessBrowserTest.ScrollBubblingFromNestedOOPIFTest,
SitePerProcessBrowserTest.ScrollLocalSubframeInOOPIF

Bug:  848825 
Change-Id: Ie8ce1f6144450c31d2673c55099e68e0979ea9bb
Reviewed-on: https://chromium-review.googlesource.com/1141987
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Commit-Queue: Jonathan Ross <jonross@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576650}
[modify] https://crrev.com/9e9d680e28af98d88320126067072b33588c47d0/content/browser/site_per_process_browsertest.cc
[modify] https://crrev.com/9e9d680e28af98d88320126067072b33588c47d0/testing/buildbot/filters/viz.android.content_browsertests.filter
[modify] https://crrev.com/9e9d680e28af98d88320126067072b33588c47d0/testing/buildbot/filters/viz.content_browsertests.filter

Status: Fixed (was: Started)

Sign in to add a comment