Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

be more explicit that jobs always run asynchronously #861

Closed
wanderview opened this issue Mar 29, 2016 · 5 comments
Closed

be more explicit that jobs always run asynchronously #861

wanderview opened this issue Mar 29, 2016 · 5 comments
Labels
Milestone

Comments

@wanderview
Copy link
Member

Previously @mattto stated that Chrome always run jobs asynchronously. The spec is a bit fuzzy on this saying things like:

If job’s job type is register, invoke Register with job and continue running these steps in parallel.

This suggests that perhaps Register gets to keep running sync and then the current steps are completed potentially in parallel.

I think it would be better for Run Job to explicitly queue a task to invoke the algorithm for the job.

This matters for things like:

registration.unregister().then(function() {
  var p =navigator.serviceWorker.register(urlThat404s, scope: registration.scope)

  // Removing last controlled client would allow registration with uninstalling flag to
  // be purged.
  controlledFrame.remove();

  return p;
}).then(function() {
  // if register can run its initial steps sync, then it can clear the uninstalling
  // flag before the controlled client is removed.  This results in the original
  //  service worker script remaining active.

  // If register always runs async, then the controlled client is always removed
  // before the uninstalling flag could be cleared.  This results in no registration
  // being present since the new script 404s.
});
@wanderview wanderview added this to the Version 1 milestone Mar 29, 2016
@wanderview
Copy link
Member Author

Also, I am currently switching gecko to always run jobs async since it provides more consistent behavior.

@jakearchibald
Copy link
Contributor

F2F: Make sure all jobs are scheduled async

@wanderview
Copy link
Member Author

No gecko bug as we have already implemented this.

@wanderview
Copy link
Member Author

I'm sorry, but I'm not sure I agree with the change in spec language. I think scheduling the job should be immediate, but actually executing the job should be asynchronous. I think the "queue a task" language belongs in the "Run Job" algorithm.

This also ensures an async step between consecutive jobs in the queue which is important for flushing event runnables and whatnot.

jungkees added a commit that referenced this issue Apr 20, 2016
Move queuing a task step for running a job from each method and Soft Update to
Run Job to address #861 (comment)
@jungkees
Copy link
Collaborator

I think scheduling the job should be immediate, but actually executing the job should be asynchronous.

Yes, that makes sense. Please see if e68be6e satisfies your expectation.

Also, I think this note should still be there, right?

For a register job and an update job, the user agent delays queuing a task for running the job until after the document initiated the job has been dispatched DOMContentLoaded event.

Or is it guaranteed the task queued in Run Job is always queued after the task for dispatching the DOMContentLoaded event?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants