Bug 1336988: Correctly handle dead callback objects when iterating over event listeners. r=peterv a=gchang
authorKris Maglione <maglione.k@gmail.com>
Mon, 06 Feb 2017 11:34:56 -0800
changeset 376176 8750d50052adc918390f6ff5ce5f5aa793e028ea
parent 376175 37e8200f6c4b5f27334892d9d5f5ba897385bb7d
child 376177 3e192f30cabc4b71e2bafb971d6a4ab952388158
push id6996
push userjlorenzo@mozilla.com
push dateMon, 06 Mar 2017 20:48:21 +0000
treeherdermozilla-beta@d89512dab048 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspeterv, gchang
bugs1336988
milestone53.0a2
Bug 1336988: Correctly handle dead callback objects when iterating over event listeners. r=peterv a=gchang 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
@@ -1588,20 +1588,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();
 });