715103 - Move parser unblocking management from nsContentSink to nsScriptLoader

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Henri Sivonen (:hsivonen)]

Currently, nsContentSink keeps track of script nodes that might be parser-blocking scripts. This makes no sense in the light of HTML5 script execution semantics, because there can be only one parser-blocking script. It also makes the responsibilities of various classes just bizarre and interferes with reasonable refactorings like bug 714777.

nsScriptLoader alone should manage the parser-blocking script and should be responsible for unblocking the parser as appropriate.

Comment 1

[Boris Zbarsky [:bzbarsky]]

nsContentSink also keeps track of parser-blocking sheets, no?

We used to not do that, but it turned out that unblocking the parser from "outside" could reenter callers in weird ways.  Maybe that's not an issue anymore.... Just something to watch out for.

Comment 2

[Henri Sivonen (:hsivonen)]

(In reply to Boris Zbarsky (:bz) from comment #1)
> nsContentSink also keeps track of parser-blocking sheets, no?

Yeah, that's a questionable design, too. (Those sheets aren't parser-blocking per se. They are script-blocking, so maybe nsScriptLoader should keep track of those, too.)

> We used to not do that, but it turned out that unblocking the parser from
> "outside" could reenter callers in weird ways.  Maybe that's not an issue
> anymore.... Just something to watch out for.

With HTML5-compliant script execution, nsScriptLoader now knows which script is the parser-blocking script and there can be only one at a time. Furthermore, document.write calls can't go through at wrong times to the way they used to. If we have some re-entrancy left that doesn't conform to HTML5-compliant script execution, we need to discover it instead of papering over it. But I have trouble coming up with harmful re-entrancy, since unblocking should only involve flipping a boolean and posting a runnable, so it should be impossible for it to re-enter anything in a bad way.

Comment 3

[Boris Zbarsky [:bzbarsky]]

> since unblocking should only involve flipping a boolean and posting a runnable

If that's all it does things are fine.  Is that true for the XML parser too?

Comment 4

[Henri Sivonen (:hsivonen)]

