Bug 1336988: Correctly handle dead callback objects when iterating over event listeners. r=peterv
authorKris Maglione <maglione.k@gmail.com>
Mon, 06 Feb 2017 11:34:56 -0800
changeset 342540 db46b66a8c3b0fe571dad420652531d5b548c538
parent 342539 94a2ac4a5806ed8549885866537f813d8af9cb96
child 342541 3bfcb88ae88d2372fc8dd14203194cc965652512
push id37313
push usermaglione.k@gmail.com
push dateMon, 13 Feb 2017 18:40:00 +0000
treeherderautoland@db46b66a8c3b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspeterv
bugs1336988
milestone54.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1336988: Correctly handle dead callback objects when iterating over event listeners. r=peterv MozReview-Commit-ID: 5vGlPL1p3uh
dom/bindings/CallbackObject.cpp
dom/bindings/CallbackObject.h
dom/events/EventListenerManager.cpp
js/xpconnect/tests/unit/test_nuke_sandbox_event_listeners.js
--- a/dom/bindings/CallbackObject.cpp
+++ b/dom/bindings/CallbackObject.cpp
@@ -360,16 +360,19 @@ CallbackObjectHolderBase::ToXPCOMCallbac
 
   // We don't init the AutoJSAPI with our callback because we don't want it
   // reporting errors to its global's onerror handlers.
   AutoJSAPI jsapi;
   jsapi.Init();
   JSContext* cx = jsapi.cx();
 
   JS::Rooted<JSObject*> callback(cx, aCallback->CallbackOrNull());
+  if (!callback) {
+    return nullptr;
+  }
 
   JSAutoCompartment ac(cx, callback);
   RefPtr<nsXPCWrappedJS> wrappedJS;
   nsresult rv =
     nsXPCWrappedJS::GetNewOrUsed(callback, aIID, getter_AddRefs(wrappedJS));
   if (NS_FAILED(rv) || !wrappedJS) {
     return nullptr;
   }
--- a/dom/bindings/CallbackObject.h
+++ b/dom/bindings/CallbackObject.h
@@ -506,18 +506,21 @@ public:
     if (!HasWebIDLCallback()) {
       RefPtr<XPCOMCallbackT> callback = GetXPCOMCallback();
       return callback.forget();
     }
 
     nsCOMPtr<nsISupports> supp =
       CallbackObjectHolderBase::ToXPCOMCallback(GetWebIDLCallback(),
                                                 NS_GET_TEMPLATE_IID(XPCOMCallbackT));
-    // ToXPCOMCallback already did the right QI for us.
-    return supp.forget().downcast<XPCOMCallbackT>();
+    if (supp) {
+      // ToXPCOMCallback already did the right QI for us.
+      return supp.forget().downcast<XPCOMCallbackT>();
+    }
+    return nullptr;
   }
 
   // Try to return a WebIDLCallbackT version of this object.
   already_AddRefed<WebIDLCallbackT> ToWebIDLCallback() const
   {
     if (HasWebIDLCallback()) {
       RefPtr<WebIDLCallbackT> callback = GetWebIDLCallback();
       return callback.forget();
--- a/dom/events/EventListenerManager.cpp
+++ b/dom/events/EventListenerManager.cpp
@@ -1567,20 +1567,26 @@ EventListenerManager::GetListenerInfo(ns
                                   nullptr);
     }
     nsAutoString eventType;
     if (listener.mAllEvents) {
       eventType.SetIsVoid(true);
     } else {
       eventType.Assign(Substring(nsDependentAtomString(listener.mTypeAtom), 2));
     }
+    nsCOMPtr<nsIDOMEventListener> callback = listener.mListener.ToXPCOMCallback();
+    if (!callback) {
+      // This will be null for cross-compartment event listeners which have been
+      // destroyed.
+      continue;
+    }
     // EventListenerInfo is defined in XPCOM, so we have to go ahead
     // and convert to an XPCOM callback here...
     RefPtr<EventListenerInfo> info =
-      new EventListenerInfo(eventType, listener.mListener.ToXPCOMCallback(),
+      new EventListenerInfo(eventType, callback.forget(),
                             listener.mFlags.mCapture,
                             listener.mFlags.mAllowUntrustedEvents,
                             listener.mFlags.mInSystemGroup);
     aList->AppendElement(info.forget());
   }
   return NS_OK;
 }
 
--- a/js/xpconnect/tests/unit/test_nuke_sandbox_event_listeners.js
+++ b/js/xpconnect/tests/unit/test_nuke_sandbox_event_listeners.js
@@ -62,20 +62,24 @@ add_task(function*() {
   fromTestPromise = promiseEvent(window, "FromTest");
   window.dispatchEvent(new window.CustomEvent("FromTest"));
   yield fromTestPromise;
   do_print("Got event from test");
 
 
   // Force cycle collection, which should cause our callback reference
   // to be dropped, and dredge up potential issues there.
+  Cu.forceGC();
   Cu.forceCC();
 
 
   do_print("Dispatch FromTest event");
   fromTestPromise = promiseEvent(window, "FromTest");
   window.dispatchEvent(new window.CustomEvent("FromTest"));
   yield fromTestPromise;
   do_print("Got event from test");
 
+  let listeners = Services.els.getListenerInfoFor(window);
+  ok(!listeners.some(info => info.type == "FromTest"),
+     "No 'FromTest' listeners returned for nuked sandbox");
 
   webnav.close();
 });