Bug 1446246 part 1. Use a single handwritten HTMLConstructor implementation, instead of code-generating lots of very similar implementations. r=peterv
authorBoris Zbarsky <bzbarsky@mit.edu>
Tue, 27 Mar 2018 15:49:02 -0400
changeset 773601 b470762b46768b7867824cda8e0190037913b3fe
parent 773600 d9b7fe99807c6325bc73db92b548e29f37889898
child 773602 9ed0718ede32ea01c67a47b8f143152e860a8be2
push id104266
push userbmo:hsivonen@hsivonen.fi
push dateWed, 28 Mar 2018 07:33:03 +0000
reviewerspeterv
bugs1446246, 1274159
milestone61.0a1
Bug 1446246 part 1. Use a single handwritten HTMLConstructor implementation, instead of code-generating lots of very similar implementations. r=peterv The codegen changes are mostly a backout of the changes made in bug 1274159. The HTMLConstructor implementation is mostly copied from one of the code-generated ones, with a few modifications: * Derive the interface name from the proto id instead of hardcoding it. * Use the proto/constructor ids to get constructor and prototype objects. * Use ErrorResult instead of FastErrorResult; we don't want the precedent of using FastErrorResult in non-generated code. There will be further changes to combine HTMLConstructor and CreateXULOrHTMLElement, in a separate changeset. MozReview-Commit-ID: 44D0qw23ioP
dom/bindings/BindingUtils.cpp
dom/bindings/BindingUtils.h
dom/bindings/Codegen.py
--- a/dom/bindings/BindingUtils.cpp
+++ b/dom/bindings/BindingUtils.cpp
@@ -3506,16 +3506,112 @@ GetDesiredProto(JSContext* aCx, const JS
     return true;
   }
 
   aDesiredProto.set(&protoVal.toObject());
   return true;
 }
 
 // https://html.spec.whatwg.org/multipage/dom.html#htmlconstructor
