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

Synchronous script loading during HTML parsing #3721

Merged
merged 5 commits into from
Oct 29, 2014

Conversation

mbrubeck
Copy link
Contributor

Fixes #3356. This makes script elements fetch their source synchronously and execute immediately by default. It also lays some groundwork for future deferred or async script loading.

(Depends on #3716 to fix a content test failure caused by running script during parsing.)

@hoppipolla-critic-bot
Copy link

Critic review: https://critic.hoppipolla.co.uk/r/2904

This is an external review system which you may optionally use for the code review of your pull request.

In order to help critic track your changes, please do not make in-place history rewrites (e.g. via git rebase -i or git commit --amend) when updating this pull request.

@mbrubeck mbrubeck force-pushed the script-sync branch 2 times, most recently from 585b7cf to b807bcb Compare October 29, 2014 19:51
bors-servo pushed a commit that referenced this pull request Oct 29, 2014
Fixes #3356.  This makes script elements fetch their source synchronously and execute immediately by default.  It also lays some groundwork for future deferred or async script loading.

(Depends on #3716 to fix a content test failure caused by running script during parsing.)
@larsbergstrom
Copy link
Contributor

Possibly intermittent?


/dom/nodes/Document-createElement-namespace.html
------------------------------------------------
CRASH expected TIMEOUT [Parent]
/html/browsers/the-window-object/accessing-other-browsing-contexts/indexed-browsing-contexts-01.html
----------------------------------------------------------------------------------------------------
TIMEOUT expected CRASH [Parent]
/html/dom/documents/resource-metadata-management/document-readyState.html
-------------------------------------------------------------------------
PASS expected FAIL readystatechange event is fired each time document.readyState changes
16:36.71 LOG: MainThread INFO Closing logging queue

This implements the parts of the "prepare a script element" algorithm that are
required for synchronous scripts.  It also adds some infrastructure for future
support of the `async` and `defer` attributes.
This removes the old code for asyncronously loading scripts during HTML
parsing and then executing them afterward.

Fixes servo#3356.
This fixes a performance regression caused by the previous patches.  Once we
allowed script and layout to run during parsing, it was running too often
(every time the document changed and called window.reflow).

Fixes servo#1269.
bors-servo pushed a commit that referenced this pull request Oct 29, 2014
Fixes #3356.  This makes script elements fetch their source synchronously and execute immediately by default.  It also lays some groundwork for future deferred or async script loading.

(Depends on #3716 to fix a content test failure caused by running script during parsing.)
@bors-servo bors-servo closed this Oct 29, 2014
@bors-servo bors-servo merged commit b2c211e into servo:master Oct 29, 2014
// the origin of the script element's node document, and the default origin
// behaviour set to taint.
match load_whole_resource(&page.resource_task, url) {
Ok((metadata, bytes)) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the JS event loop get to spin while this happens?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, this blocks the script thread. We need to fix this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Filed #4088.

@mbrubeck mbrubeck deleted the script-sync branch May 11, 2016 03:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Move script loading out of the HTML parser
5 participants