Now that we have the support for the "New Window" command in Marionette, we might want to make use of it.

Wes, can you add this to your todo list and coordinate with whimboo if you have any questions.

Note that bug 1523234 isn't needed here because wptrunner expects to have the new tab focused.

Would that be the various open() calls in https://searchfox.org/mozilla-central/source/testing/web-platform/tests/tools/wptrunner/wptrunner/executors/executormarionette.py#636 ?

All the window.open() calls, which are not only in the py files, but also in various JS modules like:

https://searchfox.org/mozilla-central/source/testing/web-platform/tests/tools/wptrunner/wptrunner/executors/reftest.js

It's not all, since it can only be used with resources that depend on executormarionette.

I put together a quick patch at https://treeherder.mozilla.org/#/jobs?repo=try&revision=2f2ee65df66a329d7d5acda114a30ddf96a623d7

There are a couple of problems:

1. The new tab opened in this way appears to show the default nee tab page and not about:blank as required by the spec. This is probably slowing tests down a little.
2. This depends on an unreleased marionette_driver. If the plan is to not make more releases then we'll need to either avoid using the client methods here or put a copy of the client in [testing/web-platform/tests]tools/third_party for wptrunner to use.

Re 1, I'd suggest setting the new tab and homepage prefs to be about:blank in one of the unittest profiles that wpt depends on.

Re 2, I will get a new marionette driver package released via bug 1530794.

Here's a try push of James's patch plus a change for the homepage: https://treeherder.mozilla.org/#/jobs?repo=try&group_state=expanded&searchStr=wpt&revision=d47b22c1be92a3a3728c9dd6c613d5ae86e2079d

That last try push looks like a complete fail someone needs to be investigate. I'm happy to fix a possible underlying issue with Marionette.

I just released marionette_driver 2.8.0 to PyPI, which includes support for the new window command. So this unblocks the implementation here.

Depends on D23972

Here's jgraham's patch and a patch to set the homepage to about:blank in the wpt testing profile, rebased to current central.

And a try push to see what that looks like: https://treeherder.mozilla.org/#/jobs?repo=try&group_state=expanded&revision=3940a503f1c911c6fb5c8c212e18d8d5fada5a7d

Do I need to do anything to pick up the new marionette_driver now that bug 1530794 is completed? Are there any parts of jgraham's patch that can be dropped now?

You have to make sure to at least bump the dependency for marionette_driver.

Here's a try push with the dependency bumped: https://treeherder.mozilla.org/#/jobs?repo=try&group_state=expanded&selectedJob=234671713&revision=11d24ebc5935bc60a540d1dfee13189ed6ec896f

Looks like everything's still broken, much in the same way it was for jgraham's original try push from comment 5. It's not every single test failing, though. There's one chunk that passed completely, and there are several TEST-OKs in each of the failing chunks. I'm not sure why things are failing. Are they occasionally getting the wrong window or something?

Eh I think it was just a silly error; for some reason I thought Create Window switched to the new window, but it turns out it doesn't. So https://treeherder.mozilla.org/#/jobs?repo=try&revision=608fb12a70a9b1ce8451fdacd51fabd114d15818 is a try push with that fixed.

Looks better. Still seems like there are several failures, and I'm seeing some of the JavascriptException: TypeError: window.__wptrunner_process_next_event is not a functions that we've been seeing in bug 1522790.

Some seem to be known intermittents, some UNEXPECTED-PASSes. Maybe this fixed some of our tests? Maybe we're just on a bad base revision?

(In reply to James Graham [:jgraham] from comment #16)

Eh I think it was just a silly error; for some reason I thought Create Window switched to the new window, but it turns out it doesn't.

Not by default. If you want that behavior you have to specify focus=True. I can see that this was already used in Wes' try push, so what exactly did you change?

Not by default. If you want that behavior you have to specify focus=True. I can see that this was already used in Wes' try push, so what exactly did you change?

I mean "switched the webdriver active browsing context to"

Still seems like there are several failures, and I'm seeing some of the JavascriptException: TypeError: window.__wptrunner_process_next_event is not a functions that we've been seeing in bug 1522790.

I'm pretty sure there's a race condition that's causing the __process_next_event thing; I'm investigating.

https://treeherder.mozilla.org/#/jobs?repo=try&author=james%40hoppipolla.co.uk&selectedJob=235205397 is a try push that ports the manual logic to make the runner wait for pending loads over to marionette (we were already using this for Chrome which doesn't support the right pageLoadStragtegy). This looks like it improved things, although there are still quite a few changed results which could be due to focus changes or something; this needs some investigation. That same patch could be a workaround in bug 1501562 so I suggest trying it there and see if it helps. But in any case it's clearly working around a bug in marionette in each case, so it's not the correct long-term solution.

So on Geckoview, if I don't also have a time.sleep(2) prior to the navigate(), wait_for_load() ends up in an infinite loop. location.href is 'about:blank', and document.readyState is 'complete'.

If I do have the time.sleep(2) up there, wait_for_load() is called once. location.href is the wpt test url, and document.readyState is 'complete'.

FTR the try push mentioned in my previous comment is https://treeherder.mozilla.org/#/jobs?repo=try&revision=2b229af02e908f1ff528260098c54ec22a6344d5

James, we should be good to go now with the changes for this bug on both desktop and Android. Are you still interested in updating the existing patch?

I rebased the patch, and updated it.

The problem now seems to be that all the focus-related tests are failing; when we open the window with window.open we end up with the document in the new window getting focus, whereas with the new window approach, it seems like the focus ends up in the chrome area, and so tests that expect to be able to call element.focus() and have it change the focus don't work.

I guess the spec doesn't make it clear what should happen here, but it seems reasonable for marionette at least that if we pass focus=True we end up with focus in the content area.

James, this should be unblocked now. Please try again after rebasing to latest central.