1648444 - Refactor reftest navigation code to use JSWindowActor

Status: RESOLVED FIXED

(Remote Protocol :: Marionette, task, P1)

Description

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Reftests in Marionette have their own logic in waiting for the page being loaded. It can be found at:

https://searchfox.org/mozilla-central/rev/a87a1c3b543475276e6d57a7a80cb02f3e42b6ed/testing/marionette/listener.js#1767-1801

Once bug 1612831 is fixes and all the navigation related code lives in the parent process, we should also move this logic, and maybe combine with the existent page load observing code.

Comment 1

[Chris Peterson [:cpeterson]]

Tracking for Fission M6b Nightly milestone.

Comment 2

[Neha Kochar [:neha]]

Henrik, what features does this enable? We need to assess Fission impact.

Comment 3

[James Graham [:jgraham]]

This feature is the implementation of wpt reftests. It currently works in fission, but the whole of marionette is being refactored to use the actor model, so this bug tracks that change.

Comment 4

[Henrik Skupin [:whimboo][⌚️UTC+1]]

I will have to work on that bug once bug 1612831 has been fixed. Hopefully this will be done soon.

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+1]]

The original reftest harnesses uses a JSWindowActor pair right now. Maybe we can just re-use or adapt it to our needs:

https://searchfox.org/mozilla-central/source/layout/tools/reftest/ReftestFissionParent.jsm
https://searchfox.org/mozilla-central/source/layout/tools/reftest/ReftestFissionChild.jsm

Comment 6

[Chris Peterson [:cpeterson]]

Moving marionette-fission-mvp bugs from Fission Nightly Experiment milestone (M6b) to Fission Beta milestone (M7).

Comment 7

[Julian Descottes [:jdescottes]]

Attachment: Bug 1648444 - [marionette] Use JSWindowActors for marionette reftests navigation

The current logic for reftest navigation is handled fully in the content process, in listener.js
The goal is to leverage the parent process navigation logic added to navigate.js in Bug 1612831.
A JSWindowActor is added to handle the reftest specific part.
Most importantly, waiting for the "reftest-wait" classname to be removed from the document element.

Comment 8

[Julian Descottes [:jdescottes]]

Attachment: Bug 1648444 - [marionette] Add expectedUrl optional parameter to waitForNavigationCompleted helper

Depends on D92566

waitForNavigationCompleted might incorrectly resolve for an intermediary about:blank load.
Allow the caller to specify the expected URL to avoid resolving too early.

Comment 9

[Julian Descottes [:jdescottes]]

Attachment: Bug 1648444 - [marionette] Use JSWindowActors for marionette reftests navigation

This is an alternative to D92566, which is closer to a 1:1 port of the reftest navigation logic from listener.js
There are still some differences, mainly caused by the fact that JSWindowActors follow the same lifecycle as the window global.

Comment 10

[Julian Descottes [:jdescottes]]

Attachment: Bug 1648444 - [marionette] Create MarionetteReftest actor on load event

Depends on D92648

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cbae6d44d4da
[marionette] Use JSWindowActors for marionette reftests navigation r=marionette-reviewers,whimboo,jgraham

Comment 12

[Sandor Molnar[:smolnar]]

Backed out for causing test_reftest.py failures

Backout link: https://hg.mozilla.org/integration/autoland/rev/4ecfcfc48960719aaa954895f5bbc609fe785c11

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&searchStr=mn&revision=cbae6d44d4da91c174774414ba4b04d114ab814b

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=318582968&repo=autoland&lineNumber=46942