(In reply to Boris Zbarsky (:bz) from comment #3)
> > since unblocking should only involve flipping a boolean and posting a runnable
> 
> If that's all it does things are fine.  Is that true for the XML parser too?

Yes, the XML parser works like that, too.

Comment 5

[Henri Sivonen (:hsivonen)]

Attachment: Move unblocking to nsScriptLoader

The patch has no tests, because it's not supposed to change anything observable. It's all green on try.

This patch still retains the blocking state in the parser objects. Removal is a follow-up: bug 717206. I didn't remove the blocking state in this iteration, because the old parser is annoying to modify and the follow-up modification wasn't necessary to unobstruct the refactorings I want to make that this patch unobstructs.

Comment 6

[Henri Sivonen (:hsivonen)]

Regarding IRC question: nsScriptLoadRequest::mElement is non-null, because it never gets nulled out. For preloads, it may start as null, but it becomes non-null when a preload becomes a non-speculative request. A request can't go from a preload to being a parser-blocking request without becoming non-speculative first.

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 588615 [details] [diff] [review]
Move unblocking to nsScriptLoader

># HG changeset patch
># User Henri Sivonen <[e-mail]>
># Date 1326534026 -7200
># Node ID bbc317cc845e9d960b244b82b9bb0819776e0e22
># Parent  b3fbded068826c708da99d8752983c376bafc03f
>Bug 715103 - Move parser unblocking management from nsContentSink to nsScriptLoader. r=NOT_REVIEWED.
>
>diff --git a/content/base/public/nsIScriptElement.h b/content/base/public/nsIScriptElement.h
>--- a/content/base/public/nsIScriptElement.h
>+++ b/content/base/public/nsIScriptElement.h
>@@ -44,18 +44,18 @@
> #include "nsCOMPtr.h"
> #include "nsIScriptLoaderObserver.h"
> #include "nsWeakPtr.h"
> #include "nsIParser.h"
> #include "nsContentCreatorFunctions.h"
> #include "nsIDOMHTMLScriptElement.h"
> 
> #define NS_ISCRIPTELEMENT_IID \
>-{ 0x5bb3b905, 0x5988, 0x476f, \
>-  { 0x95, 0x4f, 0x99, 0x02, 0x59, 0x82, 0x24, 0x67 } }
>+{ 0x24ab3ff2, 0xd75e, 0x4be4, \
>+  { 0x8d, 0x50, 0xd6, 0x75, 0x31, 0x29, 0xab, 0x65 } }
> 
> /**
>  * Internal interface implemented by script elements
>  */
> class nsIScriptElement : public nsIScriptLoaderObserver {
> public:
>   NS_DECLARE_STATIC_IID_ACCESSOR(NS_ISCRIPTELEMENT_IID)
> 
>@@ -182,16 +182,38 @@ public:
>   }
> 
>   void SetCreatorParser(nsIParser* aParser)
>   {
>     mCreatorParser = getter_AddRefs(NS_GetWeakReference(aParser));
>   }
> 
>   /**
>+   * Unblocks the creator parser
>+   */
>+  void UnblockParser()
>+  {
>+    nsCOMPtr<nsIParser> parser = do_QueryReferent(mCreatorParser);
>+    if (parser) {
>+      parser->UnblockParser();
>+    }
>+  }
>+
>+  /**
>+   * Attempts to resume parsing asynchronously
>+   */
>+  void ContinueParserAsync()
>+  {
>+    nsCOMPtr<nsIParser> parser = do_QueryReferent(mCreatorParser);
>+    if (parser) {
>+      parser->ContinueInterruptedParsingAsync();
>+    }
>+  }
>+
>+  /**
>    * Informs the creator parser that the evaluation of this script is starting
>    */
>   void BeginEvaluating()
>   {
>     nsCOMPtr<nsIParser> parser = do_QueryReferent(mCreatorParser);
>     if (parser) {
>       parser->BeginEvaluatingParserInsertedScript();
>     }
>diff --git a/content/base/src/nsContentSink.cpp b/content/base/src/nsContentSink.cpp
>--- a/content/base/src/nsContentSink.cpp
>+++ b/content/base/src/nsContentSink.cpp
>@@ -104,98 +104,44 @@
> #include "nsISupportsPrimitives.h"
> #include "mozilla/Preferences.h"
> #include "nsParserConstants.h"
> 
> using namespace mozilla;
> 
> PRLogModuleInfo* gContentSinkLogModuleInfo;
> 
>-class nsScriptLoaderObserverProxy : public nsIScriptLoaderObserver
>-{
>-public:
>-  nsScriptLoaderObserverProxy(nsIScriptLoaderObserver* aInner)
>-    : mInner(do_GetWeakReference(aInner))
>-  {
>-  }
>-  virtual ~nsScriptLoaderObserverProxy()
>-  {
>-  }
>-  
>-  NS_DECL_ISUPPORTS
>-  NS_DECL_NSISCRIPTLOADEROBSERVER
>-
>-  nsWeakPtr mInner;
>-};
>-
>-NS_IMPL_ISUPPORTS1(nsScriptLoaderObserverProxy, nsIScriptLoaderObserver)
>-
>-NS_IMETHODIMP
>-nsScriptLoaderObserverProxy::ScriptAvailable(nsresult aResult,
>-                                             nsIScriptElement *aElement,
>-                                             bool aIsInline,
>-                                             nsIURI *aURI,
>-                                             PRInt32 aLineNo)
>-{
>-  nsCOMPtr<nsIScriptLoaderObserver> inner = do_QueryReferent(mInner);
>-
>-  if (inner) {
>-    return inner->ScriptAvailable(aResult, aElement, aIsInline, aURI,
>-                                  aLineNo);
>-  }
>-
>-  return NS_OK;
>-}
>-
>-NS_IMETHODIMP
>-nsScriptLoaderObserverProxy::ScriptEvaluated(nsresult aResult,
>-                                             nsIScriptElement *aElement,
>-                                             bool aIsInline)
>-{
>-  nsCOMPtr<nsIScriptLoaderObserver> inner = do_QueryReferent(mInner);
>-
>-  if (inner) {
>-    return inner->ScriptEvaluated(aResult, aElement, aIsInline);
>-  }
>-
>-  return NS_OK;
>-}
>-
>-
> NS_IMPL_CYCLE_COLLECTING_ADDREF(nsContentSink)
> NS_IMPL_CYCLE_COLLECTING_RELEASE(nsContentSink)
> 
> NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(nsContentSink)
>   NS_INTERFACE_MAP_ENTRY(nsICSSLoaderObserver)
>   NS_INTERFACE_MAP_ENTRY(nsISupportsWeakReference)
>-  NS_INTERFACE_MAP_ENTRY(nsIScriptLoaderObserver)
>   NS_INTERFACE_MAP_ENTRY(nsIDocumentObserver)
>   NS_INTERFACE_MAP_ENTRY(nsIMutationObserver)
>   NS_INTERFACE_MAP_ENTRY(nsITimerCallback)
>-  NS_INTERFACE_MAP_ENTRY_AMBIGUOUS(nsISupports, nsIScriptLoaderObserver)
>+  NS_INTERFACE_MAP_ENTRY_AMBIGUOUS(nsISupports, nsIDocumentObserver)
> NS_INTERFACE_MAP_END
> 
> NS_IMPL_CYCLE_COLLECTION_CLASS(nsContentSink)
> NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(nsContentSink)
>   if (tmp->mDocument) {
>     tmp->mDocument->RemoveObserver(tmp);
>   }
>   NS_IMPL_CYCLE_COLLECTION_UNLINK_NSCOMPTR(mDocument)
>   NS_IMPL_CYCLE_COLLECTION_UNLINK_NSCOMPTR(mParser)
>   NS_IMPL_CYCLE_COLLECTION_UNLINK_NSCOMPTR(mNodeInfoManager)
>   NS_IMPL_CYCLE_COLLECTION_UNLINK_NSCOMPTR(mScriptLoader)
>-  NS_IMPL_CYCLE_COLLECTION_UNLINK_NSCOMARRAY(mScriptElements)
> NS_IMPL_CYCLE_COLLECTION_UNLINK_END
> NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(nsContentSink)
>   NS_IMPL_CYCLE_COLLECTION_TRAVERSE_NSCOMPTR(mDocument)
>   NS_IMPL_CYCLE_COLLECTION_TRAVERSE_NSCOMPTR(mParser)
>   NS_IMPL_CYCLE_COLLECTION_TRAVERSE_NATIVE_MEMBER(mNodeInfoManager,
>                                                   nsNodeInfoManager)
>   NS_IMPL_CYCLE_COLLECTION_TRAVERSE_NSCOMPTR(mScriptLoader)
>-  NS_IMPL_CYCLE_COLLECTION_TRAVERSE_NSCOMARRAY(mScriptElements)
> NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
> 
> 
> nsContentSink::nsContentSink()
> {
>   // We have a zeroing operator new
>   NS_ASSERTION(!mLayoutStarted, "What?");
>   NS_ASSERTION(!mDynamicLowerValue, "What?");
>@@ -298,23 +244,16 @@ nsContentSink::Init(nsIDocument* aDoc,
>   if (!mFragmentMode) {
>     if (mDocShell) {
>       PRUint32 loadType = 0;
>       mDocShell->GetLoadType(&loadType);
>       mDocument->SetChangeScrollPosWhenScrollingToRef(
>         (loadType & nsIDocShell::LOAD_CMD_HISTORY) == 0);
>     }
> 
>-    // use this to avoid a circular reference sink->document->scriptloader->sink
>-    nsCOMPtr<nsIScriptLoaderObserver> proxy =
>-      new nsScriptLoaderObserverProxy(this);
>-    NS_ENSURE_TRUE(proxy, NS_ERROR_OUT_OF_MEMORY);
>-
>-    mScriptLoader->AddObserver(proxy);
>-
>     ProcessHTTPHeaders(aChannel);
>   }
> 
>   mCSSLoader = aDoc->CSSLoader();
> 
>   mNodeInfoManager = aDoc->NodeInfoManager();
> 
>   mBackoffCount = sBackoffCount;
>@@ -361,100 +300,16 @@ nsContentSink::StyleSheetLoaded(nsCSSSty
>     }
>     
>     mScriptLoader->RemoveExecuteBlocker();
>   }
> 
>   return NS_OK;
> }
> 
>-NS_IMETHODIMP
>-nsContentSink::ScriptAvailable(nsresult aResult,
>-                               nsIScriptElement *aElement,
>-                               bool aIsInline,
>-                               nsIURI *aURI,
>-                               PRInt32 aLineNo)
>-{
>-  PRUint32 count = mScriptElements.Count();
>-
>-  // aElement will not be in mScriptElements if a <script> was added
>-  // using the DOM during loading or if DoneAddingChildren did not return
>-  // NS_ERROR_HTMLPARSER_BLOCK.
>-  NS_ASSERTION(count == 0 ||
>-               mScriptElements.IndexOf(aElement) == PRInt32(count - 1) ||
>-               mScriptElements.IndexOf(aElement) == -1,
>-               "script found at unexpected position");
>-
>-  // Check if this is the element we were waiting for
>-  if (count == 0 || aElement != mScriptElements[count - 1]) {
>-    return NS_OK;
>-  }
>-
>-  NS_ASSERTION(!aElement->GetScriptDeferred(), "defer script was in mScriptElements");
>-  NS_ASSERTION(!aElement->GetScriptAsync(), "async script was in mScriptElements");
>-
>-  if (mParser && !mParser->IsParserEnabled()) {
>-    // make sure to unblock the parser before evaluating the script,
>-    // we must unblock the parser even if loading the script failed or
>-    // if the script was empty, if we don't, the parser will never be
>-    // unblocked.
>-    mParser->UnblockParser();
>-  }
>-
>-  if (NS_SUCCEEDED(aResult)) {
>-    PreEvaluateScript();
>-  } else {
>-    mScriptElements.RemoveObjectAt(count - 1);
>-
>-    if (mParser && aResult != NS_BINDING_ABORTED) {
>-      // Loading external script failed!. So, resume parsing since the parser
>-      // got blocked when loading external script. See
>-      // http://bugzilla.mozilla.org/show_bug.cgi?id=94903.
>-      //
>-      // XXX We don't resume parsing if we get NS_BINDING_ABORTED from the
>-      //     script load, assuming that that error code means that the user
>-      //     stopped the load through some action (like clicking a link). See
>-      //     http://bugzilla.mozilla.org/show_bug.cgi?id=243392.
>-      ContinueInterruptedParsingAsync();
>-    }
>-  }
>-
>-  return NS_OK;
>-}
>-
>-NS_IMETHODIMP
>-nsContentSink::ScriptEvaluated(nsresult aResult,
>-                               nsIScriptElement *aElement,
>-                               bool aIsInline)
>-{
>-  mDeflectedCount = sPerfDeflectCount;
>-
>-  // Check if this is the element we were waiting for
>-  PRInt32 count = mScriptElements.Count();
>-  if (count == 0 || aElement != mScriptElements[count - 1]) {
>-    return NS_OK;
>-  }
>-
>-  NS_ASSERTION(!aElement->GetScriptDeferred(), "defer script was in mScriptElements");
>-  NS_ASSERTION(!aElement->GetScriptAsync(), "async script was in mScriptElements");
>-
>-  // Pop the script element stack
>-  mScriptElements.RemoveObjectAt(count - 1); 
>-
>-  if (NS_SUCCEEDED(aResult)) {
>-    PostEvaluateScript(aElement);
>-  }
>-
>-  if (mParser && mParser->IsParserEnabled()) {
>-    ContinueInterruptedParsingAsync();
>-  }
>-
>-  return NS_OK;
>-}
>-
> nsresult
> nsContentSink::ProcessHTTPHeaders(nsIChannel* aChannel)
> {
>   nsCOMPtr<nsIHttpChannel> httpchannel(do_QueryInterface(aChannel));
>   
>   if (!httpchannel) {
>     return NS_OK;
>   }
>@@ -1699,35 +1554,16 @@ nsContentSink::WillBuildModelImpl()
> 
>   if (mProcessLinkHeaderEvent.get()) {
>     mProcessLinkHeaderEvent.Revoke();
> 
>     DoProcessLinkHeader();
>   }
> }
> 
>-void
>-nsContentSink::ContinueInterruptedParsingIfEnabled()
>-{
>-  // This shouldn't be called in the HTML5 case.
>-  if (mParser && mParser->IsParserEnabled()) {
>-    mParser->ContinueInterruptedParsing();
>-  }
>-}
>-
>-// Overridden in the HTML5 case
>-void
>-nsContentSink::ContinueInterruptedParsingAsync()
>-{
>-  nsCOMPtr<nsIRunnable> ev = NS_NewRunnableMethod(this,
>-    &nsContentSink::ContinueInterruptedParsingIfEnabled);
>-
>-  NS_DispatchToCurrentThread(ev);
>-}
>-
> /* static */
> void
> nsContentSink::NotifyDocElementCreated(nsIDocument* aDoc)
> {
>   nsCOMPtr<nsIObserverService> observerService =
>     mozilla::services::GetObserverService();
>   if (observerService) {
>     nsCOMPtr<nsIDOMDocument> domDoc = do_QueryInterface(aDoc);
>diff --git a/content/base/src/nsContentSink.h b/content/base/src/nsContentSink.h
>--- a/content/base/src/nsContentSink.h
>+++ b/content/base/src/nsContentSink.h
>@@ -41,17 +41,16 @@
>  */
> 
> #ifndef _nsContentSink_h_
> #define _nsContentSink_h_
> 
> // Base class for contentsink implementations.
> 
> #include "nsICSSLoaderObserver.h"
>-#include "nsIScriptLoaderObserver.h"
> #include "nsWeakReference.h"
> #include "nsCOMPtr.h"
> #include "nsCOMArray.h"
> #include "nsString.h"
> #include "nsAutoPtr.h"
> #include "nsGkAtoms.h"
> #include "nsTHashtable.h"
> #include "nsHashKeys.h"
>@@ -59,16 +58,17 @@
> #include "nsITimer.h"
> #include "nsStubDocumentObserver.h"
> #include "nsIParserService.h"
> #include "nsIContentSink.h"
> #include "prlog.h"
> #include "nsIRequest.h"
> #include "nsCycleCollectionParticipant.h"
> #include "nsThreadUtils.h"
>+#include "nsIScriptElement.h"
> 
> class nsIDocument;
> class nsIURI;
> class nsIChannel;
> class nsIDocShell;
> class nsIParser;
> class nsIAtom;
> class nsIChannel;
>@@ -108,26 +108,23 @@ extern PRLogModuleInfo* gContentSinkLogM
> #undef SINK_NO_INCREMENTAL
> 
> //----------------------------------------------------------------------
> 
> // 1/2 second fudge factor for window creation
> #define NS_DELAY_FOR_WINDOW_CREATION  500000
> 
> class nsContentSink : public nsICSSLoaderObserver,
>-                      public nsIScriptLoaderObserver,
>                       public nsSupportsWeakReference,
>                       public nsStubDocumentObserver,
>                       public nsITimerCallback
> {
>   NS_DECL_CYCLE_COLLECTING_ISUPPORTS
>   NS_DECL_CYCLE_COLLECTION_CLASS_AMBIGUOUS(nsContentSink,
>-                                           nsIScriptLoaderObserver)
>-  NS_DECL_NSISCRIPTLOADEROBSERVER
>-
>+                                           nsICSSLoaderObserver)
>     // nsITimerCallback
>   NS_DECL_NSITIMERCALLBACK
> 
>   // nsICSSLoaderObserver
>   NS_IMETHOD StyleSheetLoaded(nsCSSStyleSheet* aSheet, bool aWasAlternate,
>                               nsresult aStatus);
> 
>   virtual nsresult ProcessMETATag(nsIContent* aContent);
>@@ -285,48 +282,43 @@ protected:
>   {
>     if (mDynamicLowerValue) {
>       return 1000;
>     }
> 
>     return sNotificationInterval;
>   }
> 
>-  // Overridable hooks into script evaluation
>-  virtual void PreEvaluateScript()                            {return;}
>-  virtual void PostEvaluateScript(nsIScriptElement *aElement) {return;}
>-
>   virtual nsresult FlushTags() = 0;
> 
>   // Later on we might want to make this more involved somehow
>   // (e.g. stop waiting after some timeout or whatnot).
>   bool WaitForPendingSheets() { return mPendingSheetCount > 0; }
> 
>   void DoProcessLinkHeader();
> 
>+  void StopDeflecting() {
>+    mDeflectedCount = sPerfDeflectCount;
>+  }
>+
> private:
>   // People shouldn't be allocating this class directly.  All subclasses should
>   // be allocated using a zeroing operator new.
>   void* operator new(size_t sz) CPP_THROW_NEW;  // Not to be implemented
> 
> protected:
> 
>-  virtual void ContinueInterruptedParsingAsync();
>-  void ContinueInterruptedParsingIfEnabled();
>-
>   nsCOMPtr<nsIDocument>         mDocument;
>   nsCOMPtr<nsIParser>           mParser;
>   nsCOMPtr<nsIURI>              mDocumentURI;
>   nsCOMPtr<nsIDocShell>         mDocShell;
>   nsRefPtr<mozilla::css::Loader> mCSSLoader;
>   nsRefPtr<nsNodeInfoManager>   mNodeInfoManager;
>   nsRefPtr<nsScriptLoader>      mScriptLoader;
> 
>-  nsCOMArray<nsIScriptElement> mScriptElements;
>-
>   // back off timer notification after count
>   PRInt32 mBackoffCount;
> 
>   // Time of last notification
>   // Note: mLastNotificationTime is only valid once mLayoutStarted is true.
>   PRTime mLastNotificationTime;
> 
>   // Timer used for notification
>diff --git a/content/base/src/nsScriptLoader.cpp b/content/base/src/nsScriptLoader.cpp
>--- a/content/base/src/nsScriptLoader.cpp
>+++ b/content/base/src/nsScriptLoader.cpp
>@@ -936,18 +936,19 @@ nsScriptLoader::ProcessPendingRequestsAs
> void
> nsScriptLoader::ProcessPendingRequests()
> {
>   nsRefPtr<nsScriptLoadRequest> request;
>   if (mParserBlockingRequest &&
>       !mParserBlockingRequest->mLoading &&
>       ReadyToExecuteScripts()) {
>     request.swap(mParserBlockingRequest);
>-    // nsContentSink::ScriptAvailable unblocks the parser
>+    UnblockParser(request);
>     ProcessRequest(request);
>+    ContinueParserAsync(request);
>   }
> 
>   while (ReadyToExecuteScripts() && 
>          !mXSLTRequests.IsEmpty() && 
>          !mXSLTRequests[0]->mLoading) {
>     request.swap(mXSLTRequests[0]);
>     mXSLTRequests.RemoveElementAt(0);
>     ProcessRequest(request);
>@@ -1163,29 +1164,42 @@ nsScriptLoader::OnStreamComplete(nsIStre
>   if (NS_FAILED(rv)) {
>     if (mDeferRequests.RemoveElement(request) ||
>         mAsyncRequests.RemoveElement(request) ||
>         mNonAsyncExternalScriptInsertedRequests.RemoveElement(request) ||
>         mXSLTRequests.RemoveElement(request)) {
>       FireScriptAvailable(rv, request);
>     } else if (mParserBlockingRequest == request) {
>       mParserBlockingRequest = nsnull;
>-      // nsContentSink::ScriptAvailable unblocks the parser
>+      UnblockParser(request);
>       FireScriptAvailable(rv, request);
>+      ContinueParserAsync(request);
>     } else {
>       mPreloads.RemoveElement(request, PreloadRequestComparator());
>     }
>   }
> 
>   // Process our request and/or any pending ones
>   ProcessPendingRequests();
> 
>   return NS_OK;
> }
> 
>+void
>+nsScriptLoader::UnblockParser(nsScriptLoadRequest* aParserBlockingRequest)
>+{
>+  aParserBlockingRequest->mElement->UnblockParser();
>+}
>+
>+void
>+nsScriptLoader::ContinueParserAsync(nsScriptLoadRequest* aParserBlockingRequest)
>+{
>+  aParserBlockingRequest->mElement->ContinueParserAsync();
>+}
>+
> nsresult
> nsScriptLoader::PrepareLoadedRequest(nsScriptLoadRequest* aRequest,
>                                      nsIStreamLoader* aLoader,
>                                      nsresult aStatus,
>                                      PRUint32 aStringLen,
>                                      const PRUint8* aString)
> {
>   if (NS_FAILED(aStatus)) {
>diff --git a/content/base/src/nsScriptLoader.h b/content/base/src/nsScriptLoader.h
>--- a/content/base/src/nsScriptLoader.h
>+++ b/content/base/src/nsScriptLoader.h
>@@ -239,16 +239,27 @@ public:
>    * @param aCharset The charset parameter for the script.
>    * @param aType The type parameter for the script.
>    */
>   virtual void PreloadURI(nsIURI *aURI, const nsAString &aCharset,
>                           const nsAString &aType);
> 
> private:
>   /**
>+   * Unblocks the creator parser of the parser-blocking scripts.
>+   */
>+  void UnblockParser(nsScriptLoadRequest* aParserBlockingRequest);
>+
>+  /**
>+   * Asynchronously resumes the creator parser of the parser-blocking scripts.
>+   */
>+  void ContinueParserAsync(nsScriptLoadRequest* aParserBlockingRequest);
>+
>+
>+  /**
>    * Helper function to check the content policy for a given request.
>    */
>   static nsresult CheckContentPolicy(nsIDocument* aDocument,
>                                      nsISupports *aContext,
>                                      nsIURI *aURI,
>                                      const nsAString &aType);
> 
>   /**
>diff --git a/content/html/document/src/nsHTMLContentSink.cpp b/content/html/document/src/nsHTMLContentSink.cpp
>--- a/content/html/document/src/nsHTMLContentSink.cpp
>+++ b/content/html/document/src/nsHTMLContentSink.cpp
>@@ -1694,18 +1694,16 @@ HTMLContentSink::DidBuildModel(bool aTer
> 
>     if (!bDestroying) {
>       StartLayout(false);
>     }
>   }
> 
>   ScrollToRef();
> 
>-  mDocument->ScriptLoader()->RemoveObserver(this);
>-
>   // Make sure we no longer respond to document mutations.  We've flushed all
>   // our notifications out, so there's no need to do anything else here.
> 
>   // XXXbz I wonder whether we could End() our contexts here too, or something,
>   // just to make sure we no longer notify...  Or is the mIsDocumentObserver
>   // thing sufficient?
>   mDocument->RemoveObserver(this);
>   mIsDocumentObserver = false;
>diff --git a/content/xml/document/src/nsXMLContentSink.cpp b/content/xml/document/src/nsXMLContentSink.cpp
>--- a/content/xml/document/src/nsXMLContentSink.cpp
>+++ b/content/xml/document/src/nsXMLContentSink.cpp
>@@ -322,17 +322,16 @@ nsXMLContentSink::DidBuildModel(bool aTe
>     nsCOMPtr<nsIDOMDocument> currentDOMDoc(do_QueryInterface(mDocument));
>     mXSLTProcessor->SetSourceContentModel(currentDOMDoc);
>     // Since the processor now holds a reference to us we drop our reference
>     // to it to avoid owning cycles
>     mXSLTProcessor = nsnull;
>   }
>   else {
>     // Kick off layout for non-XSLT transformed documents.
>-    mDocument->ScriptLoader()->RemoveObserver(this);
> 
>     // Check if we want to prettyprint
>     MaybePrettyPrint();
> 
>     bool startLayout = true;
>     
>     if (mPrettyPrinting) {
>       NS_ASSERTION(!mPendingSheetCount, "Shouldn't have pending sheets here!");
>@@ -414,18 +413,16 @@ nsXMLContentSink::OnTransformDone(nsresu
>     // document to display.
>     mDocument = aResultDocument;
>     nsCOMPtr<nsIHTMLDocument> htmlDoc = do_QueryInterface(mDocument);
>     if (htmlDoc) {
>       htmlDoc->SetDocWriteDisabled(false);
>     }
>   }
> 
>-  originalDocument->ScriptLoader()->RemoveObserver(this);
>-
>   // Notify document observers that all the content has been stuck
>   // into the document.  
>   // XXX do we need to notify for things like PIs?  Or just the
>   // documentElement?
>   nsIContent *rootElement = mDocument->GetRootElement();
>   if (rootElement) {
>     NS_ASSERTION(mDocument->IndexOf(rootElement) != -1,
>                  "rootElement not in doc?");
>@@ -591,26 +588,23 @@ nsXMLContentSink::CloseElement(nsIConten
>     mConstrainSize = true; 
>     nsCOMPtr<nsIScriptElement> sele = do_QueryInterface(aContent);
> 
>     if (mPreventScriptExecution) {
>       sele->PreventExecution();
>       return NS_OK;
>     }
> 
>+    // Always check the clock in nsContentSink right after a script
>+    StopDeflecting();
>+
>     // Now tell the script that it's ready to go. This may execute the script
>     // or return true, or neither if the script doesn't need executing.
>     bool block = sele->AttemptToExecute();
> 
>-    // If the act of insertion evaluated the script, we're fine.
>-    // Else, block the parser till the script has loaded.
>-    if (block) {
>-      mScriptElements.AppendObject(sele);
>-    }
>-
>     // If the parser got blocked, make sure to return the appropriate rv.
>     // I'm not sure if this is actually needed or not.
>     if (mParser && !mParser->IsParserEnabled()) {
>       // XXX The HTML sink doesn't call BlockParser here, why do we?
>       mParser->BlockParser();
>       block = true;
>     }
> 
>@@ -1676,8 +1670,26 @@ nsXMLContentSink::IsMonolithicContainer(
>           (aNodeInfo->NameAtom() == nsGkAtoms::tr ||
>            aNodeInfo->NameAtom() == nsGkAtoms::select ||
>            aNodeInfo->NameAtom() == nsGkAtoms::object ||
>            aNodeInfo->NameAtom() == nsGkAtoms::applet)) ||
>           (aNodeInfo->NamespaceID() == kNameSpaceID_MathML &&
>           (aNodeInfo->NameAtom() == nsGkAtoms::math))
>           );
> }
>+
>+void
>+nsXMLContentSink::ContinueInterruptedParsingIfEnabled()
>+{
>+  if (mParser && mParser->IsParserEnabled()) {
>+    mParser->ContinueInterruptedParsing();
>+  }
>+}
>+
>+void
>+nsXMLContentSink::ContinueInterruptedParsingAsync()
>+{
>+  nsCOMPtr<nsIRunnable> ev = NS_NewRunnableMethod(this,
>+    &nsXMLContentSink::ContinueInterruptedParsingIfEnabled);
>+
>+  NS_DispatchToCurrentThread(ev);
>+}
>+
>diff --git a/content/xml/document/src/nsXMLContentSink.h b/content/xml/document/src/nsXMLContentSink.h
>--- a/content/xml/document/src/nsXMLContentSink.h
>+++ b/content/xml/document/src/nsXMLContentSink.h
>@@ -97,29 +97,33 @@ public:
>   NS_IMETHOD DidBuildModel(bool aTerminated);
>   NS_IMETHOD WillInterrupt(void);
>   NS_IMETHOD WillResume(void);
>   NS_IMETHOD SetParser(nsIParser* aParser);  
>   virtual void FlushPendingNotifications(mozFlushType aType);
>   NS_IMETHOD SetDocumentCharset(nsACString& aCharset);
>   virtual nsISupports *GetTarget();
>   virtual bool IsScriptExecuting();
>+  virtual void ContinueInterruptedParsingAsync();
> 
>   // nsITransformObserver
>   NS_IMETHOD OnDocumentCreated(nsIDocument *aResultDocument);
>   NS_IMETHOD OnTransformDone(nsresult aResult, nsIDocument *aResultDocument);
> 
>   // nsICSSLoaderObserver
>   NS_IMETHOD StyleSheetLoaded(nsCSSStyleSheet* aSheet, bool aWasAlternate,
>                               nsresult aStatus);
>   static bool ParsePIData(const nsString &aData, nsString &aHref,
>                           nsString &aTitle, nsString &aMedia,
>                           bool &aIsAlternate);
> 
> protected:
>+
>+  void ContinueInterruptedParsingIfEnabled();
>+
>   // Start layout.  If aIgnorePendingSheets is true, this will happen even if
>   // we still have stylesheet loads pending.  Otherwise, we'll wait until the
>   // stylesheets are all done loading.
>   virtual void MaybeStartLayout(bool aIgnorePendingSheets);
> 
>   virtual nsresult AddAttributes(const PRUnichar** aNode, nsIContent* aContent);
>   nsresult AddText(const PRUnichar* aString, PRInt32 aLength);
> 
>diff --git a/parser/html/nsHtml5Parser.cpp b/parser/html/nsHtml5Parser.cpp
>--- a/parser/html/nsHtml5Parser.cpp
>+++ b/parser/html/nsHtml5Parser.cpp
>@@ -192,16 +192,23 @@ nsHtml5Parser::BlockParser()
> {
>   mBlocked = true;
> }
> 
> NS_IMETHODIMP_(void)
> nsHtml5Parser::UnblockParser()
> {
>   mBlocked = false;
>+  mExecutor->ContinueInterruptedParsingAsync();
>+}
>+
>+NS_IMETHODIMP_(void)
>+nsHtml5Parser::ContinueInterruptedParsingAsync()
>+{
>+  mExecutor->ContinueInterruptedParsingAsync();
> }
> 
> NS_IMETHODIMP_(bool)
> nsHtml5Parser::IsParserEnabled()
> {
>   return !mBlocked;
> }
> 
>diff --git a/parser/html/nsHtml5Parser.h b/parser/html/nsHtml5Parser.h
>--- a/parser/html/nsHtml5Parser.h
>+++ b/parser/html/nsHtml5Parser.h
>@@ -144,16 +144,21 @@ class nsHtml5Parser : public nsIParser,
>     NS_IMETHOD_(void) BlockParser();
> 
>     /**
>      * Unblocks the parser.
>      */
>     NS_IMETHOD_(void) UnblockParser();
> 
>     /**
>+     * Asynchronously continues parsing.
>+     */
>+    NS_IMETHOD_(void) ContinueInterruptedParsingAsync();
>+
>+    /**
>      * Query whether the parser is enabled (i.e. not blocked) or not.
>      */
>     NS_IMETHOD_(bool) IsParserEnabled();
> 
>     /**
>      * Query whether the parser thinks it's done with parsing.
>      */
>     NS_IMETHOD_(bool) IsComplete();
>diff --git a/parser/html/nsHtml5TreeOpExecutor.cpp b/parser/html/nsHtml5TreeOpExecutor.cpp
>--- a/parser/html/nsHtml5TreeOpExecutor.cpp
>+++ b/parser/html/nsHtml5TreeOpExecutor.cpp
>@@ -153,17 +153,16 @@ nsHtml5TreeOpExecutor::DidBuildModel(boo
>     }
> 
>     if (!destroying) {
>       nsContentSink::StartLayout(false);
>     }
>   }
> 
>   ScrollToRef();
>-  mDocument->ScriptLoader()->RemoveObserver(this);
>   mDocument->RemoveObserver(this);
>   if (!mParser) {
>     // DidBuildModelImpl may cause mParser to be nulled out
>     // Return early to avoid unblocking the onload event too many times.
>     return NS_OK;
>   }
>   mDocument->EndLoad();
>   DropParserAndPerfHint();
>@@ -551,17 +550,18 @@ nsHtml5TreeOpExecutor::RunFlushLoop()
>       // The parse ended already.
>       return;
>     }
> 
>     if (scriptElement) {
>       // must be tail call when mFlushState is eNotFlushing
>       RunScript(scriptElement);
>       
>-      // The script execution machinery makes sure this doesn't get deflected.
>+      // Always check the clock in nsContentSink right after a script
>+      StopDeflecting();
>       if (nsContentSink::DidProcessATokenImpl() == 
>           NS_ERROR_HTMLPARSER_INTERRUPTED) {
>         #ifdef DEBUG_NS_HTML5_TREE_OP_EXECUTOR_FLUSH
>           printf("REFLUSH SCHEDULED (after script): %d\n", 
>             ++sTimesFlushLoopInterrupted);
>         #endif
>         nsHtml5TreeOpExecutor::ContinueInterruptedParsingAsync();
>         return;      
>@@ -749,17 +749,16 @@ nsHtml5TreeOpExecutor::RunScript(nsICont
>   // Copied from nsXMLContentSink
>   // Now tell the script that it's ready to go. This may execute the script
>   // or return true, or neither if the script doesn't need executing.
>   bool block = sele->AttemptToExecute();
> 
>   // If the act of insertion evaluated the script, we're fine.
>   // Else, block the parser till the script has loaded.
>   if (block) {
>-    mScriptElements.AppendObject(sele);
>     if (mParser) {
>       mParser->BlockParser();
>     }
>   } else {
>     // mParser may have been nulled out by now, but the flusher deals
> 
>     // If this event isn't needed, it doesn't do anything. It is sometimes
>     // necessary for the parse to continue after complex situations.
>diff --git a/parser/htmlparser/public/nsIContentSink.h b/parser/htmlparser/public/nsIContentSink.h
>--- a/parser/htmlparser/public/nsIContentSink.h
>+++ b/parser/htmlparser/public/nsIContentSink.h
>@@ -51,20 +51,19 @@
>  */
> #include "nsISupports.h"
> #include "nsStringGlue.h"
> #include "mozFlushType.h"
> #include "nsIDTD.h"
> 
> class nsIParser;
> 
>-// 57b395ad-4276-408c-9f98-7044b5025c3d
> #define NS_ICONTENT_SINK_IID \
>-{ 0x57b395ad, 0x4276, 0x408c, \
>-  { 0x9f, 0x98, 0x70, 0x44, 0xb5, 0x02, 0x5c, 0x3d } }
>+{ 0xcf9a7cbb, 0xfcbc, 0x4e13, \
>+  { 0x8e, 0xf5, 0x18, 0xef, 0x2d, 0x3d, 0x58, 0x29 } }
> 
> class nsIContentSink : public nsISupports {
> public:
> 
>   NS_DECLARE_STATIC_IID_ACCESSOR(NS_ICONTENT_SINK_IID)
> 
>   /**
>    * This method is called by the parser when it is entered from
>@@ -150,13 +149,17 @@ public:
>    * Returns true if there's currently script executing that we need to hold
>    * parsing for.
>    */
>   virtual bool IsScriptExecuting()
>   {
>     return false;
>   }
>   
>+  /**
>+   * Posts a runnable that continues parsing.
>+   */
>+  virtual void ContinueInterruptedParsingAsync() {};
> };
> 
> NS_DEFINE_STATIC_IID_ACCESSOR(nsIContentSink, NS_ICONTENT_SINK_IID)
> 
> #endif /* nsIContentSink_h___ */
>diff --git a/parser/htmlparser/public/nsIParser.h b/parser/htmlparser/public/nsIParser.h
>--- a/parser/htmlparser/public/nsIParser.h
>+++ b/parser/htmlparser/public/nsIParser.h
>@@ -50,18 +50,18 @@
> #include "nsISupports.h"
> #include "nsIStreamListener.h"
> #include "nsIDTD.h"
> #include "nsStringGlue.h"
> #include "nsTArray.h"
> #include "nsIAtom.h"
> 
> #define NS_IPARSER_IID \
>-{ 0x30ffba62, 0x0928, 0x4503, \
>-  { 0xa8, 0x95, 0xd1, 0x32, 0x76, 0x40, 0x2a, 0x2a } }
>+{ 0x11a4f41f, 0x7044, 0x4c57, \
>+  { 0xb3, 0xa4, 0xed, 0x79, 0xc7, 0xc4, 0x61, 0x99 } }
> 
> // {41421C60-310A-11d4-816F-000064657374}
> #define NS_IDEBUG_DUMP_CONTENT_IID \
> { 0x41421c60, 0x310a, 0x11d4, \
> { 0x81, 0x6f, 0x0, 0x0, 0x64, 0x65, 0x73, 0x74 } }
> 
> class nsIContentSink;
> class nsIRequestObserver;
>@@ -214,16 +214,21 @@ class nsIParser : public nsISupports {
>     NS_IMETHOD_(void) BlockParser() = 0;
>     
>     // Open up the parser for tokenization, building up content 
>     // model..etc. However, this method does not resume parsing 
>     // automatically. It's the callers' responsibility to restart
>     // the parsing engine.
>     NS_IMETHOD_(void) UnblockParser() = 0;
> 
>+    /**
>+     * Asynchronously continues parsing.
>+     */
>+    NS_IMETHOD_(void) ContinueInterruptedParsingAsync() = 0;
>+
>     NS_IMETHOD_(bool) IsParserEnabled() = 0;
>     NS_IMETHOD_(bool) IsComplete() = 0;
>     
>     NS_IMETHOD Parse(nsIURI* aURL,
>                      nsIRequestObserver* aListener = nsnull,
>                      void* aKey = 0,
>                      nsDTDMode aMode = eDTDMode_autodetect) = 0;
>     NS_IMETHOD Parse(const nsAString& aSourceBuffer,
>diff --git a/parser/htmlparser/src/nsParser.cpp b/parser/htmlparser/src/nsParser.cpp
>--- a/parser/htmlparser/src/nsParser.cpp
>+++ b/parser/htmlparser/src/nsParser.cpp
>@@ -1187,16 +1187,22 @@ nsParser::UnblockParser()
> {
>   if (!(mFlags & NS_PARSER_FLAG_PARSER_ENABLED)) {
>     mFlags |= NS_PARSER_FLAG_PARSER_ENABLED;
>   } else {
>     NS_WARNING("Trying to unblock an unblocked parser.");
>   }
> }
> 
>+NS_IMETHODIMP_(void)
>+nsParser::ContinueInterruptedParsingAsync()
>+{
>+  mSink->ContinueInterruptedParsingAsync();
>+}
>+
> /**
>  * Call this to query whether the parser is enabled or not.
>  */
> NS_IMETHODIMP_(bool)
> nsParser::IsParserEnabled()
> {
>   return (mFlags & NS_PARSER_FLAG_PARSER_ENABLED) != 0;
> }
>diff --git a/parser/htmlparser/src/nsParser.h b/parser/htmlparser/src/nsParser.h
>--- a/parser/htmlparser/src/nsParser.h
>+++ b/parser/htmlparser/src/nsParser.h
>@@ -211,16 +211,17 @@ class nsParser : public nsIParser,
>      * @update	gess5/11/98
>      * @return  YES if model building went well -- NO otherwise.
>      */
>     NS_IMETHOD BuildModel(void);
> 
>     NS_IMETHOD        ContinueInterruptedParsing();
>     NS_IMETHOD_(void) BlockParser();
>     NS_IMETHOD_(void) UnblockParser();
>+    NS_IMETHOD_(void) ContinueInterruptedParsingAsync();
>     NS_IMETHOD        Terminate(void);
> 
>     /**
>      * Call this to query whether the parser is enabled or not.
>      *
>      *  @update  vidur 4/12/99
>      *  @return  current state
>      */

Comment 8

[Henri Sivonen (:hsivonen)]

Thanks.

https://hg.mozilla.org/integration/mozilla-inbound/rev/3d760b97157b

Comment 9

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/3d760b97157b