Bug 960519 - Make it safe to refcount the HTML parser's nsIStreamListener off-the-main-thread. r=smaug.

2014-04-16 08:41:39 +03:00 · 2014-04-16 08:41:39 +03:00 · 8751f19db8
--- a/parser/html/moz.build
+++ b/parser/html/moz.build
@ -35,8 +35,10 @@ EXPORTS += [
    'nsHtml5Parser.h',
    'nsHtml5PendingNotification.h',
    'nsHtml5PlainTextUtils.h',
+    'nsHtml5RefPtr.h',
    'nsHtml5Speculation.h',
    'nsHtml5SpeculativeLoad.h',
+    'nsHtml5StreamListener.h',
    'nsHtml5StreamParser.h',
    'nsHtml5StringParser.h',
    'nsHtml5SVGLoadDispatcher.h',
@ -75,6 +77,7 @@ UNIFIED_SOURCES += [
    'nsHtml5SpeculativeLoad.cpp',
    'nsHtml5StackNode.cpp',
    'nsHtml5StateSnapshot.cpp',
+    'nsHtml5StreamListener.cpp',
    'nsHtml5StreamParser.cpp',
    'nsHtml5StringParser.cpp',
    'nsHtml5SVGLoadDispatcher.cpp',
--- a/parser/html/nsHtml5Parser.cpp
+++ b/parser/html/nsHtml5Parser.cpp
@ -26,7 +26,7 @@ NS_IMPL_CYCLE_COLLECTION_CLASS(nsHtml5Parser)

 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(nsHtml5Parser)
  NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mExecutor)
-  NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mStreamParser)
+  NS_IMPL_CYCLE_COLLECTION_TRAVERSE_RAWPTR(GetStreamParser())
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END

 NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(nsHtml5Parser)
