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

Implement HTML5 task sources #7959

Closed
jdm opened this issue Oct 10, 2015 · 7 comments
Closed

Implement HTML5 task sources #7959

jdm opened this issue Oct 10, 2015 · 7 comments

Comments

@jdm
Copy link
Member

@jdm jdm commented Oct 10, 2015

Currently there's a generic script event loop sender for all non-specialized script events (ie. those that do not come from devtools, constellation, etc.) Like #503 we should add additional senders for each task source the spec defines and provide convenient APIs to allow people implementing DOM specs to do the right thing.

Note that this will likely be more complex than the generic select mechanism we currently use in the script event loop, due to ordering concerns about queued events in multiple task sources.

@jdm
Copy link
Member Author

@jdm jdm commented Oct 10, 2015

https://html.spec.whatwg.org/multipage/webappapis.html#generic-task-sources
Then again, the spec says:
"Select the oldest task on one of the event loop's task queues, if any, ignoring, in the case of a browsing context event loop, tasks whose associated Documents are not fully active. The user agent may pick any task queue."
Which suggests that only ordering inside of any given queue matters.

@KiChjang KiChjang self-assigned this Dec 4, 2015
@KiChjang
Copy link
Member

@KiChjang KiChjang commented Dec 4, 2015

Sounds interesting, I'm taking a look into this.

@jdm
Copy link
Member Author

@jdm jdm commented Dec 4, 2015

This would be super valuable!

@jdm
Copy link
Member Author

@jdm jdm commented Dec 4, 2015

I recommend we start by defining methods on Window/ScriptTask/GlobalRef for specific task sources (DOM, network, user input, etc.) and making them return the existing script channel, then finding callers of script_chan() that should be using those specific task sources and having them call the methods to get their channel.

@KiChjang
Copy link
Member

@KiChjang KiChjang commented Dec 4, 2015

Yes, that sounds like a good first PR with a lot of elbow-grease.

After that, I was thinking that we may want to create new message enums (similar to the style for the constellation) for each individual task source. Does that sound good to you?

@jdm
Copy link
Member Author

@jdm jdm commented Dec 4, 2015

Yep!

@KiChjang KiChjang added the C-assigned label Dec 5, 2015
bors-servo added a commit that referenced this issue Dec 5, 2015
Split fn script_chan into 5 different task channel fn

Partial #7959.

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/8853)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue Dec 7, 2015
Split fn script_chan into 5 different task channel fn

Partial #7959.

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/8853)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue Dec 9, 2015
Split script_chan into 5 different task source channels

This is **not** complete. I really need feedback right away since I felt that the direction I'm heading is very wrong.

Partial #7959.

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/8871)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue Jan 1, 2016
Add 5 different task source channels

This is **not** complete. I really need feedback right away since I felt that the direction I'm heading is very wrong.

Partial #7959.

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/8871)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue Mar 10, 2016
Redesign ScriptMsg to be more specific to DOMManipulationTaskSource

This is a large-ish PR that contains the following:
* A new directory is created under `components/script/` called `task_source`, which houses all the stuff for different task sources. Note that the ones that I have now aren't exhaustive - there are more task sources than just the generic ones.
* A `DOMManipulationTaskMsg` which eliminates some usage of `Runnable`s to fire events. Instead, they send event information to the `DOMManipulationTaskSource` and lets the `ScriptTask` handle all the event firing.
* Re-added `fn script_chan`, since I can't think of any other way to give `Trusted` values an appropriate sender.
* Rewrote step 7 of [the end](https://html.spec.whatwg.org/multipage/syntax.html#the-end) to make use of the `DOMManipulationTaskSource`

Partial #7959

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/9217)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue May 11, 2016
Implement user interaction task source

Part of #7959.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/10714)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue May 12, 2016
Implement user interaction task source

Part of #7959.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/10714)
<!-- Reviewable:end -->
@jdm jdm mentioned this issue Jul 14, 2016
4 of 4 tasks complete
bors-servo added a commit that referenced this issue Jul 14, 2016
Implement file reading task source

Implement the task source API for the File Reader task source, enabling using task sources from non-main threads.

---
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix (partially) #7959 (github issue number if applicable).
- [X] These changes do not require tests because they're refactoring existing code

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/12448)
<!-- Reviewable:end -->
@KiChjang
Copy link
Member

@KiChjang KiChjang commented Dec 11, 2016

I think this is essentially done except for HistoryTraversalTaskSource (which is still currently a stub using the ScriptChan trait) which is tracked in #10994 and other various small task sources littered around the spec.

@KiChjang KiChjang closed this Dec 11, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.