Bug 547399 - 'Workers: Don't let worker messages run if the worker is suspended'. r+sr=jst, a=beltzner.
authorBen Turner <bent.mozilla@gmail.com>
Mon, 05 Apr 2010 15:36:00 -0700
changeset 33801 9a9c3d0b69e8
parent 33800 c2a5d27c0c5b
child 33802 a138c2d30a58
push id1200
push userbturner@mozilla.com
push dateMon, 05 Apr 2010 22:36:11 +0000
reviewersbeltzner
bugs547399
milestone1.9.2.4pre
Bug 547399 - 'Workers: Don't let worker messages run if the worker is suspended'. r+sr=jst, a=beltzner.
dom/src/threads/nsDOMWorker.cpp
dom/src/threads/nsDOMWorker.h
dom/src/threads/test/Makefile.in
dom/src/threads/test/suspend_iframe.html
dom/src/threads/test/suspend_worker.js
dom/src/threads/test/test_closeOnGC.html
dom/src/threads/test/test_suspend.html
dom/src/threads/test/test_terminate.html
--- a/dom/src/threads/nsDOMWorker.cpp
+++ b/dom/src/threads/nsDOMWorker.cpp
@@ -930,16 +930,27 @@ public:
       nsDOMWorker* targetWorker = mToInner ? mWorker.get() : mWorker->mParent;
       NS_ASSERTION(currentWorker == targetWorker, "Wrong worker!");
     }
 #endif
     if (mWorker->IsCanceled()) {
       return NS_ERROR_ABORT;
     }
 
+    // If the worker is suspended and we're running on the main thread then we
+    // can't actually dispatch the event yet. Instead we queue it for whenever
+    // we resume.
+    if (mWorker->IsSuspended() && NS_IsMainThread()) {
+      if (!mWorker->QueueSuspendedRunnable(this)) {
+        NS_ERROR("Out of memory?!");
+        return NS_ERROR_ABORT;
+      }
+      return NS_OK;
+    }
+
     nsCOMPtr<nsIDOMEventTarget> target = mToInner ?
       static_cast<nsDOMWorkerMessageHandler*>(mWorker->GetInnerScope()) :
       static_cast<nsDOMWorkerMessageHandler*>(mWorker);
 
     NS_ASSERTION(target, "Null target!");
     NS_ENSURE_TRUE(target, NS_ERROR_FAILURE);
 
     mEvent->SetTarget(target);
@@ -1040,16 +1051,17 @@ nsDOMWorker::~nsDOMWorker()
     mPool->NoteDyingWorker(this);
   }
 
   if (mLock) {
     nsAutoLock::DestroyLock(mLock);
   }
 
   NS_ASSERTION(!mFeatures.Length(), "Live features!");
+  NS_ASSERTION(!mQueuedRunnables.Length(), "Events that never ran!");
 
   nsCOMPtr<nsIThread> mainThread;
   NS_GetMainThread(getter_AddRefs(mainThread));
 
   nsIPrincipal* principal;
   mPrincipal.forget(&principal);
   if (principal) {
     NS_ProxyRelease(mainThread, principal, PR_FALSE);
@@ -1344,16 +1356,19 @@ nsDOMWorker::Kill()
     mFeatures.Clear();
   }
 
   count = features.Length();
   for (index = 0; index < count; index++) {
     features[index]->Cancel();
   }
 
+  // Make sure we kill any queued runnables that we never had a chance to run.
+  mQueuedRunnables.Clear();
+
   // We no longer need to keep our inner scope.
   mInnerScope = nsnull;
   mScopeWN = nsnull;
   mGlobal = NULL;
 
   // And we can let our parent die now too.
   mParent = nsnull;
   mParentWN = nsnull;
@@ -1395,22 +1410,39 @@ nsDOMWorker::Resume()
 #endif
     shouldResumeFeatures = mSuspended;
     mSuspended = PR_FALSE;
   }
 
   if (shouldResumeFeatures) {
     ResumeFeatures();
   }
