1253738 - ServiceWorkerManager stores a single RegistrationDataPerPrincipal for all principals with same origin attributes

Status: RESOLVED FIXED

(Core :: DOM: Service Workers, defect)

Description

[Ben Kelly [:bkelly, not reviewing]]

While investigating another bug I found that we are not actually storing separate RegistrationDataPerPrincipal structures for each principal.  Instead we are storing a single struct for all principals with the same extended origin attributes.  In desktop this mean nearly all principals end up with a single struct.

This will mostly work, but it means all domains are sharing job queues, etc.  We also have much slower lookup of registrations by scope since its O(n) across all registrations in the system.

Comment 1

[Ben Kelly [:bkelly, not reviewing]]

Attachment: Store separate service worker registration per principal correctly. r=baku

https://treeherder.mozilla.org/#/jobs?repo=try&revision=234a9f4c3d03

Comment 2

[Ben Kelly [:bkelly, not reviewing]]

Attachment: Store separate service worker registration per principal correctly. r=baku

Comment 3

[Ben Kelly [:bkelly, not reviewing]]

Try is looking pretty green:

  https://treeherder.mozilla.org/#/jobs?repo=try&revision=a7c9f05f070e

I'm going to clean up the patch for review.

Comment 4

[Ben Kelly [:bkelly, not reviewing]]

Attachment: P1 Require an explicit principal when looking up a service worker registration. r=baku

In preparation for using something other than origin attributes for the scope key, require all clients to pass a full nsIPrincipal when looking up a registration.

Comment 5

[Ben Kelly [:bkelly, not reviewing]]

Attachment: P2 Consistently use "scope key" terminology in ServiceWorkerManager. r=baku

Also rename variable to use the "scope key" terminology instead of referencing the origin suffix.

I stopped short of creating an opaque ScopeKey type because its nice to be able to just pass strings around.

Comment 6

[Ben Kelly [:bkelly, not reviewing]]

Attachment: P3 Use origin the ServiceWorkerManager scope key. r=baku

Change PrincipalToScopeKey() to use nsIPrincipal::GetOrigin().  This ensures that the hash table keys for different principals do not collide.  The origin string produced by nsIPrincipal is guaranteed to contain the suffix if present.

Also assert that we are not setting empty scope keys in the future.

Comment 7

[Ben Kelly [:bkelly, not reviewing]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=8c9de29d0937

Comment 8

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e35f587fa328
https://hg.mozilla.org/integration/mozilla-inbound/rev/ffb0e778ab75
https://hg.mozilla.org/integration/mozilla-inbound/rev/8ccab7d6447f

Comment 9

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

https://hg.mozilla.org/mozilla-central/rev/e35f587fa328
https://hg.mozilla.org/mozilla-central/rev/ffb0e778ab75
https://hg.mozilla.org/mozilla-central/rev/8ccab7d6447f