Bug 777385 - Support (some) Paris bindings objects as weak map keys. r=peterv

This adds support for many kinds of Paris bindings objects as weak map keys.
This patch supports nsISupports objects as well as non-cycle-collected
non-nsISupports objects. What is needed for support is to preserve any wrapper,
if the object is wrapper cached. In other cases, we don't need to do anything.

dom/bindings/BindingUtils.cpp

@@ -492,6 +492,26 @@ NativeInterface2JSObjectAndThrowIfFailed(JSContext* aCx,
   return true;
 }
 
+bool
+TryPreserveWrapper(JSObject* obj)
+{
+  nsISupports* native;
+  if (UnwrapDOMObjectToISupports(obj, native)) {
+    nsWrapperCache* cache = nullptr;
+    CallQueryInterface(native, &cache);
+    if (cache) {
+      nsContentUtils::PreserveWrapper(native, cache);
+    }
+    return true;
+  }
+
+  // If this DOMClass is not cycle collected, then it isn't wrappercached,
+  // so it does not need to be preserved. If it is cycle collected, then
+  // we can't tell if it is wrappercached or not, so we just return false.
+  const DOMClass* domClass = GetDOMClass(obj);
+  return domClass && !domClass->mParticipant;
+}
+
 // Can only be called with the immediate prototype of the instance object. Can
 // only be called on the prototype of an object known to be a DOM instance.
 JSBool

dom/bindings/BindingUtils.h

@@ -839,6 +839,17 @@ ClearWrapper(T* p, void*)
   ClearWrapper(p, cache);
 }
 
+// Attempt to preserve the wrapper, if any, for a Paris DOM bindings object.
+// Return true if we successfully preserved the wrapper, or there is no wrapper
+// to preserve. In the latter case we don't need to preserve the wrapper, because
+// the object can only be obtained by JS once, or they cannot be meaningfully
+// owned from the native side.
+//
+// This operation will return false only for non-nsISupports cycle-collected
+// objects, because we cannot determine if they are wrappercached or not.
+bool
+TryPreserveWrapper(JSObject* obj);
+
 // Can only be called with the immediate prototype of the instance object. Can
 // only be called on the prototype of an object known to be a DOM instance.
 JSBool

js/src/jsweakmap.cpp

@@ -291,7 +291,10 @@ WeakMap_set_impl(JSContext *cx, CallArgs args)
     }
 
     // Preserve wrapped native keys to prevent wrapper optimization.
-    if (key->getClass()->ext.isWrappedNative) {
+    if (key->getClass()->ext.isWrappedNative ||
+        (key->getClass()->flags & JSCLASS_IS_DOMJSCLASS) ||
+        (key->isProxy() && GetProxyHandler(key)->family() == GetListBaseHandlerFamily()))
+    {
         JS_ASSERT(cx->runtime->preserveWrapperCallback);
         if (!cx->runtime->preserveWrapperCallback(cx, key)) {
             JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL, JSMSG_BAD_WEAKMAP_KEY);

js/xpconnect/src/XPCJSRuntime.cpp

@@ -2292,18 +2292,39 @@ CompartmentNameCallback(JSRuntime *rt, JSCompartment *comp,
 
 bool XPCJSRuntime::gExperimentalBindingsEnabled;
 
-bool PreserveWrapper(JSContext *cx, JSObject *obj)
+static bool
+PreserveWrapper(JSContext *cx, JSObject *obj)
 {
-    MOZ_ASSERT(IS_WRAPPER_CLASS(js::GetObjectClass(obj)));
-    nsISupports *native = nsXPConnect::GetXPConnect()->GetNativeOfWrapper(cx, obj);
-    if (!native)
+    MOZ_ASSERT(cx);
+    MOZ_ASSERT(obj);
+    MOZ_ASSERT(js::GetObjectClass(obj)->ext.isWrappedNative ||
+               mozilla::dom::IsDOMObject(obj));
+
+    XPCCallContext ccx(NATIVE_CALLER, cx);
+    if (!ccx.IsValid())
         return false;
-    nsresult rv;
-    nsCOMPtr<nsINode> node = do_QueryInterface(native, &rv);
-    if (NS_FAILED(rv))
+
+    JSObject *obj2 = nullptr;
+    nsIXPConnectWrappedNative *wrapper =
+        XPCWrappedNative::GetWrappedNativeOfJSObject(cx, obj, nullptr, &obj2);
+    nsISupports *supports = nullptr;
+
+    if (wrapper) {
+        supports = wrapper->Native();
+    } else if (obj2) {
+        supports = static_cast<nsISupports*>(xpc_GetJSPrivate(obj2));
+    }
+
+    if (supports) {
+        // For pre-Paris DOM bindings objects, we only support Node.
+        if (nsCOMPtr<nsINode> node = do_QueryInterface(supports)) {
+            nsContentUtils::PreserveWrapper(supports, node);
+            return true;
+        }
         return false;
-    nsContentUtils::PreserveWrapper(native, node);
-    return true;
+    }
+
+    return mozilla::dom::TryPreserveWrapper(obj);
 }
 
 static nsresult

js/xpconnect/tests/chrome/Makefile.in

@@ -70,6 +70,7 @@ MOCHITEST_CHROME_FILES = \
 		test_weakmap_keys_preserved2.xul \
 		test_weakref.xul \
 		test_wrappers.xul \
+		test_paris_weakmap_keys.xul \
 		$(NULL)
 
 ifneq ($(OS_ARCH),WINNT)

js/xpconnect/tests/chrome/test_paris_weakmap_keys.xul

@@ -0,0 +1,83 @@
+<?xml version="1.0"?>
+<?xml-stylesheet type="text/css" href="chrome://global/skin"?>
+<?xml-stylesheet type="text/css" href="chrome://mochikit/content/tests/SimpleTest/test.css"?>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=777385
+-->
+<window title="Mozilla Bug 777385"
+        xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
+  <div></div>
+  <div id="mydivname"></div>
+  <script type="application/javascript" src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"/>
+
+  <!-- test results are displayed in the html:body -->
+  <body xmlns="http://www.w3.org/1999/xhtml">
+  <a href="https://bugzilla.mozilla.org/show_bug.cgi?id="
+     target="_blank">Mozilla Bug 777385</a>
+  </body>
+
+  <!-- test code goes here -->
+  <script type="application/javascript">
+  <![CDATA[
+  /** Test for Bug 777385 **/
+
+  let Cu = Components.utils;
+
+  let live_map = new WeakMap;
+
+  let get_div_style = function () {
+    return document.getElementById("mydivname").style;
+  }
+
+  let make_live_map = function () {
+    let my_div_style = get_div_style();
+    let div_fail = false;
+    try {
+      live_map.set(my_div_style, 12345);
+    } catch (e) {
+      div_fail = true;
+    }
+    ok(!div_fail, "Using elem.style as a weak map key should not produce an exception.");
+
+    is(live_map.get(get_div_style()), 12345, "Live map should have live style with right value before GC.");
+
+  }
+
+  make_live_map();
+
+
+  // AudioContext is the only Paris bindings non-nsISupports refCounted class.
+  SpecialPowers.setBoolPref("media.webaudio.enabled", true);
+
+  // non-nsISupports cycle-collected classes should fail as weak map keys.
+  let context = new mozAudioContext();
+  let contextFail = false;
+  try {
+    live_map.set(context, 2);
+  } catch (e) {
+    contextFail = true;
+  }
+
+  ok(contextFail, "Cycle collected non-nsISupports classes aren't allowed as weak map keys.");
+
+  /* Set up for running precise GC/CC then check the results. */
+
+  SimpleTest.waitForExplicitFinish();
+
+  Cu.schedulePreciseGC(function () {
+    SpecialPowers.DOMWindowUtils.cycleCollect();
+    SpecialPowers.DOMWindowUtils.garbageCollect();
+    SpecialPowers.DOMWindowUtils.garbageCollect();
+
+    is(Cu.nondeterministicGetWeakMapKeys(live_map).length, 1,
+       "Live nsISupports new DOM bindings wrappercached native weak map key should not be removed.");
+
+    is(live_map.get(get_div_style()), 12345, "Live map should have live style with right value after GC.");
+
+    SpecialPowers.clearUserPref("media.webaudio.enabled");
+    SimpleTest.finish();
+  });
+
+  ]]>
+  </script>
+</window>

js/xpconnect/tests/chrome/test_weakmaps.xul

@@ -209,6 +209,7 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=668855
   let make_live_map = function () {
     let live = get_live_dom();
     wn_live_map.set(live, {});
+    ok(wn_live_map.has(get_live_dom()), "Live map should have live DOM node before GC.");
   }
 
   make_live_map();
@@ -237,15 +238,9 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=668855
   SimpleTest.waitForExplicitFinish();
 
   Cu.schedulePreciseGC(function () {
-    window.QueryInterface(Ci.nsIInterfaceRequestor)
-         .getInterface(Ci.nsIDOMWindowUtils)
-         .cycleCollect();
-    window.QueryInterface(Ci.nsIInterfaceRequestor)
-         .getInterface(Ci.nsIDOMWindowUtils)
-         .garbageCollect();
-    window.QueryInterface(Ci.nsIInterfaceRequestor)
-         .getInterface(Ci.nsIDOMWindowUtils)
-         .garbageCollect();
+    SpecialPowers.DOMWindowUtils.cycleCollect();
+    SpecialPowers.DOMWindowUtils.garbageCollect();
+    SpecialPowers.DOMWindowUtils.garbageCollect();
 
     ok(weak_ref_dead(weakref), "Garbage gray cycle should be collected.");
 
@@ -263,6 +258,8 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=668855
     is(Cu.nondeterministicGetWeakMapKeys(wn_live_map).length, 1,
        "Live weak map wrapped native key should not be removed.");
 
+    ok(wn_live_map.has(get_live_dom()), "Live map should have live dom.");
+
     SimpleTest.finish();
   });