@ -96,11 +96,11 @@ nsHtml5Parser::SetDocumentCharset(const nsACString& aCharset,
 {
  NS_PRECONDITION(!mExecutor->HasStarted(),
                  "Document charset set too late.");
-  NS_PRECONDITION(mStreamParser, "Setting charset on a script-only parser.");
+  NS_PRECONDITION(GetStreamParser(), "Setting charset on a script-only parser.");
  nsAutoCString trimmed;
  trimmed.Assign(aCharset);
  trimmed.Trim(" \t\r\n\f");
-  mStreamParser->SetDocumentCharset(trimmed, aCharsetSource);
+  GetStreamParser()->SetDocumentCharset(trimmed, aCharsetSource);
  mExecutor->SetDocumentCharsetAndSource(trimmed,
                                         aCharsetSource);
 }
@ -108,8 +108,8 @@ nsHtml5Parser::SetDocumentCharset(const nsACString& aCharset,
 NS_IMETHODIMP
 nsHtml5Parser::GetChannel(nsIChannel** aChannel)
 {
-  if (mStreamParser) {
-    return mStreamParser->GetChannel(aChannel);
+  if (GetStreamParser()) {
+    return GetStreamParser()->GetChannel(aChannel);
  } else {
    return NS_ERROR_NOT_AVAILABLE;
  }
@ -125,7 +125,7 @@ nsHtml5Parser::GetDTD(nsIDTD** aDTD)
 nsIStreamListener*
 nsHtml5Parser::GetStreamListener()
 {
-  return mStreamParser;
+  return mStreamListener;
 }

 NS_IMETHODIMP
@ -178,11 +178,11 @@ nsHtml5Parser::Parse(nsIURI* aURL,
   */
  NS_PRECONDITION(!mExecutor->HasStarted(), 
                  "Tried to start parse without initializing the parser.");
-  NS_PRECONDITION(mStreamParser, 
+  NS_PRECONDITION(GetStreamParser(),
                  "Can't call this Parse() variant on script-created parser");
-  mStreamParser->SetObserver(aObserver);
-  mStreamParser->SetViewSourceTitle(aURL); // In case we're viewing source
-  mExecutor->SetStreamParser(mStreamParser);
+  GetStreamParser()->SetObserver(aObserver);
+  GetStreamParser()->SetViewSourceTitle(aURL); // In case we're viewing source
+  mExecutor->SetStreamParser(GetStreamParser());
  mExecutor->SetParser(this);
  return NS_OK;
 }
@ -208,11 +208,11 @@ nsHtml5Parser::Parse(const nsAString& aSourceBuffer,
  
  // Gripping the other objects just in case, since the other old grip
  // required grips to these, too.
-  nsRefPtr<nsHtml5StreamParser> streamKungFuDeathGrip(mStreamParser);
+  nsRefPtr<nsHtml5StreamParser> streamKungFuDeathGrip(GetStreamParser());
  nsRefPtr<nsHtml5TreeOpExecutor> treeOpKungFuDeathGrip(mExecutor);

  if (!mExecutor->HasStarted()) {
-    NS_ASSERTION(!mStreamParser,
+    NS_ASSERTION(!GetStreamParser(),
                 "Had stream parser but document.write started life cycle.");
    // This is the first document.write() on a document.open()ed document
    mExecutor->SetParser(this);
@ -247,7 +247,7 @@ nsHtml5Parser::Parse(const nsAString& aSourceBuffer,

  if (aLastCall && aSourceBuffer.IsEmpty() && !aKey) {
    // document.close()
-    NS_ASSERTION(!mStreamParser,
+    NS_ASSERTION(!GetStreamParser(),
                 "Had stream parser but got document.close().");
    if (mDocumentClosed) {
      // already closed
@ -266,7 +266,7 @@ nsHtml5Parser::Parse(const nsAString& aSourceBuffer,
  NS_ASSERTION(IsInsertionPointDefined(),
               "Doc.write reached parser with undefined insertion point.");

-  NS_ASSERTION(!(mStreamParser && !aKey),
+  NS_ASSERTION(!(GetStreamParser() && !aKey),
               "Got a null key in a non-script-created parser");

  // XXX is this optimization bogus?
@ -359,7 +359,7 @@ nsHtml5Parser::Parse(const nsAString& aSourceBuffer,
    mLastWasCR = false;
    if (stackBuffer.hasMore()) {
      int32_t lineNumberSave;
-      bool inRootContext = (!mStreamParser && !aKey);
+      bool inRootContext = (!GetStreamParser() && !aKey);
      if (inRootContext) {
        mTokenizer->setLineNumber(mRootContextLineNumber);
      } else {
@ -496,10 +496,10 @@ nsHtml5Parser::Terminate()
  // XXX - [ until we figure out a way to break parser-sink circularity ]
  // Hack - Hold a reference until we are completely done...
  nsCOMPtr<nsIParser> kungFuDeathGrip(this);
-  nsRefPtr<nsHtml5StreamParser> streamKungFuDeathGrip(mStreamParser);
+  nsRefPtr<nsHtml5StreamParser> streamKungFuDeathGrip(GetStreamParser());
  nsRefPtr<nsHtml5TreeOpExecutor> treeOpKungFuDeathGrip(mExecutor);
-  if (mStreamParser) {
-    mStreamParser->Terminate();
+  if (GetStreamParser()) {
+    GetStreamParser()->Terminate();
  }
  return mExecutor->DidBuildModel(true);
 }
@ -543,7 +543,7 @@ bool
 nsHtml5Parser::IsInsertionPointDefined()
 {
  return !mExecutor->IsFlushing() &&
-    (!mStreamParser || mParserInsertedScriptsBeingEvaluated);
+    (!GetStreamParser() || mParserInsertedScriptsBeingEvaluated);
 }

 void
@ -561,7 +561,7 @@ nsHtml5Parser::EndEvaluatingParserInsertedScript()
 void
 nsHtml5Parser::MarkAsNotScriptCreated(const char* aCommand)
 {
-  NS_PRECONDITION(!mStreamParser, "Must not call this twice.");
+  NS_PRECONDITION(!mStreamListener, "Must not call this twice.");
  eParserMode mode = NORMAL;
  if (!nsCRT::strcmp(aCommand, "view-source")) {
    mode = VIEW_SOURCE_HTML;
@ -581,13 +581,14 @@ nsHtml5Parser::MarkAsNotScriptCreated(const char* aCommand)
                 "Unsupported parser command!");
  }
 #endif
-  mStreamParser = new nsHtml5StreamParser(mExecutor, this, mode);
+  mStreamListener =
+    new nsHtml5StreamListener(new nsHtml5StreamParser(mExecutor, this, mode));
 }

 bool
 nsHtml5Parser::IsScriptCreated()
 {
-  return !mStreamParser;
+  return !GetStreamParser();
 }

 /* End nsIParser  */
@ -613,7 +614,7 @@ nsHtml5Parser::ParseUntilBlocked()
          return;
        }
        if (mDocumentClosed) {
-          NS_ASSERTION(!mStreamParser,
+          NS_ASSERTION(!GetStreamParser(),
                       "This should only happen with script-created parser.");
          mTokenizer->eof();
          mTreeBuilder->StreamEnded();
@ -625,12 +626,12 @@ nsHtml5Parser::ParseUntilBlocked()
        // never release the last buffer.
        NS_ASSERTION(!mLastBuffer->getStart() && !mLastBuffer->getEnd(),
                     "Sentinel buffer had its indeces changed.");
-        if (mStreamParser) {
+        if (GetStreamParser()) {
          if (mReturnToStreamParserPermitted &&
              !mExecutor->IsScriptExecuting()) {
            mTreeBuilder->Flush();
            mReturnToStreamParserPermitted = false;
-            mStreamParser->ContinueAfterScripts(mTokenizer,
+            GetStreamParser()->ContinueAfterScripts(mTokenizer,
                                                mTreeBuilder,
                                                mLastWasCR);
          }
@ -656,7 +657,7 @@ nsHtml5Parser::ParseUntilBlocked()
    mFirstBuffer->adjust(mLastWasCR);
    mLastWasCR = false;
    if (mFirstBuffer->hasMore()) {
-      bool inRootContext = (!mStreamParser && !mFirstBuffer->key);
+      bool inRootContext = (!GetStreamParser() && !mFirstBuffer->key);
      if (inRootContext) {
        mTokenizer->setLineNumber(mRootContextLineNumber);
      }
@ -715,8 +716,8 @@ nsHtml5Parser::InitializeDocWriteParserState(nsAHtml5TreeBuilderState* aState,
 void
 nsHtml5Parser::ContinueAfterFailedCharsetSwitch()
 {
-  NS_PRECONDITION(mStreamParser, 
+  NS_PRECONDITION(GetStreamParser(),
    "Tried to continue after failed charset switch without a stream parser");
-  mStreamParser->ContinueAfterFailedCharsetSwitch();
+  GetStreamParser()->ContinueAfterFailedCharsetSwitch();
 }

--- a/parser/html/nsHtml5Parser.h
+++ b/parser/html/nsHtml5Parser.h
@ -25,6 +25,7 @@
 #include "nsHtml5StreamParser.h"
 #include "nsHtml5AtomTable.h"
 #include "nsWeakReference.h"
+#include "nsHtml5StreamListener.h"

 class nsHtml5Parser : public nsIParser,
                      public nsSupportsWeakReference
@ -239,9 +240,10 @@ class nsHtml5Parser : public nsIParser,

    void DropStreamParser()
    {
-      if (mStreamParser) {
-        mStreamParser->DropTimer();
-        mStreamParser = nullptr;
+      if (GetStreamParser()) {
+        GetStreamParser()->DropTimer();
+        mStreamListener->DropDelegate();
+        mStreamListener = nullptr;
      }
    }
    
@ -251,7 +253,10 @@ class nsHtml5Parser : public nsIParser,

    nsHtml5StreamParser* GetStreamParser()
    {
-      return mStreamParser;
+      if (!mStreamListener) {
+        return nullptr;
+      }
+      return mStreamListener->GetDelegate();
    }

    /**
@ -334,9 +339,9 @@ class nsHtml5Parser : public nsIParser,
    nsAutoPtr<nsHtml5Tokenizer>   mDocWriteSpeculativeTokenizer;

    /**
-     * The stream parser.
+     * The stream listener holding the stream parser.
     */
-    nsRefPtr<nsHtml5StreamParser>       mStreamParser;
+    nsRefPtr<nsHtml5StreamListener>     mStreamListener;

    /**
     *
--- a/parser/html/nsHtml5RefPtr.h
+++ b/parser/html/nsHtml5RefPtr.h
@ -6,7 +6,7 @@
 #ifndef nsHtml5RefPtr_h
 #define nsHtml5RefPtr_h

-#include "nsIThread.h"
+#include "nsThreadUtils.h"

 template <class T>
 class nsHtml5RefPtrReleaser : public nsRunnable
--- a/parser/html/nsHtml5StreamListener.cpp
+++ b/parser/html/nsHtml5StreamListener.cpp
@ -0,0 +1,82 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * 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 "nsHtml5StreamListener.h"
+
+NS_IMPL_ADDREF(nsHtml5StreamListener)
+NS_IMPL_RELEASE(nsHtml5StreamListener)
+
+NS_INTERFACE_MAP_BEGIN(nsHtml5StreamListener)
+  NS_INTERFACE_MAP_ENTRY_AMBIGUOUS(nsISupports, nsIRequestObserver)
+  NS_INTERFACE_MAP_ENTRY(nsIRequestObserver)
+  NS_INTERFACE_MAP_ENTRY(nsIStreamListener)
+  NS_INTERFACE_MAP_ENTRY(nsIThreadRetargetableStreamListener)
+NS_INTERFACE_MAP_END_THREADSAFE
+
+nsHtml5StreamListener::nsHtml5StreamListener(nsHtml5StreamParser* aDelegate)
+ : mDelegate(aDelegate)
+{
+}
+
+nsHtml5StreamListener::~nsHtml5StreamListener()
+{
+}
+
+void
+nsHtml5StreamListener::DropDelegate()
+{
+  MOZ_ASSERT(NS_IsMainThread(),
+             "Must not call DropDelegate from non-main threads.");
+  mDelegate = nullptr;
+}
+
+NS_IMETHODIMP
+nsHtml5StreamListener::CheckListenerChain()
+{
+  if (MOZ_UNLIKELY(!mDelegate)) {
+    return NS_ERROR_NOT_AVAILABLE;
+  }
+  return mDelegate->CheckListenerChain();
+}
+
+NS_IMETHODIMP
+nsHtml5StreamListener::OnStartRequest(nsIRequest* aRequest,
+                                      nsISupports* aContext)
+{
+  if (MOZ_UNLIKELY(!mDelegate)) {
+    return NS_ERROR_NOT_AVAILABLE;
+  }
+  return mDelegate->OnStartRequest(aRequest, aContext);
+}
+
+NS_IMETHODIMP
+nsHtml5StreamListener::OnStopRequest(nsIRequest* aRequest,
+                                     nsISupports* aContext,
+                                     nsresult aStatus)
+{
+  if (MOZ_UNLIKELY(!mDelegate)) {
+    return NS_ERROR_NOT_AVAILABLE;
+  }
+  return mDelegate->OnStopRequest(aRequest,
+                                  aContext,
+                                  aStatus);
+}
+
+NS_IMETHODIMP
+nsHtml5StreamListener::OnDataAvailable(nsIRequest* aRequest,
+                                       nsISupports* aContext,
+                                       nsIInputStream* aInStream,
+                                       uint64_t aSourceOffset,
+                                       uint32_t aLength)
+{
+  if (MOZ_UNLIKELY(!mDelegate)) {
+    return NS_ERROR_NOT_AVAILABLE;
+  }
+  return mDelegate->OnDataAvailable(aRequest,
+                                    aContext,
+                                    aInStream,
+                                    aSourceOffset,
+                                    aLength);
+}
+
--- a/parser/html/nsHtml5StreamListener.h
+++ b/parser/html/nsHtml5StreamListener.h
@ -0,0 +1,54 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * 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/. */
+
+#ifndef nsHtml5StreamListener_h
+#define nsHtml5StreamListener_h
+
+#include "nsIStreamListener.h"
+#include "nsIThreadRetargetableStreamListener.h"
+#include "nsHtml5RefPtr.h"
+#include "nsHtml5StreamParser.h"
+
+/**
+ * The purpose of this class is to reconcile the problem that
+ * nsHtml5StreamParser is a cycle collection participant, which means that it
+ * can only be refcounted on the main thread, but
+ * nsIThreadRetargetableStreamListener can be refcounted from another thread,
+ * so nsHtml5StreamParser being an nsIThreadRetargetableStreamListener was
+ * a memory corruption problem.
+ *
+ * mDelegate is an nsHtml5RefPtr, which releases the object that it points
+ * to from a runnable on the main thread. DropDelegate() is only called on
+ * the main thread. This call will finish before the main-thread derefs the
+ * nsHtml5StreamListener itself, so there is no risk of another thread making
+ * the refcount of nsHtml5StreamListener go to zero and running the destructor
+ * concurrently. Other than that, the thread-safe nsISupports implementation
+ * takes care of the destructor not running concurrently from different
+ * threads, so there is no need to have a mutex around nsHtml5RefPtr to
+ * prevent it from double-releasing nsHtml5StreamParser.
+ */
+class nsHtml5StreamListener : public nsIStreamListener,
+                              public nsIThreadRetargetableStreamListener
+{
+public:
+  nsHtml5StreamListener(nsHtml5StreamParser* aDelegate);
+  virtual ~nsHtml5StreamListener();
+
+  NS_DECL_THREADSAFE_ISUPPORTS
+  NS_DECL_NSIREQUESTOBSERVER
+  NS_DECL_NSISTREAMLISTENER
+  NS_DECL_NSITHREADRETARGETABLESTREAMLISTENER
+
+  inline nsHtml5StreamParser* GetDelegate()
+  {
+    return mDelegate;
+  }
+
+  void DropDelegate();
+
+private:
+  nsHtml5RefPtr<nsHtml5StreamParser> mDelegate;
+};
+
+#endif // nsHtml5StreamListener_h
--- a/parser/html/nsHtml5StreamParser.cpp
+++ b/parser/html/nsHtml5StreamParser.cpp
@ -73,10 +73,8 @@ NS_IMPL_CYCLE_COLLECTING_ADDREF(nsHtml5StreamParser)
 NS_IMPL_CYCLE_COLLECTING_RELEASE(nsHtml5StreamParser)

 NS_INTERFACE_TABLE_HEAD(nsHtml5StreamParser)
-  NS_INTERFACE_TABLE3(nsHtml5StreamParser,
-                      nsIStreamListener, 
-                      nsICharsetDetectionObserver,
-                      nsIThreadRetargetableStreamListener)
+  NS_INTERFACE_TABLE1(nsHtml5StreamParser,
+                      nsICharsetDetectionObserver)
  NS_INTERFACE_TABLE_TO_MAP_SEGUE_CYCLE_COLLECTION(nsHtml5StreamParser)
 NS_INTERFACE_MAP_END

@ -110,7 +108,7 @@ NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(nsHtml5StreamParser)
  // hack: count self if held by mChardet
  if (tmp->mChardet) {
    NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(cb, "mChardet->mObserver");
-    cb.NoteXPCOMChild(static_cast<nsIStreamListener*>(tmp));
+    cb.NoteXPCOMChild(static_cast<nsICharsetDetectionObserver*>(tmp));
  }
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END

@ -852,7 +850,6 @@ nsHtml5StreamParser::WriteStreamBytes(const uint8_t* aFromSegment,
  }
 }

-// nsIRequestObserver methods:
 nsresult
 nsHtml5StreamParser::OnStartRequest(nsIRequest* aRequest, nsISupports* aContext)
 {
@ -974,7 +971,7 @@ nsHtml5StreamParser::OnStartRequest(nsIRequest* aRequest, nsISupports* aContext)
  return NS_OK;
 }

-NS_IMETHODIMP
+nsresult
 nsHtml5StreamParser::CheckListenerChain()
 {
  NS_ASSERTION(NS_IsMainThread(), "Should be on the main thread!");
@ -1125,13 +1122,12 @@ class nsHtml5DataAvailable : public nsRunnable
    }
 };

-// nsIStreamListener method:
 nsresult
 nsHtml5StreamParser::OnDataAvailable(nsIRequest* aRequest,
-                               nsISupports* aContext,
-                               nsIInputStream* aInStream,
-                               uint64_t aSourceOffset,
-                               uint32_t aLength)
+                                     nsISupports* aContext,
+                                     nsIInputStream* aInStream,
+                                     uint64_t aSourceOffset,
+                                     uint32_t aLength)
 {
  nsresult rv;
  if (NS_FAILED(rv = mExecutor->IsBroken())) {
--- a/parser/html/nsHtml5StreamParser.h
+++ b/parser/html/nsHtml5StreamParser.h
@ -8,7 +8,6 @@

 #include "nsAutoPtr.h"
 #include "nsCOMPtr.h"
-#include "nsIStreamListener.h"
 #include "nsICharsetDetectionObserver.h"
 #include "nsHtml5MetaScanner.h"
 #include "nsIUnicodeDecoder.h"
@ -20,7 +19,6 @@
 #include "nsHtml5Speculation.h"
 #include "nsITimer.h"
 #include "nsICharsetDetector.h"
-#include "nsIThreadRetargetableStreamListener.h"

 class nsHtml5Parser;

@ -101,9 +99,7 @@ enum eHtml5StreamState {
  STREAM_ENDED = 2
 };

-class nsHtml5StreamParser : public nsIStreamListener,
-                            public nsIThreadRetargetableStreamListener,
-                            public nsICharsetDetectionObserver {
+class nsHtml5StreamParser : public nsICharsetDetectionObserver {

  friend class nsHtml5RequestStopper;
  friend class nsHtml5DataAvailable;
@ -113,7 +109,8 @@ class nsHtml5StreamParser : public nsIStreamListener,
  public:
    NS_DECL_AND_IMPL_ZEROING_OPERATOR_NEW
    NS_DECL_CYCLE_COLLECTING_ISUPPORTS
-    NS_DECL_CYCLE_COLLECTION_CLASS_AMBIGUOUS(nsHtml5StreamParser, nsIStreamListener)
+    NS_DECL_CYCLE_COLLECTION_CLASS_AMBIGUOUS(nsHtml5StreamParser,
+                                             nsICharsetDetectionObserver)

    static void InitializeStatics();

@ -123,13 +120,21 @@ class nsHtml5StreamParser : public nsIStreamListener,
                        
    virtual ~nsHtml5StreamParser();

-    // nsIRequestObserver methods:
-    NS_DECL_NSIREQUESTOBSERVER
-    // nsIStreamListener methods:
-    NS_DECL_NSISTREAMLISTENER
-    // nsIThreadRetargetableStreamListener methods:
-    NS_DECL_NSITHREADRETARGETABLESTREAMLISTENER
-    
+    // Methods that nsHtml5StreamListener calls
+    nsresult CheckListenerChain();
+
+    nsresult OnStartRequest(nsIRequest* aRequest, nsISupports* aContext);
+
+    nsresult OnDataAvailable(nsIRequest* aRequest,
+                             nsISupports* aContext,
+                             nsIInputStream* aInStream,
+                             uint64_t aSourceOffset,
+                             uint32_t aLength);
+
+    nsresult OnStopRequest(nsIRequest* aRequest,
+                           nsISupports* aContext,
+                           nsresult status);
+
    // nsICharsetDetectionObserver
    /**
     * Chardet calls this to report the detection result