From b324bd24fed1d2e2306ceab7251bacfd9b40506a Mon Sep 17 00:00:00 2001 From: edgul Date: Tue, 17 Oct 2023 08:27:48 +0000 Subject: [PATCH] Bug 1851992 - moving html5 stream parser's OnStopRequest off main thread. r=necko-reviewers,edgul,jesup,valentin,hsivonen Depends on D187668 Differential Revision: https://phabricator.services.mozilla.com/D187689 --- parser/html/nsHtml5StreamListener.cpp | 15 +++++++++- parser/html/nsHtml5StreamListener.h | 3 +- parser/html/nsHtml5StreamParser.cpp | 41 ++++++++++++++++++++++----- parser/html/nsHtml5StreamParser.h | 19 +++++++++++-- 4 files changed, 66 insertions(+), 12 deletions(-) diff --git a/parser/html/nsHtml5StreamListener.cpp b/parser/html/nsHtml5StreamListener.cpp index 86e18c0296b6..09812978ef1b 100644 --- a/parser/html/nsHtml5StreamListener.cpp +++ b/parser/html/nsHtml5StreamListener.cpp @@ -75,7 +75,8 @@ nsHtml5StreamListener::OnStopRequest(nsIRequest* aRequest, nsresult aStatus) { if (MOZ_UNLIKELY(!mDelegate)) { return NS_ERROR_NOT_AVAILABLE; } - return ((nsHtml5StreamParser*)mDelegate)->OnStopRequest(aRequest, aStatus); + return ((nsHtml5StreamParser*)mDelegate) + ->OnStopRequest(aRequest, aStatus, autoEnter); } NS_IMETHODIMP @@ -90,3 +91,15 @@ nsHtml5StreamListener::OnDataAvailable(nsIRequest* aRequest, return ((nsHtml5StreamParser*)mDelegate) ->OnDataAvailable(aRequest, aInStream, aSourceOffset, aLength); } + +NS_IMETHODIMP +nsHtml5StreamListener::OnDataFinished(nsresult aStatus) { + mozilla::ReentrantMonitorAutoEnter autoEnter(mDelegateMonitor); + + if (MOZ_UNLIKELY(!mDelegate)) { + return NS_ERROR_NOT_AVAILABLE; + } + + return ((nsHtml5StreamParser*)mDelegate) + ->OnStopRequest(nullptr, aStatus, autoEnter); +} diff --git a/parser/html/nsHtml5StreamListener.h b/parser/html/nsHtml5StreamListener.h index 46056eacee23..59425ffec738 100644 --- a/parser/html/nsHtml5StreamListener.h +++ b/parser/html/nsHtml5StreamListener.h @@ -28,8 +28,7 @@ * threads, so there is no need to have a mutex around nsHtml5StreamParserPtr to * prevent it from double-releasing nsHtml5StreamParser. */ -class nsHtml5StreamListener : public nsIStreamListener, - public nsIThreadRetargetableStreamListener { +class nsHtml5StreamListener : public nsIThreadRetargetableStreamListener { public: explicit nsHtml5StreamListener(nsHtml5StreamParser* aDelegate); diff --git a/parser/html/nsHtml5StreamParser.cpp b/parser/html/nsHtml5StreamParser.cpp index 18a995aa5a83..d678683d3f69 100644 --- a/parser/html/nsHtml5StreamParser.cpp +++ b/parser/html/nsHtml5StreamParser.cpp @@ -28,6 +28,8 @@ #include "mozilla/Services.h" #include "mozilla/StaticPrefs_html5.h" #include "mozilla/StaticPrefs_intl.h" +#include "mozilla/StaticPrefs_network.h" +#include "mozilla/TaskCategory.h" #include "mozilla/TextUtils.h" #include "mozilla/UniquePtrExtensions.h" @@ -1389,14 +1391,39 @@ class nsHtml5RequestStopper : public Runnable { } }; -nsresult nsHtml5StreamParser::OnStopRequest(nsIRequest* aRequest, - nsresult status) { - MOZ_ASSERT(mRequest == aRequest, "Got Stop on wrong stream."); - MOZ_ASSERT(NS_IsMainThread(), "Wrong thread!"); - nsCOMPtr stopper = new nsHtml5RequestStopper(this); - if (NS_FAILED(mEventTarget->Dispatch(stopper, nsIThread::DISPATCH_NORMAL))) { - NS_WARNING("Dispatching StopRequest event failed."); +nsresult nsHtml5StreamParser::OnStopRequest( + nsIRequest* aRequest, nsresult status, + const mozilla::ReentrantMonitorAutoEnter& aProofOfLock) { + MOZ_ASSERT_IF(aRequest, mRequest == aRequest); + if (mOnStopCalled) { + return NS_OK; } + + mOnStopCalled = true; + + if (MOZ_UNLIKELY(NS_IsMainThread())) { + nsCOMPtr stopper = new nsHtml5RequestStopper(this); + if (NS_FAILED( + mEventTarget->Dispatch(stopper, nsIThread::DISPATCH_NORMAL))) { + NS_WARNING("Dispatching StopRequest event failed."); + } + return NS_OK; + } + + if (!StaticPrefs::network_send_OnDataFinished_html5parser()) { + // Let the MainThread event handle this, even though it will just + // send it back to this thread, so we can accurately judge the impact + // of this change. This should eventually be removed once the PoC is + // complete + mOnStopCalled = false; + return NS_OK; + } + + MOZ_ASSERT(IsParserThread(), "Wrong thread!"); + + mozilla::MutexAutoLock autoLock(mTokenizerMutex); + DoStopRequest(); + PostLoadFlusher(); return NS_OK; } diff --git a/parser/html/nsHtml5StreamParser.h b/parser/html/nsHtml5StreamParser.h index 40a33fab4033..8da45cde3945 100644 --- a/parser/html/nsHtml5StreamParser.h +++ b/parser/html/nsHtml5StreamParser.h @@ -11,9 +11,11 @@ #include "MainThreadUtils.h" #include "mozilla/AlreadyAddRefed.h" #include "mozilla/Assertions.h" +#include "mozilla/Atomics.h" #include "mozilla/Encoding.h" #include "mozilla/Mutex.h" #include "mozilla/NotNull.h" +#include "mozilla/ReentrantMonitor.h" #include "mozilla/RefPtr.h" #include "mozilla/Span.h" #include "mozilla/UniquePtr.h" @@ -205,8 +207,14 @@ class nsHtml5StreamParser final : public nsISupports { nsresult OnDataAvailable(nsIRequest* aRequest, nsIInputStream* aInStream, uint64_t aSourceOffset, uint32_t aLength); - - nsresult OnStopRequest(nsIRequest* aRequest, nsresult status); + /** + * ReentrantMonitorAutoEnter is used for protecting access to + * nsHtml5StreamParser::mOnStopCalled and should be obtained from + * nsHtml5StreamListener::mDelegateMonitor + */ + nsresult OnStopRequest( + nsIRequest* aRequest, nsresult status, + const mozilla::ReentrantMonitorAutoEnter& aProofOfLock); // EncodingDeclarationHandler // https://hg.mozilla.org/projects/htmlparser/file/tip/src/nu/validator/htmlparser/common/EncodingDeclarationHandler.java @@ -756,6 +764,13 @@ class nsHtml5StreamParser final : public nsISupports { * If content is being sent to the devtools, an encoded UUID for the parser. */ nsString mUUIDForDevtools; + + /** + * prevent multiple calls to OnStopRequest + * This field can be called from multiple threads and is protected by + * nsHtml5StreamListener::mDelegateMonitor passed in the OnStopRequest + */ + bool mOnStopCalled{false}; }; #endif // nsHtml5StreamParser_h