Closed Bug 1593170 Opened 5 years ago Closed 5 years ago

make the reftest harness flush layout and update layer trees in fission child oop iframes

Categories

(Testing :: Reftest, task)

Version 3
task
Not set
normal

Tracking

(Fission Milestone:M6, firefox72 fixed)

RESOLVED FIXED
mozilla72
Fission Milestone M6
Tracking Status
firefox72 --- fixed

People

(Reporter: tnikkel, Assigned: tnikkel)

References

Details

Attachments

(8 files, 1 obsolete file)

(deleted), text/x-phabricator-request
Details
(deleted), text/x-phabricator-request
Details
(deleted), text/x-phabricator-request
Details
(deleted), text/x-phabricator-request
Details
(deleted), text/x-phabricator-request
Details
(deleted), text/x-phabricator-request
Details
(deleted), text/x-phabricator-request
Details
(deleted), text/x-phabricator-request
Details
No description provided.
Depends on: 1593171

This changes them to return a promise that resolves when the work is done, but we still need to change the callers to handle this new return type and do the right thing when these functions do their work async-ly.

To do this we add a JSWindowActor called ReftestFission. reftest-content.js communicates with this actor via reftest.jsm.

Depends on D51342

The code comment mostly explains the design. Basically, we force nothing to happen while we wait for the promises to finish and instead record what we need to do once the promise is finished, and do those pending tasks when it's finished.

Depends on D51343

I don't think this is strictly necessary but it lets us avoid a bunch of useless work, especially with webrender where these rects are always the full window size.

Depends on D51345

These tests test that moving around the plugin causes it to still draw correctly. If we don't have the test plugin then they fail and are marked as such.

The tests start by moving the plugin and then waiting for MozAfterPaint or MozPaintWaitFinished. When there is no plugin these events don't come.

Without the test plugin this was working because they got an unrelated MozAfterPaint (it was probably one of the early paints of the page).

After we update the reftest harness for fission and make it more async in general we no longer catch this early MozAfterPaint and the tests don't start.

We should just skip them, there is nothing to test with a plugin.

Depends on D51346

The asserts are

NS_ASSERTION(mDidInitialize, "should have done initial reflow by now");

in

layout/base/PresShell.cpp

in a couple of different places.

The stacks are slightly different from the existing case in bug 566159. In bug 566159 the stacks are as a result of asking to scroll. In this case the stacks are from an autofocus form element. The element is actually in our about:neterror page when the test tries to navigate to "href". The reason they show up now is that the Fission work on the reftest harness, in general, makes things more async. So in this test we previously tore down the page and moved on to the next test before being able to hit these assertions.

Depends on D51347

Attachment #9105692 - Attachment description: Bug 1593170. Make the reftest-content.js functions FlushRendering and SynchronizeForSnapshot work on Fission child oop iframes. r?mattwoodrow → Bug 1593170. Make the reftest-content.js functions FlushRendering and SynchronizeForSnapshot work on Fission child oop iframes. r?mattwoodrow! r?kmag!
Attachment #9105696 - Attachment description: Bug 1593170. Skip plugin-background-*-step.html when we do not have the test plugin. r?mattwoodrow → Bug 1593170. Skip plugin-background-*-step.html when we do not have the test plugin. r=mattwoodrow
Attachment #9105697 - Attachment description: Bug 1593170. Annotate new expected asserts in docshell/base/crashtests/1257730-1.html as a result of Fission work in reftest harness. r?mattwoodrow → Bug 1593170. Annotate new expected asserts in docshell/base/crashtests/1257730-1.html as a result of Fission work in reftest harness. r=mattwoodrow
Attachment #9105692 - Attachment description: Bug 1593170. Make the reftest-content.js functions FlushRendering and SynchronizeForSnapshot work on Fission child oop iframes. r?mattwoodrow! r?kmag! → Bug 1593170. Make the reftest-content.js functions FlushRendering and SynchronizeForSnapshot work on Fission child oop iframes. r=mattwoodrow! r?kmag!
Attachment #9105694 - Attachment description: Bug 1593170. Adjust how we deal with the reftest events for async plugin drawing for the fission changes to the reftest harness. r?mattwoodrow → Bug 1593170. Adjust how we deal with the reftest events for async plugin drawing for the fission changes to the reftest harness. r=mattwoodrow
Attachment #9105695 - Attachment description: Bug 1593170. Avoid useless painting work in queued up rects to paint by ignoring new rects that are already covered. r?mattwoodrow → Bug 1593170. Avoid useless painting work in queued up rects to paint by ignoring new rects that are already covered. r=mattwoodrow

With the fission changes everything is more async, meaning some tests can run longer. Some crashtests navigate to a new location meaning the original root element we have a variable for is no longer around. When we try to access it we get an error saying we can't access a dead object.

Not all of these try catches are probably necessary but most of them are.

Depends on D51346

The changes to make the test harness avoid busy waiting with setTimeout(0)'s made this test fail on Android 8.0 debug webrender. In order to get an active layer the test tweaks a transform slightly that has no visual effect every 74 ms. This is necessary to test the bug as far as I can tell (I wrote the test). The test times out because MakeProgress never makes any progress, there is always an afterpaint pending or an after paint has fired and we need to update the canvas for it. The painting and running through the settimeouts etc of the reftest harness take slightly too long. Before the changes to remove the busy waits we were just barely passing this test, it took 76 seconds in once instance that I checked and hundreds of iterations before we could make progress. Haven't debugged exactly why removing the busywaits makes this fail but it doesn't seem important.

