1169249 - Unregister service worker registration when uninstalling a service-worker-enabled application

Status: RESOLVED FIXED

(Core :: DOM: Service Workers, defect)

Description

[José Antonio Olivera Ortega [:jaoo]]

STR required b2g:

1. In the browser app, opens appA.
2. appA registers a SW1.
3. about:sw in b2g shows up SW1.
3. Install AppA.
4. Open appA directly, appA registers SW2.
5. Uninstall appA.
6. about:sw in b2g shows up both SW1 and Sw2.

Expected:

about:sw in b2g shows up only SW1.


IMHO we need somehow to unregister the service worker registration no longer in use. TBH I don't know how to accomplish that.

Comment 1

[Noemí Freire (:noemi)]

Hi,

As agreed, moving B2G blockers to v2. Thanks!

Comment 2

[Fernando Jiménez Moreno [:ferjm]]

I can do this one on the next sprint

Comment 3

[Fernando Jiménez Moreno [:ferjm]]

Attachment: v1

Comment 4

[Fernando Jiménez Moreno [:ferjm]]

Attachment: v1

Rebased on top of bug 1168783

Comment 5

[Ben Kelly [:bkelly, not reviewing]]

Do you plan to also wipe the origin's storage?

Comment 6

[Fernando Jiménez Moreno [:ferjm]]

Attachment: v1

Comment 7

[Fernando Jiménez Moreno [:ferjm]]

Attachment: Tests

Comment 8

[Fernando Jiménez Moreno [:ferjm]]

(In reply to Ben Kelly [:bkelly] from comment #5)
> Do you plan to also wipe the origin's storage?

Not here, but we should. I'll file a new bug for that.

Comment 9

[Fernando Jiménez Moreno [:ferjm]]

Bug 1172562

Comment 10

[Fernando Jiménez Moreno [:ferjm]]

Attachment: v1

Comment 11

[Andrea Marchesini [:baku]]

Comment on attachment 8616751 [details] [diff] [review]
v1

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

r- just for the ServiceWorkerRegistrar in child process. The rest is ok.

::: dom/workers/ServiceWorkerManager.cpp
@@ +4112,5 @@
> +  AssertIsOnMainThread();
> +
> +  MOZ_ASSERT(aPrincipal);
> +
> +  nsRefPtr<ServiceWorkerRegistrar> swr = ServiceWorkerRegistrar::Get();

ServiceWorkerRegistrar cannot run in the child process.
You should use mRegistrationInfos instead.

::: dom/workers/ServiceWorkerManager.h
@@ +526,5 @@
>    RemoveRegistrationInternal(ServiceWorkerRegistrationInfo* aRegistration);
>  
> +  // Removes all service worker registrations for a given principal.
> +  void
> +  RemoveAll(nsIPrincipal* aPrincipal);

RemoveAllRegistations ?

Comment 12

[Fernando Jiménez Moreno [:ferjm]]

Attachment: v2

Comment 13

[Andrea Marchesini [:baku]]

Comment on attachment 8620934 [details] [diff] [review]
v2

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

::: dom/workers/ServiceWorkerManager.cpp
@@ +4117,5 @@
> +  return PL_DHASH_NEXT;
> +}
> +
> +PLDHashOperator
> +UnregisterIfMatchesPrincipalPerPrincipal(const nsACString& aKey,

Maybe a better name here.

@@ +4122,5 @@
> +                                         ServiceWorkerManager::RegistrationDataPerPrincipal* aData,
> +                                         void* aUserData)
> +{
> +  UnregisterIfMatchesHostOrPrincipalData data(aData, aUserData);
> +  aData->mInfos.EnumerateRead(UnregisterIfMatchesPrincipal, &data);

We can use EnumerateRead because ForceUnregister (and Unregister) are async. Otherwise doing some R/W operations on an hashtable during an EnumerateRead will crash. Maybe add a comment.

Comment 14

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/82951c8ba8c5
https://hg.mozilla.org/integration/mozilla-inbound/rev/3257bc1b5498

Comment 15

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

https://hg.mozilla.org/mozilla-central/rev/82951c8ba8c5
https://hg.mozilla.org/mozilla-central/rev/3257bc1b5498

Comment 16

[Noemí Freire (:noemi)]

Hi,

checked with today's (6/12) master build and the service worker still appears registered when uninstalling the application. It has been reported in Bug 1174110


Environmental variables:
Flame device
Build Id: 20150612080105
Gecko: f5cca86
Gaia: fcd90a0
Platform version: 41.0a1