1228723 - "self.registration.pushManager.getSubscription()" in Service Worker freezes Firefox

Status: RESOLVED FIXED

(Core :: DOM: Push Subscriptions, defect)

Description

[Steffen Weber]

User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/46.0.2490.86 Safari/537.36

Steps to reproduce:

I'm testing push notifications in Firefox 44 Developer Edition. I opened a website with a service worker that contains the following line of code:

self.registration.pushManager.getSubscription();

The actual service worker was more complex but the reduced test-case consisting of just this single line is enough to reproduce the problem.

The following line does _not_ cause any problems:

self.registration.pushManager;


Actual results:

Firefox 44.0a2 (2015-11-27) on Linux freezed and I had to force quit. Firefox 44.0a2 (2015-11-27) on Windows loads the page but "navigator.serviceWorker.ready" never resolves.


Expected results:

Firefox should not have crashed.

Comment 1

[Gingerbread Man]

Can you reproduce the issue in the latest Nightly with a brand new profile?
https://support.mozilla.org/kb/profile-manager-create-and-remove-firefox-profiles
https://nightly.mozilla.org

In case of a crash, please submit a crash report, then post the report ID from about:crashes in a comment here.
https://developer.mozilla.org/docs/How_to_get_a_stacktrace_for_a_bug_report

Comment 2

[Steffen Weber]

Using the latest Firefox 45 Nightly on Windows (2015-11-27) the browser does not freeze. But I can still reproduce the issue that "navigator.serviceWorker.ready" does not resolve if the service worker contains the mentioned line of code.

(I'll test this Firefox issue on Windows from now on because on Linux I get mixed and strange results that probably won't help.)

Comment 3

[Steffen Weber]

Attachment: Test cases

I have created a reduced test case for this bug. First, the working test case:

1. Start Firefox Developer Edition 44.0a2 (2015-11-27) or Firefox Nightly 45.0a1 (2015-11-27) with a clean profile.
2. Open index.html
3. Click the first button labelled "Register Service Worker 1"
4. Notice that two messages are logged: "Registering Service Worker …" and "Service Worker Ready ✓". Everything is fine.

Now the failing test case:

1. See above
2. See above
3. Click the second button labelled "Register Service Worker 2"
4. Notice that only the message "Registering Service Worker …" is logged. But the "navigator.serviceWorker.ready" promise never gets resolved.

Comment 4

[Steffen Weber]

Obviously, calling self.registration.pushManager.getSubscription() outside an event handler is stupid. I have fixed my code and it now works. Maybe I should have expected the service worker to fail in such a case. Then this bug can be closed. Sorry for the confusion.

But maybe you could display an error message in the developer console if an error occurs while registering the service worker?

Comment 5

[Gingerbread Man]

Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:45.0) Gecko/20100101 Firefox/45.0
20151129030359

(In reply to Steffen Weber from comment #3)
> I have created a reduced test case for this bug.

Clicking either button produces the following errors:

Security Error: Content at file:///C:/firefox-bug-1228723-test-case/index.html may not load data from file:///C:/firefox-bug-1228723-test-case/.
SecurityError: The operation is insecure. index.html:30:0

(In reply to Steffen Weber from comment #4)
> I have fixed my code and it now works. Maybe I should have expected
> the service worker to fail in such a case. Then this bug can be closed.

Resolving as Invalid.

> But maybe you could display an error message in the developer console if an
> error occurs while registering the service worker?

Bug 1188675, bug 1214248.

Comment 6

[Steffen Weber]

> Clicking either button produces the following errors: [...]

That's because Service Workers require a secure origin (HTTPS or http://localhost/). The file:// URIs are not considered secure by Firefox. You have to host the testcase on an HTTPS server or on localhost.

> Bug 1188675, bug 1214248.

Thanks.

Comment 7

[Olivier Favre]

I'm facing this very issue within an "activate" event (or a message exchange), where I'm trying to determine the actual subscription status of a user, in the case where the permission is not granted (ie. default or denied).
It freezes the content of the tabs, the developer console no longer evaluates code, and switching to other tabs only shows a spinner.

I've updated Steffen Weber's test case.

Steps to reproduce:
Flavor 1
- Have the notification permission either unsettled or denied.
- Register the Service Worker 1, then click "Ask Service Worker for subscription". The page posts a message to the Service Worker, it reads the subscription, and Firefox freezes.
Flavor 2
- Have the notification permission either unsettled or denied.
- Register the Service Worker 2. At the activation step the Service Worker reads the subscription, and Firefox freezes.

I'm using Firefox Aurora 45.0a2 (2016-01-25) under a KDE Debian Jessie.

Please remove the resolved invalid status from this bug. (or should I open a new one?)

Comment 8

[Olivier Favre]

Attachment: Test case inside the activate or message events

Comment 9

[Lina Butler (ex-Mozilla)]

Thanks for the test case! Reopening to investigate.

Comment 10

[Lina Butler (ex-Mozilla)]

Attachment: MozReview Request: Bug 1228723 - Avoid a promise worker proxy deadlock caused by synchronous Push callbacks. r?catalinb

Review commit: https://reviewboard.mozilla.org/r/32515/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/32515/

Comment 11

[Lina Butler (ex-Mozilla)]

https://reviewboard.mozilla.org/r/32515/#review29239

::: dom/push/PushManager.cpp:493
(Diff revision 1)
> +    if (!principal) {

I'm not sure if this is necessary. Can I just `MOZ_ASSERT(principal)`?

Comment 12

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

Comment on attachment 8712344 [details]
MozReview Request: Bug 1228723 - Avoid a promise worker proxy deadlock caused by synchronous Push callbacks. r?catalinb

https://reviewboard.mozilla.org/r/32515/#review29397

Comment 13

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

https://reviewboard.mozilla.org/r/32515/#review29239

> I'm not sure if this is necessary. Can I just `MOZ_ASSERT(principal)`?

Yes, as long as the proxy wasn't cleaned up, the workerPrivate should always have a principal.

Comment 14

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/bd7fad53ccc7

Comment 15

[Carsten Book [:Tomcat]]

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

Comment 16

[Carsten Book [:Tomcat]]

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