Bug 551344 part 4 - Address more review comments from sicking in the C++ parts of the HTML5 parser. r=jonas.
authorHenri Sivonen <hsivonen@iki.fi>
Fri, 16 Apr 2010 13:52:06 +0300
changeset 41395 9732d2b1666230f86278c12a3b3b5b965454b849
parent 41394 1aa5c9dc5e24b7399fce52bac5aa666e555de11b
child 41396 a1c53e4881ca7f9543638b5e3e3d5b5be85351a1
push idunknown
push userunknown
push dateunknown
reviewersjonas
bugs551344
milestone1.9.3a5pre
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
Bug 551344 part 4 - Address more review comments from sicking in the C++ parts of the HTML5 parser. r=jonas.
intl/uconv/public/nsIUnicodeDecoder.h
modules/libpref/src/init/all.js
parser/html/nsHtml5Parser.h
parser/html/nsHtml5StreamParser.cpp
parser/html/nsHtml5StreamParser.h
--- a/intl/uconv/public/nsIUnicodeDecoder.h
+++ b/intl/uconv/public/nsIUnicodeDecoder.h
@@ -118,17 +118,20 @@ public:
    * again for continuing the conversion. This way, the caller will not have to
    * worry about managing incomplete input data by mergeing it with the next 
    * buffer.
    *
    * Error conditions: 
    * If the read value does not belong to this character set, one should 
    * replace it with the Unicode special 0xFFFD. When an actual input error is 
    * encountered, like a format error, the converter stop and return error.
-   * Hoever, we should keep in mind that we need to be lax in decoding.
+   * However, we should keep in mind that we need to be lax in decoding. When
+   * a decoding error is returned to the caller, it is the caller's
+   * responsibility to advance over the bad byte and reset the decoder before
+   * trying to call the decoder again.
    *
    * Converter required behavior:
    * In this order: when output space is full - return right away. When input
    * data is wrong, return input pointer right after the wrong byte. When 
    * partial input, it will be consumed and cached. All the time input pointer
    * will show how much was actually consumed and how much was actually 
    * written.
    *
@@ -139,17 +142,19 @@ public:
    * @param aDestLength [IN/OUT] the length of the destination data buffer;
    *                    after conversion will contain the number of Unicode
    *                    characters written
    * @return            NS_PARTIAL_MORE_INPUT if only a partial conversion was
    *                    done; more input is needed to continue
    *                    NS_PARTIAL_MORE_OUTPUT if only  a partial conversion
    *                    was done; more output space is needed to continue
    *                    NS_ERROR_ILLEGAL_INPUT if an illegal input sequence
