1973269 - OpenNewWindow does not open the requested URL when startup session restore happens

Status: RESOLVED FIXED

(Core :: DOM: Service Workers, defect)

Description

[Kagami Rosylight [:saschanaz] (they/them)]

What's happening is that the attempt to load the URI from WaitForLoad is immediately overridden by SessionStoreUtils:

xul.dll!mozilla::dom::BrowsingContext::LoadURI(nsDocShellLoadState * aLoadState, bool aSetNavigating) Line 2012 (d:\gecko\docshell\base\BrowsingContext.cpp:2012)
xul.dll!nsSHistory::LoadURIOrBFCache(nsSHistory::LoadEntryResult & aLoadEntry) Line 1455 (d:\gecko\docshell\shistory\nsSHistory.cpp:1455)
xul.dll!nsSHistory::LoadURIs(nsTArray<nsSHistory::LoadEntryResult> & aLoadResults) Line 1459 (d:\gecko\docshell\shistory\nsSHistory.cpp:1459)
xul.dll!nsSHistory::ReloadCurrentEntry() Line 1525 (d:\gecko\docshell\shistory\nsSHistory.cpp:1525)
xul.dll!mozilla::dom::SessionStoreUtils::InitializeRestore(const mozilla::dom::GlobalObject & aGlobal, mozilla::dom::CanonicalBrowsingContext & aContext, nsISessionStoreRestoreData * aData, mozilla::ErrorResult & aError) Line 1667 (d:\gecko\toolkit\components\sessionstore\SessionStoreUtils.cpp:1667)
xul.dll!mozilla::dom::SessionStoreUtils_Binding::initializeRestore(JSContext * cx_, unsigned int argc, JS::Value * vp) Line 1783 (d:\gecko\obj-x86_64-pc-windows-msvc\dom\bindings\SessionStoreUtilsBinding.cpp:1783)
xul.dll!mozilla::dom::StaticMethodPromiseWrapper(JSContext * cx, unsigned int argc, JS::Value * vp) Line 3345 (d:\gecko\dom\bindings\BindingUtils.cpp:3345)
[Inline Frame] xul.dll!CallJSNative(JSContext * cx, bool(*)(JSContext *, unsigned int, JS::Value *) native, js::CallReason reason, const JS::CallArgs & args) Line 501 (d:\gecko\js\src\vm\Interpreter.cpp:501)
xul.dll!js::InternalCallOrConstruct(JSContext * cx, const JS::CallArgs & args, js::MaybeConstruct construct, js::CallReason reason) Line 597 (d:\gecko\js\src\vm\Interpreter.cpp:597)
xul.dll!InternalCall(JSContext * cx, const js::AnyInvokeArgs & args, js::CallReason reason) Line 664 (d:\gecko\js\src\vm\Interpreter.cpp:664)
xul.dll!js::CallFromStack(JSContext * cx, const JS::CallArgs & args, js::CallReason reason) Line 669 (d:\gecko\js\src\vm\Interpreter.cpp:669)
xul.dll!js::Interpret(JSContext * cx, js::RunState & state) Line 3287 (d:\gecko\js\src\vm\Interpreter.cpp:3287)
[Inline Frame] xul.dll!MaybeEnterInterpreterTrampoline(JSContext * cx, js::RunState & state) Line 395 (d:\gecko\js\src\vm\Interpreter.cpp:395)
xul.dll!js::RunScript(JSContext * cx, js::RunState & state) Line 471 (d:\gecko\js\src\vm\Interpreter.cpp:471)
xul.dll!js::InternalCallOrConstruct(JSContext * cx, const JS::CallArgs & args, js::MaybeConstruct construct, js::CallReason reason) Line 629 (d:\gecko\js\src\vm\Interpreter.cpp:629)
xul.dll!InternalCall(JSContext * cx, const js::AnyInvokeArgs & args, js::CallReason reason) Line 664 (d:\gecko\js\src\vm\Interpreter.cpp:664)
xul.dll!js::CallFromStack(JSContext * cx, const JS::CallArgs & args, js::CallReason reason) Line 669 (d:\gecko\js\src\vm\Interpreter.cpp:669)
xul.dll!js::jit::DoCallFallback(JSContext * cx, js::jit::BaselineFrame * frame, js::jit::ICFallbackStub * stub, unsigned int argc, JS::Value * vp, JS::MutableHandle<JS::Value> res) Line 1705 (d:\gecko\js\src\jit\BaselineIC.cpp:1705)
00000155d1733a02() (Unknown Source:0)
00000155d1739d66() (Unknown Source:0)

LoadURI is called for the same BrowsingContext, and thus the first load by service worker is overridden.

Comment 1

[Kagami Rosylight [:saschanaz] (they/them)]

Attachment: Bug 1973269 - Pass the URI directly to browser chrome page r=asuth,gijs

Currently we load the URI using the resulting BrowsingContext, but that causes race with session restore. Passing the URI as an argument to the browser chrome page solves the issue.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/a3f9b39ece14
https://hg.mozilla.org/integration/autoland/rev/1ea6add18585
Pass the URI directly to browser chrome page r=asuth,Gijs

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/4c2685c03cc1
https://hg.mozilla.org/integration/autoland/rev/3e2d9b5cfd28
Revert "Bug 1973269 - Pass the URI directly to browser chrome page r=asuth,Gijs" for causing junit failures @openWindowSameSession.

Comment 4

[agoloman]

Backed out for causing junit failures @openWindowSameSession.

• Backout link
• Push with failures
• Failure Log

Comment 5

[Kagami Rosylight [:saschanaz] (they/them)]

... why? This is #ifndef geckoview?

Comment 6

[Kagami Rosylight [:saschanaz] (they/them)]

Maybe shouldLoadURI should be always true for GV.

Comment 7

[agoloman]

Also these.

Comment 8

[Kagami Rosylight [:saschanaz] (they/them)]

Probably the same reason as comment #6, thanks!

Comment 9

[Kagami Rosylight [:saschanaz] (they/them)]

(because GeckoViewOpenWindow never sets bc at all... 😅)

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/8c1f5ef3ad29
https://hg.mozilla.org/integration/autoland/rev/1bb1e4403434
Pass the URI directly to browser chrome page r=asuth,Gijs

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/d774d641a8ca
https://hg.mozilla.org/integration/autoland/rev/f0be9859bd7b
Revert "Bug 1973269 - Pass the URI directly to browser chrome page r=asuth,Gijs" for causing mochitest failures @ test_notification_serviceworker_openWindow.html

Comment 12

[Sandor Molnar[:smolnar]]

Backed out for causing mochitest failures @ test_notification_serviceworker_openWindow.html

Backout link: https://hg-edge.mozilla.org/integration/autoland/rev/f0be9859bd7b66952fe44f2fbc699f4faba04d88

Push with failures

Failure log -> TEST-UNEXPECTED-FAIL | dom/notification/test/mochitest/test_notification_serviceworker_openWindow.html | Test timed out

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/41b7ab7321cf
https://hg.mozilla.org/integration/autoland/rev/8b3156548518
Pass the URI directly to browser chrome page r=asuth,Gijs

Comment 14

[Kagami Rosylight [:saschanaz] (they/them)]

No backout so far and I hope it stay that way 🙏🏻

Comment 15

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/8b3156548518