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.

Sign up for GitHub

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

Jump to bottom

Worker cleanup #164

Closed
11 tasks done
annevk opened this issue Sep 17, 2015 · 3 comments
Closed
11 tasks done

Worker cleanup #164

annevk opened this issue Sep 17, 2015 · 3 comments
Labels
clarification Standard could be clearer normative change

Comments

@annevk
Copy link
Member

annevk commented Sep 17, 2015

I've been looking at workers today.

  • base URL should be response's url https://www.w3.org/Bugs/Public/show_bug.cgi?id=28835 My idea for fixing this is to make the environment settings object return global object's [[url]]. We mutate global object's [[url]] after fetching.
  • SharedWorker needs to be keyed on input URL (not location) as otherwise redirects make for a confusing and racy model. Implementations also do not match the specification here and use input URL. We should probably make it more explicit what the SharedWorker map is and where it lives. (Unit of browsing contexts?)
  • Need to investigate whether we want to keep URLMismatchError. Gecko gets away with not implementing it.
  • Kill a worker and terminate a worker should xref "in parallel".
  • Need to decide on a global object IDL strategy: https://www.w3.org/Bugs/Public/show_bug.cgi?id=28765#c3.
  • Inline URLReadonlyUtils per discussion with @domenic.
  • Can we forbid username/password in the URL still?
  • SharedWorker name argument defaulting should move to IDL.
  • Look into whether fragment identifiers are ignored when comparing URLs.
  • Define 404 handling for importScripts() better: https://www.w3.org/Bugs/Public/show_bug.cgi?id=27276.
  • Define muted errors flag for importScripts(): https://www.w3.org/Bugs/Public/show_bug.cgi?id=28961.

Anything else?
annevk added a commit that referenced this issue Sep 18, 2015
@annevk
Give workers a base URL and clean up shared workers
152884d 
This fixes the following from #164:

* It gives workers a base URL that matches the response's url. Per https://www.w3.org/Bugs/Public/show_bug.cgi?id=28835 this matches Firefox and would also be more consistent with how the platform determines base URLs in general. (The fix is slightly ugly as it puts the URL on the global object so it can be overridden later. In HTML Standard theory the global object is optional and non-JavaScript languages need not provide it, but I think we should move away from that and while file a follow up issue on the matter.)

* It keys shared worker on constructor URL rather than "location". The latter takes redirects into account and if implemented results in racy behavior. Fortunately, neither Chrome nor Firefox implemented this and instead already use this new "constructor url" concept.

* It references "in parallel" where used.

* It defaults name to the empty string through IDL.
@annevk annevk mentioned this issue Sep 18, 2015
Merged
annevk added a commit that referenced this issue Sep 18, 2015
@annevk
Throw NetworkError for cross-origin importScripts() exceptions
f29b22b 
This fixes the following issues from #164:

* importScripts() can fetch “no-cors” cross-origin resources. If those
fail to parse or throw an exception we shouldn’t simply rethrow that
without muting it as that would leak data that <script> does not leak.
This makes them throw a NetworkError instead.

* This also clarifies that a NetworkError is thrown if the response is
not an ok status using the Fetch terminology.
@annevk annevk mentioned this issue Sep 18, 2015
Closed
@annevk
Copy link
Member Author

annevk commented Sep 18, 2015

Fragment identifiers are included as part of the constructor url and as part of self.location. (As @Ms2ger said Chrome does indeed not consider redirects for self.location, Firefox does.)

@annevk
Copy link
Member Author

annevk commented Sep 18, 2015

@kinu, @horo-t, @sof, thoughts on removing URLMismatchError in Chromium?

@domenic domenic added clarification Standard could be clearer normative change labels Sep 18, 2015
annevk added a commit that referenced this issue Sep 21, 2015
@annevk
Give workers a base URL and clean up shared workers
b620471 
This fixes the following from #164:

