1561766 - Support fission in the reftest harness

Status: RESOLVED FIXED

(Testing :: Reftest, task, P2)

Description

[Matt Woodrow (:mattwoodrow)]

The reftest harness currently runs as two scripts, reftest.jsm in the parent process, and reftest-content.js as a frame script in the content process.

reftest.jsm handles loading the test files, taking snapshots, comparing snapshots and recording results.

reftest-content.js handles reftest specific attributes, figuring out when the test is done, flushes layers to the compositor, asks the parent to take snapshots and ends the test.

If we run an iframe in a separate process, then we won't have reftest-content there, so reftest attributes (like reftest-displayport) won't work within the iframe, and we're likely to have race conditions since we don't wait for layers to be sent to the compositor.

We could probably do a partial solution that just solves the race, but it appears that a fair few async scrolling tests really want the displayport attributes inside an iframe.

Given that, it's probably worth trying to run all of reftest-content's logic within OOP iframes.

One way to do this would be have reftest-content.js manage recursive instances of itself, another would be to change reftest.jsm to know how to manage multiple reftest-content.js instances at the same time.

I suspect the latter will be easier, since reftest.jsm already has the logic for handling all of reftest-content.js's output, we'd just need to upgrade it to know how to wait for all the dependencies to be resolved.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

Bugbug thinks this bug is a enhancement, but please change it back in case of error.

Comment 2

[Matt Woodrow (:mattwoodrow)]

kats, looks like you wrote all the FissionTestHelper stuff, do you have any ideas/suggestions on the best way to make this work?

Comment 3

[Kartikaya Gupta (email:[e-mail])]

I'm going to defer to Nika here, since she guided me through writing the FissionTestHelper stuff. The underlying mechanism there is the WindowActor API which allows the parent process to communicate with any content process reliably in a Fission world. It looks like the reftest harness currently uses the messagemanager API, which Nika said wasn't as reliable with Fission. So most likely we'll want to migrate the reftest harness to also use the WindowActor API which will allow the parent to talk to all the content processes and coordinate everything.

Nika, do you have any particular thoughts on this?

Comment 4

[Nika Layzell [:nika] (ni? for response)]

(In reply to Kartikaya Gupta (email:[e-mail]) (away 17-Jul-2019 to Feb-2020) from comment #3)

I'm going to defer to Nika here, since she guided me through writing the FissionTestHelper stuff. The underlying mechanism there is the WindowActor API which allows the parent process to communicate with any content process reliably in a Fission world. It looks like the reftest harness currently uses the messagemanager API, which Nika said wasn't as reliable with Fission. So most likely we'll want to migrate the reftest harness to also use the WindowActor API which will allow the parent to talk to all the content processes and coordinate everything.

Nika, do you have any particular thoughts on this?

If the RefTest harness is currently using MessageManager-based APIs, we'll want to migrate them to using the JSWindowActor-based APIs for communication, yes. In general IPC stuff which is not per-process should probably be migrated over if it's in JS code.

Comment 5

[Chris Peterson [:cpeterson]]

Roll unfixed test bugs from Fission Milestone M4 to M4.1

Comment 6

[Timothy Nikkel (:tnikkel)]

I landed bug 1593170 which concludes the work I'm planning on this for the immediate future. So un-assigning from myself. I would mostly call this bug fixed depending on what you want to track with this bug. Fission is now supported in the reftest harness. Bug 1585803 causes reftest fission tests to be orange, but it doesn't seem to be a harness issue. The only other blocker from reftest (related tests) from being green with fission is a crash during crashttests, but that seems like a general fission issue and nothing with the reftest harness (and I think it regressed at some point, but I'd have to look back at my try pushes).

So depending on what you want to track with this bug it is either fixed or depending on those two issues being resolved.

Comment 7

[Chris Peterson [:cpeterson]]

(In reply to Timothy Nikkel (:tnikkel) from comment #6)

So depending on what you want to track with this bug it is either fixed or depending on those two issues being resolved.

SGTM! I'll resolve this bug as fixed because we're now running reftests. We're tracking individual failures (like ClearType bug 1585803) or crashes in other bugs.