+namespace binding_detail {
+bool
+HTMLConstructor(JSContext* aCx, unsigned aArgc, JS::Value* aVp,
+                constructors::id::ID aConstructorId,
+                prototypes::id::ID aProtoId,
+                CreateInterfaceObjectsMethod aCreator)
+{
+  JS::CallArgs args = JS::CallArgsFromVp(aArgc, aVp);
+  JS::Rooted<JSObject*> obj(aCx, &args.callee());
+  if (!args.isConstructing()) {
+    return ThrowConstructorWithoutNew(aCx,
+                                      NamesOfInterfacesWithProtos(aProtoId));
+  }
+
+  GlobalObject global(aCx, obj);
+  if (global.Failed()) {
+    return false;
+  }
+
+  // The newTarget might be a cross-compartment wrapper. Get the underlying object
+  // so we can do the spec's object-identity checks.
+  JS::Rooted<JSObject*> newTarget(aCx, js::CheckedUnwrap(&args.newTarget().toObject()));
+  if (!newTarget) {
+    return ThrowErrorMessage(aCx, MSG_ILLEGAL_CONSTRUCTOR);
+  }
+
+  // Step 2 of https://html.spec.whatwg.org/multipage/dom.html#htmlconstructor.
+  // Enter the compartment of our underlying newTarget object, so we end
+  // up comparing to the constructor object for our interface from that global.
+  {
+    JSAutoCompartment ac(aCx, newTarget);
+    JS::Handle<JSObject*> constructor =
+      GetPerInterfaceObjectHandle(aCx, aConstructorId, aCreator,
+                                  true);
+    if (!constructor) {
+      return false;
+    }
+    if (newTarget == constructor) {
+      return ThrowErrorMessage(aCx, MSG_ILLEGAL_CONSTRUCTOR);
+    }
+  }
+
+  JS::Rooted<JSObject*> desiredProto(aCx);
+  if (!GetDesiredProto(aCx, args, &desiredProto)) {
+    return false;
+  }
+  if (!desiredProto) {
+    // Step 7 of https://html.spec.whatwg.org/multipage/dom.html#htmlconstructor.
+    // This fallback behavior is designed to match analogous behavior for the
+    // JavaScript built-ins. So we enter the compartment 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-compartment with the proxy, or bound functions, etc).
+    // https://bugzilla.mozilla.org/show_bug.cgi?id=1317658
+    {
+      JSAutoCompartment ac(aCx, newTarget);
+      desiredProto = GetPerInterfaceObjectHandle(aCx, aProtoId, aCreator, true);
+      if (!desiredProto) {
+          return false;
+      }
+    }
+
+    // desiredProto is in the compartment of the underlying newTarget object.
+    // Wrap it into the context compartment.
+    if (!JS_WrapObject(aCx, &desiredProto)) {
+      return false;
+    }
+  }
+
+  bool objIsXray = xpc::WrapperFactory::IsXrayWrapper(obj);
+  Maybe<JSAutoCompartment> ac;
+  if (objIsXray) {
+    obj = js::CheckedUnwrap(obj);
+    if (!obj) {
+      return false;
+    }
+    ac.emplace(aCx, obj);
+    if (!JS_WrapObject(aCx, &desiredProto)) {
+      return false;
+    }
+  }
+  ErrorResult rv;
+  RefPtr<Element> result = CreateXULOrHTMLElement(global, args, desiredProto, rv);
+  if (MOZ_UNLIKELY(rv.MaybeSetPendingException(aCx))) {
+    return false;
+  }
+  MOZ_ASSERT(!JS_IsExceptionPending(aCx));
+  if (!GetOrCreateDOMReflector(aCx, result, args.rval(), desiredProto)) {
+    MOZ_ASSERT(JS_IsExceptionPending(aCx));
+    return false;
+  }
+  return true;
+}
+} // namespace binding_detail
+
 already_AddRefed<Element>
 CreateXULOrHTMLElement(const GlobalObject& aGlobal, const JS::CallArgs& aCallArgs,
                        JS::Handle<JSObject*> aGivenProto, ErrorResult& aRv)
 {
   // Step 1.
   nsCOMPtr<nsPIDOMWindowInner> window = do_QueryInterface(aGlobal.GetAsSupports());
   if (!window) {
     aRv.Throw(NS_ERROR_UNEXPECTED);
--- a/dom/bindings/BindingUtils.h
+++ b/dom/bindings/BindingUtils.h
@@ -18,16 +18,17 @@
 #include "mozilla/DeferredFinalize.h"
 #include "mozilla/dom/BindingDeclarations.h"
 #include "mozilla/dom/CallbackObject.h"
 #include "mozilla/dom/DOMJSClass.h"
 #include "mozilla/dom/DOMJSProxyHandler.h"
 #include "mozilla/dom/Exceptions.h"
 #include "mozilla/dom/NonRefcountedDOMObject.h"
 #include "mozilla/dom/Nullable.h"
+#include "mozilla/dom/PrototypeList.h"
 #include "mozilla/dom/RootedDictionary.h"
 #include "mozilla/SegmentedVector.h"
 #include "mozilla/ErrorResult.h"
 #include "mozilla/Likely.h"
 #include "mozilla/MemoryReporting.h"
 #include "nsAutoPtr.h"
 #include "nsIDocument.h"
 #include "nsIGlobalObject.h"
@@ -3442,14 +3443,21 @@ namespace binding_detail {
 // environment controlled by a hostile adversary.  This is because in the worker
 // case the global is in fact the worker global, so it and its standard objects
 // are controlled by the worker script.  This is why this function is in the
 // binding_detail namespace.  Any use of this function MUST be very carefully
 // reviewed by someone who is sufficiently devious and has a very good
 // understanding of all the code that will run while we're using the return
 // value, including the SpiderMonkey parts.
 JSObject* UnprivilegedJunkScopeOrWorkerGlobal();
+
+// Implementation of the [HTMLConstructor] extended attribute.
+bool
+HTMLConstructor(JSContext* aCx, unsigned aArgc, JS::Value* aVp,
+                constructors::id::ID aConstructorId,
+                prototypes::id::ID aProtoId,
+                CreateInterfaceObjectsMethod aCreator);
 } // namespace binding_detail
 
 } // namespace dom
 } // namespace mozilla
 
 #endif /* mozilla_dom_BindingUtils_h__ */