-   *                    was encountered and the behavior was set to "signal"
+   *                    was encountered and the behavior was set to "signal";
+   *                    the caller must skip over one byte, reset the decoder
+   *                    and retry.
    */
   NS_IMETHOD Convert(const char * aSrc, PRInt32 * aSrcLength, 
       PRUnichar * aDest, PRInt32 * aDestLength) = 0;
 
   /**
    * Returns a quick estimation of the size of the buffer needed to hold the
    * converted data. Remember: this estimation is >= with the actual size of 
    * the buffer needed. It will be computed for the "worst case"
--- a/modules/libpref/src/init/all.js
+++ b/modules/libpref/src/init/all.js
@@ -2868,25 +2868,23 @@ pref("geo.enabled", true);
 
 // Enable/Disable the orientation API for content
 pref("accelerometer.enabled", true);
 
 // Enable/Disable HTML5 parser
 pref("html5.enable", false);
 // Toggle which thread the HTML5 parser uses for stream parsing
 pref("html5.offmainthread", true);
-// Time in milliseconds between the start of the network stream and the 
-// first time the flush timer fires in the off-the-main-thread HTML5 parser.
-pref("html5.flushtimer.startdelay", 200);
-// Time in milliseconds between the return to non-speculating more and the 
-// first time the flush timer fires thereafter.
-pref("html5.flushtimer.continuedelay", 150);
-// Time in milliseconds between timer firings once the timer has starting 
-// firing.
-pref("html5.flushtimer.interval", 120);
+// Time in milliseconds between the time a network buffer is seen and the 
+// timer firing when the timer hasn't fired previously in this parse in the 
+// off-the-main-thread HTML5 parser.
+pref("html5.flushtimer.initialdelay", 200);
+// Time in milliseconds between the time a network buffer is seen and the 
+// timer firing when the timer has already fired previously in this parse.
+pref("html5.flushtimer.subsequentdelay", 120);
 
 // Push/Pop/Replace State prefs
 pref("browser.history.allowPushState", true);
 pref("browser.history.allowReplaceState", true);
 pref("browser.history.allowPopState", true);
 pref("browser.history.maxStateObjectSize", 655360);
 
 pref("network.buffer.cache.count", 24);
--- a/parser/html/nsHtml5Parser.h
+++ b/parser/html/nsHtml5Parser.h
@@ -299,17 +299,20 @@ class nsHtml5Parser : public nsIParser,
 
     inline nsHtml5Tokenizer* GetTokenizer() {
       return mTokenizer;
     }
 
     void InitializeDocWriteParserState(nsAHtml5TreeBuilderState* aState, PRInt32 aLine);
 
     void DropStreamParser() {
-      mStreamParser = nsnull;
+      if (mStreamParser) {
+        mStreamParser->DropTimer();
+        mStreamParser = nsnull;
+      }
     }
     
     void StartTokenizer(PRBool aScriptingEnabled);
     
     void ContinueAfterFailedCharsetSwitch();
 
     nsHtml5StreamParser* GetStreamParser() {
       return mStreamParser;
--- a/parser/html/nsHtml5StreamParser.cpp
+++ b/parser/html/nsHtml5StreamParser.cpp
@@ -49,49 +49,67 @@
 #include "nsHtml5Parser.h"
 #include "nsHtml5TreeBuilder.h"
 #include "nsHtml5AtomTable.h"
 #include "nsHtml5Module.h"
 #include "nsHtml5RefPtr.h"
 
 static NS_DEFINE_CID(kCharsetAliasCID, NS_CHARSETALIAS_CID);
 
-PRInt32 nsHtml5StreamParser::sTimerStartDelay = 200;
-PRInt32 nsHtml5StreamParser::sTimerContinueDelay = 150;
-PRInt32 nsHtml5StreamParser::sTimerInterval = 100;
+PRInt32 nsHtml5StreamParser::sTimerInitialDelay = 200;
+PRInt32 nsHtml5StreamParser::sTimerSubsequentDelay = 100;
 
 // static
 void
 nsHtml5StreamParser::InitializeStatics()
 {
-  nsContentUtils::AddIntPrefVarCache("html5.flushtimer.startdelay", 
-                                     &sTimerStartDelay);
-  nsContentUtils::AddIntPrefVarCache("html5.flushtimer.continuedelay", 
-                                     &sTimerContinueDelay);
-  nsContentUtils::AddIntPrefVarCache("html5.flushtimer.interval",
-                                     &sTimerInterval);
+  nsContentUtils::AddIntPrefVarCache("html5.flushtimer.initialdelay",
+                                     &sTimerInitialDelay);
+  nsContentUtils::AddIntPrefVarCache("html5.flushtimer.subsequentdelay",
+                                     &sTimerSubsequentDelay);
 }
 
+/*
+ * Note that nsHtml5StreamParser implements cycle collecting AddRef and
+ * Release. Therefore, nsHtml5StreamParser must never be refcounted from
+ * the parser thread!
+ *
+ * To work around this limitation, runnables posted by the main thread to the
+ * parser thread hold their reference to the stream parser in an
+ * nsHtml5RefPtr. Upon creation, nsHtml5RefPtr addrefs the object it holds
+ * just like a regular nsRefPtr. This is OK, since the creation of the
+ * runnable and the nsHtml5RefPtr happens on the main thread.
+ *
+ * When the runnable is done on the parser thread, the destructor of
+ * nsHtml5RefPtr runs there. It doesn't call Release on the held object
+ * directly. Instead, it posts another runnable back to the main thread where
+ * that runnable calls Release on the wrapped object.
+ *
+ * When posting runnables in the other direction, the runnables have to be
+ * created on the main thread when nsHtml5StreamParser is instantiated and
+ * held for the lifetime of the nsHtml5StreamParser. This works, because the
+ * same runnabled can be dispatched multiple times and currently runnables
+ * posted from the parser thread to main thread don't need to wrap any
+ * runnable-specific data. (In the other direction, the runnables most notably
+ * wrap the byte data of the stream.)
+ */
 NS_IMPL_CYCLE_COLLECTING_ADDREF(nsHtml5StreamParser)
 NS_IMPL_CYCLE_COLLECTING_RELEASE(nsHtml5StreamParser)
 
 NS_INTERFACE_TABLE_HEAD(nsHtml5StreamParser)
   NS_INTERFACE_TABLE2(nsHtml5StreamParser, 
                       nsIStreamListener, 
                       nsICharsetDetectionObserver)
   NS_INTERFACE_TABLE_TO_MAP_SEGUE_CYCLE_COLLECTION(nsHtml5StreamParser)
 NS_INTERFACE_MAP_END
 
 NS_IMPL_CYCLE_COLLECTION_CLASS(nsHtml5StreamParser)
 
 NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(nsHtml5StreamParser)
