Bug 1496475 - Object.defineProperty needs to be able to return false when trying to define a non-configurable property on a WindowProxy. r=jorendorff
authorTom Schuster <evilpies@gmail.com>
Tue, 27 Nov 2018 11:38:28 +0000
changeset 507457 5da532fab9bacb44b81ae0de6b7f026d187e54c7
parent 507456 cafc30a40d9f93762372ad805443ec8ba7efecdc
child 507458 6b2aceb0979a49dbcce80db1b59ef8238bd0d2b8
push id1905
push userffxbld-merge
push dateMon, 21 Jan 2019 12:33:13 +0000
treeherdermozilla-release@c2fca1944d8c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjorendorff
bugs1496475
milestone65.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 1496475 - Object.defineProperty needs to be able to return false when trying to define a non-configurable property on a WindowProxy. r=jorendorff Differential Revision: https://phabricator.services.mozilla.com/D12948
js/public/Class.h
js/src/builtin/Object.js
js/src/js.msg
js/src/jsapi.cpp
js/src/vm/SelfHosting.cpp
--- a/js/public/Class.h
+++ b/js/public/Class.h
@@ -202,16 +202,19 @@ class ObjectOpResult
     JS_PUBLIC_API bool failCantDeleteWindowElement();
     JS_PUBLIC_API bool failCantDeleteWindowNamedProperty();
     JS_PUBLIC_API bool failCantPreventExtensions();
     JS_PUBLIC_API bool failCantSetProto();
     JS_PUBLIC_API bool failNoNamedSetter();
     JS_PUBLIC_API bool failNoIndexedSetter();
     JS_PUBLIC_API bool failNotDataDescriptor();
 
+    // Careful: This case has special handling in Object.defineProperty.
+    JS_PUBLIC_API bool failCantDefineWindowNonConfigurable();
+
     uint32_t failureCode() const {
         MOZ_ASSERT(!ok());
         return uint32_t(code_);
     }
 
     /*
      * Report an error or warning if necessary; return true to proceed and
      * false if an error was reported. Call this when failure should cause
--- a/js/src/builtin/Object.js
+++ b/js/src/builtin/Object.js
@@ -276,17 +276,20 @@ function ObjectOrReflectDefineProperty(o
     // Step 4 (generic property descriptor or data property without value).
     return _DefineProperty(obj, propertyKey, attrs, undefined, undefined, strict);
 }
 
 // ES2017 draft rev 6859bb9ccaea9c6ede81d71e5320e3833b92cb3e
 // 19.1.2.4 Object.defineProperty ( O, P, Attributes )
 function ObjectDefineProperty(obj, propertyKey, attributes) {
     // Steps 1-4.
-    ObjectOrReflectDefineProperty(obj, propertyKey, attributes, true);
+    if (!ObjectOrReflectDefineProperty(obj, propertyKey, attributes, true)) {
+        // Not standardized yet: https://github.com/tc39/ecma262/pull/688
+        return false;
+    }
 
     // Step 5.
     return obj;
 }
 
 // Proposal https://tc39.github.io/proposal-object-from-entries/
 // 1. Object.fromEntries ( iterable )
 function ObjectFromEntries(iter) {
--- a/js/src/js.msg
+++ b/js/src/js.msg
@@ -598,16 +598,17 @@ MSG_DEF(JSMSG_ATOMICS_TOO_LONG,         
 MSG_DEF(JSMSG_ATOMICS_WAIT_NOT_ALLOWED,  0, JSEXN_TYPEERR, "waiting is not allowed on this thread")
 
 // XPConnect wrappers and DOM bindings
 MSG_DEF(JSMSG_CANT_SET_INTERPOSED,       1, JSEXN_TYPEERR, "unable to set interposed data property '{0}'")
 MSG_DEF(JSMSG_CANT_DEFINE_WINDOW_ELEMENT, 0, JSEXN_TYPEERR, "can't define elements on a Window object")
 MSG_DEF(JSMSG_CANT_DELETE_WINDOW_ELEMENT, 0, JSEXN_TYPEERR, "can't delete elements from a Window object")
 MSG_DEF(JSMSG_CANT_DELETE_WINDOW_NAMED_PROPERTY, 1, JSEXN_TYPEERR, "can't delete property {0} from window's named properties object")
 MSG_DEF(JSMSG_CANT_PREVENT_EXTENSIONS,   0, JSEXN_TYPEERR, "can't prevent extensions on this proxy object")
+MSG_DEF(JSMSG_CANT_DEFINE_WINDOW_NC,     0, JSEXN_TYPEERR, "can't define non-configurable property on WindowProxy")
 MSG_DEF(JSMSG_NO_NAMED_SETTER,           2, JSEXN_TYPEERR, "{0} doesn't have a named property setter for '{1}'")
 MSG_DEF(JSMSG_NO_INDEXED_SETTER,         2, JSEXN_TYPEERR, "{0} doesn't have an indexed property setter for '{1}'")
 MSG_DEF(JSMSG_NOT_DATA_DESCRIPTOR,       2, JSEXN_TYPEERR, "can't define a getter/setter for element '{1}' of {0} object")
 
 // Super
 MSG_DEF(JSMSG_CANT_DELETE_SUPER, 0, JSEXN_REFERENCEERR, "invalid delete involving 'super'")
 MSG_DEF(JSMSG_REINIT_THIS,       0, JSEXN_REFERENCEERR, "super() called twice in derived class constructor")
 
--- a/js/src/jsapi.cpp
+++ b/js/src/jsapi.cpp
@@ -256,16 +256,22 @@ JS::ObjectOpResult::failCantDeleteWindow
 
 JS_PUBLIC_API bool
 JS::ObjectOpResult::failCantDeleteWindowNamedProperty()
 {
     return fail(JSMSG_CANT_DELETE_WINDOW_NAMED_PROPERTY);
 }
 
 JS_PUBLIC_API bool
+JS::ObjectOpResult::failCantDefineWindowNonConfigurable()
+{
+    return fail(JSMSG_CANT_DEFINE_WINDOW_NC);
+}
+
+JS_PUBLIC_API bool
 JS::ObjectOpResult::failCantPreventExtensions()
 {
     return fail(JSMSG_CANT_PREVENT_EXTENSIONS);
 }
 
 JS_PUBLIC_API bool
 JS::ObjectOpResult::failCantSetProto()
 {
--- a/js/src/vm/SelfHosting.cpp
+++ b/js/src/vm/SelfHosting.cpp
@@ -710,18 +710,26 @@ intrinsic_DefineProperty(JSContext* cx, 
     desc.assertValid();
 
     ObjectOpResult result;
     if (!DefineProperty(cx, obj, id, desc, result)) {
         return false;
     }
 
     bool strict = args[5].toBoolean();
-    if (strict && !result.checkStrict(cx, obj, id)) {
-        return false;
+    if (strict && !result.ok()) {
+        // We need to tell our caller Object.defineProperty,
+        // that this operation failed, without actually throwing
+        // for web-compatibility reasons.
+        if (result.failureCode() == JSMSG_CANT_DEFINE_WINDOW_NC) {
+            args.rval().setBoolean(false);
+            return true;
+        }
+
+        return result.reportError(cx, obj, id);
     }
 
     args.rval().setBoolean(result.reallyOk());
     return true;
 }
 
 static bool
 intrinsic_ObjectHasPrototype(JSContext* cx, unsigned argc, Value* vp)