Block the HTML parser on external stylesheets

Block the HTML parser on external stylesheets

**** Not yet ready for commit - Initial/experimental test ****

Closed in favor of https://codereview.chromium.org/1903803002/

By blocking the main parser on external stylesheets Chrome can allow
rendering to continue for styles and the part of the DOM tree that has
already been built without needing to worry about content referenced
after the style.  This mirror's The Edge/IE behavior.

Behind an experimental blink feature flag:
- Pending CSS will not block render
- Background parser breaks chunks after style/link tags so the main
  parser can cleanly pause parsing if either triggered a stylesheet to
  load.
- Main parser pauses parsing while blocking stylesheets are pending
  (keeping the existing logic for tracking pending blocking stylesheets).
- Parsing is resumed when the pending sheet(s) have loaded.

BUG=481122

[Pat Meenan, 2016-03-07 19:44:35]

pmeenan@chromium.org changed reviewers:
+ esprehn@chromium.org

[Pat Meenan, 2016-03-07 19:44:35]

esprehn@ could you take a quick peek and see if this is what you had in mind
directionally or if I'm violating layering of some kind by reaching into the
styleEngine from the HTML parser to see if there are pending sheets?

Functionally it works as expected though I haven't run it against the layout
tests yet (expect there may be some breakage if they are timing sensitive or
explicitly tested for the other behavior).

[esprehn, 2016-03-07 21:35:51]

https://codereview.chromium.org/1772853002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/dom/Document.cpp (right):

https://codereview.chromium.org/1772853002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/dom/Document.cpp:2100: return haveImportsLoaded()
&& (RuntimeEnabledFeatures::htmlParserBlocksOnCSSEnabled() ||
haveStylesheetsLoaded());
I don't think you want to do this, isRenderingReady() prevents painting, but the
parser being blocked on sheet shouldn't block painting. You want the content to
paint.

https://codereview.chromium.org/1772853002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/html/parser/HTMLDocumentParser.cpp (right):

https://codereview.chromium.org/1772853002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/html/parser/HTMLDocumentParser.cpp:999: return
RuntimeEnabledFeatures::htmlParserBlocksOnCSSEnabled() &&
document()->styleEngine().hasPendingSheets();
you need to block on imports too, does this do that?

[Pat Meenan, 2016-03-07 21:49:53]

https://codereview.chromium.org/1772853002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/dom/Document.cpp (right):

https://codereview.chromium.org/1772853002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/dom/Document.cpp:2100: return haveImportsLoaded()
&& (RuntimeEnabledFeatures::htmlParserBlocksOnCSSEnabled() ||
haveStylesheetsLoaded());
On 2016/03/07 21:35:50, esprehn wrote:
> I don't think you want to do this, isRenderingReady() prevents painting, but
the
> parser being blocked on sheet shouldn't block painting. You want the content
to
> paint.

Right.  The change basically removes the check of stylesheets having been loaded
from the "isRenderingReady" check.  When the new code is enabled, the pending
stylesheets won't be checked for "isRenderingReady".

Given that I need to look at blocking on imports as well this will change a bit
though and neither will block rendering when the code is enabled.

https://codereview.chromium.org/1772853002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/html/parser/HTMLDocumentParser.cpp (right):

https://codereview.chromium.org/1772853002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/html/parser/HTMLDocumentParser.cpp:999: return
RuntimeEnabledFeatures::htmlParserBlocksOnCSSEnabled() &&
document()->styleEngine().hasPendingSheets();
On 2016/03/07 21:35:50, esprehn wrote:
> you need to block on imports too, does this do that?

No, thanks.  The chunks are set up so imports will also be able to be blocked on
but the code as it stands wouldn't.  I'll get that added tomorrow.

[Pat Meenan, 2016-03-08 15:34:54]

It now blocks on HTML5 imports as well.  There are a collection of test pages
that exercise CSS/Import at various document locations and injected by various
means: http://test.patrickmeenan.com/css/

I'm a little worried about the perf implications of blocking the parser on
imports in the head (particularly if a lot are referenced).

Do you happen to know if they are currently processed serially (in which case it
would be a no-op) or are they allowed to parse/execute asyc from each other (in
which case this could be a huge regression)?

Right now it also blocks on imports that are "appended" to the DOM through JS. 
It's better than the current behavior that turns off paints entirely but didn't
know if that should be completely async like it is for most append operations
(script and stylesheets notably).

[Pat Meenan, 2016-03-10 16:16:26]

The CQ bit was checked by pmeenan@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-03-10 16:17:23]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1772853002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1772853002/40001

[commit-bot: I haz the power, 2016-03-10 17:01:52]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-03-10 17:01:52]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Pat Meenan, 2016-03-14 15:01:18]

The CQ bit was checked by pmeenan@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-03-14 15:01:37]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1772853002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1772853002/60001

[commit-bot: I haz the power, 2016-03-14 15:43:13]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-03-14 15:43:14]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Pat Meenan, 2016-03-15 17:50:11]

The CQ bit was checked by pmeenan@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-03-15 17:50:31]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1772853002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1772853002/80001

