From cfbff77873b2e58ca7d679282f69467ef03dd6c7 Mon Sep 17 00:00:00 2001 From: Henri Sivonen Date: Wed, 5 Nov 2014 17:44:37 +0200 Subject: [PATCH] Bug 1088635. r=smaug. --- dom/base/nsDocument.cpp | 2 +- parser/html/nsHtml5Parser.cpp | 34 ++++++++++++------- parser/html/nsHtml5Parser.h | 2 +- parser/html/nsHtml5StreamParser.cpp | 16 +++++---- parser/html/nsHtml5StreamParser.h | 2 +- parser/html/nsHtml5TreeBuilderCppSupplement.h | 4 +-- parser/html/nsHtml5TreeBuilderHSupplement.h | 2 +- parser/html/nsHtml5TreeOpExecutor.cpp | 20 +++++++---- parser/html/nsHtml5TreeOpExecutor.h | 2 +- parser/html/nsHtml5TreeOperation.cpp | 7 ++-- parser/html/nsHtml5TreeOperation.h | 12 ++++++- 11 files changed, 67 insertions(+), 36 deletions(-) diff --git a/dom/base/nsDocument.cpp b/dom/base/nsDocument.cpp index 8f4032e479b..87e1d4d2971 100644 --- a/dom/base/nsDocument.cpp +++ b/dom/base/nsDocument.cpp @@ -4016,7 +4016,7 @@ nsDocument::InsertChildAt(nsIContent* aKid, uint32_t aIndex, bool aNotify) { if (aKid->IsElement() && GetRootElement()) { - NS_ERROR("Inserting element child when we already have one"); + NS_WARNING("Inserting root element when we already have one"); return NS_ERROR_DOM_HIERARCHY_REQUEST_ERR; } diff --git a/parser/html/nsHtml5Parser.cpp b/parser/html/nsHtml5Parser.cpp index 5bb0c4ca790..3be3389d212 100644 --- a/parser/html/nsHtml5Parser.cpp +++ b/parser/html/nsHtml5Parser.cpp @@ -238,7 +238,8 @@ nsHtml5Parser::Parse(const nsAString& aSourceBuffer, * WillBuildModel to be called before the document has had its * script global object set. */ - mExecutor->WillBuildModel(eDTDMode_unknown); + rv = mExecutor->WillBuildModel(eDTDMode_unknown); + NS_ENSURE_SUCCESS(rv, rv); } // Return early if the parser has processed EOF @@ -256,7 +257,7 @@ nsHtml5Parser::Parse(const nsAString& aSourceBuffer, } mDocumentClosed = true; if (!mBlocked && !mInDocumentWrite) { - ParseUntilBlocked(); + return ParseUntilBlocked(); } return NS_OK; } @@ -379,7 +380,8 @@ nsHtml5Parser::Parse(const nsAString& aSourceBuffer, if (mTreeBuilder->HasScript()) { mTreeBuilder->Flush(); // Move ops to the executor - mExecutor->FlushDocumentWrite(); // run the ops + rv = mExecutor->FlushDocumentWrite(); // run the ops + NS_ENSURE_SUCCESS(rv, rv); // Flushing tree ops can cause all sorts of things. // Return early if the parser got terminated. if (mExecutor->IsComplete()) { @@ -438,7 +440,8 @@ nsHtml5Parser::Parse(const nsAString& aSourceBuffer, "Buffer wasn't tokenized to completion?"); // Scripting semantics require a forced tree builder flush here mTreeBuilder->Flush(); // Move ops to the executor - mExecutor->FlushDocumentWrite(); // run the ops + rv = mExecutor->FlushDocumentWrite(); // run the ops + NS_ENSURE_SUCCESS(rv, rv); } else if (stackBuffer.hasMore()) { // The buffer wasn't tokenized to completion. Tokenize the untokenized // content in order to preload stuff. This content will be retokenized @@ -596,11 +599,13 @@ nsHtml5Parser::IsScriptCreated() /* End nsIParser */ // not from interface -void +nsresult nsHtml5Parser::ParseUntilBlocked() { - if (mBlocked || mExecutor->IsComplete() || NS_FAILED(mExecutor->IsBroken())) { - return; + nsresult rv = mExecutor->IsBroken(); + NS_ENSURE_SUCCESS(rv, rv); + if (mBlocked || mExecutor->IsComplete()) { + return NS_OK; } NS_ASSERTION(mExecutor->HasStarted(), "Bad life cycle."); NS_ASSERTION(!mInDocumentWrite, @@ -613,7 +618,7 @@ nsHtml5Parser::ParseUntilBlocked() if (mFirstBuffer == mLastBuffer) { if (mExecutor->IsComplete()) { // something like cache manisfests stopped the parse in mid-flight - return; + return NS_OK; } if (mDocumentClosed) { NS_ASSERTION(!GetStreamParser(), @@ -622,8 +627,10 @@ nsHtml5Parser::ParseUntilBlocked() mTreeBuilder->StreamEnded(); mTreeBuilder->Flush(); mExecutor->FlushDocumentWrite(); + // The below call does memory cleanup, so call it even if the + // parser has been marked as broken. mTokenizer->end(); - return; + return NS_OK; } // never release the last buffer. NS_ASSERTION(!mLastBuffer->getStart() && !mLastBuffer->getEnd(), @@ -645,14 +652,14 @@ nsHtml5Parser::ParseUntilBlocked() NS_ASSERTION(mExecutor->IsInFlushLoop(), "How did we come here without being in the flush loop?"); } - return; // no more data for now but expecting more + return NS_OK; // no more data for now but expecting more } mFirstBuffer = mFirstBuffer->next; continue; } if (mBlocked || mExecutor->IsComplete()) { - return; + return NS_OK; } // now we have a non-empty buffer @@ -669,10 +676,11 @@ nsHtml5Parser::ParseUntilBlocked() } if (mTreeBuilder->HasScript()) { mTreeBuilder->Flush(); - mExecutor->FlushDocumentWrite(); + nsresult rv = mExecutor->FlushDocumentWrite(); + NS_ENSURE_SUCCESS(rv, rv); } if (mBlocked) { - return; + return NS_OK; } } continue; diff --git a/parser/html/nsHtml5Parser.h b/parser/html/nsHtml5Parser.h index d79bdbde463..d9e085be5b4 100644 --- a/parser/html/nsHtml5Parser.h +++ b/parser/html/nsHtml5Parser.h @@ -261,7 +261,7 @@ class nsHtml5Parser MOZ_FINAL : public nsIParser, /** * Parse until pending data is exhausted or a script blocks the parser */ - void ParseUntilBlocked(); + nsresult ParseUntilBlocked(); private: diff --git a/parser/html/nsHtml5StreamParser.cpp b/parser/html/nsHtml5StreamParser.cpp index 6eae6d8bd17..9c58ef10e02 100644 --- a/parser/html/nsHtml5StreamParser.cpp +++ b/parser/html/nsHtml5StreamParser.cpp @@ -794,7 +794,7 @@ nsHtml5StreamParser::WriteStreamBytes(const uint8_t* aFromSegment, // NS_HTML5_STREAM_PARSER_READ_BUFFER_SIZE. if (!mLastBuffer) { NS_WARNING("mLastBuffer should not be null!"); - MarkAsBroken(); + MarkAsBroken(NS_ERROR_NULL_POINTER); return NS_ERROR_NULL_POINTER; } if (mLastBuffer->getEnd() == NS_HTML5_STREAM_PARSER_READ_BUFFER_SIZE) { @@ -900,7 +900,8 @@ nsHtml5StreamParser::OnStartRequest(nsIRequest* aRequest, nsISupports* aContext) * WillBuildModel to be called before the document has had its * script global object set. */ - mExecutor->WillBuildModel(eDTDMode_unknown); + rv = mExecutor->WillBuildModel(eDTDMode_unknown); + NS_ENSURE_SUCCESS(rv, rv); nsRefPtr newBuf = nsHtml5OwningUTF16Buffer::FalliblyCreate( @@ -1009,8 +1010,9 @@ nsHtml5StreamParser::DoStopRequest() if (!mUnicodeDecoder) { uint32_t writeCount; - if (NS_FAILED(FinalizeSniffing(nullptr, 0, &writeCount, 0))) { - MarkAsBroken(); + nsresult rv; + if (NS_FAILED(rv = FinalizeSniffing(nullptr, 0, &writeCount, 0))) { + MarkAsBroken(rv); return; } } else if (mFeedChardet) { @@ -1082,7 +1084,7 @@ nsHtml5StreamParser::DoDataAvailable(const uint8_t* aBuffer, uint32_t aLength) rv = SniffStreamBytes(aBuffer, aLength, &writeCount); } if (NS_FAILED(rv)) { - MarkAsBroken(); + MarkAsBroken(rv); return; } NS_ASSERTION(writeCount == aLength, "Wrong number of stream bytes written/sniffed."); @@ -1668,13 +1670,13 @@ nsHtml5StreamParser::TimerFlush() } void -nsHtml5StreamParser::MarkAsBroken() +nsHtml5StreamParser::MarkAsBroken(nsresult aRv) { NS_ASSERTION(IsParserThread(), "Wrong thread!"); mTokenizerMutex.AssertCurrentThreadOwns(); Terminate(); - mTreeBuilder->MarkAsBroken(); + mTreeBuilder->MarkAsBroken(aRv); mozilla::DebugOnly hadOps = mTreeBuilder->Flush(false); NS_ASSERTION(hadOps, "Should have had the markAsBroken op!"); if (NS_FAILED(NS_DispatchToMainThread(mExecutorFlusher))) { diff --git a/parser/html/nsHtml5StreamParser.h b/parser/html/nsHtml5StreamParser.h index 6954cf3e8cf..e4ff429194c 100644 --- a/parser/html/nsHtml5StreamParser.h +++ b/parser/html/nsHtml5StreamParser.h @@ -217,7 +217,7 @@ class nsHtml5StreamParser : public nsICharsetDetectionObserver { } #endif - void MarkAsBroken(); + void MarkAsBroken(nsresult aRv); /** * Marks the stream parser as interrupted. If you ever add calls to this diff --git a/parser/html/nsHtml5TreeBuilderCppSupplement.h b/parser/html/nsHtml5TreeBuilderCppSupplement.h index 9908371a4ce..553532fda02 100644 --- a/parser/html/nsHtml5TreeBuilderCppSupplement.h +++ b/parser/html/nsHtml5TreeBuilderCppSupplement.h @@ -929,14 +929,14 @@ nsHtml5TreeBuilder::DropHandles() } void -nsHtml5TreeBuilder::MarkAsBroken() +nsHtml5TreeBuilder::MarkAsBroken(nsresult aRv) { if (MOZ_UNLIKELY(mBuilder)) { MOZ_ASSERT_UNREACHABLE("Must not call this with builder."); return; } mOpQueue.Clear(); // Previous ops don't matter anymore - mOpQueue.AppendElement()->Init(eTreeOpMarkAsBroken); + mOpQueue.AppendElement()->Init(aRv); } void diff --git a/parser/html/nsHtml5TreeBuilderHSupplement.h b/parser/html/nsHtml5TreeBuilderHSupplement.h index 6ffa71b9d7b..c47fe562670 100644 --- a/parser/html/nsHtml5TreeBuilderHSupplement.h +++ b/parser/html/nsHtml5TreeBuilderHSupplement.h @@ -223,4 +223,4 @@ void errEndWithUnclosedElements(nsIAtom* aName); - void MarkAsBroken(); + void MarkAsBroken(nsresult aRv); diff --git a/parser/html/nsHtml5TreeOpExecutor.cpp b/parser/html/nsHtml5TreeOpExecutor.cpp index fef3c4987b6..007b6904216 100644 --- a/parser/html/nsHtml5TreeOpExecutor.cpp +++ b/parser/html/nsHtml5TreeOpExecutor.cpp @@ -413,7 +413,11 @@ nsHtml5TreeOpExecutor::RunFlushLoop() GetParser()->GetStreamParser(); // Now parse content left in the document.write() buffer queue if any. // This may generate tree ops on its own or dequeue a speculation. - GetParser()->ParseUntilBlocked(); + nsresult rv = GetParser()->ParseUntilBlocked(); + if (NS_FAILED(rv)) { + MarkAsBroken(rv); + return; + } } if (mOpQueue.IsEmpty()) { @@ -496,21 +500,24 @@ nsHtml5TreeOpExecutor::RunFlushLoop() } } -void +nsresult nsHtml5TreeOpExecutor::FlushDocumentWrite() { + nsresult rv = IsBroken(); + NS_ENSURE_SUCCESS(rv, rv); + FlushSpeculativeLoads(); // Make sure speculative loads never start after the // corresponding normal loads for the same URLs. if (MOZ_UNLIKELY(!mParser)) { // The parse has ended. mOpQueue.Clear(); // clear in order to be able to assert in destructor - return; + return rv; } if (mFlushState != eNotFlushing) { // XXX Can this happen? In case it can, let's avoid crashing. - return; + return rv; } mFlushState = eInFlush; @@ -543,7 +550,7 @@ nsHtml5TreeOpExecutor::FlushDocumentWrite() } NS_ASSERTION(mFlushState == eInDocUpdate, "Tried to perform tree op outside update batch."); - nsresult rv = iter->Perform(this, &scriptElement); + rv = iter->Perform(this, &scriptElement); if (NS_FAILED(rv)) { MarkAsBroken(rv); break; @@ -558,13 +565,14 @@ nsHtml5TreeOpExecutor::FlushDocumentWrite() if (MOZ_UNLIKELY(!mParser)) { // Ending the doc update caused a call to nsIParser::Terminate(). - return; + return rv; } if (scriptElement) { // must be tail call when mFlushState is eNotFlushing RunScript(scriptElement); } + return rv; } // copied from HTML content sink diff --git a/parser/html/nsHtml5TreeOpExecutor.h b/parser/html/nsHtml5TreeOpExecutor.h index 2d739b63986..eecba61469d 100644 --- a/parser/html/nsHtml5TreeOpExecutor.h +++ b/parser/html/nsHtml5TreeOpExecutor.h @@ -178,7 +178,7 @@ class nsHtml5TreeOpExecutor MOZ_FINAL : public nsHtml5DocumentBuilder, void RunFlushLoop(); - void FlushDocumentWrite(); + nsresult FlushDocumentWrite(); void MaybeSuspend(); diff --git a/parser/html/nsHtml5TreeOperation.cpp b/parser/html/nsHtml5TreeOperation.cpp index ead4bc3b4a8..162eb772155 100644 --- a/parser/html/nsHtml5TreeOperation.cpp +++ b/parser/html/nsHtml5TreeOperation.cpp @@ -197,6 +197,10 @@ nsHtml5TreeOperation::AppendToDocument(nsIContent* aNode, nsIDocument* doc = aBuilder->GetDocument(); uint32_t childCount = doc->GetChildCount(); rv = doc->AppendChildTo(aNode, false); + if (rv == NS_ERROR_DOM_HIERARCHY_REQUEST_ERR) { + aNode->SetParserHasNotified(); + return NS_OK; + } NS_ENSURE_SUCCESS(rv, rv); aNode->SetParserHasNotified(); nsNodeUtils::ContentInserted(doc, aNode, childCount); @@ -726,8 +730,7 @@ nsHtml5TreeOperation::Perform(nsHtml5TreeOpExecutor* aBuilder, return NS_OK; } case eTreeOpMarkAsBroken: { - aBuilder->MarkAsBroken(NS_ERROR_OUT_OF_MEMORY); - return NS_OK; + return mOne.result; } case eTreeOpRunScript: { nsIContent* node = *(mOne.node); diff --git a/parser/html/nsHtml5TreeOperation.h b/parser/html/nsHtml5TreeOperation.h index 95861c041ee..0ff354f2859 100644 --- a/parser/html/nsHtml5TreeOperation.h +++ b/parser/html/nsHtml5TreeOperation.h @@ -432,6 +432,15 @@ class nsHtml5TreeOperation { mFour.integer = aInt; } + inline void Init(nsresult aRv) + { + NS_PRECONDITION(mOpCode == eTreeOpUninitialized, + "Op code must be uninitialized when initializing."); + NS_PRECONDITION(NS_FAILED(aRv), "Initialized tree op with non-failure."); + mOpCode = eTreeOpMarkAsBroken; + mOne.result = aRv; + } + inline void InitAddClass(nsIContentHandle* aNode, const char16_t* aClass) { NS_PRECONDITION(mOpCode == eTreeOpUninitialized, @@ -484,11 +493,12 @@ class nsHtml5TreeOperation { nsIAtom* atom; nsHtml5HtmlAttributes* attributes; nsHtml5DocumentMode mode; - char16_t* unicharPtr; + char16_t* unicharPtr; char* charPtr; nsHtml5TreeOperationStringPair* stringPair; nsAHtml5TreeBuilderState* state; int32_t integer; + nsresult result; } mOne, mTwo, mThree, mFour; };