Bug 1317658 part 2. Pass enough information to bindings' GetDesiredProto to allow it to return the prototype the spec says it should. r=edgar

Differential Revision: https://phabricator.services.mozilla.com/D28518

--HG--
extra : moz-landing-system : lando

dom/bindings/BindingUtils.cpp

@@ -3435,7 +3435,12 @@ static inline prototypes::ID GetProtoIdForNewtarget(
 }
 
 bool GetDesiredProto(JSContext* aCx, const JS::CallArgs& aCallArgs,
+                     prototypes::id::ID aProtoId,
+                     CreateInterfaceObjectsMethod aCreator,
                      JS::MutableHandle<JSObject*> aDesiredProto) {
+  // This basically implements
+  // https://heycam.github.io/webidl/#internally-create-a-new-object-implementing-the-interface
+  // step 3.
   MOZ_ASSERT(aCallArgs.isConstructing(), "How did we end up here?");
 
   // The desired prototype depends on the actual constructor that was invoked,
@@ -3448,6 +3453,7 @@ bool GetDesiredProto(JSContext* aCx, const JS::CallArgs& aCallArgs,
   // property is non-configurable and non-writable, so we don't have to do the
   // slow JS_GetProperty call.
   JS::Rooted<JSObject*> newTarget(aCx, &aCallArgs.newTarget().toObject());
+  MOZ_ASSERT(JS::IsCallable(newTarget));
   JS::Rooted<JSObject*> originalNewTarget(aCx, newTarget);
   // See whether we have a known DOM constructor here, such that we can take a
   // fast path.
@@ -3474,26 +3480,44 @@ bool GetDesiredProto(JSContext* aCx, const JS::CallArgs& aCallArgs,
 
   // Slow path.  This basically duplicates the ES6 spec's
   // GetPrototypeFromConstructor except that instead of taking a string naming
-  // the fallback prototype we just fall back to using null and assume that our
-  // caller will then pick the right default.  The actual defaulting behavior
-  // here still needs to be defined in the Web IDL specification.
+  // the fallback prototype we determine the fallback based on the proto id we
+  // were handed.
   //
   // Note that it's very important to do this property get on originalNewTarget,
   // not our unwrapped newTarget, since we want to get Xray behavior here as
   // needed.
   // XXXbz for speed purposes, using a preinterned id here sure would be nice.
+  // We can't use GetJSIDByIndex, because that only works on the main thread,
+  // not workers.
   JS::Rooted<JS::Value> protoVal(aCx);
   if (!JS_GetProperty(aCx, originalNewTarget, "prototype", &protoVal)) {
     return false;
   }
 
-  if (!protoVal.isObject()) {
-    aDesiredProto.set(nullptr);
+  if (protoVal.isObject()) {
+    aDesiredProto.set(&protoVal.toObject());
     return true;
   }
 
-  aDesiredProto.set(&protoVal.toObject());
-  return true;
+  // Fall back to getting the proto for our given proto id in the realm that
+  // GetFunctionRealm(newTarget) returns.
+  JS::Rooted<JS::Realm*> realm(aCx, JS::GetFunctionRealm(aCx, newTarget));
+  if (!realm) {
+    return false;
+  }
+
+  {
+    // JS::GetRealmGlobalOrNull should not be returning null here, because we
+    // have live objects in the Realm.
+    JSAutoRealm ar(aCx, JS::GetRealmGlobalOrNull(realm));
+    aDesiredProto.set(
+        GetPerInterfaceObjectHandle(aCx, aProtoId, aCreator, true));
+    if (!aDesiredProto) {
+      return false;
+    }
+  }
+
+  return MaybeWrapObject(aCx, aDesiredProto);
 }
 
 namespace {
@@ -3628,7 +3652,7 @@ bool HTMLConstructor(JSContext* aCx, unsigned aArgc, JS::Value* aVp,
     return ThrowErrorMessage(aCx, MSG_ILLEGAL_CONSTRUCTOR);
   }
 
-  // Steps 4 and 5 do some sanity checks on our callee.  We add to those a
+  // Steps 4, 5, 6 do some sanity checks on our callee.  We add to those a
   // determination of what sort of element we're planning to construct.
   // Technically, this should happen (implicitly) in step 8, but this
   // determination is side-effect-free, so it's OK.
@@ -3714,35 +3738,13 @@ bool HTMLConstructor(JSContext* aCx, unsigned aArgc, JS::Value* aVp,
     }
   }
 
-  // Step 6.
+  // Steps 7 and 8.
   JS::Rooted<JSObject*> desiredProto(aCx);
-  if (!GetDesiredProto(aCx, args, &desiredProto)) {
+  if (!GetDesiredProto(aCx, args, aProtoId, aCreator, &desiredProto)) {
     return false;
   }
 
-  // Step 7.
-  if (!desiredProto) {
-    // This fallback behavior is designed to match analogous behavior for the
-    // JavaScript built-ins. So we enter the realm of our underlying newTarget
-    // object and fall back to the prototype object from that global.
-    // XXX The spec says to use GetFunctionRealm(), which is not actually
-    // the same thing as what we have here (e.g. in the case of scripted
-    // callable proxies whose target is not same-realm with the proxy, or bound
-    // functions, etc). https://bugzilla.mozilla.org/show_bug.cgi?id=1317658
-    {
-      JSAutoRealm ar(aCx, newTarget);
-      desiredProto = GetPerInterfaceObjectHandle(aCx, aProtoId, aCreator, true);
-      if (!desiredProto) {
-        return false;
-      }
-    }
-
-    // desiredProto is in the realm of the underlying newTarget object.
-    // Wrap it into the context realm.
-    if (!JS_WrapObject(aCx, &desiredProto)) {
-      return false;
-    }
-  }
+  MOZ_ASSERT(desiredProto, "How could we not have a prototype by now?");
 
   // We need to do some work to actually return an Element, so we do step 8 on
   // one branch and steps 9-12 on another branch, then common up the "return

dom/bindings/BindingUtils.h

@@ -3063,8 +3063,12 @@ bool GetSetlikeBackingObject(JSContext* aCx, JS::Handle<JSObject*> aObj,
                              bool* aBackingObjCreated);
 
 // Get the desired prototype object for an object construction from the given
-// CallArgs.  Null is returned if the default prototype should be used.
+// CallArgs.  The CallArgs must be for a constructor call.  The
+// aProtoId/aCreator arguments are used to get a default if we don't find a
+// prototype on the newTarget of the callargs.
 bool GetDesiredProto(JSContext* aCx, const JS::CallArgs& aCallArgs,
+                     prototypes::id::ID aProtoId,
+                     CreateInterfaceObjectsMethod aCreator,
                      JS::MutableHandle<JSObject*> aDesiredProto);
 
 // This function is expected to be called from the constructor function for an

dom/bindings/Codegen.py

@@ -1881,12 +1881,16 @@ class CGClassConstructor(CGAbstractStaticMethod):
             }
 
             JS::Rooted<JSObject*> desiredProto(cx);
-            if (!GetDesiredProto(cx, args, &desiredProto)) {
+            if (!GetDesiredProto(cx, args,
+                                 prototypes::id::${name},
+                                 CreateInterfaceObjects,
+                                 &desiredProto)) {
               return false;
             }
             """,
             chromeOnlyCheck=chromeOnlyCheck,
-            ctorName=ctorName)
+            ctorName=ctorName,
+            name=self.descriptor.name)
 
         name = self._ctor.identifier.name
         nativeName = MakeNativeName(self.descriptor.binaryNameFor(name))

testing/web-platform/meta/WebIDL/ecmascript-binding/constructors.html.ini

@@ -1,7 +0,0 @@
-[constructors.html]
-  [Constructor in child window with bad NewTarget from parent window]
-    expected: FAIL
-
-  [Constructor in parent window with bad NewTarget from child window]
-    expected: FAIL
-

testing/web-platform/tests/WebIDL/ecmascript-binding/constructors.html

@@ -89,6 +89,42 @@ async_test(function() {
       assert_equals(Object.getPrototypeOf(dp), child.DOMParser.prototype);
       assert_equals(object_realm(dp), "parent window");
     }, "Constructor in parent window with bad NewTarget from child window");
+
+    test(function() {
+      var badNewTarget = Function.prototype.bind.call(new child.Function());
+      badNewTarget.prototype = 7;
+
+      var dp = Reflect.construct(DOMParser, [], badNewTarget);
+      assert_equals(Object.getPrototypeOf(dp), child.DOMParser.prototype);
+      assert_equals(object_realm(dp), "parent window");
+    }, "Constructor in parent window with bad NewTarget from parent window that's a bound child window function");
+
+    test(function() {
+      var badNewTarget = child.Function.prototype.bind.call(new Function());
+      badNewTarget.prototype = 7;
+
+      var dp = Reflect.construct(child.DOMParser, [], badNewTarget);
+      assert_equals(Object.getPrototypeOf(dp), DOMParser.prototype);
+      assert_equals(object_realm(dp), "child window");
+    }, "Constructor in child window with bad NewTarget from child window that's a bound parent window function");
+
+    test(function() {
+      var badNewTarget = new Proxy(new child.Function(), {});
+      badNewTarget.prototype = 7;
+
+      var dp = Reflect.construct(DOMParser, [], badNewTarget);
+      assert_equals(Object.getPrototypeOf(dp), child.DOMParser.prototype);
+      assert_equals(object_realm(dp), "parent window");
+    }, "Constructor in parent window with bad NewTarget from parent window that's a proxy for a child window function");
+
+    test(function() {
+      var badNewTarget = new child.Proxy(new Function(), {});
+      badNewTarget.prototype = 7;
+
+      var dp = Reflect.construct(child.DOMParser, [], badNewTarget);
+      assert_equals(Object.getPrototypeOf(dp), DOMParser.prototype);
+      assert_equals(object_realm(dp), "child window");
+    }, "Constructor in child window with bad NewTarget from child window that's a proxy for a parent window function");
   });
   iframe.src = "constructors-support.html";
   document.body.appendChild(iframe);

testing/web-platform/tests/custom-elements/htmlconstructor/newtarget.html

@@ -91,6 +91,38 @@ test_with_window(w => {
     returnNotAnObject = true;
     new ElementWithDynamicPrototype();
   }, "If prototype is not object (" + notAnObject + "), derives the fallback from NewTarget's realm (autonomous custom elements)");
+
+  test_with_window(w => {
+    // We have to return an object during define(), but not during super()
+    let returnNotAnObject = false;
+
+    function TestElement() {
+      const o = Reflect.construct(w.HTMLElement, [], new.target);
+
+      assert_equals(Object.getPrototypeOf(new.target), window.Function.prototype);
+      assert_equals(Object.getPrototypeOf(o), window.HTMLElement.prototype,
+        "Must use the HTMLElement from the realm of NewTarget");
+      assert_not_equals(Object.getPrototypeOf(o), w.HTMLElement.prototype,
+        "Must not use the HTMLElement from the realm of the active function object (w.HTMLElement)");
+
+      return o;
+    }
+
+    // Create the proxy in the subframe, which should not affect what our
+    // prototype ends up as.
+    const ElementWithDynamicPrototype = new w.Proxy(TestElement, {
+      get: function (target, name) {
+        if (name == "prototype")
+          return returnNotAnObject ? notAnObject : {};
+        return target[name];
+      }
+    });
+
+    w.customElements.define("test-element", ElementWithDynamicPrototype);
+
+    returnNotAnObject = true;
+    new ElementWithDynamicPrototype();
+  }, "If prototype is not object (" + notAnObject + "), derives the fallback from NewTarget's GetFunctionRealm (autonomous custom elements)");
 });
 
 [null, undefined, 5, "string"].forEach(function (notAnObject) {
@@ -122,6 +154,37 @@ test_with_window(w => {
     returnNotAnObject = true;
     new ElementWithDynamicPrototype();
   }, "If prototype is not object (" + notAnObject + "), derives the fallback from NewTarget's realm (customized built-in elements)");
+
+  test_with_window(w => {
+    // We have to return an object during define(), but not during super()
+    let returnNotAnObject = false;
+
+    function TestElement() {
+      const o = Reflect.construct(w.HTMLParagraphElement, [], new.target);
+
+      assert_equals(Object.getPrototypeOf(o), window.HTMLParagraphElement.prototype,
+        "Must use the HTMLParagraphElement from the realm of NewTarget");
+      assert_not_equals(Object.getPrototypeOf(o), w.HTMLParagraphElement.prototype,
+        "Must not use the HTMLParagraphElement from the realm of the active function object (w.HTMLParagraphElement)");
+
+      return o;
+    }
+
+    // Create the proxy in the subframe, which should not affect what our
+    // prototype ends up as.
+    const ElementWithDynamicPrototype = new w.Proxy(TestElement, {
+      get: function (target, name) {
+        if (name == "prototype")
+          return returnNotAnObject ? notAnObject : {};
+        return target[name];
+      }
+    });
+
+    w.customElements.define("test-element", ElementWithDynamicPrototype, { extends: "p" });
+
+    returnNotAnObject = true;
+    new ElementWithDynamicPrototype();
+  }, "If prototype is not object (" + notAnObject + "), derives the fallback from NewTarget's GetFunctionRealm (customized built-in elements)");
 });
 
 test_with_window(w => {