Bug 677273 - 'Crash in mozilla::dom::workers::WorkerPrivate::RescheduleTimeoutTimer'. r=sicking.
authorBen Turner <bent.mozilla@gmail.com>
Fri, 26 Aug 2011 00:34:10 -0700
changeset 75870 ca029820f74064a480b2065092e3863dfe0e49fd
parent 75869 62fd8154f8de12569f080a84d8d58e8e1ca668c0
child 75871 c72e3869695026552cad35f63523fd34e41d387d
push id3
push userfelipc@gmail.com
push dateFri, 30 Sep 2011 20:09:13 +0000
reviewerssicking
bugs677273
milestone9.0a1
Bug 677273 - 'Crash in mozilla::dom::workers::WorkerPrivate::RescheduleTimeoutTimer'. r=sicking.
dom/workers/WorkerPrivate.cpp
dom/workers/test/close_worker.js
--- a/dom/workers/WorkerPrivate.cpp
+++ b/dom/workers/WorkerPrivate.cpp
@@ -3222,105 +3222,104 @@ WorkerPrivate::RunExpiredTimeouts(JSCont
   if (mRunningExpiredTimeouts || !mTimerRunning) {
     return true;
   }
 
   NS_ASSERTION(!mTimeouts.IsEmpty(), "Should have some work to do!");
 
   bool retval = true;
 
-  TimeStamp now = TimeStamp::Now();
+  AutoPtrComparator<TimeoutInfo> comparator = GetAutoPtrComparator(mTimeouts);
+  JSObject* global = JS_GetGlobalObject(aCx);
+  JSPrincipals* principal = GetWorkerPrincipal();
 
   // We want to make sure to run *something*, even if the timer fired a little
   // early. Fudge the value of now to at least include the first timeout.
-  now = NS_MAX(now, mTimeouts[0]->mTargetTime);
+  const TimeStamp now = NS_MAX(TimeStamp::Now(), mTimeouts[0]->mTargetTime);
 
   nsAutoTArray<TimeoutInfo*, 10> expiredTimeouts;
   for (PRUint32 index = 0; index < mTimeouts.Length(); index++) {
     nsAutoPtr<TimeoutInfo>& info = mTimeouts[index];
-    if (info->mTargetTime <= now || info->mCanceled) {
-      expiredTimeouts.AppendElement(info);
+    if (info->mTargetTime > now) {
+      break;
     }
+    expiredTimeouts.AppendElement(info);
   }
 
   // Guard against recursion.
   mRunningExpiredTimeouts = true;
 
   // Run expired timeouts.
   for (PRUint32 index = 0; index < expiredTimeouts.Length(); index++) {
     TimeoutInfo*& info = expiredTimeouts[index];
 
     if (info->mCanceled) {
       continue;
     }
 
-    JSObject* global = JS_GetGlobalObject(aCx);
+    // Always call JS_ReportPendingException if something fails, and if
+    // JS_ReportPendingException returns false (i.e. uncatchable exception) then
+    // break out of the loop.
 
     if (JSVAL_IS_STRING(info->mTimeoutVal)) {
       JSString* expression = JSVAL_TO_STRING(info->mTimeoutVal);
 
       size_t stringLength;
       const jschar* string = JS_GetStringCharsAndLength(aCx, expression,
                                                         &stringLength);
-      if (!string) {
-        if (!JS_ReportPendingException(aCx)) {
-          retval = false;
-          break;
-        }
-        continue;
-      }
-
-      if (!JS_EvaluateUCScriptForPrincipals(aCx, global, GetWorkerPrincipal(),
-                                            string, stringLength,
-                                            info->mFilename.get(),
-                                            info->mLineNumber, nsnull)) {
-        if (!JS_ReportPendingException(aCx)) {
-          retval = false;
-          break;
-        }
-        continue;
+
+      if ((!string ||
+           !JS_EvaluateUCScriptForPrincipals(aCx, global, principal, string,
+                                             stringLength,
+                                             info->mFilename.get(),
+                                             info->mLineNumber, nsnull)) &&
+          !JS_ReportPendingException(aCx)) {
+        retval = false;
+        break;
       }
     }
     else {
       jsval rval;
       if (!JS_CallFunctionValue(aCx, global, info->mTimeoutVal,
                                 info->mExtraArgVals.Length(),
-                                info->mExtraArgVals.Elements(), &rval)) {
-        if (!JS_ReportPendingException(aCx)) {
-          retval = false;
-          break;
-        }
+                                info->mExtraArgVals.Elements(), &rval) &&
+          !JS_ReportPendingException(aCx)) {
+        retval = false;
+        break;
       }
     }
+
+    // Reschedule intervals.
+    if (info->mIsInterval) {
+      PRUint32 timeoutIndex = mTimeouts.IndexOf(info);
+      NS_ASSERTION(timeoutIndex != PRUint32(-1),
+                   "Should still be in the main list!");
+
+      mTimeouts[timeoutIndex].forget();
+      mTimeouts.RemoveElementAt(timeoutIndex);
+
+      NS_ASSERTION(!mTimeouts.Contains(info), "Shouldn't have duplicates!");
+
+      info->mTargetTime += info->mInterval;
+      mTimeouts.InsertElementSorted(info, comparator);
+    }
   }
 
   // No longer possible to be called recursively.
   mRunningExpiredTimeouts = false;
 
-  // Clean up expired and canceled timeouts, reschedule intervals.
-  for (PRUint32 index = 0; index < expiredTimeouts.Length(); index++) {
-    TimeoutInfo*& info = expiredTimeouts[index];
-    if (info->mCanceled || !info->mIsInterval) {
+  // Now remove canceled and expired timeouts from the main list.
+  for (PRUint32 index = 0; index < mTimeouts.Length(); ) {
+    nsAutoPtr<TimeoutInfo>& info = mTimeouts[index];
+    if (info->mTargetTime <= now || info->mCanceled) {
       mTimeouts.RemoveElement(info);
-      continue;
     }
-
-    PRUint32 timeoutIndex = mTimeouts.IndexOf(info);
-    NS_ASSERTION(timeoutIndex != PRUint32(-1),
-                 "Should still be in the other list!");
-
-    mTimeouts[timeoutIndex].forget();
-    mTimeouts.RemoveElementAt(timeoutIndex);
-
-    NS_ASSERTION(!mTimeouts.Contains(info), "Shouldn't have duplicates!");
-
-    AutoPtrComparator<TimeoutInfo> comparator = GetAutoPtrComparator(mTimeouts);
-
-    info->mTargetTime += info->mInterval;
-    mTimeouts.InsertElementSorted(info, comparator);
+    else {
+      index++;
+    }
   }
 
   // Signal the parent that we're no longer using timeouts or reschedule the
   // timer.
   if (mTimeouts.IsEmpty()) {
     if (!ModifyBusyCountFromWorker(aCx, false)) {
       retval = false;
     }
--- a/dom/workers/test/close_worker.js
+++ b/dom/workers/test/close_worker.js
@@ -1,9 +1,14 @@
 /**
  * Any copyright is dedicated to the Public Domain.
  * http://creativecommons.org/publicdomain/zero/1.0/
  */
 onclose = function() {
   postMessage("closed");
 };
 
-setTimeout(function() { close(); }, 1000);
+setTimeout(function () {
+  setTimeout(function () {
+    throw new Error("I should never run!");
+  }, 1000);
+  close();
+}, 1000);