From 3c195ec374422497fdce1f98528dab9e7ebbeb9b Mon Sep 17 00:00:00 2001 From: inikulin Date: Wed, 27 Jan 2016 16:24:00 +0300 Subject: [PATCH] Script handling rewrite (fixes #101). Fix benchmark. Bump version. --- docs/07_version_history.md | 3 +++ lib/parser/index.js | 38 +++++++++++++++++++------------ lib/parser/stream.js | 24 +++++++++----------- package.json | 2 +- test/benchmark/bench-micro.js | 13 ++++------- test/fixtures/scripting_test.js | 40 ++++++++++++++++++++++++--------- 6 files changed, 71 insertions(+), 49 deletions(-) diff --git a/docs/07_version_history.md b/docs/07_version_history.md index 65a4a7868..71a639f97 100644 --- a/docs/07_version_history.md +++ b/docs/07_version_history.md @@ -1,5 +1,8 @@ # Version history +## 2.1.5 + * Fixed: ParserStream accidentally hangs up on scripts (GH [#101](https://github.com/inikulin/parse5/issues/101)). + ## 2.1.4 * Fixed: Keep ParserStream sync for the inline scripts (GH [#98](https://github.com/inikulin/parse5/issues/98) follow up). diff --git a/lib/parser/index.js b/lib/parser/index.js index 3ce878eeb..d7cc522ac 100644 --- a/lib/parser/index.js +++ b/lib/parser/index.js @@ -354,11 +354,11 @@ function getSearchableIndexInputAttrs(isindexStartTagToken) { } //Parser -var Parser = module.exports = function (options, scriptHandler) { +var Parser = module.exports = function (options) { this.options = mergeOptions(DEFAULT_OPTIONS, options); this.treeAdapter = this.options.treeAdapter; - this.scriptHandler = scriptHandler; + this.pendingScript = null; if (this.options.locationInfo) locationInfoMixin.assign(this); @@ -370,7 +370,7 @@ Parser.prototype.parse = function (html) { this._bootstrap(document, null); this.tokenizer.write(html, true); - this._runParsingLoop(); + this._runParsingLoop(null, null); return document; }; @@ -396,7 +396,7 @@ Parser.prototype.parseFragment = function (html, fragmentContext) { this._resetInsertionMode(); this._findFormInFragmentContext(); this.tokenizer.write(html, true); - this._runParsingLoop(); + this._runParsingLoop(null, null); var rootElement = this.treeAdapter.getFirstChild(documentMock), fragment = this.treeAdapter.createDocumentFragment(); @@ -411,7 +411,6 @@ Parser.prototype._bootstrap = function (document, fragmentContext) { this.tokenizer = new Tokenizer(this.options); this.stopped = false; - this.pausedByScript = false; this.insertionMode = INITIAL_MODE; this.originalInsertionMode = ''; @@ -438,13 +437,13 @@ Parser.prototype._bootstrap = function (document, fragmentContext) { }; //Parsing loop -Parser.prototype._runParsingLoop = function (callback) { - while (!this.stopped && !this.pausedByScript) { +Parser.prototype._runParsingLoop = function (writeCallback, scriptHandler) { + while (!this.stopped) { this._setupTokenizerCDATAMode(); var token = this.tokenizer.getNextToken(); - if(token.type === Tokenizer.HIBERNATION_TOKEN) + if (token.type === Tokenizer.HIBERNATION_TOKEN) break; if (this.skipNextNewLine) { @@ -463,10 +462,23 @@ Parser.prototype._runParsingLoop = function (callback) { else this._processToken(token); + + if (scriptHandler && this.pendingScript) + break; + } + + if (scriptHandler && this.pendingScript) { + var script = this.pendingScript; + + this.pendingScript = null; + + scriptHandler(script); + + return; } - if(callback && !this.pausedByScript) - callback(); + if (writeCallback) + writeCallback(); }; //Text parsing @@ -2079,13 +2091,11 @@ function eofInBody(p, token) { //12.2.5.4.8 The "text" insertion mode //------------------------------------------------------------------ function endTagInText(p, token) { - var closedElement = p.openElements.current; + if (token.tagName === $.SCRIPT) + p.pendingScript = p.openElements.current; p.openElements.pop(); p.insertionMode = p.originalInsertionMode; - - if (p.scriptHandler && token.tagName === $.SCRIPT) - p.scriptHandler(closedElement); } diff --git a/lib/parser/stream.js b/lib/parser/stream.js index 0b19bba3a..3122190c7 100644 --- a/lib/parser/stream.js +++ b/lib/parser/stream.js @@ -30,11 +30,11 @@ var WritableStream = require('stream').Writable, var ParserStream = module.exports = function (options) { WritableStream.call(this); - this.parser = new Parser(options, this.scriptHandler.bind(this)); + this.parser = new Parser(options); this.lastChunkWritten = false; this.writeCallback = null; - this.parserLoopLock = false; + this.pausedByScript = false; /** * The resulting document node. @@ -48,6 +48,7 @@ var ParserStream = module.exports = function (options) { this._resume = this._resume.bind(this); this._documentWrite = this._documentWrite.bind(this); + this._scriptHandler = this._scriptHandler.bind(this); this.parser._bootstrap(this.document, null); }; @@ -68,18 +69,11 @@ ParserStream.prototype.end = function (chunk, encoding, callback) { //Scriptable parser implementation ParserStream.prototype._runParsingLoop = function () { - // NOTE: This helps avoid reentrant invocation of parsing loop - // in cases then `resume` called synchronously - // (see: https://github.com/inikulin/parse5/issues/98). - if (!this.parserLoopLock) { - this.parserLoopLock = true; - this.parser._runParsingLoop(this.writeCallback); - this.parserLoopLock = false; - } + this.parser._runParsingLoop(this.writeCallback, this._scriptHandler); }; ParserStream.prototype._resume = function () { - if (!this.parser.pausedByScript) + if (!this.pausedByScript) throw new Error('Parser was already resumed'); while (this.pendingHtmlInsertions.length) { @@ -88,7 +82,7 @@ ParserStream.prototype._resume = function () { this.parser.tokenizer.insertHtmlAtCurrentPos(html); } - this.parser.pausedByScript = false; + this.pausedByScript = false; //NOTE: keep parsing if we don't wait for the next input chunk if (this.parser.tokenizer.active) @@ -100,9 +94,9 @@ ParserStream.prototype._documentWrite = function (html) { this.pendingHtmlInsertions.push(html); }; -ParserStream.prototype.scriptHandler = function (scriptElement) { +ParserStream.prototype._scriptHandler = function (scriptElement) { if (this.listeners('script').length) { - this.parser.pausedByScript = true; + this.pausedByScript = true; /** * Raised then parser encounters a `'); + + process.nextTick(done); + }; + + + _test['Regression - Parsing loop lock causes accidental hang ups (GH-101)'] = function (done) { + var parser = new ParserStream({treeAdapter: treeAdapter}); + + parser.once('finish', function () { + done(); + }); + + parser.on('script', function (scriptElement, documentWrite, resume) { + process.nextTick(function () { + resume(); + }); + }); + + parser.write(''); + parser.end('dawg'); + }; +}); - parser.end(''); - process.nextTick(done); -};