Bug 1520831. Fix handling of Symbol-named properties on objects with named setters. r=qdot
authorBoris Zbarsky <bzbarsky@mit.edu>
Tue, 19 Feb 2019 19:08:32 +0000
changeset 519467 4dcf351df9b8a01752a62002e0985aab9033ac1e
parent 519466 4d9ae84e669496a00d0d26b5f40ad9b9a9dd077d
child 519468 b03d042670c0d5241e8e57e5af9706e2a57b4c08
push id10862
push userffxbld-merge
push dateMon, 11 Mar 2019 13:01:11 +0000
treeherdermozilla-beta@a2e7f5c935da [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersqdot
bugs1520831
milestone67.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 1520831. Fix handling of Symbol-named properties on objects with named setters. r=qdot Per spec these should just go directly to the expando object; we were ignoring them instead. Differential Revision: https://phabricator.services.mozilla.com/D16930
dom/bindings/Codegen.py
testing/web-platform/meta/html/webappapis/dynamic-markup-insertion/opening-the-input-stream/no-new-global.window.js.ini
--- a/dom/bindings/Codegen.py
+++ b/dom/bindings/Codegen.py
@@ -11458,24 +11458,28 @@ class CGProxyNamedOperation(CGProxySpeci
     Class to generate a call to a named operation.
 
     'value' is the jsval to use for the name; None indicates that it should be
     gotten from the property id.
 
     resultVar: See the docstring for CGCallGenerator.
 
     foundVar: See the docstring for CGProxySpecialOperation.
+
+    tailCode: if we end up with a non-symbol string id, run this code after
+              we do all our other work.
     """
     def __init__(self, descriptor, name, value=None, argumentHandleValue=None,
-                 resultVar=None, foundVar=None):
+                 resultVar=None, foundVar=None, tailCode=""):
         CGProxySpecialOperation.__init__(self, descriptor, name,
                                          argumentHandleValue=argumentHandleValue,
                                          resultVar=resultVar,
                                          foundVar=foundVar)
         self.value = value
+        self.tailCode = tailCode
 
     def define(self):
         # Our first argument is the id we're getting.
         argName = self.arguments[0].identifier.name
         if argName == "id":
             # deal with the name collision
             decls = "JS::Rooted<jsid> id_(cx, id);\n"
             idName = "id_"
@@ -11484,19 +11488,21 @@ class CGProxyNamedOperation(CGProxySpeci
             idName = "id"
 
         decls += "FakeString %s;\n" % argName
 
         main = fill(
             """
             ${nativeType}* self = UnwrapProxy(proxy);
             $*{op}
+            $*{tailCode}
             """,
             nativeType=self.descriptor.nativeType,
-            op=CGProxySpecialOperation.define(self))
+            op=CGProxySpecialOperation.define(self),
+            tailCode=self.tailCode)
 
         if self.value is None:
             return fill(
                 """
                 $*{decls}
                 bool isSymbol;
                 if (!ConvertIdToString(cx, ${idName}, ${argName}, isSymbol)) {
                   return false;
@@ -11560,19 +11566,20 @@ class CGProxyNamedPresenceChecker(CGProx
         CGProxyNamedGetter.__init__(self, descriptor, foundVar=foundVar)
         self.cgRoot.append(CGGeneric("(void)result;\n"))
 
 
 class CGProxyNamedSetter(CGProxyNamedOperation):
     """
     Class to generate a call to a named setter.
     """
-    def __init__(self, descriptor, argumentHandleValue=None):
+    def __init__(self, descriptor, tailCode, argumentHandleValue=None):
         CGProxyNamedOperation.__init__(self, descriptor, 'NamedSetter',
-                                       argumentHandleValue=argumentHandleValue)
+                                       argumentHandleValue=argumentHandleValue,
+                                       tailCode=tailCode)
 
 
 class CGProxyNamedDeleter(CGProxyNamedOperation):
     """
     Class to generate a call to a named deleter.
 
     resultVar: See the docstring for CGCallGenerator.
 
@@ -11795,26 +11802,22 @@ class CGDOMJSProxyHandler_defineProperty
                 """)
 
         namedSetter = self.descriptor.operations['NamedSetter']
         if namedSetter:
             if self.descriptor.hasUnforgeableMembers:
                 raise TypeError("Can't handle a named setter on an interface "
                                 "that has unforgeables.  Figure out how that "
                                 "should work!")
-            # If we support indexed properties, we won't get down here for
-            # indices, so we can just do our setter unconditionally here.
-            set += fill(
+            tailCode = dedent(
                 """
                 *defined = true;
-                $*{callSetter}
-
                 return opresult.succeed();
-                """,
-                callSetter=CGProxyNamedSetter(self.descriptor).define())
+                """)
+            set += CGProxyNamedSetter(self.descriptor, tailCode).define()
         else:
             # We allow untrusted content to prevent Xrays from setting a
             # property if that property is already a named property on the
             # object and we have no named setter.  That's how the object would
             # normally behave if you tried to set the property on it.  That
             # means we don't need to do anything special for Xrays here.
             if self.descriptor.supportsNamedProperties():
                 set += fill(
@@ -11831,17 +11834,20 @@ class CGDOMJSProxyHandler_defineProperty
             if self.descriptor.isMaybeCrossOriginObject():
                 set += dedent(
                     """
                     MOZ_ASSERT(IsPlatformObjectSameOrigin(cx, proxy),
                                "Why did the MaybeCrossOriginObject defineProperty override fail?");
                     MOZ_ASSERT(js::IsObjectInContextCompartment(proxy, cx),
                                "Why did the MaybeCrossOriginObject defineProperty override fail?");
                     """)
-            set += ("return mozilla::dom::DOMProxyHandler::defineProperty(%s);\n" %
+
+        # In all cases we want to tail-call to our base class; we can
+        # always land here for symbols.
+        set += ("return mozilla::dom::DOMProxyHandler::defineProperty(%s);\n" %
                     ", ".join(a.name for a in self.args))
         return set
 
 
 def getDeleterBody(descriptor, type, foundVar=None):
     """
     type should be "Named" or "Indexed"
 
@@ -12453,21 +12459,31 @@ class CGDOMJSProxyHandler_setCustom(Clas
             # Check assumptions.
             if self.descriptor.supportsIndexedProperties():
                 raise ValueError("In interface " + self.descriptor.name + ": " +
                                  "Can't cope with [OverrideBuiltins] and an indexed getter")
             if self.descriptor.hasUnforgeableMembers:
                 raise ValueError("In interface " + self.descriptor.name + ": " +
                                  "Can't cope with [OverrideBuiltins] and unforgeable members")
 
-            callSetter = CGProxyNamedSetter(self.descriptor, argumentHandleValue="v")
-            return (assertion +
-                    callSetter.define() +
-                    "*done = true;\n"
-                    "return true;\n")
+            tailCode = dedent(
+                """
+                *done = true;
+                return true;
+                """)
+            callSetter = CGProxyNamedSetter(self.descriptor, tailCode, argumentHandleValue="v")
+            return fill(
+                """
+                $*{assertion}
+                $*{callSetter}
+                *done = false;
+                return true;
+                """,
+                assertion=assertion,
+                callSetter=callSetter.define())
 
         # As an optimization, if we are going to call an IndexedSetter, go
         # ahead and call it and have done.
         indexedSetter = self.descriptor.operations['IndexedSetter']
         if indexedSetter is not None:
             setIndexed = fill(
                 """
                 uint32_t index = GetArrayIndexFromId(id);
deleted file mode 100644
--- a/testing/web-platform/meta/html/webappapis/dynamic-markup-insertion/opening-the-input-stream/no-new-global.window.js.ini
+++ /dev/null
@@ -1,6 +0,0 @@
-[no-new-global.window.html]
-  [sessionStorage maintains its prototype and properties through open()]
-    expected: FAIL
-
-  [localStorage maintains its prototype and properties through open()]
-    expected: FAIL