Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(198)

Issue 2614663004: Pause HTML parser for external stylesheets in the body (Closed)

Created:
3 years, 11 months ago by Pat Meenan
Modified:
3 years, 11 months ago
CC:
chromium-reviews, blink-reviews-html_chromium.org, blink-reviews-style_chromium.org, sof, eae+blinkwatch, loading-reviews+parser_chromium.org, blink-reviews-dom_chromium.org, dglazkov+blink, blink-reviews, kinuko+watch, rwlbuis
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Pause HTML parser for external stylesheets in the body When in-body CSS does not block paint (experiment flag) this change pauses the HTML parser to make sure paints can continue to happen but no DOM nodes that may depend on the stylesheet will get created. BUG=481122 Review-Url: https://codereview.chromium.org/2614663004 Cr-Commit-Position: refs/heads/master@{#445060} Committed: https://chromium.googlesource.com/chromium/src/+/79ff7175017a071ada9073ee9f37ca2d90ec89a2

Patch Set 1 #

Patch Set 2 : Fixed interaction with imports #

Total comments: 4

Patch Set 3 : Removed url check and combined paused state check into isPaused() #

Patch Set 4 : Merge to trunk #

Patch Set 5 : Added test and fixed issues exposed by test #

Patch Set 6 : merge to trunk #

Total comments: 2

Patch Set 7 : Patched the entrypoints where css could have been added asynchronously #

Patch Set 8 : Added Tests #

Total comments: 2

Patch Set 9 : Merge to trunk #

Patch Set 10 : Enabled threaded parser #

Patch Set 11 : Full set of threaded and non-threaded tests #

Total comments: 2

Patch Set 12 : Paramaterized test cases #

Unified diffs Side-by-side diffs Delta from patch set Stats (+504 lines, -40 lines) Patch
M third_party/WebKit/Source/core/dom/Document.h View 1 2 3 4 5 6 7 8 1 chunk +2 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/dom/Document.cpp View 1 2 3 4 5 6 7 8 2 chunks +10 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/dom/ScriptableDocumentParser.h View 1 2 3 4 5 6 1 chunk +2 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/dom/StyleEngine.h View 1 2 3 4 5 6 1 chunk +1 line, -0 lines 0 comments Download
M third_party/WebKit/Source/core/dom/StyleEngine.cpp View 1 2 3 4 5 6 7 8 2 chunks +10 lines, -1 line 0 comments Download
M third_party/WebKit/Source/core/html/parser/BackgroundHTMLParser.cpp View 1 2 3 4 5 6 7 8 1 chunk +2 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/html/parser/HTMLDocumentParser.h View 1 2 3 4 5 6 6 chunks +14 lines, -4 lines 0 comments Download
M third_party/WebKit/Source/core/html/parser/HTMLDocumentParser.cpp View 1 2 3 4 5 6 18 chunks +80 lines, -34 lines 0 comments Download
M third_party/WebKit/Source/core/html/parser/HTMLTreeBuilderSimulator.h View 1 2 3 4 5 6 7 8 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/core/html/parser/HTMLTreeBuilderSimulator.cpp View 1 2 3 4 5 6 7 8 2 chunks +4 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/web/BUILD.gn View 1 2 3 4 5 6 7 1 chunk +1 line, -0 lines 0 comments Download
A third_party/WebKit/Source/web/tests/HTMLDocumentParserLoadingTest.cpp View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +377 lines, -0 lines 0 comments Download

Messages

