From 7662d973da80f41eee8e0d43eb080b21182e973e Mon Sep 17 00:00:00 2001
From: Ben Turner <bent.mozilla@gmail.com>
Date: Mon, 15 Mar 2010 13:12:40 -0700
Subject: [PATCH] Bug 552054 - 'Fix random orange in test_suspend.html, make
 Worker.terminate() block any previously queued message events'. r=sicking.

---
 dom/src/threads/nsDOMWorker.cpp          | 24 +++++++++++-
 dom/src/threads/nsDOMWorker.h            |  2 +
 dom/src/threads/test/test_closeOnGC.html |  1 -
 dom/src/threads/test/test_suspend.html   | 48 ++++++++++++++++++------
 dom/src/threads/test/test_terminate.html | 32 +++++++---------
 5 files changed, 74 insertions(+), 33 deletions(-)

diff --git a/dom/src/threads/nsDOMWorker.cpp b/dom/src/threads/nsDOMWorker.cpp
index 48f75d9b9c8d..28d3e5dac831 100644
--- a/dom/src/threads/nsDOMWorker.cpp
+++ b/dom/src/threads/nsDOMWorker.cpp
@@ -1428,6 +1428,16 @@ 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:
@@ -1988,8 +1998,18 @@ 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);
diff --git a/dom/src/threads/nsDOMWorker.h b/dom/src/threads/nsDOMWorker.h
index fbe7c9d8b0dc..17e9f64e9a3a 100644
--- a/dom/src/threads/nsDOMWorker.h
+++ b/dom/src/threads/nsDOMWorker.h
@@ -292,6 +292,8 @@ private:
 
   PRBool QueueSuspendedRunnable(nsIRunnable* aRunnable);
 
+  PRBool IsCanceledNoLock();
+
 private:
 
   // mParent will live as long as mParentWN but only mParentWN will keep the JS
diff --git a/dom/src/threads/test/test_closeOnGC.html b/dom/src/threads/test/test_closeOnGC.html
index 79d6dfd0c85b..cbdb39dbce35 100644
--- a/dom/src/threads/test/test_closeOnGC.html
+++ b/dom/src/threads/test/test_closeOnGC.html
@@ -26,7 +26,6 @@
   }
 
   var interval = setInterval(function() {
-    dump("xxxben interval\n");
     var xhr = new XMLHttpRequest();
     xhr.open("GET", "closeOnGC_server.sjs", false);
     xhr.send();
diff --git a/dom/src/threads/test/test_suspend.html b/dom/src/threads/test/test_suspend.html
index 08285671d856..9b1f5eb9bd49 100644
--- a/dom/src/threads/test/test_suspend.html
+++ b/dom/src/threads/test/test_suspend.html
@@ -19,8 +19,9 @@
   var iframe;
   var lastCount;
 
-  var suspended;
-  var resumed;
+  var suspended = false;
+  var resumed = false;
+  var finished = false;
 
   var interval;
   var oldMessageCount;
@@ -41,14 +42,22 @@
   }
 
   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);
@@ -58,19 +67,30 @@
   }
 
   function badOnloadCallback() {
+    if (finished) {
+      return;
+    }
     ok(false, "iframe didn't go into fastback cache!");
     finishTest();
   }
 
   function suspendCallback() {
+    if (finished) {
+      return;
+    }
     is(iframe.location, "about:blank", "Wrong url!");
     is(suspended, true, "Not suspended?");
+    is(resumed, false, "Already resumed?!");
     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");
@@ -80,24 +100,30 @@
         iframe.location = "about:blank";
         suspended = true;
       }
+      return;
     }
-    else {
-      var newLocation =
-        window.location.toString().replace("test_suspend.html",
-                                           "suspend_iframe.html");
-      is(iframe.location, newLocation, "Wrong url!");
-      ok(suspended && resumed, "Got message before resumed!");
-      is(lastCount, data - 1, "Missed a message, suspend failed!");
-      finishTest();
-    }
+
+    var newLocation =
+      window.location.toString().replace("test_suspend.html",
+                                         "suspend_iframe.html");
+    is(iframe.location, newLocation, "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;
 
diff --git a/dom/src/threads/test/test_terminate.html b/dom/src/threads/test/test_terminate.html
index 2d5dc03a80de..f9c03ccf222a 100644
--- a/dom/src/threads/test/test_terminate.html
+++ b/dom/src/threads/test/test_terminate.html
@@ -17,32 +17,26 @@ Tests of DOM Worker terminate feature
 <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);
     }
   };