ServiceWorkerContainer.register() should run synchronously up to Update invocation

RESOLVED WONTFIX

Status

()

Core
DOM: Service Workers
RESOLVED WONTFIX
2 years ago
2 years ago

People

(Reporter: bkelly, Unassigned)

Tracking

(Blocks: 1 bug)

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox45 affected)

Details

(Reporter)

Description

2 years ago
The spec currently says that .register() and the Register algorithm should run synchronously.  There is no "execute in parallel" in there.  There is also no synchronization on a job queue until we hit Update.

We should move code out of the ServiceWorkerRegisterJob::Start() into ServiceWorkerManager::Register().

This matters for some of the test cases in unregister-then-register-new-script.https.html.  For example:

  var registration;
  var frame;
  navigation.serviceWorker.register(a).then(function(swr) {
    registration = swr;
    return with_frame(inScopeUrl);
  }).then(function (f) {
    frame = f;
    return registration.unregister();
  }).then(function () {
    navigation.serviceWorker.register(b);
    frame.remove();
  })

Here, the uninstalling flag gets set by unregister(), but it should have no effect until the frame is removed.  The register() call should clear the uninstalling flag synchronously before the frame is removed.

In gecko today, however, we run the register() steps async so the frame is removed while uninstalling is still set.  This causes the original registration to be removed.
(Reporter)

Comment 1

2 years ago
Its unclear if we want this stuff to run synchronously or not.  Accessing this data outside of the synchronization queues might be a bad idea.

We need to examine the unregister-then-register-new-script.https.html completely and figure out what the right answers are.
(Reporter)

Updated

2 years ago
See Also: → bug 1231974
(Reporter)

Comment 2

2 years ago
I'm going to implement the new unified job queue.  We can re-evaluate this at that point, but I expect this bug will just not be necessary any more.
Depends on: 1256428
(Reporter)

Comment 3

2 years ago
Yea, this is OBE now that I implemented bug 1256428.
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.