Bug 761620 - Throw an exception for unpreservable weak map keys. r=billm
authorAndrew McCreight <amccreight@mozilla.com>
Wed, 08 Aug 2012 11:05:58 -0700
changeset 105213 236151ae351f82468be10b7b104d20f001efb49c
parent 105212 679fcc8f57d0be8cee67365f403a071717ffb51f
child 105214 55c4a3f3a6a96597f7b9ce3d698071f3820e4abb
push id55
push usershu@rfrn.org
push dateThu, 30 Aug 2012 01:33:09 +0000
reviewersbillm
bugs761620
milestone17.0a1
Bug 761620 - Throw an exception for unpreservable weak map keys. r=billm
js/src/js.msg
js/src/jsweakmap.cpp
js/xpconnect/tests/chrome/test_weakmaps.xul
--- a/js/src/js.msg
+++ b/js/src/js.msg
@@ -112,17 +112,17 @@ MSG_DEF(JSMSG_BAD_REGEXP_FLAG,         5
 MSG_DEF(JSMSG_NO_INPUT,                59, 5, JSEXN_SYNTAXERR, "no input for /{0}/{1}{2}{3}{4}")
 MSG_DEF(JSMSG_CANT_OPEN,               60, 2, JSEXN_ERR, "can't open {0}: {1}")
 MSG_DEF(JSMSG_TOO_MANY_FUN_APPLY_ARGS, 61, 0, JSEXN_RANGEERR, "arguments array passed to Function.prototype.apply is too large")
 MSG_DEF(JSMSG_UNMATCHED_RIGHT_PAREN,   62, 0, JSEXN_SYNTAXERR, "unmatched ) in regular expression")
 MSG_DEF(JSMSG_TOO_BIG_TO_ENCODE,       63, 0, JSEXN_INTERNALERR, "data are to big to encode")
 MSG_DEF(JSMSG_ARG_INDEX_OUT_OF_RANGE,  64, 1, JSEXN_RANGEERR, "argument {0} accesses an index that is out of range")
 MSG_DEF(JSMSG_SPREAD_TOO_LARGE,        65, 0, JSEXN_RANGEERR, "array too large due to spread operand(s)")
 MSG_DEF(JSMSG_SOURCE_TOO_LONG,         66, 0, JSEXN_RANGEERR, "source is too long")
-MSG_DEF(JSMSG_UNUSED67,                67, 0, JSEXN_NONE,    "")
+MSG_DEF(JSMSG_BAD_WEAKMAP_KEY,         67, 0, JSEXN_TYPEERR, "cannot use the given object as a weak map key")
 MSG_DEF(JSMSG_BAD_SCRIPT_MAGIC,        68, 0, JSEXN_INTERNALERR, "bad script XDR magic number")
 MSG_DEF(JSMSG_PAREN_BEFORE_FORMAL,     69, 0, JSEXN_SYNTAXERR, "missing ( before formal parameters")
 MSG_DEF(JSMSG_MISSING_FORMAL,          70, 0, JSEXN_SYNTAXERR, "missing formal parameter")
 MSG_DEF(JSMSG_PAREN_AFTER_FORMAL,      71, 0, JSEXN_SYNTAXERR, "missing ) after formal parameters")
 MSG_DEF(JSMSG_CURLY_BEFORE_BODY,       72, 0, JSEXN_SYNTAXERR, "missing { before function body")
 MSG_DEF(JSMSG_CURLY_AFTER_BODY,        73, 0, JSEXN_SYNTAXERR, "missing } after function body")
 MSG_DEF(JSMSG_PAREN_BEFORE_COND,       74, 0, JSEXN_SYNTAXERR, "missing ( before condition")
 MSG_DEF(JSMSG_PAREN_AFTER_COND,        75, 0, JSEXN_SYNTAXERR, "missing ) after condition")
--- a/js/src/jsweakmap.cpp
+++ b/js/src/jsweakmap.cpp
@@ -244,29 +244,30 @@ WeakMap_set_impl(JSContext *cx, CallArgs
         if (!map->init()) {
             cx->delete_(map);
             JS_ReportOutOfMemory(cx);
             return false;
         }
         thisObj->setPrivate(map);
     }
 
+    // Preserve wrapped native keys to prevent wrapper optimization.
+    if (key->getClass()->ext.isWrappedNative) {
+        MOZ_ASSERT(cx->runtime->preserveWrapperCallback, "wrapped native weak map key needs preserveWrapperCallback");
+        if (!cx->runtime->preserveWrapperCallback(cx, key)) {
+            JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL, JSMSG_BAD_WEAKMAP_KEY);
+            return false;
+        }
+    }
+
     if (!map->put(key, value)) {
         JS_ReportOutOfMemory(cx);
         return false;
     }
 
-    // Preserve wrapped native keys to prevent wrapper optimization.
-    if (key->getClass()->ext.isWrappedNative) {
-        if (!cx->runtime->preserveWrapperCallback ||
-            !cx->runtime->preserveWrapperCallback(cx, key)) {
-            JS_ReportWarning(cx, "Failed to preserve wrapper of wrapped native weak map key.");
-        }
-    }
-
     args.rval().setUndefined();
     return true;
 }
 
 JSBool
 WeakMap_set(JSContext *cx, unsigned argc, Value *vp)
 {
     CallArgs args = CallArgsFromVp(argc, vp);
--- a/js/xpconnect/tests/chrome/test_weakmaps.xul
+++ b/js/xpconnect/tests/chrome/test_weakmaps.xul
@@ -208,24 +208,42 @@ https://bugzilla.mozilla.org/show_bug.cg
 
   let make_live_map = function () {
     let live = get_live_dom();
     wn_live_map.set(live, {});
   }
 
   make_live_map();
 
+  let unpreservable_native_key = function () {
+    // We only allow natives that support wrapper preservation to be used as weak
+    // map keys.  We should be able to try to add unpreservable natives as keys without
+    // crashing (bug 711616), but we should throw an error (bug 761620).
 
-  let dummy_test_map = new WeakMap;
-  // bug 711616: should be able to do this without crashing. JS warning is expected.
-  dummy_test_map.set(document.createElement("div").style, 1);
+    let dummy_test_map = new WeakMap;
+
+    let div_fail = false;
+    try {
+      dummy_test_map.set(document.createElement("div").style, 1);
+    } catch (e) {
+      div_fail = true;
+    }
+    ok(div_fail, "Using elem.style as a weak map key should produce an exception because it can't be wrapper preserved.");
 
-  // JS warning is expected.
-  dummy_test_map.set(window.navigator, 1);
+    let navi_fail = false;
+    try {
+      dummy_test_map.set(window.navigator, 1);
+    } catch (e) {
+      navi_fail = true;
+    }
+    ok(navi_fail, "Using window.navigator as a weak map key should produce an exception because it can't be wrapper preserved.");
 
+  }
+
+  unpreservable_native_key();
 
   /* set up for running precise GC/CC then checking the results */
 
   SimpleTest.waitForExplicitFinish();
 
   Cu.schedulePreciseGC(function () {
     window.QueryInterface(Ci.nsIInterfaceRequestor)
          .getInterface(Ci.nsIDOMWindowUtils)