1102020 - e10s - investigate browserDOMWindow.openURI and browser_bug537474.js failing

Status: RESOLVED FIXED

(Firefox :: Tabbed Browser, defect)

Description

[:Gijs (he/him)]

browser_bug537474.js currently fails with:

1025 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/base/content/test/general/browser_bug537474.js | page loads in the current content window - Got null, expected [object Object]

in e10s mode. The comment says:

skip-if = e10s # Bug ?????? - test doesn't wait for document to be created before it checks it


but the test does:

function test() {
  var currentWin = content;
  var newWin =
    browserDOMWindow.openURI(makeURI("about:"), null,
                             Ci.nsIBrowserDOMWindow.OPEN_CURRENTWINDOW, null)
  is(newWin, currentWin, "page loads in the current content window");
  gBrowser.stop();
}

I actually thought that should work. I'm not sure why openURI returns *null* rather than whatever else it might return. Bill, can you clarify?

I also wonder if this is related to bug 1101546... (if someone chooses to dupe, please DONTBUILD commit an update to the comment for this test so we don't lose track of what needs to be done to enable it on e10s)

Comment 1

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Well, browserDOMWindow.openURI always returns null in e10s. The only alternative would be to return a CPOW for the content window, but we try to avoid returning CPOWs unless the caller has specifically asked for one.

Also, we don't actually receive a CPOW from the content process until it has started loading. That's too late to return it from openURI.

Finally, we no longer use openURI in e10s anymore. We use openURIInFrame, which returns the <browser> element instead. Perhaps we could re-write the test to use that instead?

Comment 2

[Neil Deakin]

Attachment: browser_bug537474

The test was written to verify an aspect of openURI but not the return value specifically, so we can rework the test a bit to still call openURI.

Comment 3

[:Gijs (he/him)]

Comment on attachment 8607505 [details] [diff] [review]
browser_bug537474

Review of attachment 8607505 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/base/content/test/general/browser_bug537474.js
@@ +1,4 @@
> +add_task(function *() {
> +  browserDOMWindow.openURI(makeURI("about:"), null,
> +                           Ci.nsIBrowserDOMWindow.OPEN_CURRENTWINDOW, null)
> +  yield BrowserTestUtils.browserLoaded(gBrowser.selectedBrowser);

Shouldn't we create this promise before calling openURI ?

Comment 4

[Albert [:albert]]

In bug 1130687 we need to open a new window / tab from a service worker context. In a non e10s environment there are no problems but when it is enabled OpenURI returns null as detailed in this bug. Reading comment 1 you advice to use openURIInFrame instead of openURI, however my concern is that openURI works as a kind of wrapper that that tries to open window or tab depending on the environment (preferences and other stuff). So with openURIInFrame funciton I'll have to check these thinks in my own and call openURIInFrame or openwindow2 if I want a tab or a window.

Am I wrong? Do I miss something?

Comment 5

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Are you trying to open a window in the parent or the child? You'll have to provide more context since I don't know anything about service workers. In any case, we should talk about it in bug 1130687.

Comment 6

[Tracy Walker [:tracy]]

Neil, should the patch here be landed or is there work to do? If the latter, I could take it with some pointers on what to do next.

Comment 7

[Neil Deakin]

I needed to address comment 3, but I'm not sure if I did that or if it didn't work properly.

Comment 8

[Tracy Walker [:tracy]]

I tried the patch locally. The test case passes.

Comment 9

[Neil Deakin]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b2e96500fbd87f0aa0b268727c5d015c77054213
Bug 1102020, fix and reenable browser_bug537474.js, r=gijs

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/b2e96500fbd8