Bug 1851992 - moving html5 stream parser's OnStopRequest off main thr…

…ead. r=necko-reviewers,edgul,jesup,valentin,hsivonen Depends on D187668 Differential Revision: https://phabricator.services.mozilla.com/D187689
dothq · Oct 27, 2023 · 827cd75 · 827cd75
1 parent 6b70b22
commit 827cd75
Show file tree

Hide file tree

Showing 4 changed files with 66 additions and 12 deletions.
diff --git 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
@@ -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
@@ -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<nsIRunnable> 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<nsIRunnable> 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
@@ -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