-  if (tmp->mFlushTimer) {
-    tmp->mFlushTimer->Cancel();
-    tmp->mFlushTimer = nsnull;
-  }
+  tmp->DropTimer();
   NS_IMPL_CYCLE_COLLECTION_UNLINK_NSCOMPTR(mObserver)
   NS_IMPL_CYCLE_COLLECTION_UNLINK_NSCOMPTR(mRequest)
   tmp->mOwner = nsnull;
   tmp->mExecutorFlusher = nsnull;
   tmp->mLoadFlusher = nsnull;
   tmp->mExecutor = nsnull;
   NS_IMPL_CYCLE_COLLECTION_UNLINK_NSCOMPTR(mChardet)
 NS_IMPL_CYCLE_COLLECTION_UNLINK_END
@@ -164,20 +182,21 @@ nsHtml5StreamParser::nsHtml5StreamParser
   , mSpeculationMutex("nsHtml5StreamParser mSpeculationMutex")
   , mTerminatedMutex("nsHtml5StreamParser mTerminatedMutex")
   , mThread(nsHtml5Module::GetStreamParserThread())
   , mExecutorFlusher(new nsHtml5ExecutorFlusher(aExecutor))
   , mLoadFlusher(new nsHtml5LoadFlusher(aExecutor))
   , mFlushTimer(do_CreateInstance("@mozilla.org/timer;1"))
 {
   NS_ASSERTION(NS_IsMainThread(), "Wrong thread!");
+  mFlushTimer->SetTarget(mThread);
   mAtomTable.Init(); // we aren't checking for OOM anyway...
-  #ifdef DEBUG
-    mAtomTable.SetPermittedLookupThread(mThread);
-  #endif
+#ifdef DEBUG
+  mAtomTable.SetPermittedLookupThread(mThread);
+#endif
   mTokenizer->setInterner(&mAtomTable);
   mTokenizer->setEncodingDeclarationHandler(this);
 
   // Chardet instantiation adapted from nsDOMFile.
   // Chardet is initialized here even if it turns out to be useless
   // to make the chardet refcount its observer (nsHtml5StreamParser)
   // on the main thread.
   const nsAdoptingString& detectorName = 
@@ -193,30 +212,29 @@ nsHtml5StreamParser::nsHtml5StreamParser
 
   // There's a zeroing operator new for everything else
 }
 
 nsHtml5StreamParser::~nsHtml5StreamParser()
 {
   NS_ASSERTION(NS_IsMainThread(), "Wrong thread!");
   mTokenizer->end();
+  NS_ASSERTION(!mFlushTimer, "Flush timer was not dropped before dtor!");
+#ifdef DEBUG
   mRequest = nsnull;
   mObserver = nsnull;
   mUnicodeDecoder = nsnull;
   mSniffingBuffer = nsnull;
   mMetaScanner = nsnull;
   mFirstBuffer = nsnull;
   mExecutor = nsnull;
   mTreeBuilder = nsnull;
   mTokenizer = nsnull;
   mOwner = nsnull;
-  if (mFlushTimer) {
-    mFlushTimer->Cancel();
-    mFlushTimer = nsnull;
-  }
+#endif
 }
 
 nsresult
 nsHtml5StreamParser::GetChannel(nsIChannel** aChannel)
 {
   NS_ASSERTION(NS_IsMainThread(), "Wrong thread!");
   return mRequest ? CallQueryInterface(mRequest, aChannel) :
                     NS_ERROR_NOT_AVAILABLE;
@@ -535,21 +553,16 @@ nsHtml5StreamParser::OnStartRequest(nsIR
   mExecutor->StartReadingFromStage();
   /*
    * If you move the following line, be very careful not to cause 
    * WillBuildModel to be called before the document has had its 
    * script global object set.
    */
   mExecutor->WillBuildModel(eDTDMode_unknown);
   
-  mFlushTimer->InitWithFuncCallback(nsHtml5StreamParser::TimerCallback, 
-                                    static_cast<void*> (this), 
-                                    sTimerStartDelay, 
-                                    nsITimer::TYPE_ONE_SHOT);
-
   nsresult rv = NS_OK;
 
   mReparseForbidden = PR_FALSE;
   nsCOMPtr<nsIHttpChannel> httpChannel(do_QueryInterface(mRequest, &rv));
   if (NS_SUCCEEDED(rv)) {
     nsCAutoString method;
     httpChannel->GetRequestMethod(method);
     // XXX does Necko have a way to renavigate POST, etc. without hitting
@@ -653,16 +666,28 @@ nsHtml5StreamParser::DoDataAvailable(PRU
   // dropping nsresult here
   NS_ASSERTION(writeCount == aLength, "Wrong number of stream bytes written/sniffed.");
 
   if (IsTerminatedOrInterrupted()) {
     return;
   }
 
   ParseAvailableData();
+
+  if (mFlushTimerArmed || mSpeculating) {
+    return;
+  }
+
+  mFlushTimer->InitWithFuncCallback(nsHtml5StreamParser::TimerCallback,
+                                    static_cast<void*> (this),
+                                    mFlushTimerEverFired ?
+                                        sTimerInitialDelay :
+                                        sTimerSubsequentDelay,
+                                    nsITimer::TYPE_ONE_SHOT);
+  mFlushTimerArmed = PR_TRUE;
 }
 
 class nsHtml5DataAvailable : public nsRunnable
 {
   private:
     nsHtml5RefPtr<nsHtml5StreamParser> mStreamParser;
     nsAutoArrayPtr<PRUint8>            mData;
     PRUint32                           mLength;
@@ -743,24 +768,37 @@ nsHtml5StreamParser::internalEncodingDec
   
   rv = calias->GetPreferred(newEncoding, preferred);
   if (NS_FAILED(rv)) {
     // the encoding name is bogus
     return;
   }
   
   mTreeBuilder->NeedsCharsetSwitchTo(preferred);
+  FlushTreeOpsAndDisarmTimer();
+  Interrupt();
+  // the tree op executor will cause the stream parser to terminate
+  // if the charset switch request is accepted or it'll uninterrupt 
+  // if the request failed.
+}
+
+void
+nsHtml5StreamParser::FlushTreeOpsAndDisarmTimer()
+{
+  NS_ASSERTION(IsParserThread(), "Wrong thread!");
+  if (mFlushTimerArmed) {
+    // avoid calling Cancel if the flush timer isn't armed to avoid acquiring
+    // a mutex
+    mFlushTimer->Cancel();
+    mFlushTimerArmed = PR_FALSE;
+  }
   mTreeBuilder->Flush();
-  Interrupt();
   if (NS_FAILED(NS_DispatchToMainThread(mExecutorFlusher))) {
     NS_WARNING("failed to dispatch executor flush event");
   }
-  // the tree op executor will cause the stream parser to terminate
-  // if the charset switch request is accepted or it'll uninterrupt 
-  // if the request failed.
 }
 
 void
 nsHtml5StreamParser::ParseAvailableData()
 {
   NS_ASSERTION(IsParserThread(), "Wrong thread!");
   mTokenizerMutex.AssertCurrentThreadOwns();
 
@@ -789,20 +827,17 @@ nsHtml5StreamParser::ParseAvailableData(
             return; // no more data for now but expecting more
           case STREAM_ENDED:
             if (mAtEOF) {
               return;
             }
             mAtEOF = PR_TRUE;
             mTokenizer->eof();
             mTreeBuilder->StreamEnded();
-            mTreeBuilder->Flush();
-            if (NS_FAILED(NS_DispatchToMainThread(mExecutorFlusher))) {
-              NS_WARNING("failed to dispatch executor flush event");
-            }
+            FlushTreeOpsAndDisarmTimer();
             return; // no more data and not expecting more
           default:
             NS_NOTREACHED("It should be impossible to reach this.");
             return;
         }
       }
       mFirstBuffer = mFirstBuffer->next;
       continue;
@@ -821,20 +856,17 @@ nsHtml5StreamParser::ParseAvailableData(
         mozilla::MutexAutoLock speculationAutoLock(mSpeculationMutex);
         nsHtml5Speculation* speculation = 
           new nsHtml5Speculation(mFirstBuffer,
                                  mFirstBuffer->getStart(),
                                  mTokenizer->getLineNumber(),
                                  mTreeBuilder->newSnapshot());
         mTreeBuilder->AddSnapshotToScript(speculation->GetSnapshot(), 
                                           speculation->GetStartLineNumber());
-        mTreeBuilder->Flush();
-        if (NS_FAILED(NS_DispatchToMainThread(mExecutorFlusher))) {
-          NS_WARNING("failed to dispatch executor flush event");
-        }
+        FlushTreeOpsAndDisarmTimer();
         mTreeBuilder->SetOpSink(speculation);
         mSpeculations.AppendElement(speculation); // adopts the pointer
         mSpeculating = PR_TRUE;
       }
       if (IsTerminatedOrInterrupted()) {
         return;
       }
     }
@@ -848,16 +880,17 @@ private:
   nsHtml5RefPtr<nsHtml5StreamParser> mStreamParser;
 public:
   nsHtml5StreamParserContinuation(nsHtml5StreamParser* aStreamParser)
     : mStreamParser(aStreamParser)
   {}
   NS_IMETHODIMP Run()
   {
     mozilla::MutexAutoLock autoLock(mStreamParser->mTokenizerMutex);
+    mStreamParser->Uninterrupt();
     mStreamParser->ParseAvailableData();
     return NS_OK;
   }
 };
 
 void
 nsHtml5StreamParser::ContinueAfterScripts(nsHtml5Tokenizer* aTokenizer, 
                                           nsHtml5TreeBuilder* aTreeBuilder,
@@ -938,21 +971,17 @@ nsHtml5StreamParser::ContinueAfterScript
                              // run here synchronously on the main thread...
 
       mTreeBuilder->flushCharacters(); // empty the pending buffer
       mTreeBuilder->ClearOps(); // now get rid of the failed ops
 
       mTreeBuilder->SetOpSink(mExecutor->GetStage());
       mExecutor->StartReadingFromStage();
       mSpeculating = PR_FALSE;
-      mFlushTimer->Cancel();
-      mFlushTimer->InitWithFuncCallback(nsHtml5StreamParser::TimerCallback, 
-                                        static_cast<void*> (this), 
-                                        sTimerContinueDelay, 
-                                        nsITimer::TYPE_ONE_SHOT);
+
       // Copy state over
       mLastWasCR = aLastWasCR;
       mTokenizer->loadState(aTokenizer);
       mTreeBuilder->loadState(aTreeBuilder, &mAtomTable);
     } else {    
       // We've got a successful speculation and at least a moment ago it was
       // the current speculation
       mSpeculations.ElementAt(0)->FlushToSink(mExecutor);
@@ -963,62 +992,110 @@ nsHtml5StreamParser::ContinueAfterScript
         "RunFlushLoop() didn't call ParseUntilBlocked() which is the "
         "only caller of this method?");
       mSpeculations.RemoveElementAt(0);
       if (mSpeculations.IsEmpty()) {
         // yes, it was still the only speculation. Now stop speculating
         mTreeBuilder->SetOpSink(mExecutor->GetStage());
         mExecutor->StartReadingFromStage();
         mSpeculating = PR_FALSE;
-        mFlushTimer->Cancel();
-        mFlushTimer->InitWithFuncCallback(nsHtml5StreamParser::TimerCallback, 
-                                          static_cast<void*> (this), 
-                                          sTimerContinueDelay, 
-                                          nsITimer::TYPE_ONE_SHOT);
       }
     }
