The HTMLTreeBuilderSimulator has strange stack behavior with <title> |
|||
Issue description
<html>
<head>
<title></title>
<script></script>
</head>
<body>
</body>
</html>
When the BackgroundHTMLParser tokenizes this chunk, it builds the following stack of open tags (HTMLTreeBuilderSimulator::State).
HTML TITLE SCRIPT
i.e. we don't properly close the title tag. There's also mode switching between html integration points that I don't fully understand here. I think we really shouldn't care about script, we just care because we think that it could change the mode of title. Not sure.
In any case, this is causing problems with us discarding our token speculations on the main thread (validateSpeculations). It is really easy the tree builder and the tree builder simulator to differ here if there is a <title> in the head, so basically any document.write will cause token invalidation afterwards. This is really bad, especially because the vast majority of document.writes are benign.
dominicc, it looks like you recently made some changes here. Can you explain the current behavior a bit?
,
Aug 12 2016
FYI: I also have a pending change in the space: https://codereview.chromium.org/2169433002/
,
Aug 18 2016
I probably broke this because I added the integration point stuff to the tree builder simulator. Let me look at this in the debugger.
,
Aug 19 2016
I don't remember the specifics but I seem to remember that this was broken even before your change. Still, it would be great to fix this in general. FYI you can monitor Parser.DiscardedTokenCount to see what the story looks like right now. I imagine if this is fixed we can see that drop quite a bit.
,
Sep 8 2016
Any progress here? I'd like to make sure pages that use document.write aren't wasting CPU retokenizing when it isn't necessary, especially as some parser experiments running now are erring for more aggressive speculation.
,
Oct 18 2016
kouhei@, what's your current plan to deal w/ HTMLTreeBuilderSimulator? Fixing this bug will be a big CPU perf win for sites using document.write.
,
Feb 6 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/38b859def1f34ec21e94248303b9c90ada4acace commit 38b859def1f34ec21e94248303b9c90ada4acace Author: csharrison <csharrison@chromium.org> Date: Mon Feb 06 03:29:06 2017 Add SimTests for HTML parser rewinding BUG= 636674 Review-Url: https://codereview.chromium.org/2674533009 Cr-Commit-Position: refs/heads/master@{#448207} [modify] https://crrev.com/38b859def1f34ec21e94248303b9c90ada4acace/third_party/WebKit/Source/web/tests/HTMLDocumentParserLoadingTest.cpp
,
Feb 6 2017
I think this is WontFix, thankfully. Not quite sure how it was fixed though. |
|||
►
Sign in to add a comment |
|||
Comment 1 by jkarlin@chromium.org
, Aug 11 2016Status: Assigned (was: Untriaged)