nagai/fune
3
0
Fork 0
forked from mirrors/gecko-dev
Code Pull requests Activity

Bug 1729005: Recursively block parsing for parser blocking scripts. r=hsivonen

Browse source 
Bug 1333990 added the ability to have multiple parser blockers at the same
time, so we no longer need to guard against recursively blocking. What's more,
if we do, and skip calling `BlockParser` while it's blocked for another reason,
we still call `UnblockParser` when we receive script data, at which point we
crash.

This patch moves the XHTML parser's behavior closer in line with the HTML
parser's.

The only way I've seen this manifest as a bug is when we have an XHTML
document with a top-level <script> element, and an extension with content
scripts that cause us to block parsing when we see that top-level element and
need to wait for them to compile.

Differential Revision: https://phabricator.services.mozilla.com/D145513
This commit is contained in:
Kris Maglione 2022-05-11 22:15:14 +00:00
parent 715c6ab9a9
commit a1fde10261
4 changed files with 126 additions and 10 deletions
repo.diff.show_diff_stats Download patch file Download diff file Expand all files Collapse all files

111
dom/base/test/unit/test_blockParsing.js Normal file
View file

@ -0,0 +1,111 @@
"use strict";
const { TestUtils } = ChromeUtils.import(
"resource://testing-common/TestUtils.jsm"
);
const { XPCShellContentUtils } = ChromeUtils.import(
"resource://testing-common/XPCShellContentUtils.jsm"
);
const { setTimeout } = ChromeUtils.import("resource://gre/modules/Timer.jsm");
XPCShellContentUtils.init(this);
function delay() {
return new Promise(resolve => {
setTimeout(resolve, 0);
});
}
const server = XPCShellContentUtils.createHttpServer({
hosts: ["example.com"],
});
// XML document with only a <script> tag as the document element.
const PAGE_URL = "http://example.com/";
server.registerPathHandler("/", (request, response) => {
response.setHeader("Content-Type", "application/xhtml+xml");
response.write(String.raw`<!DOCTYPE html>
<script xmlns="http://www.w3.org/1999/xhtml" src="slow.js"/>
`);
});
let resolveResumeScriptPromise;
let resumeScriptPromise = new Promise(resolve => {
resolveResumeScriptPromise = resolve;
});
let resolveScriptRequestPromise;
let scriptRequestPromise = new Promise(resolve => {
resolveScriptRequestPromise = resolve;
});
// An empty script which waits to complete until `resumeScriptPromise`
// resolves.
server.registerPathHandler("/slow.js", async (request, response) => {
response.processAsync();
resolveScriptRequestPromise();
await resumeScriptPromise;
response.setHeader("Content-type", "text/javascript");
response.write("");
response.finish();
});
// Tests that attempting to block parsing for a <script> tag while the
// parser is already blocked is handled correctly, and does not cause
// crashes or hangs.
add_task(async function test_nested_blockParser() {
// Wait for the document element of the page to be inserted, and block
// the parser when it is.
let resolveBlockerPromise;
let blockerPromise;
let docElementPromise = TestUtils.topicObserved(
"document-element-inserted",
doc => {
if (doc.location.href === PAGE_URL) {
blockerPromise = new Promise(resolve => {
resolveBlockerPromise = resolve;
});
doc.blockParsing(blockerPromise);
return true;
}
return false;
}
);
// Begin loading the page.
let pagePromise = XPCShellContentUtils.loadContentPage(PAGE_URL, {
remote: false,
});
// Wait for the document element to be inserted.
await docElementPromise;
// Wait for the /slow.js script request to initiate.
await scriptRequestPromise;
// Make some trips through the event loop to be safe.
await delay();
await delay();
// Allow the /slow.js script request to complete.
resolveResumeScriptPromise();
// Make some trips through the event loop so that the <script> request
// unblocks the parser.
await delay();
await delay();
// Release the parser blocker added in the observer above.
resolveBlockerPromise();
// Make some trips through the event loop to allow the parser to
// unblock.
await delay();
await delay();
// Wait for the document to finish loading, and then close it.
let page = await pagePromise;
await page.close();
});

3
dom/base/test/unit/xpcshell.ini
View file

@ -24,6 +24,9 @@ support-files =
nodelist_data_2.xhtml
test_delete_range.xml
[test_blockParsing.js]
# We fail an assertion if we block parsing on a self-closing element
skip-if = debug || toolkit == 'android'
[test_bug553888.js]
[test_bug737966.js]
[test_error_codes.js]

13
dom/xml/nsXMLContentSink.cpp
View file

@ -567,11 +567,16 @@ nsresult nsXMLContentSink::CloseElement(nsIContent* aContent) {
// Now tell the script that it's ready to go. This may execute the script
// or return true, or neither if the script doesn't need executing.
bool block = sele->AttemptToExecute();
if (mParser) {
if (block) {
GetParser()->BlockParser();
}
// If the parser got blocked, make sure to return the appropriate rv.
// I'm not sure if this is actually needed or not.
if (mParser && !mParser->IsParserEnabled()) {
block = true;
// If the parser got blocked, make sure to return the appropriate rv.
// I'm not sure if this is actually needed or not.
if (!mParser->IsParserEnabled()) {
block = true;
}
}
return block ? NS_ERROR_HTMLPARSER_BLOCK : NS_OK;

9
parser/htmlparser/nsParser.cpp
View file

@ -744,14 +744,11 @@ nsresult nsParser::ResumeParse(bool allowIteration, bool aIsFinalChunk,
// has been scanned to completion (theTokenizerResult should be kEOF).
// kEOF -> End of buffer.
// If we're told to block the parser, we disable all further parsing
// (and cache any data coming in) until the parser is re-enabled.
// If we're told the parser has been blocked, we disable all further
// parsing (and cache any data coming in) until the parser is
// re-enabled.
if (NS_ERROR_HTMLPARSER_BLOCK == result) {
mSink->WillInterrupt();
if (!mBlocked) {
// If we were blocked by a recursive invocation, don't re-block.
BlockParser();
}
return NS_OK;
}
if (NS_ERROR_HTMLPARSER_STOPPARSING == result) {