Total messages: 39 (20 generated)
Pat Meenan
esprehn@chromium.org: I think I got all of the "pause for external in-body CSS" issues sorted ...
3 years, 11 months ago (2017-01-05 17:46:35 UTC) #10
kouhei (in TOK)
https://codereview.chromium.org/2614663004/diff/20001/third_party/WebKit/Source/core/html/parser/HTMLDocumentParser.cpp File third_party/WebKit/Source/core/html/parser/HTMLDocumentParser.cpp (right): https://codereview.chromium.org/2614663004/diff/20001/third_party/WebKit/Source/core/html/parser/HTMLDocumentParser.cpp#newcode261 third_party/WebKit/Source/core/html/parser/HTMLDocumentParser.cpp:261: if (isStopped() || isWaitingForScripts() || m_isWaitingForStylesheets) Would you mind ...
3 years, 11 months ago (2017-01-06 03:32:44 UTC) #11
Pat Meenan
https://codereview.chromium.org/2614663004/diff/20001/third_party/WebKit/Source/core/html/parser/HTMLDocumentParser.cpp File third_party/WebKit/Source/core/html/parser/HTMLDocumentParser.cpp (right): https://codereview.chromium.org/2614663004/diff/20001/third_party/WebKit/Source/core/html/parser/HTMLDocumentParser.cpp#newcode261 third_party/WebKit/Source/core/html/parser/HTMLDocumentParser.cpp:261: if (isStopped() || isWaitingForScripts() || m_isWaitingForStylesheets) On 2017/01/06 03:32:43, ...
3 years, 11 months ago (2017-01-09 16:37:05 UTC) #12
Pat Meenan
On 2017/01/09 16:37:05, Pat Meenan wrote: > https://codereview.chromium.org/2614663004/diff/20001/third_party/WebKit/Source/core/html/parser/HTMLDocumentParser.cpp > File third_party/WebKit/Source/core/html/parser/HTMLDocumentParser.cpp (right): > > https://codereview.chromium.org/2614663004/diff/20001/third_party/WebKit/Source/core/html/parser/HTMLDocumentParser.cpp#newcode261 ...
3 years, 11 months ago (2017-01-09 18:51:55 UTC) #13
kouhei (in TOK)
lg % tests? On 2017/01/09 18:51:55, Pat Meenan wrote: > On 2017/01/09 16:37:05, Pat Meenan ...
3 years, 11 months ago (2017-01-10 05:52:26 UTC) #14
Pat Meenan
On 2017/01/10 05:52:26, kouhei wrote: > lg % tests? > > On 2017/01/09 18:51:55, Pat ...
3 years, 11 months ago (2017-01-11 18:07:54 UTC) #19
kouhei (in TOK)
On 2017/01/11 18:07:54, Pat Meenan wrote: > On 2017/01/10 05:52:26, kouhei wrote: > > lg ...
3 years, 11 months ago (2017-01-12 11:57:51 UTC) #20
kouhei (in TOK)
https://codereview.chromium.org/2614663004/diff/100001/third_party/WebKit/Source/core/html/parser/HTMLDocumentParser.cpp File third_party/WebKit/Source/core/html/parser/HTMLDocumentParser.cpp (right): https://codereview.chromium.org/2614663004/diff/100001/third_party/WebKit/Source/core/html/parser/HTMLDocumentParser.cpp#newcode564 third_party/WebKit/Source/core/html/parser/HTMLDocumentParser.cpp:564: if (m_isWaitingForStylesheets && it + 1 == tokens->end()) { ...
3 years, 11 months ago (2017-01-12 11:58:13 UTC) #21
Pat Meenan
I'll add a bunch more cases including the ones you called out (as well as ...
3 years, 11 months ago (2017-01-12 13:26:47 UTC) #22
kouhei (in TOK)
On 2017/01/12 13:26:47, Pat Meenan wrote: > I'll add a bunch more cases including the ...
3 years, 11 months ago (2017-01-12 13:33:00 UTC) #23
Pat Meenan
On 2017/01/12 13:33:00, kouhei wrote: > Can we omit the import cases if that makes ...
3 years, 11 months ago (2017-01-13 19:48:22 UTC) #24
Pat Meenan
PTAL. I added ~10 tests that test both cases where it should pause and cases ...
3 years, 11 months ago (2017-01-17 20:54:19 UTC) #27
kouhei (in TOK)
Thanks! New tests lg https://codereview.chromium.org/2614663004/diff/140001/third_party/WebKit/Source/web/tests/HTMLDocumentParserLoadingTest.cpp File third_party/WebKit/Source/web/tests/HTMLDocumentParserLoadingTest.cpp (right): https://codereview.chromium.org/2614663004/diff/140001/third_party/WebKit/Source/web/tests/HTMLDocumentParserLoadingTest.cpp#newcode17 third_party/WebKit/Source/web/tests/HTMLDocumentParserLoadingTest.cpp:17: class HTMLDocumentParserLoadingTest : public SimTest ...
3 years, 11 months ago (2017-01-19 04:19:41 UTC) #30
Pat Meenan
https://codereview.chromium.org/2614663004/diff/140001/third_party/WebKit/Source/web/tests/HTMLDocumentParserLoadingTest.cpp File third_party/WebKit/Source/web/tests/HTMLDocumentParserLoadingTest.cpp (right): https://codereview.chromium.org/2614663004/diff/140001/third_party/WebKit/Source/web/tests/HTMLDocumentParserLoadingTest.cpp#newcode17 third_party/WebKit/Source/web/tests/HTMLDocumentParserLoadingTest.cpp:17: class HTMLDocumentParserLoadingTest : public SimTest {}; On 2017/01/19 04:19:41, ...
3 years, 11 months ago (2017-01-19 13:49:32 UTC) #31
kouhei (in TOK)
https://codereview.chromium.org/2614663004/diff/200001/third_party/WebKit/Source/web/tests/HTMLDocumentParserLoadingTest.cpp File third_party/WebKit/Source/web/tests/HTMLDocumentParserLoadingTest.cpp (right): https://codereview.chromium.org/2614663004/diff/200001/third_party/WebKit/Source/web/tests/HTMLDocumentParserLoadingTest.cpp#newcode44 third_party/WebKit/Source/web/tests/HTMLDocumentParserLoadingTest.cpp:44: Document::setThreadedParsingEnabledForTesting(true); Please dedupe test cases. testing::TestWithParam may help here.
3 years, 11 months ago (2017-01-20 10:56:47 UTC) #32
Pat Meenan
https://codereview.chromium.org/2614663004/diff/200001/third_party/WebKit/Source/web/tests/HTMLDocumentParserLoadingTest.cpp File third_party/WebKit/Source/web/tests/HTMLDocumentParserLoadingTest.cpp (right): https://codereview.chromium.org/2614663004/diff/200001/third_party/WebKit/Source/web/tests/HTMLDocumentParserLoadingTest.cpp#newcode44 third_party/WebKit/Source/web/tests/HTMLDocumentParserLoadingTest.cpp:44: Document::setThreadedParsingEnabledForTesting(true); On 2017/01/20 10:56:47, kouhei wrote: > Please dedupe ...
3 years, 11 months ago (2017-01-20 13:21:10 UTC) #33
kouhei (in TOK)
lgtm
3 years, 11 months ago (2017-01-20 13:49:00 UTC) #34
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2614663004/220001
3 years, 11 months ago (2017-01-20 13:57:10 UTC) #36
commit-bot: I haz the power
3 years, 11 months ago (2017-01-20 15:36:38 UTC) #39
Message was sent while issue was closed.
Committed patchset #12 (id:220001) as
https://chromium.googlesource.com/chromium/src/+/79ff7175017a071ada9073ee9f37...

Powered by Google App Engine
This is Rietveld 408576698