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

matchAll() runs Request constructor off the main thread #672

Closed
annevk opened this issue Apr 6, 2015 · 7 comments
Closed

matchAll() runs Request constructor off the main thread #672

annevk opened this issue Apr 6, 2015 · 7 comments

Comments

@annevk
Copy link
Member

annevk commented Apr 6, 2015

That does not seem possible. You need to run the synchronous stuff at the start of the algorithm, before any parallel steps.

@annevk
Copy link
Member Author

annevk commented Apr 6, 2015

This is also a problem for match() by extension.

@jakearchibald jakearchibald added this to the Version 1 milestone Oct 28, 2015
@mkruisselbrink
Copy link
Collaborator

Is the same also true for pretty much every other method? In particular the ones that return Client instances, maybe also the ones that return ServiceWorker and ServiceWorkerRegistration instances?

@annevk
Copy link
Member Author

annevk commented Nov 1, 2015

Yeah could be. Depends on how things are worded/done.

@annevk
Copy link
Member Author

annevk commented Jan 21, 2016

@jungkees could you perhaps give pointers to the sections in the specification that need review? That would help a lot.

@jungkees
Copy link
Collaborator

Ah.. I had to give this pointer: 3d0ee14#commitcomment-14237642.

To sum, it'd be nice if you review whether this change makes sense:

  • Before: a queued task (to the client's event loop) captures the window's state and create a WindowClient object there and resolves the returned promise there.
  • After: a queued task (to the client's event loop) captures the window's state and only set the corresponding variables defined in the service worker environment. When the task has executed, the steps in the service worker environment create a WindowClient object and resolves using the values set in the variables.

I think WindowClient.focus() is an example that I want you to take a look:

@annevk
Copy link
Member Author

annevk commented Jan 27, 2016

It seems e.g., matchAll() still invokes the Request constructor from a section that runs in parallel. That is a little odd, because e.g., it means the promise is not guaranteed to reject at the end of the current task, which is what you'd expect.

I would expect methods to do that kind of setup work first, before returning and running some steps in parallel for lookup.

@jakearchibald
Copy link
Contributor

F2F: move the "if request is a string, upgrade to request" steps to before "run in parallel"

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

No branches or pull requests

5 participants