INFO - 1602674179754 Marionette DEBUG 7 -> [0,8,"WebDriver:ExecuteScript",{"script":"Components.utils.import("resource://gre/modules/Preferences.jsm");\n\n ... e,false],"filename":"../../venv/lib/python2.7/site-packages/marionette_driver/marionette.py","sandbox":"default","line":770}]
[task 2020-10-14T11:16:19.758Z] 11:16:19 INFO - 1602674179755 Marionette DEBUG 7 <- [1,8,null,{"value":null}]
[task 2020-10-14T11:16:19.759Z] 11:16:19 INFO - 1602674179757 Marionette DEBUG 7 -> [0,9,"Marionette:SetContext",{"value":"content"}]
[task 2020-10-14T11:16:19.759Z] 11:16:19 INFO - 1602674179757 Marionette DEBUG 7 <- [1,9,null,{"value":null}]
[task 2020-10-14T11:16:19.775Z] 11:16:19 INFO - 1602674179760 Marionette DEBUG 7 -> [0,10,"reftest:setup",{"screenshot":"unexpected"}]
[task 2020-10-14T11:16:19.775Z] 11:16:19 INFO - 1602674179760 Marionette DEBUG 7 <- [1,10,{"error":"unknown error","message":"NotSupportedError: ChromeUtils.registerWindowActor: 'MarionetteReftestFrame' actor ... t@chrome://marionette/content/server.js:241:9\n_onJSONObjectReady/<@chrome://marionette/content/transport.js:504:20\n"},null]
[task 2020-10-14T11:16:19.775Z] 11:16:19 INFO - Marionette threw an error: NotSupportedError: ChromeUtils.registerWindowActor: 'MarionetteReftestFrame' actor is already registered.
[task 2020-10-14T11:16:19.775Z] 11:16:19 INFO - setup@chrome://marionette/content/reftest.js:111:17
[task 2020-10-14T11:16:19.776Z] 11:16:19 INFO - GeckoDriver.prototype.setupReftest@chrome://marionette/content/driver.js:3742:17
[task 2020-10-14T11:16:19.776Z] 11:16:19 INFO - despatch@chrome://marionette/content/server.js:297:40
[task 2020-10-14T11:16:19.776Z] 11:16:19 INFO - execute@chrome://marionette/content/server.js:267:16
[task 2020-10-14T11:16:19.776Z] 11:16:19 INFO - onPacket/<@chrome://marionette/content/server.js:240:20
[task 2020-10-14T11:16:19.776Z] 11:16:19 INFO - onPacket@chrome://marionette/content/server.js:241:9
[task 2020-10-14T11:16:19.776Z] 11:16:19 INFO - _onJSONObjectReady/<@chrome://marionette/content/transport.js:504:20
[task 2020-10-14T11:16:19.776Z] 11:16:19 INFO - 1602674179762 Marionette DEBUG 7 -> [0,11,"Marionette:GetContext",{}]
[task 2020-10-14T11:16:19.776Z] 11:16:19 INFO - 1602674179762 Marionette DEBUG 7 <- [1,11,null,{"value":"content"}]
[task 2020-10-14T11:16:19.776Z] 11:16:19 INFO - 1602674179763 Marionette DEBUG 7 -> [0,12,"Marionette:SetContext",{"value":"chrome"}]
[task 2020-10-14T11:16:19.776Z] 11:16:19 INFO - 1602674179763 Marionette DEBUG 7 <- [1,12,null,{"value":null}]
[task 2020-10-14T11:16:19.799Z] 11:16:19 INFO - 1602674179769 Marionette DEBUG 7 -> [0,13,"WebDriver:TakeScreenshot",{"full":true,"hash":false,"id":null,"scroll":true}]
[task 2020-10-14T11:16:19.844Z] 11:16:19 INFO - 1602674179830 Marionette DEBUG 7 <- [1,13,null,{"value":"iVBORw0KGgoAAAANSUhEUgAABQAAAAQmCAYAAABvZhJoAAAgAElEQVR4nOzde3SV9Z0v/v79W3ZmzunM/E5nOjO/1XZsZ82cGXpcnUWn ... ACAAAAwJgABAAAAIAxAQgAAAAAYwIQAAAAAMYEIAAAAACMCUAAAAAAGBOAAAAAADAmAAEAAABgTAACAAAAwJgABAAAAICxAAP+iQoFGMCUAAAAAElFTkSuQmCC"}]
[task 2020-10-14T11:16:19.845Z] 11:16:19 INFO - 1602674179833 Marionette DEBUG 7 -> [0,14,"Marionette:SetContext",{"value":"content"}]
[task 2020-10-14T11:16:19.845Z] 11:16:19 INFO - 1602674179833 Marionette DEBUG 7 <- [1,14,null,{"value":null}]
[task 2020-10-14T11:16:19.845Z] 11:16:19 INFO - 1602674179833 Marionette DEBUG 7 -> [0,15,"Marionette:GetContext",{}]
[task 2020-10-14T11:16:19.845Z] 11:16:19 INFO - 1602674179834 Marionette DEBUG 7 <- [1,15,null,{"value":"content"}]
[task 2020-10-14T11:16:19.845Z] 11:16:19 INFO - 1602674179834 Marionette DEBUG 7 -> [0,16,"Marionette:SetContext",{"value":"content"}]
[task 2020-10-14T11:16:19.845Z] 11:16:19 INFO - 1602674179834 Marionette DEBUG 7 <- [1,16,null,{"value":null}]
[task 2020-10-14T11:16:19.845Z] 11:16:19 INFO - 1602674179835 Marionette DEBUG 7 -> [0,17,"WebDriver:GetPageSource",{}]
[task 2020-10-14T11:16:19.917Z] 11:16:19 INFO - TEST-UNEXPECTED-ERROR | testing/marionette/harness/marionette_harness/tests/unit/test_reftest.py TestReftest.test_url_comparison | UnknownException: NotSupportedError: ChromeUtils.registerWindowActor: 'MarionetteReftestFrame' actor is already registered.
[task 2020-10-14T11:16:19.920Z] 11:16:19 INFO - stacktrace:
[task 2020-10-14T11:16:19.920Z] 11:16:19 INFO - setup@chrome://marionette/content/reftest.js:111:17
[task 2020-10-14T11:16:19.920Z] 11:16:19 INFO - GeckoDriver.prototype.setupReftest@chrome://marionette/content/driver.js:3742:17
[task 2020-10-14T11:16:19.920Z] 11:16:19 INFO - despatch@chrome://marionette/content/server.js:297:40
[task 2020-10-14T11:16:19.920Z] 11:16:19 INFO - execute@chrome://marionette/content/server.js:267:16
[task 2020-10-14T11:16:19.920Z] 11:16:19 INFO - onPacket/<@chrome://marionette/content/server.js:240:20
[task 2020-10-14T11:16:19.920Z] 11:16:19 INFO - onPacket@chrome://marionette/content/server.js:241:9
[task 2020-10-14T11:16:19.920Z] 11:16:19 INFO - _onJSONObjectReady/<@chrome://marionette/content/transport.js:504:20

Comment 13

[Julian Descottes [:jdescottes]]

Sorry about that! Will need to unregister the new jswindow actors once the reftest is done.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0e9c47d685d3
[marionette] Use JSWindowActors for marionette reftests navigation r=marionette-reviewers,whimboo,jgraham,maja_zf

Comment 15

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/0e9c47d685d3