-    Uninterrupt();
     nsCOMPtr<nsIRunnable> event = new nsHtml5StreamParserContinuation(this);
     if (NS_FAILED(mThread->Dispatch(event, nsIThread::DISPATCH_NORMAL))) {
-      NS_WARNING("Failed to dispatch ParseAvailableData event");
+      NS_WARNING("Failed to dispatch nsHtml5StreamParserContinuation");
     }
     // A stream event might run before this event runs, but that's harmless.
     #ifdef DEBUG
       mAtomTable.SetPermittedLookupThread(mThread);
     #endif
   }
 }
 
 void
 nsHtml5StreamParser::ContinueAfterFailedCharsetSwitch()
 {
   NS_ASSERTION(NS_IsMainThread(), "Wrong thread!");
-  mozilla::MutexAutoLock tokenizerAutoLock(mTokenizerMutex);
-  Uninterrupt();
   nsCOMPtr<nsIRunnable> event = new nsHtml5StreamParserContinuation(this);
   if (NS_FAILED(mThread->Dispatch(event, nsIThread::DISPATCH_NORMAL))) {
     NS_WARNING("Failed to dispatch nsHtml5StreamParserContinuation");
   }
 }
 
+class nsHtml5TimerKungFu : public nsRunnable
+{
+private:
+  nsHtml5RefPtr<nsHtml5StreamParser> mStreamParser;
+public:
+  nsHtml5TimerKungFu(nsHtml5StreamParser* aStreamParser)
+    : mStreamParser(aStreamParser)
+  {}
+  NS_IMETHODIMP Run()
+  {
+    if (mStreamParser->mFlushTimer) {
+      mStreamParser->mFlushTimer->Cancel();
+      mStreamParser->mFlushTimer = nsnull;
+    }
+    return NS_OK;
+  }
+};
+
+void
+nsHtml5StreamParser::DropTimer()
+{
+  NS_ASSERTION(NS_IsMainThread(), "Wrong thread!");
+  /*
+   * Simply nulling out the timer wouldn't work, because if the timer is
+   * armed, it needs to be canceled first. Simply canceling it first wouldn't
+   * work, because nsTimerImpl::Cancel is not safe for calling from outside
+   * the thread where nsTimerImpl::Fire would run. It's not safe to
+   * dispatch a runnable to cancel the timer from the destructor of this
+   * class, because the timer has a weak (void*) pointer back to this instance
+   * of the stream parser and having the timer fire before the runnable
+   * cancels it would make the timer access a deleted object.
+   *
+   * This DropTimer method addresses these issues. This method must be called
+   * on the main thread before the destructor of this class is reached.
+   * The nsHtml5TimerKungFu object has an nsHtml5RefPtr that addrefs this
+   * stream parser object to keep it alive until the runnable is done.
+   * The runnable cancels the timer on the parser thread, drops the timer
+   * and lets nsHtml5RefPtr send a runnable back to the main thread to
+   * release the stream parser.
+   */
+  if (mFlushTimer) {
+    nsCOMPtr<nsIRunnable> event = new nsHtml5TimerKungFu(this);
+    if (NS_FAILED(mThread->Dispatch(event, nsIThread::DISPATCH_NORMAL))) {
+      NS_WARNING("Failed to dispatch TimerKungFu event");
+    }
+  }
+}
+
 // Using a static, because the method name Notify is taken by the chardet 
 // callback.
 void
 nsHtml5StreamParser::TimerCallback(nsITimer* aTimer, void* aClosure)
 {
-  (static_cast<nsHtml5StreamParser*> (aClosure))->PostTimerFlush();
+  (static_cast<nsHtml5StreamParser*> (aClosure))->TimerFlush();
 }
 
 void
 nsHtml5StreamParser::TimerFlush()
 {
   NS_ASSERTION(IsParserThread(), "Wrong thread!");
-  mTokenizerMutex.AssertCurrentThreadOwns();
+  mozilla::MutexAutoLock autoLock(mTokenizerMutex);
+
+  NS_ASSERTION(!mSpeculating, "Flush timer fired while speculating.");
 
-  if (mSpeculating) {
+  // The timer fired if we got here. No need to cancel it. Mark it as
+  // not armed, though.
+  mFlushTimerArmed = PR_FALSE;
+
+  mFlushTimerEverFired = PR_TRUE;
+
+  if (IsTerminatedOrInterrupted()) {
     return;
   }
 
   // we aren't speculating and we don't know when new data is
   // going to arrive. Send data to the main thread.
   // However, don't do if the current element on the stack is a 
   // foster-parenting element and there's pending text, because flushing in 
   // that case would make the tree shape dependent on where the flush points 
@@ -1026,57 +1103,8 @@ nsHtml5StreamParser::TimerFlush()
   if (mTreeBuilder->IsDiscretionaryFlushSafe()) {
     if (mTreeBuilder->Flush()) {
       if (NS_FAILED(NS_DispatchToMainThread(mExecutorFlusher))) {
         NS_WARNING("failed to dispatch executor flush event");
       }
     }
   }
 }
-
-class nsHtml5StreamParserTimerFlusher : public nsRunnable
-{
-private:
-  nsHtml5RefPtr<nsHtml5StreamParser> mStreamParser;
-public:
-  nsHtml5StreamParserTimerFlusher(nsHtml5StreamParser* aStreamParser)
-    : mStreamParser(aStreamParser)
-  {}
-  NS_IMETHODIMP Run()
-  {
-    mozilla::MutexAutoLock autoLock(mStreamParser->mTokenizerMutex);
-    mStreamParser->TimerFlush();
-    return NS_OK;
-  }
-};
-
-void
-nsHtml5StreamParser::PostTimerFlush()
-{
-  NS_ASSERTION(NS_IsMainThread(), "Wrong thread!");
-
-  mFlushTimer->Cancel();
-
-  // The following line reads a mutex-protected variable without acquiring 
-  // the mutex. This is OK, because failure to exit early here is harmless.
-  // The early exit here is merely an optimization. Note that parser thread
-  // may have set mSpeculating to true where it previously was false--not 
-  // the other way round. mSpeculating is set to false only on the main thread.
-  if (mSpeculating) {
-    // No need for timer flushes when speculating
-    return;
-  }
-
-  // Schedule the next timer shot
-  mFlushTimer->InitWithFuncCallback(nsHtml5StreamParser::TimerCallback, 
-                                    static_cast<void*> (this), 
-                                    sTimerInterval, 
-                                    nsITimer::TYPE_ONE_SHOT);
-
-  // TODO: (If the document isn't in the frontmost tab or If the user isn't 
-  // interacting with the browser) and this isn't every nth timer flush, return
-
-  nsCOMPtr<nsIRunnable> event = new nsHtml5StreamParserTimerFlusher(this);
-  if (NS_FAILED(mThread->Dispatch(event, nsIThread::DISPATCH_NORMAL))) {
-    NS_WARNING("Failed to dispatch nsHtml5StreamParserTimerFlusher");
-  }
-}
-
--- a/parser/html/nsHtml5StreamParser.h
+++ b/parser/html/nsHtml5StreamParser.h
@@ -105,17 +105,17 @@ enum eHtml5StreamState {
 class nsHtml5StreamParser : public nsIStreamListener,
                             public nsICharsetDetectionObserver,
                             public nsAHtml5EncodingDeclarationHandler
 {
 
   friend class nsHtml5RequestStopper;
   friend class nsHtml5DataAvailable;
   friend class nsHtml5StreamParserContinuation;
-  friend class nsHtml5StreamParserTimerFlusher;
+  friend class nsHtml5TimerKungFu;
 
   public:
     NS_DECL_AND_IMPL_ZEROING_OPERATOR_NEW
     NS_DECL_CYCLE_COLLECTING_ISUPPORTS
     NS_DECL_CYCLE_COLLECTION_CLASS_AMBIGUOUS(nsHtml5StreamParser, nsIStreamListener)
 
     static void InitializeStatics();
 
@@ -170,49 +170,62 @@ class nsHtml5StreamParser : public nsISt
      * when no scripts are executing and the document.written 
      * buffer has been exhausted.
      */
     void ContinueAfterScripts(nsHtml5Tokenizer* aTokenizer, 
                               nsHtml5TreeBuilder* aTreeBuilder,
                               PRBool aLastWasCR);
 
     /**
-     * Uninterrupts and continues the stream parser if the charset switch 
-     * failed.
+     * Continues the stream parser if the charset switch failed.
      */
     void ContinueAfterFailedCharsetSwitch();
 
     void Terminate() {
       mozilla::MutexAutoLock autoLock(mTerminatedMutex);
       mTerminated = PR_TRUE;
     }
     
+    void DropTimer();
+
   private:
 
 #ifdef DEBUG
     PRBool IsParserThread() {
       PRBool ret;
       mThread->IsOnCurrentThread(&ret);
       return ret;
     }
 #endif
 
+    /**
+     * Marks the stream parser as interrupted. If you ever add calls to this
+     * method, be sure to review Uninterrupt usage very, very carefully to
+     * avoid having a previous in-flight runnable cancel your Interrupt()
+     * call on the other thread too soon.
+     */
     void Interrupt() {
       mozilla::MutexAutoLock autoLock(mTerminatedMutex);
       mInterrupted = PR_TRUE;
     }
 
     void Uninterrupt() {
-      NS_ASSERTION(NS_IsMainThread(), "Wrong thread!");
+      NS_ASSERTION(IsParserThread(), "Wrong thread!");
       mTokenizerMutex.AssertCurrentThreadOwns();
       // Not acquiring mTerminatedMutex because mTokenizerMutex is already
       // held at this point and is already stronger.
       mInterrupted = PR_FALSE;      
     }
 
+    /**
+     * Flushes the tree ops from the tree builder and disarms the flush
+     * timer.
+     */
+    void FlushTreeOpsAndDisarmTimer();
+
     void ParseAvailableData();
     
     void DoStopRequest();
     
     void DoDataAvailable(PRUint8* aBuffer, PRUint32 aLength);
 
     PRBool IsTerminatedOrInterrupted() {
       mozilla::MutexAutoLock autoLock(mTerminatedMutex);
@@ -307,22 +320,16 @@ class nsHtml5StreamParser : public nsISt
                                   const char* aDecoderCharsetName);
 
     /**
      * Callback for mFlushTimer.
      */
     static void TimerCallback(nsITimer* aTimer, void* aClosure);
 
     /**
-     * Main thread entry point for (maybe) flushing the ops and posting
-     * a flush runnable back on the main thread.
-     */
-    void PostTimerFlush();
-
-    /**
      * Parser thread entry point for (maybe) flushing the ops and posting
      * a flush runnable back on the main thread.
      */
     void TimerFlush();
 
     nsCOMPtr<nsIRequest>          mRequest;
     nsCOMPtr<nsIRequestObserver>  mObserver;
 
@@ -461,29 +468,34 @@ class nsHtml5StreamParser : public nsISt
     nsCOMPtr<nsICharsetDetector>  mChardet;
 
     /**
      * Timer for flushing tree ops once in a while when not speculating.
      */
     nsCOMPtr<nsITimer>            mFlushTimer;
 
     /**
-     * The pref html5.flushtimer.startdelay: Time in milliseconds between
-     * the start of the network stream and the first time the flush timer
-     * fires.
+     * Keeps track whether mFlushTimer has been armed. Unfortunately,
+     * nsITimer doesn't enable querying this from the timer itself.
      */
-    static PRInt32                sTimerStartDelay;
+    PRBool                        mFlushTimerArmed;
+
+    /**
+     * False initially and true after the timer has fired at least once.
+     */
+    PRBool                        mFlushTimerEverFired;
 
     /**
-     * The pref html5.flushtimer.continuedelay: Time in milliseconds between
-     * the return to non-speculating more and the first time the flush timer
-     * fires thereafter.
+     * The pref html5.flushtimer.initialdelay: Time in milliseconds between
+     * the time a network buffer is seen and the timer firing when the
+     * timer hasn't fired previously in this parse.
      */
-    static PRInt32                sTimerContinueDelay;
+    static PRInt32                sTimerInitialDelay;
 
     /**
-     * The pref html5.flushtimer.interval: Time in milliseconds between
-     * timer firings once the timer has starting firing.
+     * The pref html5.flushtimer.subsequentdelay: Time in milliseconds between
+     * the time a network buffer is seen and the timer firing when the
+     * timer has already fired previously in this parse.
      */
-    static PRInt32                sTimerInterval;
+    static PRInt32                sTimerSubsequentDelay;
 };
 
 #endif // nsHtml5StreamParser_h__