1661332 - Marionette reftests have to run in content and not chrome context

Status: RESOLVED FIXED

(Remote Protocol :: Marionette, defect, P1)

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 02/17 - 02/21)]

The code as added on bug 1658696 calls into this.driver.currentURL, which returns the URL of the currently active browsing context based on the selected context. In case of reftests this currently is chrome. All reftests are run within a browser element, and as such we should actually have the content context set.

James, do you remember why chrome is selected? If not I will dive into that tomorrow.

Comment 2

[James Graham [:jgraham]]

I'm not certain, but given that it's creating a custom kind of browser and accessing the elements directly I wouldn't be surprised to find that the assumptions in content context around the existence of a tabbedbrowser &c. don't hold.

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 02/17 - 02/21)]

I'm not fully understanding your reply due to the double negative wordings, but I just tried to see what happens when running reftests in content scope and it all looks fine locally. As such there shouldn't be a need to force chrome context.

Lets see how it looks like in CI:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=9555e9f616cd1069f32270286b315a631318fd1d

Comment 4

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 02/17 - 02/21)]

As it can be seen in the try build a lot of jobs are failing because of assertions. So I spoke with James and Emilio on Matrix. It took us a while to figure out what's wrong here, and it all makes sense.

Given that until now the reftests always run in chrome context the following line was not executed in content but in chrome context:

https://searchfox.org/mozilla-central/rev/27932d4e6ebd2f4b8519865dad864c72176e4e3b/testing/web-platform/tests/tools/wptrunner/wptrunner/executors/executormarionette.py#414-415

By making use of using_context three lines above the context switched from chrome -> chrome, and once using_context has been left, chrome context is still present.

As such we now have to account for the correct number of assertions in the test ini files.

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 02/17 - 02/21)]

Attachment: Bug 1661332 - [marionette] Run reftests in content and not chrome context.

Reftests are running inside a custom browser element, which basically is
reflected as content context in Marionette. And given that some commands
like "WebDriver:GetCurrentURL" rely on the correct context to be set,
wrong data would be returned.

To allow that all commands are working based on the custom browser element
don't enforce chrome context for the Reftests code in Marionette.

This change also fixes the code which counts the assertions as seen for
debug builds. Until now only assertions from chrome context have been
collected.

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 02/17 - 02/21)]

Attachment: Bug 1661332 - [wpt] Fix assertion counts for content context for wpt-reftests.

Until now only assertions from chrome context have been counted.
With this patch we also account for content scope assertions.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3b672b3916de
[marionette] Run reftests in content and not chrome context. r=marionette-reviewers,jgraham
https://hg.mozilla.org/integration/autoland/rev/0b71328757e6
[wpt] Fix assertion counts for content context for wpt-reftests. r=jgraham

Comment 8

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/25262 for changes under testing/web-platform/tests

Comment 9

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/3b672b3916de
https://hg.mozilla.org/mozilla-central/rev/0b71328757e6

Comment 10

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 02/17 - 02/21)]

(In reply to Web Platform Test Sync Bot (Matrix: #interop:mozilla.org) from comment #8)

Created web-platform-tests PR
https://github.com/web-platform-tests/wpt/pull/25262 for changes under
testing/web-platform/tests

James, can you please check why this cannot be merged? Thanks.

Comment 11

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR was closed without merging

Comment 12

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot