1658928 - Make "WebDriver:GetCurrentURL" Fission compatible

Status: RESOLVED FIXED

(Remote Protocol :: Marionette, task, P1)

Description

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

This command should work through the JSWindowActor pair to simplify the behavior for both content and chrome.

Note that since bug 1368492 landed we do not have an end-point in listener.js, which is not pretty helpful. Also because for reftests running on Android this.driver.curBrowser.currentURI returns null. Having the command been forwarded to the content process should give us the expected data.

As such I will attach two patches. The first will partly revert the former changes, and the second one will add the Actor support.

Comment 1

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

Having the patches from bug 1654628 landed first, will kinda help here. Marking as blocking.

Comment 2

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

Attachment: Bug 1658928 - [marionette] Make "WebDriver:GetCurrentURL" Fission compatible.

By moving out the "currentURI" getter from the Browser class,
and making it available on the driver other parts of Marionette
like reftests can make use of the "getCurrentURI" helper method.
Hereby in case of content context the URL needs to be retrieved
from the framescript.

Also a couple of methods expressed the passed-around data as
instance of nsIURI where by it was a string or an instance of
URL. The patch fixes that to always pass around nsIURI instances,
and using it's appropriate properties when handling cookies.

Comment 3

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

Attachment: Bug 1658928 - [marionette] Port WebDriver:GetCurrentUrl to JSWindowActor.

The patch adds the handling of the current URI to the JSWindowActor.
Given that the documentURI is already available via the currentWindowGlobal
of the parent actor, it can be returned immediately without having
to do any IPC communication.

Depends on D87572

Comment 4

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

Actually using nsIURI isn't that elegant as using URL(). Reason is that the former raises exceptions when accessing properties like host that would not match. As example a NS_ERROR_FAILURE is raised when trying to get the host from nsIURI("about:blank").

When using new URL() the host is just an empty string and way better to handle. Given that I will revert parts of the first patch to make use of URL.

Comment 5

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

Attachment: Bug 1658928 - [wdspec] Improve tests for getCurrentURL and add site-isolation test.

Depends on D87572

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/391624a4819f
[marionette] Make "WebDriver:GetCurrentURL" Fission compatible. r=marionette-reviewers,maja_zf
https://hg.mozilla.org/integration/autoland/rev/d72e8a597ae4
[wdspec] Improve tests for getCurrentURL and add site-isolation test. r=maja_zf

Comment 7

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

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

Comment 8

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/391624a4819f
https://hg.mozilla.org/mozilla-central/rev/d72e8a597ae4

Comment 9

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

Upstream PR merged by moz-wptsync-bot