--- a/dom/bindings/Codegen.py
+++ b/dom/bindings/Codegen.py
@@ -1820,16 +1820,36 @@ class CGClassConstructor(CGAbstractStati
         if not self._ctor:
             return ""
         return CGAbstractStaticMethod.define(self)
 
     def definition_body(self):
         return self.generate_code()
 
     def generate_code(self):
+        if self._ctor.isHTMLConstructor():
+            # We better have a prototype object.  Otherwise our proto
+            # id won't make sense.
+            assert self.descriptor.interface.hasInterfacePrototypeObject()
+            # We also better have a constructor object, if this is
+            # getting called!
+            assert self.descriptor.interface.hasInterfaceObject()
+            # We can't just pass null for the CreateInterfaceObjects callback,
+            # because our newTarget might be in a different compartment, in
+            # which case we'll need to look up constructor objects in that
+            # compartment.
+            return fill(
+                """
+                return binding_detail::HTMLConstructor(cx, argc, vp,
+                                                       constructors::id::${name},
+                                                       prototypes::id::${name},
+                                                       CreateInterfaceObjects);
+                """,
+                name=self.descriptor.name)
+
         # [ChromeOnly] interfaces may only be constructed by chrome.
         chromeOnlyCheck = ""
         if isChromeOnly(self._ctor):
             chromeOnlyCheck = dedent("""
                 if (!nsContentUtils::ThreadsafeIsSystemCaller(cx)) {
                   return ThrowingConstructor(cx, argc, vp);
                 }
 
@@ -1843,126 +1863,40 @@ class CGClassConstructor(CGAbstractStati
         # the name JS sees is the interface name; for named constructors
         # identifier.name is the actual name.
         name = self._ctor.identifier.name
         if name != "constructor":
             ctorName = name
         else:
             ctorName = self.descriptor.interface.identifier.name
 
-        # [HTMLConstructor] for custom element
-        # This needs to live in bindings code because it directly examines
-        # newtarget and the callee function to do HTMLConstructor specific things.
-        if self._ctor.isHTMLConstructor():
-            htmlConstructorSanityCheck = dedent("""
-                // The newTarget might be a cross-compartment wrapper. Get the underlying object
-                // so we can do the spec's object-identity checks.
-                JS::Rooted<JSObject*> newTarget(cx, js::CheckedUnwrap(&args.newTarget().toObject()));
-                if (!newTarget) {
-                  return ThrowErrorMessage(cx, MSG_ILLEGAL_CONSTRUCTOR);
-                }
-
-                // Step 2 of https://html.spec.whatwg.org/multipage/dom.html#htmlconstructor.
-                // Enter the compartment of our underlying newTarget object, so we end
-                // up comparing to the constructor object for our interface from that global.
-                {
-                  JSAutoCompartment ac(cx, newTarget);
-                  JS::Handle<JSObject*> constructor(GetConstructorObjectHandle(cx));
-                  if (!constructor) {
-                    return false;
-                  }
-                  if (newTarget == constructor) {
-                    return ThrowErrorMessage(cx, MSG_ILLEGAL_CONSTRUCTOR);
-                  }
-                }
-
-                """)
-
-            # If we are unable to get desired prototype from newTarget, then we
-            # fall back to the interface prototype object from newTarget's realm.
-            htmlConstructorFallback = dedent("""
-                if (!desiredProto) {
-                  // Step 7 of https://html.spec.whatwg.org/multipage/dom.html#htmlconstructor.
-                  // This fallback behavior is designed to match analogous behavior for the
-                  // JavaScript built-ins. So we enter the compartment 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-compartment with the proxy, or bound functions, etc).
-                  // https://bugzilla.mozilla.org/show_bug.cgi?id=1317658
-                  {
-                    JSAutoCompartment ac(cx, newTarget);
-                    desiredProto = GetProtoObjectHandle(cx);
-                    if (!desiredProto) {
-                        return false;
-                    }
-                  }
-
-                  // desiredProto is in the compartment of the underlying newTarget object.
-                  // Wrap it into the context compartment.
-                  if (!JS_WrapObject(cx, &desiredProto)) {
-                    return false;
-                  }
-                }
-                """)
-        else:
-            htmlConstructorSanityCheck = ""
-            htmlConstructorFallback = ""
-
-
-        # If we're a constructor, "obj" may not be a function, so calling
-        # XrayAwareCalleeGlobal() on it is not safe.  Of course in the
-        # constructor case either "obj" is an Xray or we're already in the
-        # content compartment, not the Xray compartment, so just
-        # constructing the GlobalObject from "obj" is fine.
         preamble = fill(
             """
             JS::CallArgs args = JS::CallArgsFromVp(argc, vp);
             JS::Rooted<JSObject*> obj(cx, &args.callee());
             $*{chromeOnlyCheck}
             if (!args.isConstructing()) {
               // XXXbz wish I could get the name from the callee instead of
               // Adding more relocations
               return ThrowConstructorWithoutNew(cx, "${ctorName}");
             }
 
-            GlobalObject global(cx, obj);
-            if (global.Failed()) {
-              return false;
-            }
-
-            $*{htmlConstructorSanityCheck}
             JS::Rooted<JSObject*> desiredProto(cx);
             if (!GetDesiredProto(cx, args, &desiredProto)) {
               return false;
             }
-            $*{htmlConstructorFallback}
             """,
             chromeOnlyCheck=chromeOnlyCheck,
-            ctorName=ctorName,
-            htmlConstructorSanityCheck=htmlConstructorSanityCheck,
-            htmlConstructorFallback=htmlConstructorFallback)
-
-        if  self._ctor.isHTMLConstructor():
-            signatures = self._ctor.signatures()
-            assert len(signatures) == 1
-            # Given that HTMLConstructor takes no args, we can just codegen a
-            # call to CreateXULOrHTMLElement() in BindingUtils which reuses the
-            # factory thing in HTMLContentSink. Then we don't have to implement
-            # Constructor on all the HTML elements.
-            callGenerator = CGPerSignatureCall(signatures[0][0], signatures[0][1],
-                                               "CreateXULOrHTMLElement", True,
-                                               self.descriptor, self._ctor,
-                                               isConstructor=True)
-        else:
-            name = self._ctor.identifier.name
-            nativeName = MakeNativeName(self.descriptor.binaryNameFor(name))
-            callGenerator = CGMethodCall(nativeName, True, self.descriptor,
-                                         self._ctor, isConstructor=True,
-                                         constructorName=ctorName)
+            ctorName=ctorName)
+
+        name = self._ctor.identifier.name
+        nativeName = MakeNativeName(self.descriptor.binaryNameFor(name))
+        callGenerator = CGMethodCall(nativeName, True, self.descriptor,
+                                     self._ctor, isConstructor=True,
+                                     constructorName=ctorName)
         return preamble + "\n" + callGenerator.define()
 
     def profiler_label_and_jscontext(self):
         name = self._ctor.identifier.name
         if name != "constructor":
             ctorName = name
         else:
             ctorName = self.descriptor.interface.identifier.name
@@ -7757,33 +7691,36 @@ class CGPerSignatureCall(CGThing):
             cgThings.append(CGGeneric(dedent(
                 """
                 bool foundNonFiniteFloat = false;
                 """)))
             lenientFloatCode = "foundNonFiniteFloat = true;\n"
 
         argsPre = []
         if idlNode.isStatic():
-            # If we're a constructor, the GlobalObject struct will be created in
-            # CGClassConstructor.
-            if not isConstructor:
-                cgThings.append(CGGeneric(dedent(
-                    """
-                    GlobalObject global(cx, xpc::XrayAwareCalleeGlobal(obj));
-                    if (global.Failed()) {
-                      return false;
-                    }
-
-                    """)))
-
+            # If we're a constructor, "obj" may not be a function, so calling
+            # XrayAwareCalleeGlobal() on it is not safe.  Of course in the
+            # constructor case either "obj" is an Xray or we're already in the
+            # content compartment, not the Xray compartment, so just
+            # constructing the GlobalObject from "obj" is fine.
+            if isConstructor:
+                objForGlobalObject = "obj"
+            else:
+                objForGlobalObject = "xpc::XrayAwareCalleeGlobal(obj)"
+            cgThings.append(CGGeneric(fill(
+                """
+                GlobalObject global(cx, ${obj});
+                if (global.Failed()) {
+                  return false;
+                }
+
+                """,
+                obj=objForGlobalObject)))
             argsPre.append("global")
 
-        if isConstructor and idlNode.isHTMLConstructor():
-            argsPre.extend(["args", "desiredProto"])
-
         # For JS-implemented interfaces we do not want to base the
         # needsCx decision on the types involved, just on our extended
         # attributes. Also, JSContext is not needed for the static case
         # since GlobalObject already contains the context.
         needsCx = needCx(returnType, arguments, self.extendedAttributes,
                          not descriptor.interface.isJSImplemented(), static)
         if needsCx:
             argsPre.append("cx")