Commit 2e945073 authored by Richard Townsend's avatar Richard Townsend Committed by Chromium LUCI CQ

fix: disable prefetching for TextDocument

The synchronous HTMLDocumentParser mode was incorrectly dispatching
preloads for text/plain documents by interpreting their contents as
HTML. This CL extends the HTMLDocumentParser's constructor, adds a
new enum to disable this behaviour, and a Web Platform Test to show
that the unintended prefetching no longer happens.

Bug: 1160665
Change-Id: I07902d58e3bc06ce6ecc07c341e997846d6e5a64
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2613008
Commit-Queue: Richard Townsend <richard.townsend@arm.com>
Reviewed-by: default avatarMason Freed <masonfreed@chromium.org>
Cr-Commit-Position: refs/heads/master@{#841110}
parent dfe39a80
...@@ -298,8 +298,12 @@ class ScopedYieldTimer { ...@@ -298,8 +298,12 @@ class ScopedYieldTimer {
}; };
HTMLDocumentParser::HTMLDocumentParser(HTMLDocument& document, HTMLDocumentParser::HTMLDocumentParser(HTMLDocument& document,
ParserSynchronizationPolicy sync_policy) ParserSynchronizationPolicy sync_policy,
: HTMLDocumentParser(document, kAllowScriptingContent, sync_policy) { ParserPrefetchPolicy prefetch_policy)
: HTMLDocumentParser(document,
kAllowScriptingContent,
sync_policy,
prefetch_policy) {
script_runner_ = script_runner_ =
HTMLParserScriptRunner::Create(ReentryPermit(), &document, this); HTMLParserScriptRunner::Create(ReentryPermit(), &document, this);
...@@ -314,10 +318,12 @@ HTMLDocumentParser::HTMLDocumentParser(HTMLDocument& document, ...@@ -314,10 +318,12 @@ HTMLDocumentParser::HTMLDocumentParser(HTMLDocument& document,
HTMLDocumentParser::HTMLDocumentParser( HTMLDocumentParser::HTMLDocumentParser(
DocumentFragment* fragment, DocumentFragment* fragment,
Element* context_element, Element* context_element,
ParserContentPolicy parser_content_policy) ParserContentPolicy parser_content_policy,
ParserPrefetchPolicy parser_prefetch_policy)
: HTMLDocumentParser(fragment->GetDocument(), : HTMLDocumentParser(fragment->GetDocument(),
parser_content_policy, parser_content_policy,
kForceSynchronousParsing) { kForceSynchronousParsing,
parser_prefetch_policy) {
// Allow declarative shadow DOM for the fragment parser only if explicitly // Allow declarative shadow DOM for the fragment parser only if explicitly
// enabled. // enabled.
bool include_shadow_roots = bool include_shadow_roots =
...@@ -346,7 +352,8 @@ int GetMaxTokenizationBudget() { ...@@ -346,7 +352,8 @@ int GetMaxTokenizationBudget() {
HTMLDocumentParser::HTMLDocumentParser(Document& document, HTMLDocumentParser::HTMLDocumentParser(Document& document,
ParserContentPolicy content_policy, ParserContentPolicy content_policy,
ParserSynchronizationPolicy sync_policy) ParserSynchronizationPolicy sync_policy,
ParserPrefetchPolicy prefetch_policy)
: ScriptableDocumentParser(document, content_policy), : ScriptableDocumentParser(document, content_policy),
options_(&document), options_(&document),
reentry_permit_(HTMLParserReentryPermit::Create()), reentry_permit_(HTMLParserReentryPermit::Create()),
...@@ -415,7 +422,8 @@ HTMLDocumentParser::HTMLDocumentParser(Document& document, ...@@ -415,7 +422,8 @@ HTMLDocumentParser::HTMLDocumentParser(Document& document,
!document.IsPrefetchOnly()) !document.IsPrefetchOnly())
return; return;
preloader_ = MakeGarbageCollected<HTMLResourcePreloader>(document); if (prefetch_policy == kAllowPrefetching)
preloader_ = MakeGarbageCollected<HTMLResourcePreloader>(document);
} }
HTMLDocumentParser::~HTMLDocumentParser() = default; HTMLDocumentParser::~HTMLDocumentParser() = default;
...@@ -1221,7 +1229,7 @@ void HTMLDocumentParser::Append(const String& input_source) { ...@@ -1221,7 +1229,7 @@ void HTMLDocumentParser::Append(const String& input_source) {
const SegmentedString source(input_source); const SegmentedString source(input_source);
if (!preload_scanner_ && GetDocument()->Url().IsValid() && if (!preload_scanner_ && preloader_ && GetDocument()->Url().IsValid() &&
(!task_runner_state_->IsSynchronous() || (!task_runner_state_->IsSynchronous() ||
GetDocument()->IsPrefetchOnly() || IsPaused())) { GetDocument()->IsPrefetchOnly() || IsPaused())) {
// If we're operating with synchronous, budgeted foreground HTML parsing // If we're operating with synchronous, budgeted foreground HTML parsing
...@@ -1246,16 +1254,14 @@ void HTMLDocumentParser::Append(const String& input_source) { ...@@ -1246,16 +1254,14 @@ void HTMLDocumentParser::Append(const String& input_source) {
// Return after the preload scanner, do not actually parse the document. // Return after the preload scanner, do not actually parse the document.
return; return;
} }
if (preload_scanner_) { if (preload_scanner_ && preloader_) {
preload_scanner_->AppendToEnd(source); preload_scanner_->AppendToEnd(source);
if (preloader_) { if (!task_runner_state_->IsSynchronous() || IsPaused()) {
if (!task_runner_state_->IsSynchronous() || IsPaused()) { // Should scan and preload if the parser's paused and operating
// Should scan and preload if the parser's paused and operating // synchronously, or if the parser's operating in an asynchronous
// synchronously, or if the parser's operating in an asynchronous // mode.
// mode. ScanAndPreload(preload_scanner_.get());
ScanAndPreload(preload_scanner_.get()); }
}
}
} }
input_.AppendToEnd(source); input_.AppendToEnd(source);
......
...@@ -67,6 +67,13 @@ class HTMLResourcePreloader; ...@@ -67,6 +67,13 @@ class HTMLResourcePreloader;
class HTMLTreeBuilder; class HTMLTreeBuilder;
class HTMLDocumentParserState; class HTMLDocumentParserState;
enum ParserPrefetchPolicy {
// Indicates that prefetches/preloads should happen for this document type.
kAllowPrefetching,
// Indicates that prefetches are forbidden for this document type.
kDisallowPrefetching
};
// TODO(https://crbug.com/1049898): These are only exposed to make it possible // TODO(https://crbug.com/1049898): These are only exposed to make it possible
// to delete an expired histogram. The test should be rewritten to test at a // to delete an expired histogram. The test should be rewritten to test at a
// different level, so it won't have to make assertions about internal state. // different level, so it won't have to make assertions about internal state.
...@@ -78,10 +85,13 @@ class CORE_EXPORT HTMLDocumentParser : public ScriptableDocumentParser, ...@@ -78,10 +85,13 @@ class CORE_EXPORT HTMLDocumentParser : public ScriptableDocumentParser,
USING_PRE_FINALIZER(HTMLDocumentParser, Dispose); USING_PRE_FINALIZER(HTMLDocumentParser, Dispose);
public: public:
HTMLDocumentParser(HTMLDocument&, ParserSynchronizationPolicy); HTMLDocumentParser(HTMLDocument&,
ParserSynchronizationPolicy,
ParserPrefetchPolicy prefetch_policy = kAllowPrefetching);
HTMLDocumentParser(DocumentFragment*, HTMLDocumentParser(DocumentFragment*,
Element* context_element, Element* context_element,
ParserContentPolicy); ParserContentPolicy,
ParserPrefetchPolicy prefetch_policy = kAllowPrefetching);
~HTMLDocumentParser() override; ~HTMLDocumentParser() override;
void Trace(Visitor*) const override; void Trace(Visitor*) const override;
...@@ -156,7 +166,8 @@ class CORE_EXPORT HTMLDocumentParser : public ScriptableDocumentParser, ...@@ -156,7 +166,8 @@ class CORE_EXPORT HTMLDocumentParser : public ScriptableDocumentParser,
private: private:
HTMLDocumentParser(Document&, HTMLDocumentParser(Document&,
ParserContentPolicy, ParserContentPolicy,
ParserSynchronizationPolicy); ParserSynchronizationPolicy,
ParserPrefetchPolicy);
// DocumentParser // DocumentParser
void Detach() final; void Detach() final;
......
...@@ -32,7 +32,7 @@ namespace blink { ...@@ -32,7 +32,7 @@ namespace blink {
TextDocumentParser::TextDocumentParser(HTMLDocument& document, TextDocumentParser::TextDocumentParser(HTMLDocument& document,
ParserSynchronizationPolicy sync_policy) ParserSynchronizationPolicy sync_policy)
: HTMLDocumentParser(document, sync_policy), : HTMLDocumentParser(document, sync_policy, kDisallowPrefetching),
have_inserted_fake_pre_element_(false) {} have_inserted_fake_pre_element_(false) {}
TextDocumentParser::~TextDocumentParser() = default; TextDocumentParser::~TextDocumentParser() = default;
......
...@@ -85,8 +85,7 @@ crbug.com/887140 virtual/hdr/video-canvas-alpha.html [ Failure ] ...@@ -85,8 +85,7 @@ crbug.com/887140 virtual/hdr/video-canvas-alpha.html [ Failure ]
external/wpt/css/compositing/root-element-background-transparency.html [ Failure ] external/wpt/css/compositing/root-element-background-transparency.html [ Failure ]
# ====== Synchronous, budgeted HTML parser tests from here ====== # ====== Synchronous, budgeted HTML parser tests from here ======
crbug.com/901056 virtual/synchronous_html_parser/external/wpt/preload/modulepreload.html [ Failure Pass ]
### virtual/synchronous_html_parser/http/tests/preload/
# An extra line ("linear_memory_inspector") appears in the output in some cases. # An extra line ("linear_memory_inspector") appears in the output in some cases.
crbug.com/901056 http/tests/devtools/modules-load-source.js [ Failure Pass ] crbug.com/901056 http/tests/devtools/modules-load-source.js [ Failure Pass ]
# Extra line info appears in the output in some cases. # Extra line info appears in the output in some cases.
......
...@@ -171,6 +171,7 @@ ...@@ -171,6 +171,7 @@
"external/wpt/domparsing", "external/wpt/domparsing",
"external/wpt/html/syntax/parsing", "external/wpt/html/syntax/parsing",
"external/wpt/html/the-xhtml-syntax", "external/wpt/html/the-xhtml-syntax",
"external/wpt/preload",
"external/wpt/signed-exchange", "external/wpt/signed-exchange",
"scrollingcoordinator"], "scrollingcoordinator"],
"args": ["--enable-blink-features=ForceSynchronousHTMLParsing"] "args": ["--enable-blink-features=ForceSynchronousHTMLParsing"]
......
<!DOCTYPE html>
<title>Ensures content delivered with Content-Type: text/plain header is not prefetched</title>
<!-- Regression test for https://crbug.com/1160665 -->
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="/preload/resources/preload_helper.js"></script>
<body>
<script>
setup({single_test: true});
window.addEventListener("load", function() {
verifyPreloadAndRTSupport();
// This test works by loading a text/plain iframe containing a <script> tag.
// It then injects some post-load JavaScript to serialize the Performance API
// data and pass it back to this document.
var prefetchingIframe = document.getElementById('prefetching-frame');
window.addEventListener("message", function(msg) {
// Parse the Performance API data passed from the plain text iframe.
const entries = JSON.parse(msg.data);
const urls = [];
const resource_types = [];
for (const entry of entries) {
resource_types.push(entry.entryType);
urls.push(entry.name);
}
// If preloading is working correctly, should only see the text document
// represented in the performance information.
assert_array_equals(resource_types, ['navigation']);
assert_equals(urls.length, 1);
assert_equals(urls[0].endsWith("avoid-prefetching-on-text-plain-inner.html"), true);
done();
});
prefetchingIframe.addEventListener('load', function() {
// Pass performance API info back to this document, process in above event handler.
const passMsg = 'parent.postMessage(JSON.stringify(performance.getEntries()));';
prefetchingIframe.contentWindow.eval(passMsg);
});
// Start the iframe load.
prefetchingIframe.src = "avoid-prefetching-on-text-plain-inner.html";
});
</script>
<iframe id="prefetching-frame"></iframe>
</body>
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment