From e4f8183545b3aee80ca7a23485af6fb602b43721 Mon Sep 17 00:00:00 2001 From: Henri Sivonen Date: Wed, 21 Mar 2012 14:39:25 +0200 Subject: [PATCH] Bug 717488 part 1. r=smaug. --- parser/html/nsHtml5Parser.cpp | 11 ++++----- parser/html/nsHtml5StreamParser.cpp | 18 +++++++-------- parser/html/nsHtml5TreeOpExecutor.cpp | 25 ++++++++++++++++---- parser/html/nsHtml5TreeOpExecutor.h | 33 +++++++++++++-------------- parser/html/nsHtml5TreeOperation.cpp | 2 +- 5 files changed, 52 insertions(+), 37 deletions(-) diff --git a/parser/html/nsHtml5Parser.cpp b/parser/html/nsHtml5Parser.cpp index fae4c0c7cf49..b488ce776833 100644 --- a/parser/html/nsHtml5Parser.cpp +++ b/parser/html/nsHtml5Parser.cpp @@ -245,12 +245,12 @@ nsHtml5Parser::Parse(const nsAString& aSourceBuffer, bool aLastCall, nsDTDMode aMode) // ignored { - if (mExecutor->IsBroken()) { - return NS_ERROR_OUT_OF_MEMORY; + nsresult rv; + if (NS_FAILED(rv = mExecutor->IsBroken())) { + return rv; } if (aSourceBuffer.Length() > PR_INT32_MAX) { - mExecutor->MarkAsBroken(); - return NS_ERROR_OUT_OF_MEMORY; + return mExecutor->MarkAsBroken(NS_ERROR_OUT_OF_MEMORY); } // Maintain a reference to ourselves so we don't go away @@ -446,8 +446,7 @@ nsHtml5Parser::Parse(const nsAString& aSourceBuffer, heapBuffer = stackBuffer.FalliblyCopyAsOwningBuffer(); if (!heapBuffer) { // Allocation failed. The parser is now broken. - mExecutor->MarkAsBroken(); - return NS_ERROR_OUT_OF_MEMORY; + return mExecutor->MarkAsBroken(NS_ERROR_OUT_OF_MEMORY); } } diff --git a/parser/html/nsHtml5StreamParser.cpp b/parser/html/nsHtml5StreamParser.cpp index f8d2901ce62a..36c9e3de0470 100644 --- a/parser/html/nsHtml5StreamParser.cpp +++ b/parser/html/nsHtml5StreamParser.cpp @@ -947,10 +947,10 @@ nsHtml5StreamParser::OnStartRequest(nsIRequest* aRequest, nsISupports* aContext) nsHtml5OwningUTF16Buffer::FalliblyCreate( NS_HTML5_STREAM_PARSER_READ_BUFFER_SIZE); if (!newBuf) { - mExecutor->MarkAsBroken(); // marks this stream parser as terminated, - // which prevents entry to code paths that - // would use mFirstBuffer or mLastBuffer. - return NS_ERROR_OUT_OF_MEMORY; + // marks this stream parser as terminated, + // which prevents entry to code paths that + // would use mFirstBuffer or mLastBuffer. + return mExecutor->MarkAsBroken(NS_ERROR_OUT_OF_MEMORY); } NS_ASSERTION(!mFirstBuffer, "How come we have the first buffer set?"); NS_ASSERTION(!mLastBuffer, "How come we have the last buffer set?"); @@ -1143,8 +1143,9 @@ nsHtml5StreamParser::OnDataAvailable(nsIRequest* aRequest, PRUint32 aSourceOffset, PRUint32 aLength) { - if (mExecutor->IsBroken()) { - return NS_ERROR_OUT_OF_MEMORY; + nsresult rv; + if (NS_FAILED(rv = mExecutor->IsBroken())) { + return rv; } NS_ASSERTION(mRequest == aRequest, "Got data on wrong stream."); @@ -1152,10 +1153,9 @@ nsHtml5StreamParser::OnDataAvailable(nsIRequest* aRequest, const mozilla::fallible_t fallible = mozilla::fallible_t(); nsAutoArrayPtr data(new (fallible) PRUint8[aLength]); if (!data) { - mExecutor->MarkAsBroken(); - return NS_ERROR_OUT_OF_MEMORY; + return mExecutor->MarkAsBroken(NS_ERROR_OUT_OF_MEMORY); } - nsresult rv = aInStream->Read(reinterpret_cast(data.get()), + rv = aInStream->Read(reinterpret_cast(data.get()), aLength, &totalRead); NS_ENSURE_SUCCESS(rv, rv); NS_ASSERTION(totalRead <= aLength, "Read more bytes than were available?"); diff --git a/parser/html/nsHtml5TreeOpExecutor.cpp b/parser/html/nsHtml5TreeOpExecutor.cpp index 02b9be335408..fe18bd35cdf9 100644 --- a/parser/html/nsHtml5TreeOpExecutor.cpp +++ b/parser/html/nsHtml5TreeOpExecutor.cpp @@ -117,6 +117,20 @@ nsHtml5TreeOpExecutor::WillParse() return NS_ERROR_NOT_IMPLEMENTED; } +NS_IMETHODIMP +nsHtml5TreeOpExecutor::WillBuildModel(nsDTDMode aDTDMode) +{ + if (mDocShell && !GetDocument()->GetScriptGlobalObject()) { + // Not loading as data but script global object not ready + return MarkAsBroken(NS_ERROR_DOM_INVALID_STATE_ERR); + } + mDocument->AddObserver(this); + WillBuildModelImpl(); + GetDocument()->BeginLoad(); + return NS_OK; +} + + // This is called when the tree construction has ended NS_IMETHODIMP nsHtml5TreeOpExecutor::DidBuildModel(bool aTerminated) @@ -143,7 +157,9 @@ nsHtml5TreeOpExecutor::DidBuildModel(bool aTerminated) GetParser()->DropStreamParser(); // This comes from nsXMLContentSink and nsHTMLContentSink - DidBuildModelImpl(aTerminated); + // If this parser has been marked as broken, treat the end of parse as + // forced termination. + DidBuildModelImpl(aTerminated || IsBroken()); if (!mLayoutStarted) { // We never saw the body, and layout never got started. Force @@ -274,12 +290,12 @@ nsHtml5TreeOpExecutor::UpdateChildCounts() // No-op } -void -nsHtml5TreeOpExecutor::MarkAsBroken() +nsresult +nsHtml5TreeOpExecutor::MarkAsBroken(nsresult aReason) { NS_ASSERTION(NS_IsMainThread(), "Wrong thread!"); NS_ASSERTION(!mRunsToCompletion, "Fragment parsers can't be broken!"); - mBroken = true; + mBroken = aReason; if (mStreamParser) { mStreamParser->Terminate(); } @@ -293,6 +309,7 @@ nsHtml5TreeOpExecutor::MarkAsBroken() NS_WARNING("failed to dispatch executor flush event"); } } + return aReason; } nsresult diff --git a/parser/html/nsHtml5TreeOpExecutor.h b/parser/html/nsHtml5TreeOpExecutor.h index 56a43ec70abe..29ddcb362e9e 100644 --- a/parser/html/nsHtml5TreeOpExecutor.h +++ b/parser/html/nsHtml5TreeOpExecutor.h @@ -129,14 +129,17 @@ class nsHtml5TreeOpExecutor : public nsContentSink, bool mCallContinueInterruptedParsingIfEnabled; /** - * True if this parser should refuse to process any more input. - * Currently, the only way a parser can break is if it drops some input - * due to a memory allocation failure. In such a case, the whole parser - * needs to be marked as broken, because some input has been lost and - * parsing more input could lead to a DOM where pieces of HTML source + * Non-NS_OK if this parser should refuse to process any more input. + * For example, the parser needs to be marked as broken if it drops some + * input due to a memory allocation failure. In such a case, the whole + * parser needs to be marked as broken, because some input has been lost + * and parsing more input could lead to a DOM where pieces of HTML source * that weren't supposed to become scripts become scripts. + * + * Since NS_OK is actually 0, zeroing operator new takes care of + * initializing this. */ - bool mBroken; + nsresult mBroken; public: @@ -153,14 +156,7 @@ class nsHtml5TreeOpExecutor : public nsContentSink, /** * */ - NS_IMETHOD WillBuildModel(nsDTDMode aDTDMode) { - NS_ASSERTION(!mDocShell || GetDocument()->GetScriptGlobalObject(), - "Script global object not ready"); - mDocument->AddObserver(this); - WillBuildModelImpl(); - GetDocument()->BeginLoad(); - return NS_OK; - } + NS_IMETHOD WillBuildModel(nsDTDMode aDTDMode); /** * Emits EOF. @@ -263,13 +259,16 @@ class nsHtml5TreeOpExecutor : public nsContentSink, /** * Marks this parser as broken and tells the stream parser (if any) to * terminate. + * + * @return aReason for convenience */ - void MarkAsBroken(); + nsresult MarkAsBroken(nsresult aReason); /** - * Checks if this parser is broken. + * Checks if this parser is broken. Returns a non-NS_OK (i.e. non-0) + * value if broken. */ - inline bool IsBroken() { + inline nsresult IsBroken() { NS_ASSERTION(NS_IsMainThread(), "Wrong thread!"); return mBroken; } diff --git a/parser/html/nsHtml5TreeOperation.cpp b/parser/html/nsHtml5TreeOperation.cpp index d03180b9f2f3..e87d67a772c6 100644 --- a/parser/html/nsHtml5TreeOperation.cpp +++ b/parser/html/nsHtml5TreeOperation.cpp @@ -588,7 +588,7 @@ nsHtml5TreeOperation::Perform(nsHtml5TreeOpExecutor* aBuilder, return AppendToDocument(asContent, aBuilder); } case eTreeOpMarkAsBroken: { - aBuilder->MarkAsBroken(); + aBuilder->MarkAsBroken(NS_ERROR_OUT_OF_MEMORY); return rv; } case eTreeOpRunScript: {