From 622b02696018d034c10ca308e47e101cbb01295b Mon Sep 17 00:00:00 2001 From: Molnar Sandor <smolnar@mozilla.com> Date: Tue, 19 Apr 2022 14:41:03 +0300 Subject: [PATCH] Backed out changeset aa30bc1b369b (bug 1763076) for causing assertion failure in parser/html/nsHtml5TreeOpExecutor.cpp CLOSED TREE --- parser/html/nsAHtml5TreeOpSink.h | 5 +- parser/html/nsHtml5Highlighter.cpp | 18 +- parser/html/nsHtml5Highlighter.h | 17 +- parser/html/nsHtml5Parser.cpp | 35 +-- parser/html/nsHtml5Speculation.cpp | 9 +- parser/html/nsHtml5Speculation.h | 5 +- parser/html/nsHtml5StreamParser.cpp | 202 +++--------------- parser/html/nsHtml5StreamParser.h | 4 +- parser/html/nsHtml5TokenizerCppSupplement.h | 12 +- parser/html/nsHtml5TokenizerHSupplement.h | 6 +- parser/html/nsHtml5TreeBuilderCppSupplement.h | 7 +- parser/html/nsHtml5TreeBuilderHSupplement.h | 7 +- parser/html/nsHtml5TreeOpExecutor.cpp | 19 +- parser/html/nsHtml5TreeOpExecutor.h | 5 +- parser/html/nsHtml5TreeOpStage.cpp | 16 +- parser/html/nsHtml5TreeOpStage.h | 7 +- 16 files changed, 81 insertions(+), 293 deletions(-) diff --git a/parser/html/nsAHtml5TreeOpSink.h b/parser/html/nsAHtml5TreeOpSink.h index a09971a2e88e8..bdcec970c5ac9 100644 --- a/parser/html/nsAHtml5TreeOpSink.h +++ b/parser/html/nsAHtml5TreeOpSink.h @@ -19,11 +19,8 @@ class nsAHtml5TreeOpSink { /** * Flush the operations from the tree operations from the argument * queue into this sink unconditionally. - * - * Returns `true` on success and `false` on OOM. */ - [[nodiscard]] virtual bool MoveOpsFrom( - nsTArray<nsHtml5TreeOperation>& aOpQueue) = 0; + virtual void MoveOpsFrom(nsTArray<nsHtml5TreeOperation>& aOpQueue) = 0; }; #endif /* nsAHtml5TreeOpSink_h */ diff --git a/parser/html/nsHtml5Highlighter.cpp b/parser/html/nsHtml5Highlighter.cpp index 45c84b743a319..5b54829747c49 100644 --- a/parser/html/nsHtml5Highlighter.cpp +++ b/parser/html/nsHtml5Highlighter.cpp @@ -3,7 +3,6 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ #include "nsHtml5Highlighter.h" -#include "ErrorList.h" #include "nsDebug.h" #include "nsHtml5AttributeName.h" #include "nsHtml5Tokenizer.h" @@ -463,7 +462,7 @@ int32_t nsHtml5Highlighter::Transition(int32_t aState, bool aReconsume, return aState; } -[[nodiscard]] bool nsHtml5Highlighter::End() { +void nsHtml5Highlighter::End() { switch (mState) { case nsHtml5Tokenizer::COMMENT_END: case nsHtml5Tokenizer::COMMENT_END_BANG: @@ -502,7 +501,7 @@ int32_t nsHtml5Highlighter::Transition(int32_t aState, bool aReconsume, nsHtml5TreeOperation* treeOp = mOpQueue.AppendElement(); NS_ASSERTION(treeOp, "Tree op allocation failed."); treeOp->Init(mozilla::AsVariant(opStreamEnded())); - return FlushOps().isOk(); + FlushOps(); } void nsHtml5Highlighter::SetBuffer(nsHtml5UTF16Buffer* aBuffer) { @@ -619,19 +618,10 @@ void nsHtml5Highlighter::FlushCurrent() { FlushChars(); } -bool nsHtml5Highlighter::ShouldFlushOps() { - // Arbitrary threshold that doesn't have an exact justification. - // The general idea is to flush much, much sooner than reaching - // the maximum size of `nsTArray`. - return mOpQueue.Length() > 100000; -} - -mozilla::Result<bool, nsresult> nsHtml5Highlighter::FlushOps() { +bool nsHtml5Highlighter::FlushOps() { bool hasOps = !mOpQueue.IsEmpty(); if (hasOps) { - if (!mOpSink->MoveOpsFrom(mOpQueue)) { - return Err(NS_ERROR_OUT_OF_MEMORY); - } + mOpSink->MoveOpsFrom(mOpQueue); } return hasOps; } diff --git a/parser/html/nsHtml5Highlighter.h b/parser/html/nsHtml5Highlighter.h index 4966b216085c7..f50ac379d5ede 100644 --- a/parser/html/nsHtml5Highlighter.h +++ b/parser/html/nsHtml5Highlighter.h @@ -62,10 +62,8 @@ class nsHtml5Highlighter { /** * Report end of file. - * - * Returns `true` normally and `false` on OOM. */ - [[nodiscard]] bool End(); + void End(); /** * Set the current buffer being tokenized @@ -79,21 +77,12 @@ class nsHtml5Highlighter { */ void DropBuffer(int32_t aPos); - /** - * Query whether there are some many ops in the queue - * that they should be flushed now. - * - * @return true if FlushOps() should be called now - */ - bool ShouldFlushOps(); - /** * Flush the tree ops into the sink. * - * @return Ok(true) if there were ops to flush, Ok(false) - * if there were no ops to flush and Err() on OOM. + * @return true if there were ops to flush */ - mozilla::Result<bool, nsresult> FlushOps(); + bool FlushOps(); /** * Linkify the current attribute value if the attribute name is one of diff --git a/parser/html/nsHtml5Parser.cpp b/parser/html/nsHtml5Parser.cpp index 5425b7d8a7634..d830dc4cf2cc9 100644 --- a/parser/html/nsHtml5Parser.cpp +++ b/parser/html/nsHtml5Parser.cpp @@ -332,10 +332,7 @@ nsresult nsHtml5Parser::Parse(const nsAString& aSourceBuffer, void* aKey, } if (mTreeBuilder->HasScript()) { - auto r = mTreeBuilder->Flush(); // Move ops to the executor - if (r.isErr()) { - return executor->MarkAsBroken(r.unwrapErr()); - } + mTreeBuilder->Flush(); // Move ops to the executor rv = executor->FlushDocumentWrite(); // run the ops NS_ENSURE_SUCCESS(rv, rv); // Flushing tree ops can cause all sorts of things. @@ -395,10 +392,7 @@ nsresult nsHtml5Parser::Parse(const nsAString& aSourceBuffer, void* aKey, NS_ASSERTION(!stackBuffer.hasMore(), "Buffer wasn't tokenized to completion?"); // Scripting semantics require a forced tree builder flush here - auto r = mTreeBuilder->Flush(); // Move ops to the executor - if (r.isErr()) { - return executor->MarkAsBroken(r.unwrapErr()); - } + mTreeBuilder->Flush(); // Move ops to the executor rv = executor->FlushDocumentWrite(); // run the ops NS_ENSURE_SUCCESS(rv, rv); } else if (stackBuffer.hasMore()) { @@ -448,10 +442,7 @@ nsresult nsHtml5Parser::Parse(const nsAString& aSourceBuffer, void* aKey, } } - auto r = mDocWriteSpeculativeTreeBuilder->Flush(); - if (r.isErr()) { - return executor->MarkAsBroken(r.unwrapErr()); - } + mDocWriteSpeculativeTreeBuilder->Flush(); mDocWriteSpeculativeTreeBuilder->DropHandles(); executor->FlushSpeculativeLoads(); } @@ -557,10 +548,7 @@ nsresult nsHtml5Parser::ParseUntilBlocked() { mTreeBuilder->StreamEnded(); } } - auto r = mTreeBuilder->Flush(); - if (r.isErr()) { - return mExecutor->MarkAsBroken(r.unwrapErr()); - } + mTreeBuilder->Flush(); mExecutor->FlushDocumentWrite(); // The below call does memory cleanup, so call it even if the // parser has been marked as broken. @@ -573,20 +561,14 @@ nsresult nsHtml5Parser::ParseUntilBlocked() { if (GetStreamParser()) { if (mReturnToStreamParserPermitted && !mExecutor->IsScriptExecuting()) { - auto r = mTreeBuilder->Flush(); - if (r.isErr()) { - return mExecutor->MarkAsBroken(r.unwrapErr()); - } + mTreeBuilder->Flush(); mReturnToStreamParserPermitted = false; GetStreamParser()->ContinueAfterScriptsOrEncodingCommitment( mTokenizer.get(), mTreeBuilder.get(), mLastWasCR); } } else { // Script-created parser - auto r = mTreeBuilder->Flush(); - if (r.isErr()) { - return mExecutor->MarkAsBroken(r.unwrapErr()); - } + mTreeBuilder->Flush(); // No need to flush the executor, because the executor is already // in a flush NS_ASSERTION(mExecutor->IsInFlushLoop(), @@ -622,10 +604,7 @@ nsresult nsHtml5Parser::ParseUntilBlocked() { mRootContextLineNumber = mTokenizer->getLineNumber(); } if (mTreeBuilder->HasScript()) { - auto r = mTreeBuilder->Flush(); - if (r.isErr()) { - return mExecutor->MarkAsBroken(r.unwrapErr()); - } + mTreeBuilder->Flush(); rv = mExecutor->FlushDocumentWrite(); NS_ENSURE_SUCCESS(rv, rv); } diff --git a/parser/html/nsHtml5Speculation.cpp b/parser/html/nsHtml5Speculation.cpp index 07796e77b80da..00e61d604abef 100644 --- a/parser/html/nsHtml5Speculation.cpp +++ b/parser/html/nsHtml5Speculation.cpp @@ -20,11 +20,10 @@ nsHtml5Speculation::~nsHtml5Speculation() { MOZ_COUNT_DTOR(nsHtml5Speculation); } -[[nodiscard]] bool nsHtml5Speculation::MoveOpsFrom( - nsTArray<nsHtml5TreeOperation>& aOpQueue) { - return !!mOpQueue.AppendElements(std::move(aOpQueue), mozilla::fallible_t()); +void nsHtml5Speculation::MoveOpsFrom(nsTArray<nsHtml5TreeOperation>& aOpQueue) { + mOpQueue.AppendElements(std::move(aOpQueue)); } -[[nodiscard]] bool nsHtml5Speculation::FlushToSink(nsAHtml5TreeOpSink* aSink) { - return aSink->MoveOpsFrom(mOpQueue); +void nsHtml5Speculation::FlushToSink(nsAHtml5TreeOpSink* aSink) { + aSink->MoveOpsFrom(mOpQueue); } diff --git a/parser/html/nsHtml5Speculation.h b/parser/html/nsHtml5Speculation.h index 9e1b2c31f5866..3230615213c8b 100644 --- a/parser/html/nsHtml5Speculation.h +++ b/parser/html/nsHtml5Speculation.h @@ -33,10 +33,9 @@ class nsHtml5Speculation final : public nsAHtml5TreeOpSink { * Flush the operations from the tree operations from the argument * queue unconditionally. */ - [[nodiscard]] virtual bool MoveOpsFrom( - nsTArray<nsHtml5TreeOperation>& aOpQueue) override; + virtual void MoveOpsFrom(nsTArray<nsHtml5TreeOperation>& aOpQueue) override; - [[nodiscard]] bool FlushToSink(nsAHtml5TreeOpSink* aSink); + void FlushToSink(nsAHtml5TreeOpSink* aSink); private: /** diff --git a/parser/html/nsHtml5StreamParser.cpp b/parser/html/nsHtml5StreamParser.cpp index 7e9e9d1a7ceed..67c51aa5783e3 100644 --- a/parser/html/nsHtml5StreamParser.cpp +++ b/parser/html/nsHtml5StreamParser.cpp @@ -12,7 +12,6 @@ #include <new> #include <type_traits> #include <utility> -#include "ErrorList.h" #include "GeckoProfiler.h" #include "js/GCAPI.h" #include "mozilla/ArrayIterator.h" @@ -776,15 +775,9 @@ nsresult nsHtml5StreamParser::SniffStreamBytes(Span<const uint8_t> aFromSegment, MOZ_ASSERT(mLookingForMetaCharset); if (mMode == VIEW_SOURCE_HTML) { - auto r = mTokenizer->FlushViewSource(); - if (r.isErr()) { - return r.unwrapErr(); - } - } - auto r = mTreeBuilder->Flush(); - if (r.isErr()) { - return r.unwrapErr(); + mTokenizer->FlushViewSource(); } + mTreeBuilder->Flush(); // Encoding committer flushes the ops on the main thread. mozilla::MutexAutoLock speculationAutoLock(mSpeculationMutex); @@ -930,16 +923,10 @@ nsresult nsHtml5StreamParser::WriteStreamBytes( mCharsetSource = source; if (encoding != mEncoding) { mEncoding = encoding; - nsresult rv = ReDecodeLocalFile(); - if (NS_FAILED(rv)) { - return rv; - } + ReDecodeLocalFile(); } else { MOZ_ASSERT(mEncoding == UTF_8_ENCODING); - nsresult rv = CommitLocalFileToEncoding(); - if (NS_FAILED(rv)) { - return rv; - } + CommitLocalFileToEncoding(); } } return NS_OK; @@ -947,7 +934,7 @@ nsresult nsHtml5StreamParser::WriteStreamBytes( } } -[[nodiscard]] nsresult nsHtml5StreamParser::ReDecodeLocalFile() { +void nsHtml5StreamParser::ReDecodeLocalFile() { MOZ_ASSERT(mDecodingLocalFileWithoutTokenizing && !mLookingForMetaCharset); MOZ_ASSERT(mFirstBufferOfMetaScan); MOZ_ASSERT(mCharsetSource == kCharsetFromFinalAutoDetectionFile || @@ -981,19 +968,12 @@ nsresult nsHtml5StreamParser::WriteStreamBytes( } if (mMode == VIEW_SOURCE_HTML) { - auto r = mTokenizer->FlushViewSource(); - if (r.isErr()) { - return r.unwrapErr(); - } - } - auto r = mTreeBuilder->Flush(); - if (r.isErr()) { - return r.unwrapErr(); + mTokenizer->FlushViewSource(); } - return NS_OK; + mTreeBuilder->Flush(); } -[[nodiscard]] nsresult nsHtml5StreamParser::CommitLocalFileToEncoding() { +void nsHtml5StreamParser::CommitLocalFileToEncoding() { MOZ_ASSERT(mDecodingLocalFileWithoutTokenizing && !mLookingForMetaCharset); MOZ_ASSERT(mFirstBufferOfMetaScan); mDecodingLocalFileWithoutTokenizing = false; @@ -1019,16 +999,9 @@ nsresult nsHtml5StreamParser::WriteStreamBytes( mForceAutoDetection = false; // To stop feeding the detector mTreeBuilder->SetDocumentCharset(mEncoding, mCharsetSource, true); if (mMode == VIEW_SOURCE_HTML) { - auto r = mTokenizer->FlushViewSource(); - if (r.isErr()) { - return r.unwrapErr(); - } + mTokenizer->FlushViewSource(); } - auto r = mTreeBuilder->Flush(); - if (r.isErr()) { - return r.unwrapErr(); - } - return NS_OK; + mTreeBuilder->Flush(); } class MaybeRunCollector : public Runnable { @@ -1136,10 +1109,7 @@ nsresult nsHtml5StreamParser::OnStartRequest(nsIRequest* aRequest) { mTemplatePushedOrHeadPopped); // Needed to force 1024-byte sniffing // Flush the ops to put them where ContinueAfterScriptsOrEncodingCommitment // can find them. - auto r = mTreeBuilder->Flush(); - if (r.isErr()) { - return mExecutor->MarkAsBroken(r.unwrapErr()); - } + mTreeBuilder->Flush(); } else if (mMode == VIEW_SOURCE_PLAIN) { nsAutoString viewSourceTitle; CopyUTF8toUTF16(mViewSourceTitle, viewSourceTitle); @@ -1150,10 +1120,7 @@ nsresult nsHtml5StreamParser::OnStartRequest(nsIRequest* aRequest) { mTemplatePushedOrHeadPopped); // Needed to force 1024-byte sniffing // Flush the ops to put them where ContinueAfterScriptsOrEncodingCommitment // can find them. - auto r = mTreeBuilder->Flush(); - if (r.isErr()) { - return mExecutor->MarkAsBroken(r.unwrapErr()); - } + mTreeBuilder->Flush(); } else if (mMode == VIEW_SOURCE_HTML || mMode == VIEW_SOURCE_XML) { // Generate and flush the View Source document up to and including the // pre element start. @@ -1163,10 +1130,7 @@ nsresult nsHtml5StreamParser::OnStartRequest(nsIRequest* aRequest) { } // Flush the ops to put them where ContinueAfterScriptsOrEncodingCommitment // can find them. - auto r = mTokenizer->FlushViewSource(); - if (r.isErr()) { - return mExecutor->MarkAsBroken(r.unwrapErr()); - } + mTokenizer->FlushViewSource(); } /* @@ -1292,11 +1256,7 @@ void nsHtml5StreamParser::DoStopRequest() { mTreeBuilder->SetDocumentCharset(mEncoding, mCharsetSource, false); for (auto&& buffer : mBufferedBytes) { - nsresult rv = WriteStreamBytes(buffer); - if (NS_FAILED(rv)) { - MarkAsBroken(rv); - return; - } + Unused << WriteStreamBytes(buffer); } } else if (!mUnicodeDecoder) { nsresult rv; @@ -1356,20 +1316,12 @@ void nsHtml5StreamParser::DoStopRequest() { mCharsetSource = source; if (encoding != mEncoding) { mEncoding = encoding; - nsresult rv = ReDecodeLocalFile(); - if (NS_FAILED(rv)) { - MarkAsBroken(rv); - return; - } + ReDecodeLocalFile(); DoStopRequest(); return; } MOZ_ASSERT(mEncoding == UTF_8_ENCODING); - nsresult rv = CommitLocalFileToEncoding(); - if (NS_FAILED(rv)) { - MarkAsBroken(rv); - return; - } + CommitLocalFileToEncoding(); } break; } @@ -1781,23 +1733,6 @@ void nsHtml5StreamParser::PostLoadFlusher() { if (NS_FAILED(DispatchToMain(runnable.forget()))) { NS_WARNING("failed to dispatch load flush event"); } - - if ((mMode == VIEW_SOURCE_HTML || mMode == VIEW_SOURCE_XML) && - mTokenizer->ShouldFlushViewSource()) { - auto r = mTreeBuilder->Flush(); // delete useless ops - MOZ_ASSERT(r.isOk(), "Should have null sink with View Source"); - r = mTokenizer->FlushViewSource(); - if (r.isErr()) { - MarkAsBroken(r.unwrapErr()); - return; - } - if (r.unwrap()) { - nsCOMPtr<nsIRunnable> runnable(mExecutorFlusher); - if (NS_FAILED(DispatchToMain(runnable.forget()))) { - NS_WARNING("failed to dispatch executor flush event"); - } - } - } } void nsHtml5StreamParser::FlushTreeOpsAndDisarmTimer() { @@ -1812,15 +1747,9 @@ void nsHtml5StreamParser::FlushTreeOpsAndDisarmTimer() { mFlushTimerArmed = false; } if (mMode == VIEW_SOURCE_HTML || mMode == VIEW_SOURCE_XML) { - auto r = mTokenizer->FlushViewSource(); - if (r.isErr()) { - MarkAsBroken(r.unwrapErr()); - } - } - auto r = mTreeBuilder->Flush(); - if (r.isErr()) { - MarkAsBroken(r.unwrapErr()); + mTokenizer->FlushViewSource(); } + mTreeBuilder->Flush(); nsCOMPtr<nsIRunnable> runnable(mExecutorFlusher); if (NS_FAILED(DispatchToMain(runnable.forget()))) { NS_WARNING("failed to dispatch executor flush event"); @@ -2235,17 +2164,9 @@ bool nsHtml5StreamParser::ProcessLookingForMetaCharset(bool aEof) { mCharsetSource = mEncodingSwitchSource; if (mMode == VIEW_SOURCE_HTML) { - auto r = mTokenizer->FlushViewSource(); - if (r.isErr()) { - MarkAsBroken(r.unwrapErr()); - return false; - } - } - auto r = mTreeBuilder->Flush(); - if (r.isErr()) { - MarkAsBroken(r.unwrapErr()); - return false; + mTokenizer->FlushViewSource(); } + mTreeBuilder->Flush(); if (mEncoding != needsEncodingSwitchTo) { // Speculation failed @@ -2284,11 +2205,7 @@ bool nsHtml5StreamParser::ProcessLookingForMetaCharset(bool aEof) { "Must have set mLookingForMetaCharset to false to report data " "to dev tools below"); for (auto&& buffer : mBufferedBytes) { - nsresult rv = WriteStreamBytes(buffer); - if (NS_FAILED(rv)) { - MarkAsBroken(rv); - return false; - } + WriteStreamBytes(buffer); } } } else if (!mLookingForMetaCharset && !mDecodingLocalFileWithoutTokenizing) { @@ -2313,17 +2230,9 @@ bool nsHtml5StreamParser::ProcessLookingForMetaCharset(bool aEof) { mBufferedBytes.Clear(); mTreeBuilder->SetDocumentCharset(mEncoding, mCharsetSource, true); if (mMode == VIEW_SOURCE_HTML) { - auto r = mTokenizer->FlushViewSource(); - if (r.isErr()) { - MarkAsBroken(r.unwrapErr()); - return false; - } - } - auto r = mTreeBuilder->Flush(); - if (r.isErr()) { - MarkAsBroken(r.unwrapErr()); - return false; + mTokenizer->FlushViewSource(); } + mTreeBuilder->Flush(); } } return rewound; @@ -2446,9 +2355,7 @@ void nsHtml5StreamParser::ParseAvailableData() { } else { mTreeBuilder->StreamEnded(); if (mMode == VIEW_SOURCE_HTML || mMode == VIEW_SOURCE_XML) { - if (!mTokenizer->EndViewSource()) { - MarkAsBroken(NS_ERROR_OUT_OF_MEMORY); - } + mTokenizer->EndViewSource(); } } } @@ -2489,17 +2396,9 @@ void nsHtml5StreamParser::ParseAvailableData() { speculation->GetStartLineNumber()); if (mLookingForMetaCharset) { if (mMode == VIEW_SOURCE_HTML) { - auto r = mTokenizer->FlushViewSource(); - if (r.isErr()) { - MarkAsBroken(r.unwrapErr()); - return; - } - } - auto r = mTreeBuilder->Flush(); - if (r.isErr()) { - MarkAsBroken(r.unwrapErr()); - return; + mTokenizer->FlushViewSource(); } + mTreeBuilder->Flush(); } else { FlushTreeOpsAndDisarmTimer(); } @@ -2559,10 +2458,7 @@ void nsHtml5StreamParser::ContinueAfterScriptsOrEncodingCommitment( // XML View Source never needs this kind of encoding commitment. // We need to take the ops here so that they end up in the queue before // the ops that we take from a speculation later in this method. - if (!mExecutor->TakeOpsFromStage()) { - mExecutor->MarkAsBroken(NS_ERROR_OUT_OF_MEMORY); - return; - } + mExecutor->TakeOpsFromStage(); } else { #ifdef DEBUG mExecutor->AssertStageEmpty(); @@ -2599,10 +2495,7 @@ void nsHtml5StreamParser::ContinueAfterScriptsOrEncodingCommitment( if (mSpeculations.Length() > 1) { // the first speculation isn't the current speculation, so there's // no need to bother the parser thread. - if (!speculation->FlushToSink(mExecutor)) { - mExecutor->MarkAsBroken(NS_ERROR_OUT_OF_MEMORY); - return; - } + speculation->FlushToSink(mExecutor); MOZ_ASSERT(!mExecutor->IsScriptExecuting(), "ParseUntilBlocked() was supposed to ensure we don't come " "here when scripts are executing."); @@ -2677,10 +2570,7 @@ void nsHtml5StreamParser::ContinueAfterScriptsOrEncodingCommitment( } else { // We've got a successful speculation and at least a moment ago it was // the current speculation - if (!mSpeculations.ElementAt(0)->FlushToSink(mExecutor)) { - mExecutor->MarkAsBroken(NS_ERROR_OUT_OF_MEMORY); - return; - } + mSpeculations.ElementAt(0)->FlushToSink(mExecutor); MOZ_ASSERT(!mExecutor->IsScriptExecuting(), "ParseUntilBlocked() was supposed to ensure we don't come " "here when scripts are executing."); @@ -2699,11 +2589,7 @@ void nsHtml5StreamParser::ContinueAfterScriptsOrEncodingCommitment( // any pending ops straight to the executor, because otherwise // they remain unflushed until we get more data from the network. mTreeBuilder->SetOpSink(mExecutor); - auto r = mTreeBuilder->Flush(true); - if (r.isErr()) { - mExecutor->MarkAsBroken(r.unwrapErr()); - return; - } + mTreeBuilder->Flush(true); mTreeBuilder->SetOpSink(mExecutor->GetStage()); } mExecutor->StartReadingFromStage(); @@ -2799,17 +2685,8 @@ void nsHtml5StreamParser::TimerFlush() { } if (mMode == VIEW_SOURCE_HTML || mMode == VIEW_SOURCE_XML) { - auto r = mTreeBuilder->Flush(); // delete useless ops - if (r.isErr()) { - MarkAsBroken(r.unwrapErr()); - return; - } - r = mTokenizer->FlushViewSource(); - if (r.isErr()) { - MarkAsBroken(r.unwrapErr()); - return; - } - if (r.unwrap()) { + mTreeBuilder->Flush(); // delete useless ops + if (mTokenizer->FlushViewSource()) { nsCOMPtr<nsIRunnable> runnable(mExecutorFlusher); if (NS_FAILED(DispatchToMain(runnable.forget()))) { NS_WARNING("failed to dispatch executor flush event"); @@ -2818,12 +2695,7 @@ void nsHtml5StreamParser::TimerFlush() { } else { // we aren't speculating and we don't know when new data is // going to arrive. Send data to the main thread. - auto r = mTreeBuilder->Flush(true); - if (r.isErr()) { - MarkAsBroken(r.unwrapErr()); - return; - } - if (r.unwrap()) { + if (mTreeBuilder->Flush(true)) { nsCOMPtr<nsIRunnable> runnable(mExecutorFlusher); if (NS_FAILED(DispatchToMain(runnable.forget()))) { NS_WARNING("failed to dispatch executor flush event"); @@ -2838,12 +2710,8 @@ void nsHtml5StreamParser::MarkAsBroken(nsresult aRv) { Terminate(); mTreeBuilder->MarkAsBroken(aRv); - auto r = mTreeBuilder->Flush(false); - if (r.isOk()) { - MOZ_ASSERT(r.unwrap(), "Should have had the markAsBroken op!"); - } else { - MOZ_CRASH("OOM prevents propagation of OOM state"); - } + mozilla::DebugOnly<bool> hadOps = mTreeBuilder->Flush(false); + MOZ_ASSERT(hadOps, "Should have had the markAsBroken op!"); nsCOMPtr<nsIRunnable> runnable(mExecutorFlusher); if (NS_FAILED(DispatchToMain(runnable.forget()))) { NS_WARNING("failed to dispatch executor flush event"); diff --git a/parser/html/nsHtml5StreamParser.h b/parser/html/nsHtml5StreamParser.h index 6ba4f8bd645d1..79f2197670246 100644 --- a/parser/html/nsHtml5StreamParser.h +++ b/parser/html/nsHtml5StreamParser.h @@ -412,13 +412,13 @@ class nsHtml5StreamParser final : public nsISupports { * to UTF-8 as the non-speculative encoding and start processing * the decoded data. */ - [[nodiscard]] nsresult CommitLocalFileToEncoding(); + void CommitLocalFileToEncoding(); /** * When speculatively decoding from file: URL as UTF-8, redecode * using fallback and then continue normally with the fallback. */ - [[nodiscard]] nsresult ReDecodeLocalFile() REQUIRES(mTokenizerMutex); + void ReDecodeLocalFile() REQUIRES(mTokenizerMutex); /** * Potentially guess the encoding using mozilla::EncodingDetector. diff --git a/parser/html/nsHtml5TokenizerCppSupplement.h b/parser/html/nsHtml5TokenizerCppSupplement.h index 1802681961294..5ffcf75800b09 100644 --- a/parser/html/nsHtml5TokenizerCppSupplement.h +++ b/parser/html/nsHtml5TokenizerCppSupplement.h @@ -82,13 +82,7 @@ void nsHtml5Tokenizer::EnableViewSource(nsHtml5Highlighter* aHighlighter) { mViewSource = WrapUnique(aHighlighter); } -bool nsHtml5Tokenizer::ShouldFlushViewSource() { - return mViewSource->ShouldFlushOps(); -} - -mozilla::Result<bool, nsresult> nsHtml5Tokenizer::FlushViewSource() { - return mViewSource->FlushOps(); -} +bool nsHtml5Tokenizer::FlushViewSource() { return mViewSource->FlushOps(); } void nsHtml5Tokenizer::StartViewSource(const nsAutoString& aTitle) { mViewSource->Start(aTitle); @@ -98,9 +92,7 @@ void nsHtml5Tokenizer::StartViewSourceCharacters() { mViewSource->StartCharacters(); } -[[nodiscard]] bool nsHtml5Tokenizer::EndViewSource() { - return mViewSource->End(); -} +void nsHtml5Tokenizer::EndViewSource() { mViewSource->End(); } void nsHtml5Tokenizer::SetViewSourceOpSink(nsAHtml5TreeOpSink* aOpSink) { mViewSource->SetOpSink(aOpSink); diff --git a/parser/html/nsHtml5TokenizerHSupplement.h b/parser/html/nsHtml5TokenizerHSupplement.h index 5fd2451de8f0b..633a99b4ca83c 100644 --- a/parser/html/nsHtml5TokenizerHSupplement.h +++ b/parser/html/nsHtml5TokenizerHSupplement.h @@ -34,15 +34,13 @@ void StartPlainText(); void EnableViewSource(nsHtml5Highlighter* aHighlighter); -bool ShouldFlushViewSource(); - -mozilla::Result<bool, nsresult> FlushViewSource(); +bool FlushViewSource(); void StartViewSource(const nsAutoString& aTitle); void StartViewSourceCharacters(); -[[nodiscard]] bool EndViewSource(); +void EndViewSource(); void RewindViewSource(); diff --git a/parser/html/nsHtml5TreeBuilderCppSupplement.h b/parser/html/nsHtml5TreeBuilderCppSupplement.h index 5500a9c2c85a9..0c0d25723969f 100644 --- a/parser/html/nsHtml5TreeBuilderCppSupplement.h +++ b/parser/html/nsHtml5TreeBuilderCppSupplement.h @@ -4,7 +4,6 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -#include "ErrorList.h" #include "nsError.h" #include "mozilla/CheckedInt.h" #include "mozilla/Likely.h" @@ -1202,7 +1201,7 @@ bool nsHtml5TreeBuilder::HasScript() { return mOpQueue.ElementAt(len - 1).IsRunScript(); } -mozilla::Result<bool, nsresult> nsHtml5TreeBuilder::Flush(bool aDiscretionary) { +bool nsHtml5TreeBuilder::Flush(bool aDiscretionary) { if (MOZ_UNLIKELY(mBuilder)) { MOZ_ASSERT_UNREACHABLE("Must never flush with builder."); return false; @@ -1231,9 +1230,7 @@ mozilla::Result<bool, nsresult> nsHtml5TreeBuilder::Flush(bool aDiscretionary) { "Tree builder is broken but the op in queue is not marked " "as broken."); } - if (!mOpSink->MoveOpsFrom(mOpQueue)) { - return Err(NS_ERROR_OUT_OF_MEMORY); - } + mOpSink->MoveOpsFrom(mOpQueue); } return hasOps; } diff --git a/parser/html/nsHtml5TreeBuilderHSupplement.h b/parser/html/nsHtml5TreeBuilderHSupplement.h index e7dacdfaa81a1..0197f5dbd6254 100644 --- a/parser/html/nsHtml5TreeBuilderHSupplement.h +++ b/parser/html/nsHtml5TreeBuilderHSupplement.h @@ -110,12 +110,7 @@ void SetOpSink(nsAHtml5TreeOpSink* aOpSink) { mOpSink = aOpSink; } void ClearOps() { mOpQueue.Clear(); } -/** - * Flushes tree ops. - * @return Ok(true) if there were ops to flush, Ok(false) - * if there were no ops to flush and Err() on OOM. - */ -mozilla::Result<bool, nsresult> Flush(bool aDiscretionary = false); +bool Flush(bool aDiscretionary = false); void FlushLoads(); diff --git a/parser/html/nsHtml5TreeOpExecutor.cpp b/parser/html/nsHtml5TreeOpExecutor.cpp index f10e41279d649..93504fa7ffa92 100644 --- a/parser/html/nsHtml5TreeOpExecutor.cpp +++ b/parser/html/nsHtml5TreeOpExecutor.cpp @@ -22,7 +22,6 @@ #include "mozilla/StaticPrefs_view_source.h" #include "mozilla/Telemetry.h" #include "mozilla/css/Loader.h" -#include "mozilla/fallible.h" #include "nsContentUtils.h" #include "nsDocShell.h" #include "nsError.h" @@ -141,8 +140,7 @@ nsHtml5TreeOpExecutor::~nsHtml5TreeOpExecutor() { } } } - MOZ_ASSERT(NS_FAILED(mBroken) || mOpQueue.IsEmpty(), - "Somehow there's stuff in the op queue."); + NS_ASSERTION(mOpQueue.IsEmpty(), "Somehow there's stuff in the op queue."); } // nsIContentSink @@ -611,12 +609,7 @@ void nsHtml5TreeOpExecutor::RunFlushLoop() { nsTArray<nsHtml5SpeculativeLoad> speculativeLoadQueue; MOZ_RELEASE_ASSERT(mFlushState == eNotFlushing, "mOpQueue modified during flush."); - if (!mStage.MoveOpsAndSpeculativeLoadsTo(mOpQueue, - speculativeLoadQueue)) { - MarkAsBroken(nsresult::NS_ERROR_OUT_OF_MEMORY); - return; - } - + mStage.MoveOpsAndSpeculativeLoadsTo(mOpQueue, speculativeLoadQueue); // Make sure speculative loads never start after the corresponding // normal loads for the same URLs. nsHtml5SpeculativeLoad* start = speculativeLoadQueue.Elements(); @@ -850,9 +843,7 @@ void nsHtml5TreeOpExecutor::CommitToInternalEncoding() { false); } -[[nodiscard]] bool nsHtml5TreeOpExecutor::TakeOpsFromStage() { - return mStage.MoveOpsTo(mOpQueue); -} +void nsHtml5TreeOpExecutor::TakeOpsFromStage() { mStage.MoveOpsTo(mOpQueue); } // copied from HTML content sink bool nsHtml5TreeOpExecutor::IsScriptEnabled() { @@ -1046,11 +1037,11 @@ nsHtml5Parser* nsHtml5TreeOpExecutor::GetParser() { return static_cast<nsHtml5Parser*>(mParser.get()); } -[[nodiscard]] bool nsHtml5TreeOpExecutor::MoveOpsFrom( +void nsHtml5TreeOpExecutor::MoveOpsFrom( nsTArray<nsHtml5TreeOperation>& aOpQueue) { MOZ_RELEASE_ASSERT(mFlushState == eNotFlushing, "Ops added to mOpQueue during tree op execution."); - return !!mOpQueue.AppendElements(std::move(aOpQueue), mozilla::fallible_t()); + mOpQueue.AppendElements(std::move(aOpQueue)); } void nsHtml5TreeOpExecutor::ClearOpQueue() { diff --git a/parser/html/nsHtml5TreeOpExecutor.h b/parser/html/nsHtml5TreeOpExecutor.h index 65a1f619e9111..0ed81edb9cce6 100644 --- a/parser/html/nsHtml5TreeOpExecutor.h +++ b/parser/html/nsHtml5TreeOpExecutor.h @@ -184,7 +184,7 @@ class nsHtml5TreeOpExecutor final void CommitToInternalEncoding(); - [[nodiscard]] bool TakeOpsFromStage(); + void TakeOpsFromStage(); void MaybeSuspend(); @@ -220,8 +220,7 @@ class nsHtml5TreeOpExecutor final * Flush the operations from the tree operations from the argument * queue unconditionally. (This is for the main thread case.) */ - [[nodiscard]] virtual bool MoveOpsFrom( - nsTArray<nsHtml5TreeOperation>& aOpQueue) override; + virtual void MoveOpsFrom(nsTArray<nsHtml5TreeOperation>& aOpQueue) override; void ClearOpQueue(); diff --git a/parser/html/nsHtml5TreeOpStage.cpp b/parser/html/nsHtml5TreeOpStage.cpp index 6f7cb5f60888a..54a647d2b95ad 100644 --- a/parser/html/nsHtml5TreeOpStage.cpp +++ b/parser/html/nsHtml5TreeOpStage.cpp @@ -10,26 +10,22 @@ nsHtml5TreeOpStage::nsHtml5TreeOpStage() : mMutex("nsHtml5TreeOpStage mutex") {} nsHtml5TreeOpStage::~nsHtml5TreeOpStage() {} -bool nsHtml5TreeOpStage::MoveOpsFrom(nsTArray<nsHtml5TreeOperation>& aOpQueue) { +void nsHtml5TreeOpStage::MoveOpsFrom(nsTArray<nsHtml5TreeOperation>& aOpQueue) { mozilla::MutexAutoLock autoLock(mMutex); - return !!mOpQueue.AppendElements(std::move(aOpQueue), mozilla::fallible_t()); + mOpQueue.AppendElements(std::move(aOpQueue)); } -[[nodiscard]] bool nsHtml5TreeOpStage::MoveOpsAndSpeculativeLoadsTo( +void nsHtml5TreeOpStage::MoveOpsAndSpeculativeLoadsTo( nsTArray<nsHtml5TreeOperation>& aOpQueue, nsTArray<nsHtml5SpeculativeLoad>& aSpeculativeLoadQueue) { mozilla::MutexAutoLock autoLock(mMutex); - if (!aOpQueue.AppendElements(std::move(mOpQueue), mozilla::fallible_t())) { - return false; - }; + aOpQueue.AppendElements(std::move(mOpQueue)); aSpeculativeLoadQueue.AppendElements(std::move(mSpeculativeLoadQueue)); - return true; } -[[nodiscard]] bool nsHtml5TreeOpStage::MoveOpsTo( - nsTArray<nsHtml5TreeOperation>& aOpQueue) { +void nsHtml5TreeOpStage::MoveOpsTo(nsTArray<nsHtml5TreeOperation>& aOpQueue) { mozilla::MutexAutoLock autoLock(mMutex); - return !!aOpQueue.AppendElements(std::move(mOpQueue), mozilla::fallible_t()); + aOpQueue.AppendElements(std::move(mOpQueue)); } void nsHtml5TreeOpStage::MoveSpeculativeLoadsFrom( diff --git a/parser/html/nsHtml5TreeOpStage.h b/parser/html/nsHtml5TreeOpStage.h index 967ccfd008d80..37cd3ed83b812 100644 --- a/parser/html/nsHtml5TreeOpStage.h +++ b/parser/html/nsHtml5TreeOpStage.h @@ -21,18 +21,17 @@ class nsHtml5TreeOpStage : public nsAHtml5TreeOpSink { * Flush the operations from the tree operations from the argument * queue unconditionally. */ - [[nodiscard]] virtual bool MoveOpsFrom( - nsTArray<nsHtml5TreeOperation>& aOpQueue) override; + virtual void MoveOpsFrom(nsTArray<nsHtml5TreeOperation>& aOpQueue) override; /** * Retrieve the staged operations into the argument. */ - [[nodiscard]] bool MoveOpsTo(nsTArray<nsHtml5TreeOperation>& aOpQueue); + void MoveOpsTo(nsTArray<nsHtml5TreeOperation>& aOpQueue); /** * Retrieve the staged operations and speculative loads into the arguments. */ - [[nodiscard]] bool MoveOpsAndSpeculativeLoadsTo( + void MoveOpsAndSpeculativeLoadsTo( nsTArray<nsHtml5TreeOperation>& aOpQueue, nsTArray<nsHtml5SpeculativeLoad>& aSpeculativeLoadQueue); -- GitLab