+
+  // Repost any events that were queued for the main thread while suspended.
+  PRUint32 count = mQueuedRunnables.Length();
+  for (PRUint32 index = 0; index < count; index++) {
+    NS_DispatchToCurrentThread(mQueuedRunnables[index]);
+  }
+  mQueuedRunnables.Clear();
 }
 
 PRBool
 nsDOMWorker::IsCanceled()
 {
   nsAutoLock lock(mLock);
+  return IsCanceledNoLock();
+}
+
+PRBool
+nsDOMWorker::IsCanceledNoLock()
+{
+  // If we haven't started the close process then we're not canceled.
+  if (mStatus == eRunning) {
+    return PR_FALSE;
+  }
 
   // There are several conditions under which we want JS code to abort and all
   // other functions to bail:
   // 1. If we've already run our close handler then we are canceled forevermore.
   // 2. If we've been terminated then we want to pretend to be canceled until
   //    our close handler is scheduled and running.
   // 3. If we've been canceled then we pretend to be canceled until the close
   //    handler has been scheduled.
@@ -1929,16 +1961,23 @@ nsDOMWorker::SetExpirationTime(PRInterva
 PRIntervalTime
 nsDOMWorker::GetExpirationTime()
 {
   nsAutoLock lock(mLock);
   return mExpirationTime;
 }
 #endif
 
+PRBool
+nsDOMWorker::QueueSuspendedRunnable(nsIRunnable* aRunnable)
+{
+  NS_ASSERTION(NS_IsMainThread(), "Wrong thread!");
+  return mQueuedRunnables.AppendElement(aRunnable) ? PR_TRUE : PR_FALSE;
+}
+
 NS_IMETHODIMP
 nsDOMWorker::AddEventListener(const nsAString& aType,
                               nsIDOMEventListener* aListener,
                               PRBool aUseCapture)
 {
   NS_ASSERTION(mWrappedNative, "Called after Finalize!");
   if (IsCanceled()) {
     return NS_OK;
@@ -1960,18 +1999,28 @@ nsDOMWorker::RemoveEventListener(const n
   return nsDOMWorkerMessageHandler::RemoveEventListener(aType, aListener,
                                                         aUseCapture);
 }
 
 NS_IMETHODIMP
 nsDOMWorker::DispatchEvent(nsIDOMEvent* aEvent,
                            PRBool* _retval)
 {
-  if (IsCanceled()) {
-    return NS_OK;
+  {
+    nsAutoLock lock(mLock);
+    if (IsCanceledNoLock()) {
+      return NS_OK;
+    }
+    if (mStatus == eTerminated) {
+      nsCOMPtr<nsIWorkerMessageEvent> messageEvent(do_QueryInterface(aEvent));
+      if (messageEvent) {
+        // This is a message event targeted to a terminated worker. Ignore it.
+        return NS_OK;
+      }
+    }
   }
 
   return nsDOMWorkerMessageHandler::DispatchEvent(aEvent, _retval);
 }
 
 /**
  * See nsIWorker
  */
--- a/dom/src/threads/nsDOMWorker.h
+++ b/dom/src/threads/nsDOMWorker.h
@@ -59,16 +59,17 @@ class nsDOMWorker;
 class nsDOMWorkerFeature;
 class nsDOMWorkerMessageHandler;
 class nsDOMWorkerNavigator;
 class nsDOMWorkerPool;
 class nsDOMWorkerTimeout;
 class nsICancelable;
 class nsIDOMEventListener;
 class nsIEventTarget;
+class nsIRunnable;
 class nsIScriptGlobalObject;
 class nsIXPConnectWrappedNative;
 
 class nsDOMWorkerScope : public nsDOMWorkerMessageHandler,
                          public nsIWorkerScope,
                          public nsIXPCScriptable
 {
   friend class nsDOMWorker;
@@ -107,27 +108,23 @@ class nsDOMWorker : public nsDOMWorkerMe
   friend class nsDOMWorkerFeature;
   friend class nsDOMWorkerFunctions;
   friend class nsDOMWorkerScope;
   friend class nsDOMWorkerScriptLoader;
   friend class nsDOMWorkerTimeout;
   friend class nsDOMWorkerXHR;
   friend class nsDOMWorkerXHRProxy;
   friend class nsReportErrorRunnable;
+  friend class nsDOMFireEventRunnable;
 
   friend JSBool DOMWorkerOperationCallback(JSContext* aCx);
   friend void DOMWorkerErrorReporter(JSContext* aCx,
                                      const char* aMessage,
                                      JSErrorReport* aReport);
 
-#ifdef DEBUG
-  // For fun assertions.
-  friend class nsDOMFireEventRunnable;
-#endif
-
 public:
   NS_DECL_ISUPPORTS_INHERITED
   NS_DECL_NSIDOMEVENTTARGET
   NS_DECL_NSIABSTRACTWORKER
   NS_DECL_NSIWORKER
   NS_DECL_NSITIMERCALLBACK
   NS_DECL_NSIXPCSCRIPTABLE
 
@@ -148,17 +145,19 @@ public:
                               PRUint32 aArgc,
                               jsval* aArgv);
 
   void Cancel();
   void Kill();
   void Suspend();
   void Resume();
 
+  // This just calls IsCanceledNoLock with an autolock around the call.
   PRBool IsCanceled();
+
   PRBool IsClosing();
   PRBool IsSuspended();
 
   PRBool SetGlobalForContext(JSContext* aCx);
 
   void SetPool(nsDOMWorkerPool* aPool);
 
   nsDOMWorkerPool* Pool() {
@@ -271,16 +270,22 @@ private:
   nsresult Close();
 
   nsresult TerminateInternal(PRBool aFromFinalize);
 
   nsIWorkerLocation* GetLocation() {
     return mLocation;
   }
 
+  PRBool QueueSuspendedRunnable(nsIRunnable* aRunnable);
+
+  // Determines if the worker should be considered "canceled". See the large
+  // comment in the implementation for more details.
+  PRBool IsCanceledNoLock();
+
 private:
 
   // mParent will live as long as mParentWN but only mParentWN will keep the JS
   // reflection alive, so we only hold one strong reference to mParentWN.
   nsDOMWorker* mParent;
   nsCOMPtr<nsIXPConnectWrappedNative> mParentWN;
 
   PRLock* mLock;
@@ -310,16 +315,18 @@ private:
 
   // Always protected by mLock
   PRIntervalTime mExpirationTime;
 
   nsCOMPtr<nsITimer> mKillTimer;
 
   nsCOMPtr<nsIWorkerLocation> mLocation;
 
+  nsTArray<nsCOMPtr<nsIRunnable> > mQueuedRunnables;
+
   PRPackedBool mSuspended;
   PRPackedBool mCompileAttempted;
 };
 
 /**
  * A worker "feature" holds the worker alive yet can be canceled, paused, and
  * resumed by the worker. It is up to each derived class to implement these
  * methods. This class uses a custom implementation of Release in order to
--- a/dom/src/threads/test/Makefile.in
+++ b/dom/src/threads/test/Makefile.in
@@ -77,16 +77,19 @@ include $(topsrcdir)/config/rules.mk
   test_regExpStatics.html \
   regExpStatics_worker.js \
   test_relativeLoad.html \
   relativeLoad_worker.js \
   relativeLoad_worker2.js \
   relativeLoad_import.js \
   test_scopeOnerror.html \
   scopeOnerror_worker.js \
+  test_suspend.html \
+  suspend_iframe.html \
+  suspend_worker.js \
   test_simpleThread.html \
   simpleThread_worker.js \
   test_terminate.html \
   terminate_worker.js \
   test_threadErrors.html \
   threadErrors_worker1.js \
   threadErrors_worker2.js \
   threadErrors_worker3.js \
new file mode 100644
--- /dev/null
+++ b/dom/src/threads/test/suspend_iframe.html
@@ -0,0 +1,40 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+  <title>Test for DOM Worker Threads Suspending</title>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
+</head>
+<body>
+<pre id="test">
+<div id="output"></div>
+<script class="testbody" type="text/javascript">
+
+  var output = document.getElementById("output");
+
+  var worker;
+
+  function terminateWorker() {
+    if (worker) {
+      worker.terminate();
+      worker = null;
+    }
+  }
+
+  function startWorker(messageCallback, errorCallback) {
+    worker = new Worker("suspend_worker.js");
+
+    worker.onmessage = function(event) {
+      output.textContent = output.textContent + event.data + "\n";
+      messageCallback(event.data);
+    };
+
+    worker.onerror = function(event) {
+      this.terminate();
+      errorCallback(event.message);
+    };
+  }
+
+</script>
+</pre>
+</body>
+</html>
new file mode 100644
--- /dev/null
+++ b/dom/src/threads/test/suspend_worker.js
@@ -0,0 +1,5 @@
+var counter = 0;
+
+setInterval(function() {
+  postMessage(++counter);
+}, 100);
--- a/dom/src/threads/test/test_closeOnGC.html
+++ b/dom/src/threads/test/test_closeOnGC.html
@@ -21,17 +21,16 @@
 
   var worker = new Worker("closeOnGC_worker.js");
   worker.onmessage = function(event) {
     is(event.data, "ready");
     worker = null;
   }
 
   var interval = setInterval(function() {
-    dump("xxxben interval\n");
     var xhr = new XMLHttpRequest();
     xhr.open("GET", "closeOnGC_server.sjs", false);
     xhr.send();
     if (xhr.responseText != "closed") {
       CC();
       return;
     }
     clearInterval(interval);
new file mode 100644
--- /dev/null
+++ b/dom/src/threads/test/test_suspend.html
@@ -0,0 +1,140 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+  <title>Test for DOM Worker Threads</title>
+  <script type="text/javascript" src="/MochiKit/packed.js"></script>
+  <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
+</head>
+<body>
+<p id="display"></p>
+<div id="content" style="display: none"></div>
+<pre id="test">
+<iframe id="workerFrame" src="suspend_iframe.html" onload="subframeLoaded();">
+</iframe>
+<script class="testbody" type="text/javascript">
+
+  SimpleTest.waitForExplicitFinish();
+
+  var iframe;
+  var lastCount;
+
+  var suspended = false;
+  var resumed = false;
+  var finished = false;
+
+  var interval;
+  var oldMessageCount;
+  var waitCount = 0;
+
+  function setCachePref(enabled) {
+    netscape.security.PrivilegeManager.enablePrivilege('UniversalXPConnect');
+    var prefBranch = Components.classes["@mozilla.org/preferences-service;1"]
+                               .getService(Components.interfaces.nsIPrefBranch);
+    if (enabled) {
+      prefBranch.setBoolPref("browser.sessionhistory.cache_subframes", true);
+    }
+    else {
+      try {
+        prefBranch.clearUserPref("browser.sessionhistory.cache_subframes");
+      } catch (e) { /* Pref didn't exist, meh */ }
+    }
+  }
+
+  function finishTest() {
+    if (finished) {
+      return;
+    }
+    finished = true;
+    setCachePref(false);
+    iframe.terminateWorker();
+    SimpleTest.finish();
+  }
+
+  function waitInterval() {
+    if (finished) {
+      return;
+    }
+    is(iframe.location, "about:blank", "Wrong url!");
+    is(suspended, true, "Not suspended?");
+    is(resumed, false, "Already resumed?!");
+    is(lastCount, oldMessageCount, "Received a message while suspended!");
+    if (++waitCount == 5) {
+      clearInterval(interval);
+      resumed = true;
+      iframe.history.back();
+    }
+  }
+
+  function badOnloadCallback() {
+    if (finished) {
+      return;
+    }
+    ok(false, "shouldn't get here!");
+    finishTest();
+  }
+
+  function suspendCallback() {
+    if (finished) {
+      return;
+    }
+    is(iframe.location, "about:blank", "Wrong url!");
+    is(suspended, false, "Already suspended?");
+    is(resumed, false, "Already resumed?");
+    setCachePref(false);
+    suspended = true;
+    iframe.onload = badOnloadCallback;
+    oldMessageCount = lastCount;
+    interval = setInterval(waitInterval, 1000);
+  }
+
+  function messageCallback(data) {
+    if (finished) {
+      return;
+    }
+
+    if (!suspended) {
+      ok(lastCount === undefined || lastCount == data - 1,
+         "Data is inconsistent");
+      lastCount = data;
+      if (lastCount == 50) {
+        setCachePref(true);
+        iframe.location = "about:blank";
+      }
+      return;
+    }
+
+    var newLocation =
+      window.location.toString().replace("test_suspend.html",
+                                         "suspend_iframe.html");
+    is(newLocation.indexOf(iframe.location.toString()), 0, "Wrong url!");
+    is(resumed, true, "Got message before resumed!");
+    is(lastCount, data - 1, "Missed a message, suspend failed!");
+    finishTest();
+  }
+
+  function errorCallback(data) {
+    if (finished) {
+      return;
+    }
+    ok(false, "Iframe had an error: '" + data + "'");
+    finishTest();
+  }
+
+  function subframeLoaded() {
+    if (finished) {
+      return;
+    }
+    var iframeElement = document.getElementById("workerFrame");
+    iframeElement.onload = suspendCallback;
+
+    iframe = iframeElement.contentWindow;
+    ok(iframe, "No iframe?!");
+
+    iframe.startWorker(messageCallback, errorCallback);
+  }
+
+</script>
+</pre>
+</body>
+</html>
--- a/dom/src/threads/test/test_terminate.html
+++ b/dom/src/threads/test/test_terminate.html
@@ -12,42 +12,36 @@ Tests of DOM Worker terminate feature
 <body>
 <p id="display"></p>
 <div id="content" style="display: none">
 
 </div>
 <pre id="test">
 <script class="testbody" language="javascript">
 
-  var worker = new Worker("terminate_worker.js");
 
-  var count = 0;
+  var messageCount = 0;
+  var intervalCount = 0;
+
   var interval;
 
-  function maybeFinish() {
-    if (count) {
-      count = 0;
-      return;
+  function testCount() {
+    is(messageCount, 21, "Received another message after terminated!");
+    if (intervalCount++ == 5) {
+      clearInterval(interval);
+      SimpleTest.finish();
     }
-
-    clearInterval(interval);
-    ok(true, "no more messages");
-    SimpleTest.finish();
   }
 
+  var worker = new Worker("terminate_worker.js");
   worker.onmessage = function(event) {
-    if (event.data == "Still alive!") {
-      count++;
-      if (!interval && count == 20) {
-        worker.terminate();
-      }
-    }
-    else if (event.data == "Closed!") {
-      count = 0;
-      interval = setInterval(maybeFinish, 500);
+    is(event.data, "Still alive!", "Bad message!");
+    if (messageCount++ == 20) {
+      worker.terminate();
+      interval = setInterval(testCount, 1000);
     }
   };
 
   worker.onerror = function(event) {
     ok(false, "Worker had an error: " + event.data);
     SimpleTest.finish();
   }