1218080 - Fix service workers openWindow when there's no available browser window

Status: RESOLVED WORKSFORME

(Core :: DOM: Service Workers, defect, P3)

Description

[Cătălin Badea (:catalinb)]

This is a follow-up for bug 1172870 to address the case where firefox is running without a browser window.

Comment 1

[Cătălin Badea (:catalinb)]

We can run into this bug more often than previously thought. With an active private browsing window, firefox can receive notifications and run service workers but won't have a valid browser window for openWindow.

Comment 2

[Cătălin Badea (:catalinb)]

Attachment: Use the hidden XUL window when no browser window is available for ServiceWorkerClients.openWindow.

Comment 3

[Cătălin Badea (:catalinb)]

Attachment: Don't send the url to the parent process when opening new windows, because it is not actually used.

Comment 4

[Olli Pettay [:smaug][[e-mail]](vacation until July 14)]

Comment on attachment 8744901 [details] [diff] [review]
Use the hidden XUL window when no browser window is available for ServiceWorkerClients.openWindow.

So I think we should somehow explicitly tell when it is ok to not have tabparent when sending BrowserFrameOpenWindow message from child side, and only use that for sw.openWindow case, at least for now.
Should be a simple change, but could take a look at another patch.

Comment 5

[Olli Pettay [:smaug][[e-mail]](vacation until July 14)]

Comment on attachment 8744902 [details] [diff] [review]
Don't send the url to the parent process when opening new windows, because it is not actually used.

Fun. nsWindowWatcher code is so great :/
Could we add an assertion to nsWindowWatcher::OpenWindowInternal that if aOpeningTab is non-null, aURL is null.

Comment 6

[Olli Pettay [:smaug][[e-mail]](vacation until July 14)]

mike, you were thinking about cleaning up WW code. See the latter patch here as another example of the messiness of the code :)

Comment 7

[Mike Conley (:mconley) (:⚙️)]

Youch. Thanks for cleaning that up, catalinb.

Comment 8

[Cătălin Badea (:catalinb)]

(In reply to Olli Pettay [:smaug] from comment #4)
> Comment on attachment 8744901 [details] [diff] [review]
> Use the hidden XUL window when no browser window is available for
> ServiceWorkerClients.openWindow.
> 
> So I think we should somehow explicitly tell when it is ok to not have
> tabparent when sending BrowserFrameOpenWindow message from child side, and
> only use that for sw.openWindow case, at least for now.
> Should be a simple change, but could take a look at another patch.

I'm not sure I follow. Service Workers use the SendCreateWindow path. As far as I know, BrowserFrameOpenWindow is used only on Firefox OS and always expects a valid tabchild, which is enforced through asserts.

Are you suggesting adding a flag that explicitly marks that (in the child) the call is coming from a sw instead of guessing based on whether aTabOpener is null or not?

Comment 9

[Cătălin Badea (:catalinb)]

Attachment: Don't send the url to the parent process when opening new windows, because it is not actually used.

Comment 10

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/13c042a06d5c

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/13c042a06d5c

Comment 12

[Cătălin Badea (:catalinb)]

This was fixed when the openWindow code was refactored with the rest of the API for clients.