Do not block painting for in-body CSS

Do not block painting for in-body CSS

This mirrors the Firefox behavior where in-body stylesheets
are not render-blocking.  Developers can force the parser to
block and get Edge/IE behavior by inserting script tags after
the stylesheets (they will need to anyway because of Firefox).

BUG=481122
Committed: https://crrev.com/ef8540e50d8f1e1cbd8b403b393a90d29985b9c5
Cr-Commit-Position: refs/heads/master@{#393347}

[Pat Meenan, 2016-04-26 20:17:26]

Description was changed from

==========
Block HTML parser for in-body CSS

NOT READY FOR REVIEW - just staging work in progress

Currently tracks head and in-body css separately and does not block
painting for in-body css.

Still needed:

- Make sure it works for @import that was referenced from a css in the
head but only started loading after the parser already reached the body.

- Break the HTML chunks from the background parser after external style
and HTML5import link tags (take from
https://codereview.chromium.org/1772853002/)

- Pause the main parser if script is not ready to execute (i.e. pending
import or style) and the body has been inserted.

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

to

==========
Block HTML parser for in-body CSS

NOT READY FOR REVIEW - just staging work in progress

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 in the body 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 script-blocking resources are pending
  (stylesheets and imports) and the parser is in the body.
- Parsing is resumed when the pending resource(s) have loaded.

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

[Pat Meenan, 2016-04-28 17:31:15]

Description was changed from

==========
Block HTML parser for in-body CSS

NOT READY FOR REVIEW - just staging work in progress

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 in the body 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 script-blocking resources are pending
  (stylesheets and imports) and the parser is in the body.
- Parsing is resumed when the pending resource(s) have loaded.

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

to

==========
Do not block painting for in-body CSS

NOT READY FOR REVIEW - just staging work in progress

This mirrors the Firefox behavior where in-body stylesheets are not
render-blocking.  Developers can force the parser to block and get Edge/IE
behavior by inserting script tags after the stylesheets (they will need to
anyway because of Firefox).

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

[Pat Meenan, 2016-04-29 18:26:56]

Description was changed from

==========
Do not block painting for in-body CSS

NOT READY FOR REVIEW - just staging work in progress

This mirrors the Firefox behavior where in-body stylesheets are not
render-blocking.  Developers can force the parser to block and get Edge/IE
behavior by inserting script tags after the stylesheets (they will need to
anyway because of Firefox).

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

to

==========
Do not block painting for in-body CSS

This mirrors the Firefox behavior where in-body stylesheets 
are not render-blocking.  Developers can force the parser to 
block and get Edge/IE behavior by inserting script tags after 
the stylesheets (they will need to anyway because of Firefox).

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

[Pat Meenan, 2016-04-29 18:29:23]

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

[Pat Meenan, 2016-04-29 18:29:23]

esprehn@ PTAL

This does not block the parser, it just prevents in-body styles from blocking
render.  The parser blocking can be layered on top of this pretty cleanly but
can also be done independently (and is somewhat harrier).

[esprehn, 2016-05-06 21:50:15]

lgtm, but it's unfortunate we need to duplicate this logic in two places. Can we
do it inside the StyleEngine?

https://codereview.chromium.org/1903803002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/dom/StyleElement.cpp (right):

https://codereview.chromium.org/1903803002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/dom/StyleElement.cpp:195: m_beforeBody =
m_beforeBody || !document.body();
Can we move this into the StyleEngine instead?

[Pat Meenan, 2016-05-09 18:51:05]

Double-check my understanding but AFAIK we have to attach the state of "before
body" to the sheets themselves when they are created because when they finish
the parser may have gone ahead and created the body so the style engine needs to
be told that the sheet was originally started before the body exists.

https://codereview.chromium.org/1903803002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/dom/StyleElement.cpp (right):

https://codereview.chromium.org/1903803002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/dom/StyleElement.cpp:195: m_beforeBody =
m_beforeBody || !document.body();
On 2016/05/06 21:50:15, esprehn wrote:
> Can we move this into the StyleEngine instead?

The main issue is that we need to keep track of if the sheet was originally
created/requested before the body so when it finishes loading and we remove it
we also decrement the "before body" counter.  The style engine could handle the
part about when it is created but when the sheet is removed it doesn't have the
context to know (at least AFAIK).

It would be possible if the parser didn't keep going after the sheet is created
but as it stands right now, the parser can be before the body when a sheet is
created and after the body when it finishes so the state needed to be attached
to the sheet itself.

[esprehn, 2016-05-09 23:12:39]

Right, I just mean having this book keeping in all these places seems bad,
ideally we'd have a StyleCandidate object or something that both subclassed or
contained that had the shared bookkeeping. We can go forward with this though. I
guess StyleElement is that thing too, it's just ProcessingInstruction that's
weird.

[esprehn, 2016-05-09 23:13:24]

Ah no, so this has the logic in <link>, <style> and ProcessingInstruction.
Ideally they'd add have a StyleHost superclass or something.

[Pat Meenan, 2016-05-10 00:09:07]

On 2016/05/09 23:13:24, esprehn wrote:
> Ah no, so this has the logic in <link>, <style> and ProcessingInstruction.
> Ideally they'd add have a StyleHost superclass or something.

I like the idea of a StyleEngineContext object that is contained by all of the
places where the tracking is done that the StyleEngine can use to carry the
state (subclassing feels like it might be overkill).  I'll get it implemented
and sent over in the morning.

[Pat Meenan, 2016-05-10 16:07:59]

On 2016/05/10 00:09:07, Pat Meenan wrote:
> On 2016/05/09 23:13:24, esprehn wrote:
> > Ah no, so this has the logic in <link>, <style> and ProcessingInstruction.
> > Ideally they'd add have a StyleHost superclass or something.
> 
> I like the idea of a StyleEngineContext object that is contained by all of the
> places where the tracking is done that the StyleEngine can use to carry the
> state (subclassing feels like it might be overkill).  I'll get it implemented
> and sent over in the morning.

Could you PTAL again?  I just moved the before body state tracking into a
StyleEngineContext member and put the actual logic for checking it in there.

[Pat Meenan, 2016-05-10 17:05:13]

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

[commit-bot: I haz the power, 2016-05-10 17:05:58]

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

[commit-bot: I haz the power, 2016-05-10 20:58:08]

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

[commit-bot: I haz the power, 2016-05-10 20:58:09]

Dry run: This issue passed the CQ dry run.

[esprehn, 2016-05-12 17:17:16]

lgtm

[esprehn, 2016-05-12 17:18:33]

Yeah that's much better. Thanks!

[Pat Meenan, 2016-05-12 17:36:52]

The CQ bit was checked by pmeenan@chromium.org

[commit-bot: I haz the power, 2016-05-12 17:37:20]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1903803002/160001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1903803002/160001

[commit-bot: I haz the power, 2016-05-12 17:39:19]

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

[commit-bot: I haz the power, 2016-05-12 17:39:21]

Try jobs failed on following builders:
  ios-device-gn on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-gn/bui...)
  ios-simulator on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  ios-simulator-gn on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-gn/...)

