Closed Bug 1169249 Opened 6 years ago Closed 6 years ago

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

Categories

(Core :: DOM: Service Workers, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

()

RESOLVED FIXED
NGA S2 (12Jun)
Tracking Status
firefox41 --- fixed

People

(Reporter: jaoo, Assigned: ferjm)

References

Details

(Whiteboard: [s3])

Attachments

(2 files, 4 obsolete files)

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.
Hi,

As agreed, moving B2G blockers to v2. Thanks!
Blocks: ServiceWorkers-B2G
No longer blocks: ServiceWorkers-v1
I can do this one on the next sprint
Assignee: nobody → ferjmoreno
Whiteboard: [s3]
Target Milestone: --- → NGA S2 (12Jun)
Component: Gaia → DOM: Service Workers
Product: Firefox OS → Core
Status: NEW → ASSIGNED
Attached patch v1 (obsolete) — Splinter Review
Depends on: 1168783
Attached patch v1 (obsolete) — Splinter Review
Rebased on top of bug 1168783
Attachment #8614579 - Attachment is obsolete: true
Do you plan to also wipe the origin's storage?
Attached patch v1 (obsolete) — Splinter Review
Attachment #8614734 - Attachment is obsolete: true
Attachment #8616743 - Flags: review?(amarchesini)
Attached patch TestsSplinter Review
Attachment #8616744 - Flags: review?(amarchesini)
(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.
Attached patch v1 (obsolete) — Splinter Review
Attachment #8616743 - Attachment is obsolete: true
Attachment #8616743 - Flags: review?(amarchesini)
Attachment #8616751 - Flags: review?(amarchesini)
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 ?
Attachment #8616751 - Flags: review?(amarchesini) → review-
Attached patch v2Splinter Review
Attachment #8616751 - Attachment is obsolete: true
Attachment #8620934 - Flags: review?(amarchesini)
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.
Attachment #8620934 - Flags: review?(amarchesini) → review+
Attachment #8616744 - Flags: review?(amarchesini) → review+
See Also: → 1174110
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
You need to log in before you can comment on or make changes to this bug.