1002570 - Implement access to various ServiceWorkerContainer ServiceWorker instances

Status: RESOLVED FIXED

(Core :: DOM: Workers, defect)

Description

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

Based on the resolution of spec issue https://github.com/slightlyoff/ServiceWorker/issues/232

A placeholder bug to land Bug 930348 with FIXMEs.

Comment 1

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

Attachment: Return valid ServiceWorker instances for .installing, .waiting, .active and .controller

While the DOM exposed API is unintuitive when new registrations are performed, the updated spec makes this much easier to understand. I'd like to land this before making the DOM facing spec changes from https://github.com/slightlyoff/ServiceWorker/issues/365 since the ServiceWorkerManager code remains mostly unchanged.

Comment 2

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

Attachment: Return valid ServiceWorker instances for .installing, .waiting, .active and .controller

MaybeStartControlling() should only control the document if the registration has a non-null active worker.

Comment 3

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

https://hg.mozilla.org/users/nsm.nikhil_gmail.com/service-workers2/rev/9024986f644e
https://hg.mozilla.org/users/nsm.nikhil_gmail.com/service-workers2/rev/8f8677c1e4ca

Comment 4

[(no longer active)]

Comment on attachment 8461180 [details] [diff] [review]
Return valid ServiceWorker instances for .installing, .waiting, .active and .controller

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

r=me with the CC issue fixed.

::: dom/workers/ServiceWorkerContainer.cpp
@@ +24,5 @@
>  namespace dom {
>  namespace workers {
>  
>  NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION_INHERITED(ServiceWorkerContainer)
>  NS_INTERFACE_MAP_END_INHERITING(DOMEventTargetHelper)

Don't you need to hook up these new members to the CC?

::: dom/workers/ServiceWorkerManager.h
@@ +99,5 @@
> +enum WhichServiceWorker {
> +  INSTALLING_WORKER = 1,
> +  WAITING_WORKER    = 2,
> +  ACTIVE_WORKER     = 4,
> +};

It would be nice if you used a typed enum here with MOZ_MAKE_ENUM_CLASS_BITWISE_OPERATORS.  Could be a follow-up.

Comment 5

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

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

Comment 6

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

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