Skip to content
This repository has been archived by the owner on Oct 21, 2022. It is now read-only.

Commit

Permalink
In the latest Chrome release, it appears that scripts loaded dynamica…
Browse files Browse the repository at this point in the history 
…lly without an async attr will now block rendering. This change restores the non-blocking behavior that was present before the latest chrome update.
  • Loading branch information
@scottjehl
scottjehl committed Oct 15, 2014
1 parent 0d96d2d commit 45e41fb
Showing 1 changed file with 1 addition and 0 deletions.
1 change: 1 addition & 0 deletions loadJS.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ function loadJS( src, cb ){
var ref = window.document.getElementsByTagName( "script" )[ 0 ];
var script = window.document.createElement( "script" );
script.src = src;
script.async = true;
ref.parentNode.insertBefore( script, ref );
if (cb) {
script.onload = cb;
Expand Down

11 comments on commit 45e41fb

@scottjehl
Copy link
Member Author

Choose a reason for hiding this comment

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

Hey @paulirish before I tag a release for this, does this make sense? It seems like Chrome's handling of dynamically injected script tags must have changed...

@paulirish
Copy link

Choose a reason for hiding this comment

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

Hmm.. This would surprise me. Do you have have a test page where we can clearly see the difference between two Chrome versions?

cc @igrigorik @tonygentilcore

@paulirish
Copy link

Choose a reason for hiding this comment

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

Are you talking about Chrome 37 v 38 (the recent stable bump)?

@scottjehl
Copy link
Member Author

Choose a reason for hiding this comment

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

Hey Paul... yeah, super troubling.

I was seeing it when testing with my connection throttled to 3G. I was consistently seeing layout blocked up-front if that attr wasn't there. I'll make a reduced test page for ya. Not sure which version but we started seeing this very recently.

@pmeenan
Copy link

Choose a reason for hiding this comment

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

If it's done while in the head block it's probably from fixing a regression that was introduced in 34: https://code.google.com/p/chromium/issues/detail?id=391741&can=1&q=owner%3Apmeenan%40chromium.org&colspec=ID%20Pri%20M%20Iteration%20ReleaseBlock%20Cr%20Status%20Owner%20Summary%20OS%20Modified

@pmeenan
Copy link

Choose a reason for hiding this comment

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

The code that explicitly moved async/defer out of the blocking path was from this bug: https://code.google.com/p/chromium/issues/detail?id=408229&can=1&q=owner%3Apmeenan%40chromium.org&colspec=ID%20Pri%20M%20Iteration%20ReleaseBlock%20Cr%20Status%20Owner%20Summary%20OS%20Modified

@scottjehl
Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for chiming in, @pmeenan

I'm having a little trouble following the tracker notes there. Is it true that at some point, dynamically-injected script tags were bumped up in priority so that they started blocking rendering? If so, is that new behavior intentional or should I file a bug? The reason I ask is I assume it's very common to dynamically load scripts this way, and now an awful lot of sites may unintentionally have render-blocking scripts in Chrome

@scottjehl
Copy link
Member Author

Choose a reason for hiding this comment

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

Well, now I'm sort of stumped. I made an isolated test that uses loadJS to request a slow file and it's definitely not blocking rendering regardless of whether the async attr is there or not. http://scottjehl.com/misc/loadjs/

But the issue still seems to be happening in a client site, so maybe there are more complications in play

@mathiasbynens
Copy link

Choose a reason for hiding this comment

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

I doubt the change in behavior is intentional, as it’s a violation of the HTML Standard.

@pmeenan
Copy link

Choose a reason for hiding this comment

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

It doesn't block render or parse, it just keeps Chrome in it's "critical resources" loading phase if it is fetched in the head (where Chrome will push back on loading images until the higher priority resources have loaded). The spec actually dictates the main parser behavior and not the loading of the resources (this is really impacting the preload scanner-discovered resources).

There are a bunch of things at play and one unintended consequence for this fix:

  • Chrome 34 broke it's handling of delaying non-critical fetches so everything the preload scanner or parser hit would be loaded as soon as possible. Good for overall loading of resources but really bad for initial render and the early parsing because huge images from the body would compete with the css and js from the head.
  • Chrome 38 fixed the regression so that the 2 loading phases would work again.
  • Chrome 38 also lowered the priority of async resources so they don't compete with blocking scripts and don't load during the critical phase.
  • Chrome 38 also fixed an issue where it would exit the critical loading phase when the parser reached the head tag, even if the css had not finished loading (which is bad for render because css doesn't block the parser but it does block layout and paint)

The last fix had a side-effect that it also waits for any scripts that were discovered to finish loading, even if they are not parser-blocking. I opened a bug on it and should have it fixed in Canary in the next couple of days.

All of the fixes above were reasonably big wins for render and script loading performance in aggregate. I'd expect the new fix may trade off an improvement in render for a slow down in DOM Content Loaded (as scripts will be competing with images).

@pmeenan
Copy link

Choose a reason for hiding this comment

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

btw, the reason the test page doesn't show an issue is that it doesn't actually block render or the parser, it just loads images slower until the script loads. If you throw a bunch of images on the page and make the script take a long time to load you should see the images load one at a time until the script finishes loading and then they would start to load 10 at a time.

Please sign in to comment.