Bug 683885 - 'Assertion failure: self, at dom/workers/EventTarget.cpp:170'. r=sicking.
☠☠ backed out by cc0753a23f8b ☠ ☠
authorBen Turner <bent.mozilla@gmail.com>
Thu, 08 Sep 2011 17:04:57 -0700
changeset 76797 c5a9a439d72ca0811d977e0c7103b96ba14c8289
parent 76796 58d026601240cec8a26acc78a7cfdb6dc0bb55ae
child 76798 cfdf675266b2e1f81664dcef4e6fe30cb7ba43d2
push id3
push userfelipc@gmail.com
push dateFri, 30 Sep 2011 20:09:13 +0000
reviewerssicking
bugs683885
milestone9.0a1
Bug 683885 - 'Assertion failure: self, at dom/workers/EventTarget.cpp:170'. r=sicking.
dom/workers/EventTarget.cpp
dom/workers/Worker.cpp
dom/workers/Worker.h
dom/workers/WorkerPrivate.cpp
dom/workers/test/test_terminate.html
--- a/dom/workers/EventTarget.cpp
+++ b/dom/workers/EventTarget.cpp
@@ -136,17 +136,19 @@ EventTarget::FromJSObject(JSContext* aCx
 
 // static
 JSBool
 EventTarget::AddEventListener(JSContext* aCx, uintN aArgc, jsval* aVp)
 {
   JSObject* obj = JS_THIS_OBJECT(aCx, aVp);
 
   EventTarget* self = GetPrivate(aCx, obj);
-  JS_ASSERT(self);
+  if (!self) {
+    return true;
+  }
 
   JSString* type;
   JSObject* listener;
   JSBool capturing = false, wantsUntrusted = false;
   if (!JS_ConvertArguments(aCx, aArgc, JS_ARGV(aCx, aVp), "So/bb", &type,
                            &listener, &capturing, &wantsUntrusted)) {
     return false;
   }
@@ -162,17 +164,19 @@ EventTarget::AddEventListener(JSContext*
 
 // static
 JSBool
 EventTarget::RemoveEventListener(JSContext* aCx, uintN aArgc, jsval* aVp)
 {
   JSObject* obj = JS_THIS_OBJECT(aCx, aVp);
 
   EventTarget* self = GetPrivate(aCx, obj);
-  JS_ASSERT(self);
+  if (!self) {
+    return true;
+  }
 
   JSString* type;
   JSObject* listener;
   JSBool capturing = false;
   if (!JS_ConvertArguments(aCx, aArgc, JS_ARGV(aCx, aVp), "So/b", &type,
                            &listener, &capturing)) {
     return false;
   }
@@ -188,17 +192,19 @@ EventTarget::RemoveEventListener(JSConte
 
 // static
 JSBool
 EventTarget::DispatchEvent(JSContext* aCx, uintN aArgc, jsval* aVp)
 {
   JSObject* obj = JS_THIS_OBJECT(aCx, aVp);
 
   EventTarget* self = GetPrivate(aCx, obj);
-  JS_ASSERT(self);
+  if (!self) {
+    return true;
+  }
 
   JSObject* event;
   if (!JS_ConvertArguments(aCx, aArgc, JS_ARGV(aCx, aVp), "o", &event)) {
     return false;
   }
 
   bool preventDefaultCalled;
   if (!self->mListenerManager.DispatchEvent(aCx, obj, event,
--- a/dom/workers/Worker.cpp
+++ b/dom/workers/Worker.cpp
@@ -107,16 +107,37 @@ public:
                               PRIVATE_TO_JSVAL(parent))) {
         return NULL;
       }
     }
 
     return proto;
   }
 
+  static void
+  ClearPrivateSlot(JSContext* aCx, JSObject* aObj)
+  {
+    JS_ASSERT(!JS_IsExceptionPending(aCx));
+
+    WorkerPrivate* worker = GetJSPrivateSafeish<WorkerPrivate>(aCx, aObj);
+    JS_ASSERT(worker);
+
+    for (int index = 0; index < STRING_COUNT; index++) {
+      const char* name = sEventStrings[index];
+      jsval listener;
+      if (!worker->GetEventListenerOnEventTarget(aCx, name + 2, &listener) ||
+          !JS_DefineProperty(aCx, aObj, name, listener, NULL, NULL,
+                             (PROPERTY_FLAGS & ~JSPROP_SHARED))) {
+        JS_ClearPendingException(aCx);
+      }
+    }
+
+    SetJSPrivateSafeish(aCx, aObj, NULL);
+  }
+
 protected:
   static WorkerPrivate*
   GetInstancePrivate(JSContext* aCx, JSObject* aObj, const char* aFunctionName);
 
   static JSBool
   ConstructInternal(JSContext* aCx, uintN aArgc, jsval* aVp,
                     bool aIsChromeWorker)
   {
@@ -332,16 +353,22 @@ public:
                               PRIVATE_TO_JSVAL(parent))) {
         return NULL;
       }
     }
 
     return proto;
   }
 
+  static void
+  ClearPrivateSlot(JSContext* aCx, JSObject* aObj)
+  {
+    Worker::ClearPrivateSlot(aCx, aObj);
+  }
+
 private:
   // No instance of this class should ever be created so these are explicitly
   // left without an implementation to prevent linking in case someone tries to
   // make one.
   ChromeWorker();
   ~ChromeWorker();
 
   static WorkerPrivate*
@@ -420,16 +447,30 @@ namespace worker {
 
 JSObject*
 InitClass(JSContext* aCx, JSObject* aGlobal, JSObject* aProto,
           bool aMainRuntime)
 {
   return Worker::InitClass(aCx, aGlobal, aProto, aMainRuntime);
 }
 
+void
+ClearPrivateSlot(JSContext* aCx, JSObject* aObj)
+{
+  JSClass* clasp = JS_GET_CLASS(aCx, aObj);
+  JS_ASSERT(clasp == Worker::Class() || clasp == ChromeWorker::Class());
+
+  if (clasp == ChromeWorker::Class()) {
+    ChromeWorker::ClearPrivateSlot(aCx, aObj);
+  }
+  else {
+    Worker::ClearPrivateSlot(aCx, aObj);
+  }
+}
+
 } // namespace worker
 
 namespace chromeworker {
 
 bool
 InitClass(JSContext* aCx, JSObject* aGlobal, JSObject* aProto,
           bool aMainRuntime)
 {
--- a/dom/workers/Worker.h
+++ b/dom/workers/Worker.h
@@ -46,16 +46,19 @@
 BEGIN_WORKERS_NAMESPACE
 
 namespace worker {
 
 JSObject*
 InitClass(JSContext* aCx, JSObject* aGlobal, JSObject* aProto,
           bool aMainRuntime);
 
+void
+ClearPrivateSlot(JSContext* aCx, JSObject* aObj);
+
 } // namespace worker
 
 namespace chromeworker {
 
 bool
 InitClass(JSContext* aCx, JSObject* aGlobal, JSObject* aProto,
           bool aMainRuntime);
 
--- a/dom/workers/WorkerPrivate.cpp
+++ b/dom/workers/WorkerPrivate.cpp
@@ -71,16 +71,17 @@
 #include "xpcpublic.h"
 
 #include "Events.h"
 #include "Exceptions.h"
 #include "File.h"
 #include "Principal.h"
 #include "RuntimeService.h"
 #include "ScriptLoader.h"
+#include "Worker.h"
 #include "WorkerFeature.h"
 #include "WorkerScope.h"
 
 #include "WorkerInlines.h"
 
 #if 0 // Define to run GC more often.
 #define EXTRA_GC
 #endif
@@ -1868,17 +1869,17 @@ WorkerPrivateParent<Derived>::Resume(JSC
 template <class Derived>
 void
 WorkerPrivateParent<Derived>::FinalizeInstance(JSContext* aCx)
 {
   AssertIsOnParentThread();
 
   if (mJSObject) {
     // Decouple the object from the private now.
-    SetJSPrivateSafeish(aCx, mJSObject, nsnull);
+    worker::ClearPrivateSlot(aCx, mJSObject);
 
     // Clear the JS object.
     mJSObject = nsnull;
 
     // Unroot.
     RootJSObject(aCx, false);
 
     if (!Terminate(aCx)) {
--- a/dom/workers/test/test_terminate.html
+++ b/dom/workers/test/test_terminate.html
@@ -21,36 +21,62 @@ Tests of DOM Worker terminate feature
 <script class="testbody" language="javascript">
 
 
   var messageCount = 0;
   var intervalCount = 0;
 
   var interval;
 
+  var worker;
+
+  function messageListener(event) {
+    is(event.data, "Still alive!", "Correct message!");
+    if (messageCount++ == 20) {
+      ok(worker.onmessage === messageListener,
+         "Correct listener before terminate");
+
+      worker.terminate();
+
+      var exception = false;
+      try {
+        worker.addEventListener("message", messageListener, false);
+      }
+      catch (e) {
+        exception = true;
+      }
+      is(exception, false, "addEventListener didn't throw after terminate");
+
+      exception = false;
+      try {
+        worker.removeEventListener("message", messageListener, false);
+      }
+      catch (e) {
+        exception = true;
+      }
+      is(exception, false, "removeEventListener didn't throw after terminate");
+
+      ok(worker.onmessage === messageListener,
+         "Correct listener after terminate");
+
+      worker.onmessage = function(event) { }
+
+      interval = setInterval(testCount, 1000);
+    }
+  }
+
   function testCount() {
     is(messageCount, 21, "Received another message after terminated!");
     if (intervalCount++ == 5) {
       clearInterval(interval);
       SimpleTest.finish();
     }
   }
 
-  var worker = new Worker("terminate_worker.js");
-  worker.onmessage = function(event) {
-    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();
-  }
+  worker = new Worker("terminate_worker.js");
+  worker.onmessage = messageListener;
 
   SimpleTest.waitForExplicitFinish();
 
 </script>
 </pre>
 </body>
 </html>