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 101839 34ae2864bbd8cf109162c7b75595b63ceb4ee7c7
parent 101838 f4e702ef9113dbdb850f42e2ee6318c89741edc3
child 101840 7e0466ba65c45490309046d9e63173ab68a3ec38
push id23253
push userryanvm@gmail.com
push dateThu, 09 Aug 2012 01:21:41 +0000
treeherdermozilla-central@257777cf58fe [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbillm
bugs761620
milestone17.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 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;
 }
 
 static 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)