1247055 - controlled documents associated with ServiceWorkerRegistrationInfo instead of a particular active service worker

Status: RESOLVED DUPLICATE of bug 1425975

(Core :: DOM: Service Workers, defect)

Description

[Ben Kelly [:bkelly, not reviewing]]

While investigating bug 1238954 I noticed that we store documents in a hash table called mControlledDocuments.  This maps nsIDocument pointers to ServiceWorkerRegistationInfo objects.  This mapping of document to registration is problematic for a couple reasons:

1) The spec associates a particular ServiceWorker with the controlled client, not a registration.  This is important because with skipWaiting() you can have different clients controlled by different ServiceWorkers under the same scope.  By mapping to the registration we can't do that.

2) We run the risk that skipWaiting() will change the controlling worker for a document since we only map to the registration, not to the particular service worker.  So in effect we probably stealth do a clients.claim() without firing any controllerchange events.  (I haven't tested this, though.  Hopefully I'm wrong.)

3) It means we can list documents in mControlledDocuments when there is no active worker.  This creates the potential for the document to load uncontrolled, then have a worker reach .active, and suddenly the document shows as controlled.  Again, its stealth clients.claim() behavior.

We should test these scenarios and fix if they are indeed happening.

Comment 1

[Ben Kelly [:bkelly, not reviewing]]

Another issue we have here is that a document that is in mControlledDocuments, but not controlled because there is no active worker, can then summarily kill the worker when the document closes.

Comment 2

[Ben Kelly [:bkelly, not reviewing]]

This one worries me, so I'm going to work it now.

Comment 3

[Ben Kelly [:bkelly, not reviewing]]

It turns out we do implement the required observable behavior.  Step 9.1 of Active does start controlling existing controlled documents.  We also fire the controller change event.

Talking with Jake the spec used to be written with the client referencing the registration.  At some point it was changed to the worker, but its an equivalent implementation.

We could do this bug to converge with the spec, but its not a priority.