* It gives workers a base URL that matches the response's url. Per https://www.w3.org/Bugs/Public/show_bug.cgi?id=28835 this matches Firefox and would also be more consistent with how the platform determines base URLs in general. (The fix is slightly ugly as it puts the URL on the global object so it can be overridden later. In HTML Standard theory the global object is optional and non-JavaScript languages need not provide it, but I think we should move away from that: #167.)

* It keys shared worker on constructor URL rather than "location". The latter takes redirects into account and if implemented results in racy behavior. Fortunately, neither Chrome nor Firefox implemented this and instead already use this new "constructor url" concept.

* It references "in parallel" where used.

* It defaults name to the empty string through IDL.
annevk added a commit that referenced this issue Sep 22, 2015
@annevk
Make shared workers always key on constructor url and name
7738ff6 
This matches Firefox, simplifies the model, and removes the need for
URLMismatchError.

Fixes part of #164.
@annevk annevk mentioned this issue Sep 22, 2015
Merged
annevk added a commit that referenced this issue Sep 23, 2015
@annevk @domenic
Throw NetworkError for cross-origin importScripts() exceptions
04ba30e 
This fixes the following issues from #164:

* importScripts() can fetch “no-cors” cross-origin resources. If those
fail to parse or throw an exception we shouldn’t simply rethrow that
without muting it as that would leak data that <script> does not leak.
This makes them throw a NetworkError instead.

* This also clarifies that a NetworkError is thrown if the response is
not an ok status using the Fetch terminology.
annevk added a commit that referenced this issue Sep 23, 2015
@annevk @domenic
Make shared workers always key on constructor url and name
b4c3ff6 
This matches Firefox, simplifies the model, and removes the need for
URLMismatchError.

Fixes part of #164.
annevk added a commit that referenced this issue Sep 23, 2015
@annevk @domenic
Make shared workers always key on constructor url and name
b261d16 
This matches Firefox, simplifies the model, and removes the need for
URLMismatchError.

Fixes part of #164.
annevk added a commit that referenced this issue Sep 24, 2015
@annevk
Inline URLUtilsReadOnly since the URLUtils abstraction is not working…
05ce65a 
… out

See #164 and whatwg/url#62 for details.
annevk added a commit that referenced this issue Sep 24, 2015
@annevk
Throw NetworkError for cross-origin importScripts() exceptions
79c2e37 
This fixes the following issues from #164:

* importScripts() can fetch “no-cors” cross-origin resources. If those
  fail to parse or throw an exception we shouldn’t simply rethrow that
  without muting it as that would leak data that <script> does not
  leak. This makes them throw a NetworkError instead.

* This also clarifies that a NetworkError is thrown if the response is
  not an ok status using the Fetch terminology.
annevk added a commit that referenced this issue Sep 24, 2015
@annevk
Throw NetworkError for cross-origin importScripts() exceptions
1e865ed 
This fixes the following issues from #164:

* importScripts() can fetch “no-cors” cross-origin resources. If those
  fail to parse or throw an exception we shouldn’t simply rethrow that
  without muting it as that would leak data that <script> does not
  leak. This makes them throw a NetworkError instead.

* This also clarifies that a NetworkError is thrown if the response is
  not an ok status using the Fetch terminology.

Also add missing data-x="" attribute that should have been part of
8d239dd.
@annevk
Copy link
Member Author

annevk commented Sep 24, 2015

I'm going to punt on figuring out whether username/password can be forbidden. If anyone is interested in that I recommend figuring it out and then letting this repository know a change is needed. 😊

Inlining URLUtilsReadOnly is now #187 so closing this.

@annevk annevk closed this as completed Sep 24, 2015
annevk added a commit that referenced this issue Sep 25, 2015
@annevk
Inline URLUtilsReadOnly
32a7a20 
The URLUtils abstraction is not working out. See #164 and whatwg/url#62 for details.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
clarification Standard could be clearer normative change
Milestone
No milestone
Development

No branches or pull requests
2 participants
@domenic @annevk