From b7a0045e0020856b860493856499c87768abab0d Mon Sep 17 00:00:00 2001
From: Andrew McCreight <continuation@gmail.com>
Date: Wed, 18 Feb 2015 15:40:52 -0800
Subject: [PATCH] Bug 1127827, part 1 - WeakMap.get, has and delete should not
 throw when the key arg is not an object. r=Waldo

Plus add tests for this, plus the return values of some other WeakMap functions.
---
 js/src/jsweakmap.cpp                       | 46 +++++++++++-----------
 js/src/tests/js1_8_5/extensions/weakmap.js | 11 +++++-
 2 files changed, 34 insertions(+), 23 deletions(-)

diff --git a/js/src/jsweakmap.cpp b/js/src/jsweakmap.cpp
index 6a8175d08d29..dcacf2b76e90 100644
--- a/js/src/jsweakmap.cpp
+++ b/js/src/jsweakmap.cpp
@@ -200,16 +200,6 @@ ObjectValueMap::findZoneEdges()
     return true;
 }
 
-static JSObject *
-GetKeyArg(JSContext *cx, CallArgs &args)
-{
-    if (args[0].isPrimitive()) {
-        JS_ReportErrorNumber(cx, js_GetErrorMessage, nullptr, JSMSG_NOT_NONNULL_OBJECT);
-        return nullptr;
-    }
-    return &args[0].toObject();
-}
-
 MOZ_ALWAYS_INLINE bool
 IsWeakMap(HandleValue v)
 {
@@ -226,11 +216,14 @@ WeakMap_has_impl(JSContext *cx, CallArgs args)
                              "WeakMap.has", "0", "s");
         return false;
     }
-    JSObject *key = GetKeyArg(cx, args);
-    if (!key)
-        return false;
+
+    if (!args[0].isObject()) {
+        args.rval().setBoolean(false);
+        return true;
+    }
 
     if (ObjectValueMap *map = args.thisv().toObject().as<WeakMapObject>().getMap()) {
+        JSObject *key = &args[0].toObject();
         if (map->has(key)) {
             args.rval().setBoolean(true);
             return true;
@@ -279,11 +272,14 @@ WeakMap_get_impl(JSContext *cx, CallArgs args)
                              "WeakMap.get", "0", "s");
         return false;
     }
-    JSObject *key = GetKeyArg(cx, args);
-    if (!key)
-        return false;
+
+    if (!args[0].isObject()) {
+        args.rval().setUndefined();
+        return true;
+    }
 
     if (ObjectValueMap *map = args.thisv().toObject().as<WeakMapObject>().getMap()) {
+        JSObject *key = &args[0].toObject();
         if (ObjectValueMap::Ptr ptr = map->lookup(key)) {
             args.rval().set(ptr->value());
             return true;
@@ -311,11 +307,14 @@ WeakMap_delete_impl(JSContext *cx, CallArgs args)
                              "WeakMap.delete", "0", "s");
         return false;
     }
-    JSObject *key = GetKeyArg(cx, args);
-    if (!key)
-        return false;
+
+    if (!args[0].isObject()) {
+        args.rval().setBoolean(false);
+        return true;
+    }
 
     if (ObjectValueMap *map = args.thisv().toObject().as<WeakMapObject>().getMap()) {
+        JSObject *key = &args[0].toObject();
         if (ObjectValueMap::Ptr ptr = map->lookup(key)) {
             map->remove(ptr);
             args.rval().setBoolean(true);
@@ -407,10 +406,13 @@ WeakMap_set_impl(JSContext *cx, CallArgs args)
                              "WeakMap.set", "0", "s");
         return false;
     }
-    RootedObject key(cx, GetKeyArg(cx, args));
-    if (!key)
-        return false;
 
+    if (!args[0].isObject()) {
+        JS_ReportErrorNumber(cx, js_GetErrorMessage, nullptr, JSMSG_NOT_NONNULL_OBJECT);
+        return false;
+    }
+
+    RootedObject key(cx, &args[0].toObject());
     RootedValue value(cx, (args.length() > 1) ? args[1] : UndefinedValue());
     Rooted<JSObject*> thisObj(cx, &args.thisv().toObject());
     Rooted<WeakMapObject*> map(cx, &thisObj->as<WeakMapObject>());
diff --git a/js/src/tests/js1_8_5/extensions/weakmap.js b/js/src/tests/js1_8_5/extensions/weakmap.js
index 4be0c383a649..e526c628b16b 100644
--- a/js/src/tests/js1_8_5/extensions/weakmap.js
+++ b/js/src/tests/js1_8_5/extensions/weakmap.js
@@ -91,7 +91,10 @@ function test()
     gc(); gc(); gc();
 
     check(function() map.get(key) == 42);
-    map.delete(key);
+    check(function() map.delete(key) == true);
+    check(function() map.delete(key) == false);
+    check(function() map.delete({}) == false);
+
     check(function() typeof map.get(key) == "undefined");
     check(function() !map.has(key));
 
@@ -99,6 +102,12 @@ function test()
     map.set(new Object(), value);
     gc(); gc(); gc();
 
+    check(function() map.has("non-object key") == false);
+    check(function() map.get("non-object key") == undefined);
+    check(function() map.delete("non-object key") == false);
+
+    checkThrows(function() map.set("non-object key", value));
+
     print ("done");
 
     reportCompare(0, TestFailCount, "weak map tests");