Depends on D52649

Attachment #9108058 - Attachment is obsolete: true
Attachment #9105693 - Attachment description: Bug 1593170. Make the reftest harness deal with SynchronizeForSnapshot and FlushRendering returning promises. r?mattwoodrow → Bug 1593170. Make the reftest harness deal with SynchronizeForSnapshot and FlushRendering returning promises. r=mattwoodrow

With the fission changes everything is more async, meaning some tests can run longer. Some crashtests navigate to a new location meaning the original root element we have a variable for is no longer around, and chrome isn't allowed to keep content nodes alive, so we are left holding a dead wrapper that throws if we try to access anything on it.

So we check if contentRootElement has become a dead wrapper (and null it out) any time we try to access it and it could have become dead. Generally the existing code already handled a null contentRootElement.

Depends on D52828

Attachment #9108059 - Attachment description: Bug 1593170. Skip 1553571-1.html reftest on android debug webrender. r?mattwoodrow → Bug 1593170. Skip 1553571-1.html reftest on android debug webrender. r=mattwoodrow

Tentatively moving all bugs whose summaries mention "Fission" (or other Fission-related keywords) but are not assigned to a Fission Milestone to the "?" triage milestone.

This will generate a lot of bugmail, so you can filter your bugmail for the following UUID and delete them en masse:

0ee3c76a-bc79-4eb2-8d12-05dc0b68e732

Fission Milestone: --- → ?
Attachment #9105692 - Attachment description: Bug 1593170. Make the reftest-content.js functions FlushRendering and SynchronizeForSnapshot work on Fission child oop iframes. r=mattwoodrow! r?kmag! → Bug 1593170. Make the reftest-content.js functions FlushRendering and SynchronizeForSnapshot work on Fission child oop iframes. r=mattwoodrow r=kmag
Attachment #9108378 - Attachment description: Bug 1593170. Check if contentRootElement is a dead wrapper before using it. r?mattwoodrow!,dbaron! → Bug 1593170. Check if contentRootElement is a dead wrapper before using it. r=mattwoodrow r=dbaron
Pushed by tnikkel@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/78d380a2241a Make the reftest-content.js functions FlushRendering and SynchronizeForSnapshot work on Fission child oop iframes. r=mattwoodrow,kmag https://hg.mozilla.org/integration/autoland/rev/a004de649342 Make the reftest harness deal with SynchronizeForSnapshot and FlushRendering returning promises. r=mattwoodrow https://hg.mozilla.org/integration/autoland/rev/80bd8cadf835 Adjust how we deal with the reftest events for async plugin drawing for the fission changes to the reftest harness. r=mattwoodrow https://hg.mozilla.org/integration/autoland/rev/a10072d821e5 Avoid useless painting work in queued up rects to paint by ignoring new rects that are already covered. r=mattwoodrow https://hg.mozilla.org/integration/autoland/rev/d511975b764e Check if contentRootElement is a dead wrapper before using it. r=mattwoodrow,dbaron https://hg.mozilla.org/integration/autoland/rev/2c8e57a90cb8 Skip 1553571-1.html reftest on android debug webrender. r=mattwoodrow https://hg.mozilla.org/integration/autoland/rev/39d63ae4d287 Skip plugin-background-*-step.html when we do not have the test plugin. r=mattwoodrow https://hg.mozilla.org/integration/autoland/rev/2c98625f235b Annotate new expected asserts in docshell/base/crashtests/1257730-1.html as a result of Fission work in reftest harness. r=mattwoodrow
Pushed by tnikkel@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/06f14f474f28 Make the reftest-content.js functions FlushRendering and SynchronizeForSnapshot work on Fission child oop iframes. r=mattwoodrow,kmag https://hg.mozilla.org/integration/autoland/rev/449d87c47593 Make the reftest harness deal with SynchronizeForSnapshot and FlushRendering returning promises. r=mattwoodrow https://hg.mozilla.org/integration/autoland/rev/ddcfa0a1fa00 Adjust how we deal with the reftest events for async plugin drawing for the fission changes to the reftest harness. r=mattwoodrow https://hg.mozilla.org/integration/autoland/rev/285e9108032f Avoid useless painting work in queued up rects to paint by ignoring new rects that are already covered. r=mattwoodrow https://hg.mozilla.org/integration/autoland/rev/0dc7546c4a8a Check if contentRootElement is a dead wrapper before using it. r=mattwoodrow,dbaron https://hg.mozilla.org/integration/autoland/rev/249e26bfefd6 Skip 1553571-1.html reftest on android debug webrender. r=mattwoodrow https://hg.mozilla.org/integration/autoland/rev/cd93c57f87fa Skip plugin-background-*-step.html when we do not have the test plugin. r=mattwoodrow https://hg.mozilla.org/integration/autoland/rev/f663e895f995 Annotate new expected asserts in docshell/base/crashtests/1257730-1.html as a result of Fission work in reftest harness. r=mattwoodrow
Flags: needinfo?(tnikkel)

Blocks enabling Fission in Nightly (M6)

Fission Milestone: ? → M6
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: