Bug 1332713 part 4. Make Promise-returning getters return a rejected Promise on exception instead of throwing. r=qdot

dom/bindings/BindingUtils.cpp

@@ -2855,6 +2855,50 @@ GenericBindingGetter(JSContext* cx, unsigned argc, JS::Value* vp)
   return ok;
 }
 
+bool
+GenericPromiseReturningBindingGetter(JSContext* cx, unsigned argc, JS::Value* vp)
+{
+  // Make sure to save the callee before someone maybe messes with rval().
+  JS::CallArgs args = JS::CallArgsFromVp(argc, vp);
+  JS::Rooted<JSObject*> callee(cx, &args.callee());
+
+  // We could invoke GenericBindingGetter here, but that involves an
+  // extra call.  Manually inline it instead.
+  const JSJitInfo *info = FUNCTION_VALUE_TO_JITINFO(args.calleev());
+  prototypes::ID protoID = static_cast<prototypes::ID>(info->protoID);
+  if (!args.thisv().isObject()) {
+    ThrowInvalidThis(cx, args, false, protoID);
+    return ConvertExceptionToPromise(cx, xpc::XrayAwareCalleeGlobal(callee),
+                                     args.rval());
+  }
+  JS::Rooted<JSObject*> obj(cx, &args.thisv().toObject());
+
+  void* self;
+  {
+    nsresult rv = UnwrapObject<void>(obj, self, protoID, info->depth);
+    if (NS_FAILED(rv)) {
+      ThrowInvalidThis(cx, args, rv == NS_ERROR_XPC_SECURITY_MANAGER_VETO,
+                       protoID);
+      return ConvertExceptionToPromise(cx, xpc::XrayAwareCalleeGlobal(callee),
+                                       args.rval());
+    }
+  }
+  MOZ_ASSERT(info->type() == JSJitInfo::Getter);
+  JSJitGetterOp getter = info->getter;
+  bool ok = getter(cx, obj, self, JSJitGetterCallArgs(args));
+  if (ok) {
+#ifdef DEBUG
+    AssertReturnTypeMatchesJitinfo(info, args.rval());
+#endif
+    return true;
+  }
+
+  // Promise-returning getters always return objects
+  MOZ_ASSERT(info->returnType() == JSVAL_TYPE_OBJECT);
+  return ConvertExceptionToPromise(cx, xpc::XrayAwareCalleeGlobal(callee),
+                                   args.rval());
+}
+
 bool
 GenericBindingSetter(JSContext* cx, unsigned argc, JS::Value* vp)
 {

dom/bindings/BindingUtils.h

@@ -2997,6 +2997,9 @@ class PinnedStringId
 bool
 GenericBindingGetter(JSContext* cx, unsigned argc, JS::Value* vp);
 
+bool
+GenericPromiseReturningBindingGetter(JSContext* cx, unsigned argc, JS::Value* vp);
+
 bool
 GenericBindingSetter(JSContext* cx, unsigned argc, JS::Value* vp);
 

dom/bindings/Codegen.py

@@ -2652,15 +2652,38 @@ class AttrDefiner(PropertyDefiner):
 
         def getter(attr):
             if self.static:
+                if attr.type.isPromise():
+                    raise TypeError("Don't know how to handle "
+                                    "static Promise-returning "
+                                    "attribute %s.%s" %
+                                    (self.descriptor.name,
+                                     attr.identifier.name))
                 accessor = 'get_' + IDLToCIdentifier(attr.identifier.name)
                 jitinfo = "nullptr"
             else:
                 if attr.hasLenientThis():
+                    if attr.type.isPromise():
+                        raise TypeError("Don't know how to handle "
+                                        "[LenientThis] Promise-returning "
+                                        "attribute %s.%s" %
+                                        (self.descriptor.name,
+                                         attr.identifier.name))
                     accessor = "genericLenientGetter"
                 elif attr.getExtendedAttribute("CrossOriginReadable"):
+                    if attr.type.isPromise():
+                        raise TypeError("Don't know how to handle "
+                                        "cross-origin Promise-returning "
+                                        "attribute %s.%s" %
+                                        (self.descriptor.name,
+                                         attr.identifier.name))
                     accessor = "genericCrossOriginGetter"
                 elif self.descriptor.needsSpecialGenericOps():
-                    accessor = "genericGetter"
+                    if attr.type.isPromise():
+                        accessor = "genericPromiseReturningGetter"
+                    else:
+                        accessor = "genericGetter"
+                elif attr.type.isPromise():
+                    accessor = "GenericPromiseReturningBindingGetter"
                 else:
                     accessor = "GenericBindingGetter"
                 jitinfo = ("&%s_getterinfo" %
@@ -8831,6 +8854,53 @@ class CGGenericGetter(CGAbstractBindingMethod):
             """))
 
 
+class CGGenericPromiseReturningGetter(CGAbstractBindingMethod):
+    """
+    A class for generating the C++ code for an IDL getter that returns a Promise.
+
+    Does not handle cross-origin this or [LenientThis].
+    """
+    def __init__(self, descriptor):
+        unwrapFailureCode = fill(
+            """
+            ThrowInvalidThis(cx, args, %%(securityError)s, "${iface}");
+            return ConvertExceptionToPromise(cx, xpc::XrayAwareCalleeGlobal(callee),
+                                             args.rval());
+            """,
+            iface=descriptor.interface.identifier.name)
+        name = "genericPromiseReturningGetter"
+        customCallArgs = dedent(
+            """
+            JS::CallArgs args = JS::CallArgsFromVp(argc, vp);
+            // Make sure to save the callee before someone maybe messes with rval().
+            JS::Rooted<JSObject*> callee(cx, &args.callee());
+            """)
+
+        CGAbstractBindingMethod.__init__(self, descriptor, name,
+                                         JSNativeArguments(),
+                                         callArgs=customCallArgs,
+                                         unwrapFailureCode=unwrapFailureCode)
+
+    def generate_code(self):
+        return CGGeneric(dedent(
+            """
+            const JSJitInfo *info = FUNCTION_VALUE_TO_JITINFO(args.calleev());
+            MOZ_ASSERT(info->type() == JSJitInfo::Getter);
+            JSJitGetterOp getter = info->getter;
+            bool ok = getter(cx, obj, self, JSJitGetterCallArgs(args));
+            if (ok) {
+            #ifdef DEBUG
+              AssertReturnTypeMatchesJitinfo(info, args.rval());
+            #endif
+              return true;
+            }
+
+            MOZ_ASSERT(info->returnType() == JSVAL_TYPE_OBJECT);
+            return ConvertExceptionToPromise(cx, xpc::XrayAwareCalleeGlobal(callee),
+                                             args.rval());
+            """))
+
+
 class CGSpecializedGetter(CGAbstractStaticMethod):
     """
     A class for generating the code for a specialized attribute getter
@@ -8946,6 +9016,41 @@ class CGSpecializedGetter(CGAbstractStaticMethod):
         return nativeName
 
 
+class CGGetterPromiseWrapper(CGAbstractStaticMethod):
+    """
+    A class for generating a wrapper around another getter that will
+    convert exceptions to promises.
+    """
+    def __init__(self, descriptor, getterToWrap):
+        self.getter = getterToWrap
+        name = self.makeName(getterToWrap.name)
+        args = list(getterToWrap.args)
+        CGAbstractStaticMethod.__init__(self, descriptor, name, 'bool', args)
+
+    def definition_body(self):
+        return fill(
+            """
+            bool ok = ${getterName}(${args});
+            if (ok) {
+              return true;
+            }
+            JS::Rooted<JSObject*> globalForPromise(cx);
+            // We can't use xpc::XrayAwareCalleeGlobal here because we have no
+            // callee.  Use our hacky version instead.
+            if (!xpc::XrayAwareCalleeGlobalForSpecializedGetters(cx, obj,
+                                                                 &globalForPromise)) {
+              return false;
+            }
+            return ConvertExceptionToPromise(cx, globalForPromise, args.rval());
+            """,
+            getterName=self.getter.name,
+            args=", ".join(arg.name for arg in self.args))
+
+    @staticmethod
+    def makeName(getterName):
+        return getterName + "_promiseWrapper"
+
+
 class CGStaticGetter(CGAbstractStaticBindingMethod):
     """
     A class for generating the C++ code for an IDL static attribute getter.
@@ -9237,8 +9342,10 @@ class CGMemberJITInfo(CGThing):
                           IDLToCIdentifier(self.member.identifier.name))
             # We need the cast here because JSJitGetterOp has a "void* self"
             # while we have the right type.
-            getter = ("(JSJitGetterOp)get_%s" %
-                      IDLToCIdentifier(self.member.identifier.name))
+            name = IDLToCIdentifier(self.member.identifier.name)
+            if self.member.type.isPromise():
+                name = CGGetterPromiseWrapper.makeName(name)
+            getter = ("(JSJitGetterOp)get_%s" % name)
             extendedAttrs = self.descriptor.getExtendedAttributes(self.member, getter=True)
             getterinfal = "infallible" in extendedAttrs
 
@@ -12192,6 +12299,7 @@ class MemberProperties:
         self.isGenericGetter = False
         self.isLenientGetter = False
         self.isCrossOriginGetter = False
+        self.isPromiseReturningGetter = False
         self.isGenericSetter = False
         self.isLenientSetter = False
         self.isCrossOriginSetter = False
@@ -12220,7 +12328,10 @@ def memberProperties(m, descriptor):
             elif m.getExtendedAttribute("CrossOriginReadable"):
                 props.isCrossOriginGetter = True
             elif descriptor.needsSpecialGenericOps():
-                props.isGenericGetter = True
+                if m.type.isPromise():
+                    props.isPromiseReturningGetter = True
+                else:
+                    props.isGenericGetter = True
         if not m.readonly:
             if not m.isStatic() and descriptor.interface.hasInterfacePrototypeObject():
                 if m.hasLenientThis():
@@ -12262,7 +12373,8 @@ class CGDescriptor(CGThing):
         # These are set to true if at least one non-static
         # method/getter/setter or jsonifier exist on the interface.
         (hasMethod, hasGetter, hasLenientGetter, hasSetter, hasLenientSetter,
-            hasPromiseReturningMethod) = False, False, False, False, False, False
+            hasPromiseReturningMethod, hasPromiseReturningGetter) = (
+                False, False, False, False, False, False, False)
         jsonifierMethod = None
         crossOriginMethods, crossOriginGetters, crossOriginSetters = set(), set(), set()
         unscopableNames = list()
@@ -12313,7 +12425,10 @@ class CGDescriptor(CGThing):
                 elif descriptor.interface.hasInterfacePrototypeObject():
                     if isNonExposedNavigatorObjectGetter(m, descriptor):
                         continue
-                    cgThings.append(CGSpecializedGetter(descriptor, m))
+                    specializedGetter = CGSpecializedGetter(descriptor, m)
+                    cgThings.append(specializedGetter)
+                    if m.type.isPromise():
+                        cgThings.append(CGGetterPromiseWrapper(descriptor, specializedGetter))
                     if props.isCrossOriginGetter:
                         crossOriginGetters.add(m.identifier.name)
                 if not m.readonly:
@@ -12339,6 +12454,8 @@ class CGDescriptor(CGThing):
             hasMethod = hasMethod or props.isGenericMethod
             hasPromiseReturningMethod = (hasPromiseReturningMethod or
                                          props.isPromiseReturningMethod)
+            hasPromiseReturningGetter = (hasPromiseReturningGetter or
+                                         props.isPromiseReturningGetter)
             hasGetter = hasGetter or props.isGenericGetter
             hasLenientGetter = hasLenientGetter or props.isLenientGetter
             hasSetter = hasSetter or props.isGenericSetter
@@ -12357,6 +12474,8 @@ class CGDescriptor(CGThing):
                                             allowCrossOriginThis=True))
         if hasGetter:
             cgThings.append(CGGenericGetter(descriptor))
+        if hasPromiseReturningGetter:
+            cgThings.append(CGGenericPromiseReturningGetter(descriptor))
         if hasLenientGetter:
             cgThings.append(CGGenericGetter(descriptor, lenientThis=True))
         if len(crossOriginGetters):

testing/web-platform/tests/resources/idlharness.js

@@ -1117,10 +1117,12 @@ IdlInterface.prototype.test_member_const = function(member)
 //@}
 IdlInterface.prototype.test_member_attribute = function(member)
 //@{
-{
-    test(function()
+  {
+    var a_test = async_test(this.name + " interface: attribute " + member.name);
+    a_test.step(function()
     {
         if (this.is_callback() && !this.has_constants()) {
+            a_test.done()
             return;
         }
 
@@ -1133,6 +1135,7 @@ IdlInterface.prototype.test_member_attribute = function(member)
             assert_own_property(self[this.name], member.name,
                 "The interface object must have a property " +
                 format_value(member.name));
+            a_test.done();
         } else if (this.is_global()) {
             assert_own_property(self, member.name,
                 "The global object must have a property " +
@@ -1161,23 +1164,42 @@ IdlInterface.prototype.test_member_attribute = function(member)
                               "Gets on a global should not require an explicit this");
             }
 
-            this.do_interface_attribute_asserts(self, member);
+            // do_interface_attribute_asserts must be the last thing we do,
+            // since it will call done() on a_test.
+            this.do_interface_attribute_asserts(self, member, a_test);
         } else {
             assert_true(member.name in self[this.name].prototype,
                 "The prototype object must have a property " +
                 format_value(member.name));
 
             if (!member.has_extended_attribute("LenientThis")) {
-                assert_throws(new TypeError(), function() {
-                    self[this.name].prototype[member.name];
-                }.bind(this), "getting property on prototype object must throw TypeError");
+                if (member.idlType.generic !== "Promise") {
+                    assert_throws(new TypeError(), function() {
+                        self[this.name].prototype[member.name];
+                    }.bind(this), "getting property on prototype object must throw TypeError");
+                    // do_interface_attribute_asserts must be the last thing we
+                    // do, since it will call done() on a_test.
+                    this.do_interface_attribute_asserts(self[this.name].prototype, member, a_test);
+                } else {
+                    promise_rejects(a_test, new TypeError(),
+                                    self[this.name].prototype[member.name])
+                        .then(function() {
+                            // do_interface_attribute_asserts must be the last
+                            // thing we do, since it will call done() on a_test.
+                            this.do_interface_attribute_asserts(self[this.name].prototype,
+                                                                member, a_test);
+                        }.bind(this));
+                }
             } else {
                 assert_equals(self[this.name].prototype[member.name], undefined,
                               "getting property on prototype object must return undefined");
+              // do_interface_attribute_asserts must be the last thing we do,
+              // since it will call done() on a_test.
+              this.do_interface_attribute_asserts(self[this.name].prototype, member, a_test);
             }
-            this.do_interface_attribute_asserts(self[this.name].prototype, member);
+
         }
-    }.bind(this), this.name + " interface: attribute " + member.name);
+    }.bind(this));
 };
 
 //@}
@@ -1522,12 +1544,13 @@ IdlInterface.prototype.test_interface_of = function(desc, obj, exception, expect
         var member = this.members[i];
         if (member.type == "attribute" && member.isUnforgeable)
         {
-            test(function()
-            {
+            var a_test = async_test(this.name + " interface: " + desc + ' must have own property "' + member.name + '"');
+            a_test.step(function() {
                 assert_equals(exception, null, "Unexpected exception when evaluating object");
                 assert_equals(typeof obj, expected_typeof, "wrong typeof object");
-                this.do_interface_attribute_asserts(obj, member);
-            }.bind(this), this.name + " interface: " + desc + ' must have own property "' + member.name + '"');
+                // Call do_interface_attribute_asserts last, since it will call a_test.done()
+                this.do_interface_attribute_asserts(obj, member, a_test);
+            }.bind(this));
         }
         else if (member.type == "operation" &&
                  member.name &&
@@ -1646,7 +1669,7 @@ IdlInterface.prototype.has_stringifier = function()
 };
 
 //@}
-IdlInterface.prototype.do_interface_attribute_asserts = function(obj, member)
+IdlInterface.prototype.do_interface_attribute_asserts = function(obj, member, a_test)
 //@{
 {
     // This function tests WebIDL as of 2015-01-27.
@@ -1656,6 +1679,8 @@ IdlInterface.prototype.do_interface_attribute_asserts = function(obj, member)
     // it is not a global, and the global otherwise, and by test_interface_of()
     // with the object as obj.
 
+    var pendingPromises = [];
+
     // "For each exposed attribute of the interface, whether it was declared on
     // the interface itself or one of its consequential interfaces, there MUST
     // exist a corresponding property. The characteristics of this property are
@@ -1695,9 +1720,15 @@ IdlInterface.prototype.do_interface_attribute_asserts = function(obj, member)
         // attribute, then return undefined.
         // "Otherwise, throw a TypeError."
         if (!member.has_extended_attribute("LenientThis")) {
-            assert_throws(new TypeError(), function() {
-                desc.get.call({});
-            }.bind(this), "calling getter on wrong object type must throw TypeError");
+            if (member.idlType.generic !== "Promise") {
+                assert_throws(new TypeError(), function() {
+                    desc.get.call({});
+                }.bind(this), "calling getter on wrong object type must throw TypeError");
+            } else {
+                pendingPromises.push(
+                    promise_rejects(a_test, new TypeError(), desc.get.call({}),
+                                    "calling getter on wrong object type must reject the return promise with TypeError"));
+            }
         } else {
             assert_equals(desc.get.call({}), undefined,
                           "calling getter on wrong object type must return undefined");
@@ -1748,6 +1779,8 @@ IdlInterface.prototype.do_interface_attribute_asserts = function(obj, member)
         // value 1."
         assert_equals(desc.set.length, 1, "setter length must be 1");
     }
+
+    Promise.all(pendingPromises).then(a_test.done.bind(a_test));
 }
 //@}