Bug 1390159 - Remove throw-on-set setProperty hooks on WrappedNatives. r=mrbkap
authorJan de Mooij <jdemooij@mozilla.com>
Wed, 16 Aug 2017 09:01:56 +0200
changeset 374963 4ed300523cb0fcbfb773a4589835ce3e3e8b3d6a
parent 374962 9d4b16fa3dd7c242e10a7412513ba59cc222aab6
child 374964 d1d85eed4d06d513cb6e6937e7e487f4f2386543
push id93804
push userjandemooij@gmail.com
push dateWed, 16 Aug 2017 07:02:18 +0000
treeherdermozilla-inbound@4ed300523cb0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmrbkap
bugs1390159
milestone57.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 1390159 - Remove throw-on-set setProperty hooks on WrappedNatives. r=mrbkap
dom/base/nsIDOMClassInfo.h
js/xpconnect/idl/nsIXPCScriptable.idl
js/xpconnect/public/xpc_make_class.h
js/xpconnect/src/XPCRuntimeService.cpp
js/xpconnect/src/XPCWrappedNativeJSOps.cpp
js/xpconnect/tests/chrome/chrome.ini
js/xpconnect/tests/chrome/test_bug1390159.xul
--- a/dom/base/nsIDOMClassInfo.h
+++ b/dom/base/nsIDOMClassInfo.h
@@ -7,17 +7,16 @@
 #ifndef nsIDOMClassInfo_h___
 #define nsIDOMClassInfo_h___
 
 #include "nsIXPCScriptable.h"
 
 #define DOM_BASE_SCRIPTABLE_FLAGS                                          \
   (XPC_SCRIPTABLE_USE_JSSTUB_FOR_ADDPROPERTY |                             \
    XPC_SCRIPTABLE_USE_JSSTUB_FOR_DELPROPERTY |                             \
-   XPC_SCRIPTABLE_USE_JSSTUB_FOR_SETPROPERTY |                             \
    XPC_SCRIPTABLE_ALLOW_PROP_MODS_TO_PROTOTYPE |                           \
    XPC_SCRIPTABLE_DONT_ASK_INSTANCE_FOR_SCRIPTABLE |                       \
    XPC_SCRIPTABLE_DONT_REFLECT_INTERFACE_NAMES)
 
 #define DEFAULT_SCRIPTABLE_FLAGS                                           \
   (DOM_BASE_SCRIPTABLE_FLAGS |                                             \
    XPC_SCRIPTABLE_WANT_RESOLVE |                                           \
    XPC_SCRIPTABLE_WANT_PRECREATE)
--- a/js/xpconnect/idl/nsIXPCScriptable.idl
+++ b/js/xpconnect/idl/nsIXPCScriptable.idl
@@ -42,17 +42,17 @@ interface nsIXPConnectWrappedNative;
     #define XPC_SCRIPTABLE_WANT_NEWENUMERATE                (1 <<  4)
     #define XPC_SCRIPTABLE_WANT_RESOLVE                     (1 <<  5)
     #define XPC_SCRIPTABLE_WANT_FINALIZE                    (1 <<  6)
     #define XPC_SCRIPTABLE_WANT_CALL                        (1 <<  7)
     #define XPC_SCRIPTABLE_WANT_CONSTRUCT                   (1 <<  8)
     #define XPC_SCRIPTABLE_WANT_HASINSTANCE                 (1 <<  9)
     #define XPC_SCRIPTABLE_USE_JSSTUB_FOR_ADDPROPERTY       (1 << 10)
     #define XPC_SCRIPTABLE_USE_JSSTUB_FOR_DELPROPERTY       (1 << 11)
-    #define XPC_SCRIPTABLE_USE_JSSTUB_FOR_SETPROPERTY       (1 << 12)
+    // (1 << 12) is unused
     #define XPC_SCRIPTABLE_DONT_ENUM_QUERY_INTERFACE        (1 << 13)
     #define XPC_SCRIPTABLE_DONT_ASK_INSTANCE_FOR_SCRIPTABLE (1 << 14)
     #define XPC_SCRIPTABLE_CLASSINFO_INTERFACES_ONLY        (1 << 15)
     #define XPC_SCRIPTABLE_ALLOW_PROP_MODS_DURING_RESOLVE   (1 << 16)
     #define XPC_SCRIPTABLE_ALLOW_PROP_MODS_TO_PROTOTYPE     (1 << 17)
     #define XPC_SCRIPTABLE_IS_GLOBAL_OBJECT                 (1 << 18)
     #define XPC_SCRIPTABLE_DONT_REFLECT_INTERFACE_NAMES     (1 << 19)
 %}
@@ -123,17 +123,16 @@ interface nsIXPCScriptable : nsISupports
     GET_IT(WantNewEnumerate,             WANT_NEWENUMERATE)
     GET_IT(WantResolve,                  WANT_RESOLVE)
     GET_IT(WantFinalize,                 WANT_FINALIZE)
     GET_IT(WantCall,                     WANT_CALL)
     GET_IT(WantConstruct,                WANT_CONSTRUCT)
     GET_IT(WantHasInstance,              WANT_HASINSTANCE)
     GET_IT(UseJSStubForAddProperty,      USE_JSSTUB_FOR_ADDPROPERTY)
     GET_IT(UseJSStubForDelProperty,      USE_JSSTUB_FOR_DELPROPERTY)
-    GET_IT(UseJSStubForSetProperty,      USE_JSSTUB_FOR_SETPROPERTY)
     GET_IT(DontEnumQueryInterface,       DONT_ENUM_QUERY_INTERFACE)
     GET_IT(DontAskInstanceForScriptable, DONT_ASK_INSTANCE_FOR_SCRIPTABLE)
     GET_IT(ClassInfoInterfacesOnly,      CLASSINFO_INTERFACES_ONLY)
     GET_IT(AllowPropModsDuringResolve,   ALLOW_PROP_MODS_DURING_RESOLVE)
     GET_IT(AllowPropModsToPrototype,     ALLOW_PROP_MODS_TO_PROTOTYPE)
     GET_IT(IsGlobalObject,               IS_GLOBAL_OBJECT)
     GET_IT(DontReflectInterfaceNames,    DONT_REFLECT_INTERFACE_NAMES)
 
--- a/js/xpconnect/public/xpc_make_class.h
+++ b/js/xpconnect/public/xpc_make_class.h
@@ -31,24 +31,16 @@ XPC_WN_CannotDeletePropertyStub(JSContex
 bool
 XPC_WN_Helper_GetProperty(JSContext* cx, JS::HandleObject obj, JS::HandleId id,
                           JS::MutableHandleValue vp);
 
 bool
 XPC_WN_Helper_SetProperty(JSContext* cx, JS::HandleObject obj, JS::HandleId id,
                           JS::MutableHandleValue vp,
                           JS::ObjectOpResult& result);
-bool
-XPC_WN_MaybeResolvingSetPropertyStub(JSContext* cx, JS::HandleObject obj,
-                                     JS::HandleId id, JS::MutableHandleValue vp,
-                                     JS::ObjectOpResult& result);
-bool
-XPC_WN_CannotModifySetPropertyStub(JSContext* cx, JS::HandleObject obj,
-                                   JS::HandleId id, JS::MutableHandleValue vp,
-                                   JS::ObjectOpResult& result);
 
 bool
 XPC_WN_Helper_Enumerate(JSContext* cx, JS::HandleObject obj);
 bool
 XPC_WN_Shared_Enumerate(JSContext* cx, JS::HandleObject obj);
 
 bool
 XPC_WN_NewEnumerate(JSContext* cx, JS::HandleObject obj, JS::AutoIdVector& properties,
@@ -96,21 +88,17 @@ extern const js::ClassExtension XPC_WN_J
     /* getProperty */ \
     ((_flags) & XPC_SCRIPTABLE_WANT_GETPROPERTY) \
     ? XPC_WN_Helper_GetProperty \
     : nullptr, \
     \
     /* setProperty */ \
     ((_flags) & XPC_SCRIPTABLE_WANT_SETPROPERTY) \
     ? XPC_WN_Helper_SetProperty \
-    : ((_flags) & XPC_SCRIPTABLE_USE_JSSTUB_FOR_SETPROPERTY) \
-      ? nullptr \
-      : ((_flags) & XPC_SCRIPTABLE_ALLOW_PROP_MODS_DURING_RESOLVE) \
-        ? XPC_WN_MaybeResolvingSetPropertyStub \
-        : XPC_WN_CannotModifySetPropertyStub, \
+    : nullptr, \
     \
     /* enumerate */ \
     ((_flags) & XPC_SCRIPTABLE_WANT_NEWENUMERATE) \
     ? nullptr /* We will use newEnumerate set below in this case */ \
     : ((_flags) & XPC_SCRIPTABLE_WANT_ENUMERATE) \
       ? XPC_WN_Helper_Enumerate \
       : XPC_WN_Shared_Enumerate, \
     \
--- a/js/xpconnect/src/XPCRuntimeService.cpp
+++ b/js/xpconnect/src/XPCRuntimeService.cpp
@@ -28,17 +28,16 @@ NS_IMPL_RELEASE(BackstagePass)
 #define XPC_MAP_CLASSNAME         BackstagePass
 #define XPC_MAP_QUOTED_CLASSNAME "BackstagePass"
 #define XPC_MAP_FLAGS (XPC_SCRIPTABLE_WANT_RESOLVE | \
                        XPC_SCRIPTABLE_WANT_ENUMERATE | \
                        XPC_SCRIPTABLE_WANT_FINALIZE | \
                        XPC_SCRIPTABLE_WANT_PRECREATE | \
                        XPC_SCRIPTABLE_USE_JSSTUB_FOR_ADDPROPERTY |  \
                        XPC_SCRIPTABLE_USE_JSSTUB_FOR_DELPROPERTY |  \
-                       XPC_SCRIPTABLE_USE_JSSTUB_FOR_SETPROPERTY |  \
                        XPC_SCRIPTABLE_DONT_ENUM_QUERY_INTERFACE |  \
                        XPC_SCRIPTABLE_IS_GLOBAL_OBJECT |  \
                        XPC_SCRIPTABLE_DONT_REFLECT_INTERFACE_NAMES)
 #include "xpc_map_end.h" /* This will #undef the above */
 
 
 JSObject*
 BackstagePass::GetGlobalJSObject()
--- a/js/xpconnect/src/XPCWrappedNativeJSOps.cpp
+++ b/js/xpconnect/src/XPCWrappedNativeJSOps.cpp
@@ -479,23 +479,16 @@ XPC_WN_CannotModifyPropertyStub(JSContex
 bool
 XPC_WN_CannotDeletePropertyStub(JSContext* cx, HandleObject obj, HandleId id,
                                 ObjectOpResult& result)
 {
     return Throw(NS_ERROR_XPC_CANT_MODIFY_PROP_ON_WN, cx);
 }
 
 bool
-XPC_WN_CannotModifySetPropertyStub(JSContext* cx, HandleObject obj, HandleId id,
-                                   MutableHandleValue vp, ObjectOpResult& result)
-{
-    return Throw(NS_ERROR_XPC_CANT_MODIFY_PROP_ON_WN, cx);
-}
-
-bool
 XPC_WN_Shared_Enumerate(JSContext* cx, HandleObject obj)
 {
     XPCCallContext ccx(cx, obj);
     XPCWrappedNative* wrapper = ccx.GetWrapper();
     THROW_AND_RETURN_IF_BAD_WRAPPER(cx, wrapper);
 
     // Since we aren't going to enumerate tearoff names and the prototype
     // handles non-mutated members, we can do this potential short-circuit.
@@ -664,24 +657,16 @@ XPC_WN_MaybeResolvingPropertyStub(JSCont
     THROW_AND_RETURN_IF_BAD_WRAPPER(cx, wrapper);
 
     if (ccx.GetResolvingWrapper() == wrapper)
         return true;
     return Throw(NS_ERROR_XPC_CANT_MODIFY_PROP_ON_WN, cx);
 }
 
 bool
-XPC_WN_MaybeResolvingSetPropertyStub(JSContext* cx, HandleObject obj, HandleId id,
-                                     MutableHandleValue vp, ObjectOpResult& result)
-{
-    result.succeed();
-    return XPC_WN_MaybeResolvingPropertyStub(cx, obj, id, vp);
-}
-
-bool
 XPC_WN_MaybeResolvingDeletePropertyStub(JSContext* cx, HandleObject obj, HandleId id,
                                         ObjectOpResult& result)
 {
     XPCCallContext ccx(cx, obj);
     XPCWrappedNative* wrapper = ccx.GetWrapper();
     THROW_AND_RETURN_IF_BAD_WRAPPER(cx, wrapper);
 
     if (ccx.GetResolvingWrapper() == wrapper) {
--- a/js/xpconnect/tests/chrome/chrome.ini
+++ b/js/xpconnect/tests/chrome/chrome.ini
@@ -83,16 +83,17 @@ skip-if = os == 'win' || os == 'mac' || 
 [test_bug1042436.xul]
 [test_bug1050049.html]
 [test_bug1065185.html]
 [test_bug1074863.html]
 [test_bug1092477.xul]
 [test_bug1124898.html]
 [test_bug1126911.html]
 [test_bug1281071.xul]
+[test_bug1390159.xul]
 [test_chrometoSource.xul]
 [test_cloneInto.xul]
 [test_cows.xul]
 [test_discardSystemSource.xul]
 [test_documentdomain.xul]
 [test_doublewrappedcompartments.xul]
 [test_evalInSandbox.xul]
 [test_evalInWindow.xul]
new file mode 100644
--- /dev/null
+++ b/js/xpconnect/tests/chrome/test_bug1390159.xul
@@ -0,0 +1,46 @@
+<?xml version="1.0"?>
+<?xml-stylesheet href="chrome://global/skin" type="text/css"?>
+<?xml-stylesheet href="chrome://mochikit/content/tests/SimpleTest/test.css"
+                 type="text/css"?>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=1390159
+-->
+<window title="Mozilla Bug 1390159"
+  xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
+  <script type="application/javascript"
+          src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"></script>
+
+  <!-- test code goes here -->
+  <script type="application/javascript"><![CDATA[
+    function test() {
+      var xulRuntime = Components.classes["@mozilla.org/xre/app-info;1"]
+                           .getService(Components.interfaces.nsIXULRuntime);
+
+      // Make sure it has an inSafeMode property. This is just an arbitrary
+      // readonly property.
+      var oldValue = xulRuntime.inSafeMode;
+      is(typeof oldValue, "boolean", "Expected an inSafeMode property");
+
+      // Changing a readonly property doesn't throw, but shouldn't change
+      // the value.
+      xulRuntime.inSafeMode = !oldValue;
+      is(xulRuntime.inSafeMode, oldValue, "Should not have modified prop");
+
+      // Adding a new property should throw.
+      ex = null;
+      try {
+        xulRuntime.foobar = 1;
+      } catch (e) {
+        ex = e;
+      }
+      is(ex.toString().includes("Cannot modify"), true, "Expected an error");
+      is(xulRuntime.foobar, undefined, "Property not defined");
+    }
+    test();
+  ]]></script>
+
+  <!-- test results are displayed in the html:body -->
+  <body xmlns="http://www.w3.org/1999/xhtml">
+  </body>
+
+</window>