From 4bffa39c215e591d107bde3108348f3e27888055 Mon Sep 17 00:00:00 2001 From: Henri Sivonen Date: Wed, 20 Jun 2012 10:05:39 +0300 Subject: [PATCH] Bug 765620 - When parsing from stream without executing scripts, avoid script execution-related tree ops. r=smaug --- content/base/src/nsContentSink.h | 2 -- content/xml/document/src/nsXMLContentSink.h | 2 ++ parser/html/nsHtml5Parser.cpp | 4 +--- parser/html/nsHtml5StreamParser.cpp | 7 ++++++- parser/html/nsHtml5StringParser.cpp | 10 ++++++---- parser/html/nsHtml5TreeBuilderCppSupplement.h | 5 +++++ parser/html/nsHtml5TreeBuilderHSupplement.h | 5 +++++ parser/html/nsHtml5TreeOpExecutor.cpp | 14 +++++++------- parser/html/nsHtml5TreeOpExecutor.h | 15 --------------- parser/html/nsHtml5TreeOperation.cpp | 7 +++++++ parser/html/nsHtml5TreeOperation.h | 1 + 11 files changed, 40 insertions(+), 32 deletions(-) diff --git a/content/base/src/nsContentSink.h b/content/base/src/nsContentSink.h index 57d0175e5d30..76350ed013eb 100644 --- a/content/base/src/nsContentSink.h +++ b/content/base/src/nsContentSink.h @@ -303,8 +303,6 @@ protected: // True if this is parser is a fragment parser or an HTML DOMParser. // XML DOMParser leaves this to false for now! PRUint8 mRunsToCompletion : 1; - // True to call prevent script execution in the fragment mode. - PRUint8 mPreventScriptExecution : 1; // // -- Can interrupt parsing members -- diff --git a/content/xml/document/src/nsXMLContentSink.h b/content/xml/document/src/nsXMLContentSink.h index c6ea73baee58..fa42d98658e3 100644 --- a/content/xml/document/src/nsXMLContentSink.h +++ b/content/xml/document/src/nsXMLContentSink.h @@ -182,6 +182,8 @@ protected: PRUint8 mPrettyPrintHasFactoredElements : 1; PRUint8 mPrettyPrinting : 1; // True if we called PrettyPrint() and it // decided we should in fact prettyprint. + // True to call prevent script execution in the fragment mode. + PRUint8 mPreventScriptExecution : 1; nsTArray mContentStack; diff --git a/parser/html/nsHtml5Parser.cpp b/parser/html/nsHtml5Parser.cpp index 57ab42b3f3b3..ab00d3c0ef75 100644 --- a/parser/html/nsHtml5Parser.cpp +++ b/parser/html/nsHtml5Parser.cpp @@ -678,9 +678,7 @@ nsHtml5Parser::Initialize(nsIDocument* aDoc, void nsHtml5Parser::StartTokenizer(bool aScriptingEnabled) { - if (!aScriptingEnabled) { - mExecutor->PreventScriptExecution(); - } + mTreeBuilder->SetPreventScriptExecution(!aScriptingEnabled); mTreeBuilder->setScriptingEnabled(aScriptingEnabled); mTokenizer->start(); } diff --git a/parser/html/nsHtml5StreamParser.cpp b/parser/html/nsHtml5StreamParser.cpp index c154d8b44cc9..1f21301b0eed 100644 --- a/parser/html/nsHtml5StreamParser.cpp +++ b/parser/html/nsHtml5StreamParser.cpp @@ -899,6 +899,8 @@ nsHtml5StreamParser::OnStartRequest(nsIRequest* aRequest, nsISupports* aContext) false : mExecutor->IsScriptEnabled(); mOwner->StartTokenizer(scriptingEnabled); mTreeBuilder->setScriptingEnabled(scriptingEnabled); + mTreeBuilder->SetPreventScriptExecution(!((mMode == NORMAL) && + scriptingEnabled)); mTokenizer->start(); mExecutor->Start(); mExecutor->StartReadingFromStage(); @@ -1360,7 +1362,10 @@ nsHtml5StreamParser::ParseAvailableData() // Terminate, but that never happens together with script. // Can't assert that here, though, because it's possible that the main // thread has called Terminate() while this thread was parsing. - if (mMode == NORMAL && mTreeBuilder->HasScript()) { + if (mTreeBuilder->HasScript()) { + // HasScript() cannot return true if the tree builder is preventing + // script execution. + MOZ_ASSERT(mMode == NORMAL); mozilla::MutexAutoLock speculationAutoLock(mSpeculationMutex); nsHtml5Speculation* speculation = new nsHtml5Speculation(mFirstBuffer, diff --git a/parser/html/nsHtml5StringParser.cpp b/parser/html/nsHtml5StringParser.cpp index 1afcf62cc397..86e13287af29 100644 --- a/parser/html/nsHtml5StringParser.cpp +++ b/parser/html/nsHtml5StringParser.cpp @@ -60,7 +60,7 @@ nsHtml5StringParser::ParseFragment(const nsAString& aSourceBuffer, } #endif - mExecutor->EnableFragmentMode(aPreventScriptExecution); + mTreeBuilder->SetPreventScriptExecution(aPreventScriptExecution); Tokenize(aSourceBuffer, doc, true); return NS_OK; @@ -81,7 +81,7 @@ nsHtml5StringParser::ParseDocument(const nsAString& aSourceBuffer, nsnull, false); - mExecutor->PreventScriptExecution(); + mTreeBuilder->SetPreventScriptExecution(true); Tokenize(aSourceBuffer, aTargetDoc, aScriptingEnabledForNoscriptParsing); return NS_OK; @@ -113,8 +113,10 @@ nsHtml5StringParser::Tokenize(const nsAString& aSourceBuffer, if (buffer.hasMore()) { lastWasCR = mTokenizer->tokenizeBuffer(&buffer); if (mTreeBuilder->HasScript()) { - // Flush on each script, because the execution prevention code - // can handle at most one script per flush. + // If we come here, we are in createContextualFragment() or in the + // upcoming document.parse(). It's unclear if it's really necessary + // to flush here, but let's do so for consistency with other flushes + // to avoid different code paths on the executor side. mTreeBuilder->Flush(); // Move ops to the executor mExecutor->FlushDocumentWrite(); // run the ops } diff --git a/parser/html/nsHtml5TreeBuilderCppSupplement.h b/parser/html/nsHtml5TreeBuilderCppSupplement.h index 13755425e55a..b586700150f5 100644 --- a/parser/html/nsHtml5TreeBuilderCppSupplement.h +++ b/parser/html/nsHtml5TreeBuilderCppSupplement.h @@ -27,6 +27,7 @@ nsHtml5TreeBuilder::nsHtml5TreeBuilder(nsAHtml5TreeOpSink* aOpSink, , mHandlesUsed(0) , mSpeculativeLoadStage(aStage) , mCurrentHtmlScriptIsAsyncOrDefer(false) + , mPreventScriptExecution(false) #ifdef DEBUG , mActive(false) #endif @@ -470,6 +471,10 @@ nsHtml5TreeBuilder::elementPopped(PRInt32 aNamespace, nsIAtom* aName, nsIContent } // we now have only SVG and HTML if (aName == nsHtml5Atoms::script) { + if (mPreventScriptExecution) { + mOpQueue.AppendElement()->Init(eTreeOpPreventScriptExecution, aElement); + return; + } if (mCurrentHtmlScriptIsAsyncOrDefer) { NS_ASSERTION(aNamespace == kNameSpaceID_XHTML, "Only HTML scripts may be async/defer."); diff --git a/parser/html/nsHtml5TreeBuilderHSupplement.h b/parser/html/nsHtml5TreeBuilderHSupplement.h index e31ac9b8be84..35194fc7f6a4 100644 --- a/parser/html/nsHtml5TreeBuilderHSupplement.h +++ b/parser/html/nsHtml5TreeBuilderHSupplement.h @@ -15,6 +15,7 @@ nsHtml5TreeOpStage* mSpeculativeLoadStage; nsIContent** mDeepTreeSurrogateParent; bool mCurrentHtmlScriptIsAsyncOrDefer; + bool mPreventScriptExecution; #ifdef DEBUG bool mActive; #endif @@ -96,6 +97,10 @@ void DropHandles(); + void SetPreventScriptExecution(bool aPrevent) { + mPreventScriptExecution = aPrevent; + } + void EnableViewSource(nsHtml5Highlighter* aHighlighter); void errStrayStartTag(nsIAtom* aName); diff --git a/parser/html/nsHtml5TreeOpExecutor.cpp b/parser/html/nsHtml5TreeOpExecutor.cpp index e63a37b95a46..0d80c4e67af8 100644 --- a/parser/html/nsHtml5TreeOpExecutor.cpp +++ b/parser/html/nsHtml5TreeOpExecutor.cpp @@ -759,6 +759,13 @@ nsHtml5TreeOpExecutor::StartLayout() { void nsHtml5TreeOpExecutor::RunScript(nsIContent* aScriptElement) { + if (mRunsToCompletion) { + // We are in createContextualFragment() or in the upcoming document.parse(). + // Do nothing. Let's not even mark scripts malformed here, because that + // could cause serialization weirdness later. + return; + } + NS_ASSERTION(aScriptElement, "No script to run"); nsCOMPtr sele = do_QueryInterface(aScriptElement); @@ -770,13 +777,6 @@ nsHtml5TreeOpExecutor::RunScript(nsIContent* aScriptElement) return; } - if (mPreventScriptExecution) { - sele->PreventExecution(); - } - if (mRunsToCompletion) { - return; - } - if (sele->GetScriptDeferred() || sele->GetScriptAsync()) { DebugOnly block = sele->AttemptToExecute(); NS_ASSERTION(!block, "Defer or async script tried to block."); diff --git a/parser/html/nsHtml5TreeOpExecutor.h b/parser/html/nsHtml5TreeOpExecutor.h index d43de8eb29d8..b38e336ba013 100644 --- a/parser/html/nsHtml5TreeOpExecutor.h +++ b/parser/html/nsHtml5TreeOpExecutor.h @@ -212,21 +212,6 @@ class nsHtml5TreeOpExecutor : public nsContentSink, bool IsScriptEnabled(); - /** - * Enables the fragment mode. - * - * @param aPreventScriptExecution if true, scripts are prevented from - * executing; don't set to false when parsing a fragment directly into - * a document--only when parsing to an actual DOM fragment - */ - void EnableFragmentMode(bool aPreventScriptExecution) { - mPreventScriptExecution = aPreventScriptExecution; - } - - void PreventScriptExecution() { - mPreventScriptExecution = true; - } - bool BelongsToStringParser() { return mRunsToCompletion; } diff --git a/parser/html/nsHtml5TreeOperation.cpp b/parser/html/nsHtml5TreeOperation.cpp index b4b15afd84b8..6262384ea5b4 100644 --- a/parser/html/nsHtml5TreeOperation.cpp +++ b/parser/html/nsHtml5TreeOperation.cpp @@ -562,6 +562,13 @@ nsHtml5TreeOperation::Perform(nsHtml5TreeOpExecutor* aBuilder, aBuilder->RunScript(node); return rv; } + case eTreeOpPreventScriptExecution: { + nsIContent* node = *(mOne.node); + nsCOMPtr sele = do_QueryInterface(node); + MOZ_ASSERT(sele); + sele->PreventExecution(); + return rv; + } case eTreeOpDoneAddingChildren: { nsIContent* node = *(mOne.node); node->DoneAddingChildren(aBuilder->HaveNotified(node)); diff --git a/parser/html/nsHtml5TreeOperation.h b/parser/html/nsHtml5TreeOperation.h index 6bcaf56f2edc..7b0ef4024834 100644 --- a/parser/html/nsHtml5TreeOperation.h +++ b/parser/html/nsHtml5TreeOperation.h @@ -38,6 +38,7 @@ enum eHtml5TreeOperation { eTreeOpMarkAsBroken, eTreeOpRunScript, eTreeOpRunScriptAsyncDefer, + eTreeOpPreventScriptExecution, eTreeOpDoneAddingChildren, eTreeOpDoneCreatingElement, eTreeOpFlushPendingAppendNotifications,