[commit-bot: I haz the power, 2016-03-15 18:32:08]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-03-15 18:32:09]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[esprehn, 2016-03-18 05:08:08]

On 2016/03/08 at 15:34:54, pmeenan wrote:
> It now blocks on HTML5 imports as well.  There are a collection of test pages
that exercise CSS/Import at various document locations and injected by various
means: http://test.patrickmeenan.com/css/
> 
> I'm a little worried about the perf implications of blocking the parser on
imports in the head (particularly if a lot are referenced).
> 
> Do you happen to know if they are currently processed serially (in which case
it would be a no-op) or are they allowed to parse/execute asyc from each other
(in which case this could be a huge regression)?

They're processed in parallel which means this is a bad regression, I think we
want the parser to never block on sheets in the <head> ?

> 
> Right now it also blocks on imports that are "appended" to the DOM through JS.
 It's better than the current behavior that turns off paints entirely but didn't
know if that should be completely async like it is for most append operations
(script and stylesheets notably).

[esprehn, 2016-04-06 21:51:12]

esprehn@chromium.org changed reviewers:
+ kouhei@chromium.org

[esprehn, 2016-04-06 21:51:13]

kouhei@ Can you look this over?

[kouhei (in TOK), 2016-04-26 23:56:35]

pmeenan: Would you tell me the status of this CL?

[Pat Meenan, 2016-04-27 00:55:00]

On 2016/04/26 23:56:35, kouhei wrote:
> pmeenan: Would you tell me the status of this CL?

I'm in the process of replacing it with this one which only pauses when in the
body and is somewhat cleaner: https://codereview.chromium.org/1903803002/

That said, I'm super-sketchy about the pausing and resuming of the parser and
getting the edge cases working. If it is too hairy I will fall back to just not
blocking paints but letting the parser continue. 

I was going to get the unit/layout tests working and then send it around but if
you have time to peek at it and let me know if it's a good idea to do or not it
would be appreciated.

[Pat Meenan, 2016-04-28 17:32:05]

Description was changed from

==========
Block the HTML parser on external stylesheets

**** Not yet ready for commit - Initial/experimental test ****

By blocking the main parser on external stylesheets Chrome can allow
rendering to continue for styles and the part of the DOM tree that has
already been built without needing to worry about content referenced
after the style.  This mirror's The Edge/IE behavior.

Behind an experimental blink feature flag:
- Pending CSS will not block render
- Background parser breaks chunks after style/link tags so the main
  parser can cleanly pause parsing if either triggered a stylesheet to
  load.
- Main parser pauses parsing while blocking stylesheets are pending
  (keeping the existing logic for tracking pending blocking stylesheets).
- Parsing is resumed when the pending sheet(s) have loaded.

BUG=481122
==========

to

==========
Block the HTML parser on external stylesheets

**** Not yet ready for commit - Initial/experimental test ****

By blocking the main parser on external stylesheets Chrome can allow
rendering to continue for styles and the part of the DOM tree that has
already been built without needing to worry about content referenced
after the style.  This mirror's The Edge/IE behavior.

Behind an experimental blink feature flag:
- Pending CSS will not block render
- Background parser breaks chunks after style/link tags so the main
  parser can cleanly pause parsing if either triggered a stylesheet to
  load.
- Main parser pauses parsing while blocking stylesheets are pending
  (keeping the existing logic for tracking pending blocking stylesheets).
- Parsing is resumed when the pending sheet(s) have loaded.

BUG=481122
==========

[Pat Meenan, 2016-04-28 17:32:32]

Description was changed from

==========
Block the HTML parser on external stylesheets

**** Not yet ready for commit - Initial/experimental test ****

By blocking the main parser on external stylesheets Chrome can allow
rendering to continue for styles and the part of the DOM tree that has
already been built without needing to worry about content referenced
after the style.  This mirror's The Edge/IE behavior.

Behind an experimental blink feature flag:
- Pending CSS will not block render
- Background parser breaks chunks after style/link tags so the main
  parser can cleanly pause parsing if either triggered a stylesheet to
  load.
- Main parser pauses parsing while blocking stylesheets are pending
  (keeping the existing logic for tracking pending blocking stylesheets).
- Parsing is resumed when the pending sheet(s) have loaded.

BUG=481122
==========

to

==========
Block the HTML parser on external stylesheets

**** Not yet ready for commit - Initial/experimental test ****

Closed in favor of https://codereview.chromium.org/1903803002/

By blocking the main parser on external stylesheets Chrome can allow
rendering to continue for styles and the part of the DOM tree that has
already been built without needing to worry about content referenced
after the style.  This mirror's The Edge/IE behavior.

Behind an experimental blink feature flag:
- Pending CSS will not block render
- Background parser breaks chunks after style/link tags so the main
  parser can cleanly pause parsing if either triggered a stylesheet to
  load.
- Main parser pauses parsing while blocking stylesheets are pending
  (keeping the existing logic for tracking pending blocking stylesheets).
- Parsing is resumed when the pending sheet(s) have loaded.

BUG=481122
==========