[Pat Meenan, 2016-05-12 19:29:47]

The CQ bit was checked by pmeenan@chromium.org

[Pat Meenan, 2016-05-12 19:29:47]

The patchset sent to the CQ was uploaded after l-g-t-m from esprehn@chromium.org
Link to the patchset: https://codereview.chromium.org/1903803002/#ps200001
(title: "Fixed merge conflict")

[commit-bot: I haz the power, 2016-05-12 19:30:08]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1903803002/200001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1903803002/200001

[commit-bot: I haz the power, 2016-05-12 20:53:31]

Description was changed from

==========
Do not block painting for in-body CSS

This mirrors the Firefox behavior where in-body stylesheets 
are not render-blocking.  Developers can force the parser to 
block and get Edge/IE behavior by inserting script tags after 
the stylesheets (they will need to anyway because of Firefox).

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

to

==========
Do not block painting for in-body CSS

This mirrors the Firefox behavior where in-body stylesheets 
are not render-blocking.  Developers can force the parser to 
block and get Edge/IE behavior by inserting script tags after 
the stylesheets (they will need to anyway because of Firefox).

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

[commit-bot: I haz the power, 2016-05-12 20:53:32]

Committed patchset #11 (id:200001)

[commit-bot: I haz the power, 2016-05-12 20:54:55]

Description was changed from

==========
Do not block painting for in-body CSS

This mirrors the Firefox behavior where in-body stylesheets 
are not render-blocking.  Developers can force the parser to 
block and get Edge/IE behavior by inserting script tags after 
the stylesheets (they will need to anyway because of Firefox).

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

to

==========
Do not block painting for in-body CSS

This mirrors the Firefox behavior where in-body stylesheets
are not render-blocking.  Developers can force the parser to
block and get Edge/IE behavior by inserting script tags after
the stylesheets (they will need to anyway because of Firefox).

BUG=481122

Committed: https://crrev.com/ef8540e50d8f1e1cbd8b403b393a90d29985b9c5
Cr-Commit-Position: refs/heads/master@{#393347}
==========

[commit-bot: I haz the power, 2016-05-12 20:54:56]

Patchset 11 (id:??) landed as
https://crrev.com/ef8540e50d8f1e1cbd8b403b393a90d29985b9c5
Cr-Commit-Position: refs/heads/master@{#393347}

[rune, 2016-05-13 09:15:30]

On 2016/05/12 20:54:56, commit-bot: I haz the power wrote:
> Patchset 11 (id:??) landed as
> https://crrev.com/ef8540e50d8f1e1cbd8b403b393a90d29985b9c5
> Cr-Commit-Position: refs/heads/master@{#393347}

Should we really have landed this without tests?

[esprehn, 2016-05-13 18:45:03]

On 2016/05/13 at 09:15:30, rune wrote:
> On 2016/05/12 20:54:56, commit-bot: I haz the power wrote:
> > Patchset 11 (id:??) landed as
> > https://crrev.com/ef8540e50d8f1e1cbd8b403b393a90d29985b9c5
> > Cr-Commit-Position: refs/heads/master@{#393347}
> 
> Should we really have landed this without tests?

Err yeah that was a mistake, This needs some SimTests. Can you write them
pmeenan@ ? You can look at DocumentLoadingRenderingTest for how to do it.

[Pat Meenan, 2016-05-13 19:43:52]

On 2016/05/13 18:45:03, esprehn wrote:
> On 2016/05/13 at 09:15:30, rune wrote:
> > On 2016/05/12 20:54:56, commit-bot: I haz the power wrote:
> > > Patchset 11 (id:??) landed as
> > > https://crrev.com/ef8540e50d8f1e1cbd8b403b393a90d29985b9c5
> > > Cr-Commit-Position: refs/heads/master@{#393347}
> > 
> > Should we really have landed this without tests?
> 
> Err yeah that was a mistake, This needs some SimTests. Can you write them
> pmeenan@ ? You can look at DocumentLoadingRenderingTest for how to do it.

Sorry, working on it now.  Also taking a look at rune@'s request to unify the
logic for render-blocking and parser-blocking if possible.