Bug 1643457 - Support ChromeOnly properties on remote proxies. r=mccr8
authorPeter Van der Beken <peterv@propagandism.org>
Fri, 05 Jun 2020 12:45:40 +0000
changeset 534126 9c1d646353b318e1fbd2c2dc112eccfdd0720f80
parent 534125 58e98c4dbecb3c03e68ff1cd94002cb06844c4be
child 534127 17f21c2824f08e1d32167db63cfed84e18c586ba
push id37483
push userapavel@mozilla.com
push dateFri, 05 Jun 2020 21:40:11 +0000
treeherdermozilla-central@dadc7312128e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmccr8
bugs1643457
milestone79.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 1643457 - Support ChromeOnly properties on remote proxies. r=mccr8 Differential Revision: https://phabricator.services.mozilla.com/D78360
docshell/base/BrowsingContext.cpp
dom/base/MaybeCrossOriginObject.cpp
dom/base/MaybeCrossOriginObject.h
dom/base/RemoteOuterWindowProxy.cpp
dom/base/nsGlobalWindowOuter.cpp
dom/bindings/Codegen.py
dom/bindings/RemoteObjectProxy.h
--- a/docshell/base/BrowsingContext.cpp
+++ b/docshell/base/BrowsingContext.cpp
@@ -1451,18 +1451,17 @@ NS_IMPL_CYCLE_COLLECTION_UNLINK_END
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(BrowsingContext)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(
       mDocShell, mParentWindow, mGroup, mEmbedderElement, mWindowContexts,
       mCurrentWindowContext, mSessionStorageManager, mChildSessionHistory)
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
 
 class RemoteLocationProxy
     : public RemoteObjectProxy<BrowsingContext::LocationProxy,
-                               Location_Binding::sCrossOriginAttributes,
-                               Location_Binding::sCrossOriginMethods> {
+                               Location_Binding::sCrossOriginProperties> {
  public:
   typedef RemoteObjectProxy Base;
 
   constexpr RemoteLocationProxy()
       : RemoteObjectProxy(prototypes::id::Location) {}
 
   void NoteChildren(JSObject* aProxy,
                     nsCycleCollectionTraversalCallback& aCb) const override {
--- a/dom/base/MaybeCrossOriginObject.cpp
+++ b/dom/base/MaybeCrossOriginObject.cpp
@@ -230,17 +230,17 @@ bool MaybeCrossOriginObjectMixins::Cross
 
   // Step 4.
   return ReportCrossOriginDenial(cx, id, NS_LITERAL_CSTRING("set"));
 }
 
 /* static */
 bool MaybeCrossOriginObjectMixins::EnsureHolder(
     JSContext* cx, JS::Handle<JSObject*> obj, size_t slot,
-    JSPropertySpec* attributes, JSFunctionSpec* methods,
+    const CrossOriginProperties& properties,
     JS::MutableHandle<JSObject*> holder) {
   MOZ_ASSERT(!IsPlatformObjectSameOrigin(cx, obj) || IsRemoteObjectProxy(obj),
              "Why are we calling this at all in same-origin cases?");
   // We store the holders in a weakmap stored in obj's slot.  Our object is
   // always a proxy, so we can just go ahead and use GetProxyReservedSlot here.
   JS::Rooted<JS::Value> weakMapVal(cx, js::GetProxyReservedSlot(obj, slot));
   if (weakMapVal.isUndefined()) {
     // Enter the Realm of "obj" when we allocate the WeakMap, since we are going
@@ -306,19 +306,24 @@ bool MaybeCrossOriginObjectMixins::Ensur
 
   // We didn't find a usable holder.  Go ahead and allocate one.  At this point
   // we have two options: we could allocate the holder in the current Realm and
   // store a cross-compartment wrapper for it in the map as needed, or we could
   // allocate the holder in the Realm of the map and have it hold
   // cross-compartment references to all the methods it holds, since those
   // methods need to be in our current Realm.  It seems better to allocate the
   // holder in our current Realm.
+  bool isChrome = xpc::AccessCheck::isChrome(js::GetContextRealm(cx));
   holder.set(JS_NewObjectWithGivenProto(cx, nullptr, nullptr));
-  if (!holder || !JS_DefineProperties(cx, holder, attributes) ||
-      !JS_DefineFunctions(cx, holder, methods)) {
+  if (!holder || !JS_DefineProperties(cx, holder, properties.mAttributes) ||
+      !JS_DefineFunctions(cx, holder, properties.mMethods) ||
+      (isChrome && properties.mChromeOnlyAttributes &&
+       !JS_DefineProperties(cx, holder, properties.mChromeOnlyAttributes)) ||
+      (isChrome && properties.mChromeOnlyMethods &&
+       !JS_DefineFunctions(cx, holder, properties.mChromeOnlyMethods))) {
     return false;
   }
 
   holderVal.setObject(*holder);
   {  // Scope for working with the map
     JSAutoRealm ar(cx, map);
 
     // Key is already in the right Realm, but we need to wrap the value.
--- a/dom/base/MaybeCrossOriginObject.h
+++ b/dom/base/MaybeCrossOriginObject.h
@@ -29,16 +29,32 @@
 
 #include "js/Class.h"
 #include "js/TypeDecls.h"
 #include "nsStringFwd.h"
 
 namespace mozilla {
 namespace dom {
 
+/**
+ * "mAttributes" and "mMethods" are the cross-origin attributes and methods we
+ * care about, which should get defined on holders.
+ *
+ * "mChromeOnlyAttributes" and "mChromeOnlyMethods" are the cross-origin
+ * attributes and methods we care about, which should get defined on holders
+ * for the chrome realm, in addition to the properties that are in
+ * "mAttributes" and "mMethods".
+ */
+struct CrossOriginProperties {
+  const JSPropertySpec* mAttributes;
+  const JSFunctionSpec* mMethods;
+  const JSPropertySpec* mChromeOnlyAttributes;
+  const JSFunctionSpec* mChromeOnlyMethods;
+};
+
 // Methods that MaybeCrossOriginObject wants that do not depend on the "Base"
 // template parameter.  We can avoid having multiple instantiations of them by
 // pulling them out into this helper class.
 class MaybeCrossOriginObjectMixins {
  public:
   /**
    * Implementation of
    * <https://html.spec.whatwg.org/multipage/browsers.html#isplatformobjectsameorigin-(-o-)>.
@@ -113,22 +129,21 @@ class MaybeCrossOriginObjectMixins {
    *
    * When this is called, "cx" and "obj" are _always_ different-Realm, because
    * this is only used in cross-origin situations.  The "holder" return value is
    * always in the Realm of "cx".
    *
    * "obj" is the object which has space to store the collection of holders in
    * the given slot.
    *
-   * "attributes" and "methods" are the cross-origin attributes and methods we
-   * care about, which should get defined on holders.
+   * "properties" are the cross-origin attributes and methods we care about,
+   * which should get defined on holders.
    */
   static bool EnsureHolder(JSContext* cx, JS::Handle<JSObject*> obj,
-                           size_t slot, JSPropertySpec* attributes,
-                           JSFunctionSpec* methods,
+                           size_t slot, const CrossOriginProperties& properties,
                            JS::MutableHandle<JSObject*> holder);
 
   /**
    * Ensures we have a holder object for the current Realm.  When this is
    * called, "obj" is guaranteed to not be same-Realm with "cx", because this
    * is only used for cross-origin cases.
    *
    * Subclasses are expected to implement this by calling our static
--- a/dom/base/RemoteOuterWindowProxy.cpp
+++ b/dom/base/RemoteOuterWindowProxy.cpp
@@ -19,18 +19,17 @@ namespace dom {
  * Window objects that live in a different process.
  *
  * RemoteOuterWindowProxy holds a BrowsingContext, which is cycle collected.
  * This reference is declared to the cycle collector via NoteChildren().
  */
 
 class RemoteOuterWindowProxy
     : public RemoteObjectProxy<BrowsingContext,
-                               Window_Binding::sCrossOriginAttributes,
-                               Window_Binding::sCrossOriginMethods> {
+                               Window_Binding::sCrossOriginProperties> {
  public:
   typedef RemoteObjectProxy Base;
 
   constexpr RemoteOuterWindowProxy()
       : RemoteObjectProxy(prototypes::id::Window) {}
 
   // Standard internal methods
   bool getOwnPropertyDescriptor(
--- a/dom/base/nsGlobalWindowOuter.cpp
+++ b/dom/base/nsGlobalWindowOuter.cpp
@@ -1076,18 +1076,17 @@ bool nsOuterWindowProxy::AppendIndexedPr
 
   return true;
 }
 
 bool nsOuterWindowProxy::EnsureHolder(
     JSContext* cx, JS::Handle<JSObject*> proxy,
     JS::MutableHandle<JSObject*> holder) const {
   return EnsureHolder(cx, proxy, HOLDER_WEAKMAP_SLOT,
-                      Window_Binding::sCrossOriginAttributes,
-                      Window_Binding::sCrossOriginMethods, holder);
+                      Window_Binding::sCrossOriginProperties, holder);
 }
 
 size_t nsOuterWindowProxy::objectMoved(JSObject* obj, JSObject* old) const {
   nsGlobalWindowOuter* outerWindow = GetOuterWindow(obj);
   if (outerWindow) {
     outerWindow->UpdateWrapper(obj, old);
     BrowsingContext* bc = outerWindow->GetBrowsingContext();
     if (bc) {
--- a/dom/bindings/Codegen.py
+++ b/dom/bindings/Codegen.py
@@ -4319,83 +4319,130 @@ class CGClearCachedValueMethod(CGAbstrac
             slotIndex=slotIndex,
             clearXrayExpandoSlots=clearXrayExpandoSlots,
             regetMember=regetMember)
 
 
 class CGCrossOriginProperties(CGThing):
     def __init__(self, descriptor):
         attrs = []
+        chromeOnlyAttrs = []
         methods = []
+        chromeOnlyMethods = []
         for m in descriptor.interface.members:
             if m.isAttr() and (m.getExtendedAttribute("CrossOriginReadable") or
                                m.getExtendedAttribute("CrossOriginWritable")):
                 if m.isStatic():
                     raise TypeError("Don't know how to deal with static method %s" %
                                     m.identifier.name)
                 if PropertyDefiner.getControllingCondition(m, descriptor).hasDisablers():
                     raise TypeError("Don't know how to deal with disabler for %s" %
                                     m.identifier.name)
                 if len(m.bindingAliases) > 0:
                     raise TypeError("Don't know how to deal with aliases for %s" %
                                     m.identifier.name)
-                attrs.extend(AttrDefiner.attrData(m, overrideFlags="0"))
+                if m.getExtendedAttribute("ChromeOnly") is not None:
+                    chromeOnlyAttrs.extend(AttrDefiner.attrData(m, overrideFlags="0"))
+                else:
+                    attrs.extend(AttrDefiner.attrData(m, overrideFlags="0"))
             elif m.isMethod() and m.getExtendedAttribute("CrossOriginCallable"):
                 if m.isStatic():
                     raise TypeError("Don't know how to deal with static method %s" %
                                     m.identifier.name)
                 if PropertyDefiner.getControllingCondition(m, descriptor).hasDisablers():
                     raise TypeError("Don't know how to deal with disabler for %s" %
                                     m.identifier.name)
                 if len(m.aliases) > 0:
                     raise TypeError("Don't know how to deal with aliases for %s" %
                                     m.identifier.name)
-                methods.append(MethodDefiner.methodData(m, descriptor, overrideFlags="JSPROP_READONLY"))
+                if m.getExtendedAttribute("ChromeOnly") is not None:
+                    chromeOnlyMethods.append(MethodDefiner.methodData(m, descriptor, overrideFlags="JSPROP_READONLY"))
+                else:
+                    methods.append(MethodDefiner.methodData(m, descriptor, overrideFlags="JSPROP_READONLY"))
 
         if len(attrs) > 0:
             self.attributeSpecs, _ = PropertyDefiner.generatePrefableArrayValues(
                 attrs, descriptor, AttrDefiner.formatSpec, '  JS_PS_END\n',
                 AttrDefiner.condition, functools.partial(AttrDefiner.specData, crossOriginOnly=True))
         else:
             self.attributeSpecs = [' JS_PS_END\n']
         if len(methods) > 0:
             self.methodSpecs, _ = PropertyDefiner.generatePrefableArrayValues(
                 methods, descriptor, MethodDefiner.formatSpec, '  JS_FS_END\n',
                 MethodDefiner.condition, MethodDefiner.specData)
         else:
             self.methodSpecs = ['  JS_FS_END\n']
 
+        if len(chromeOnlyAttrs) > 0:
+            self.chromeOnlyAttributeSpecs, _ = PropertyDefiner.generatePrefableArrayValues(
+                chromeOnlyAttrs, descriptor, AttrDefiner.formatSpec, '  JS_PS_END\n',
+                AttrDefiner.condition, functools.partial(AttrDefiner.specData, crossOriginOnly=True))
+        else:
+            self.chromeOnlyAttributeSpecs = []
+        if len(chromeOnlyMethods) > 0:
+            self.chromeOnlyMethodSpecs, _ = PropertyDefiner.generatePrefableArrayValues(
+                chromeOnlyMethods, descriptor, MethodDefiner.formatSpec, '  JS_FS_END\n',
+                MethodDefiner.condition, MethodDefiner.specData)
+        else:
+            self.chromeOnlyMethodSpecs = []
+
     def declare(self):
-        return fill("""
-            extern JSPropertySpec sCrossOriginAttributes[${attributesLength}];
-            extern JSFunctionSpec sCrossOriginMethods[${methodsLength}];
-            """,
-            attributesLength=len(self.attributeSpecs),
-            methodsLength=len(self.methodSpecs))
+        return dedent("""
+            extern const CrossOriginProperties sCrossOriginProperties;
+            """)
 
     def define(self):
+        def defineChromeOnly(name, specs, specType):
+            if len(specs) == 0:
+                return ("", "nullptr")
+            name = "sChromeOnlyCrossOrigin" + name
+            define = fill(
+                """
+                static const ${specType} ${name}[] = {
+                  $*{specs}
+                };
+                """,
+                specType=specType,
+                name=name,
+                specs=",\n".join(specs))
+            return (define, name)
+
+        chromeOnlyAttributes = defineChromeOnly("Attributes", self.chromeOnlyAttributeSpecs, "JSPropertySpec")
+        chromeOnlyMethods = defineChromeOnly("Methods", self.chromeOnlyMethodSpecs, "JSFunctionSpec")
         return fill(
             """
             // We deliberately use brace-elision to make Visual Studio produce better initalization code.
             #if defined(__clang__)
             #pragma clang diagnostic push
             #pragma clang diagnostic ignored "-Wmissing-braces"
             #endif
-            JSPropertySpec sCrossOriginAttributes[] = {
+            static const JSPropertySpec sCrossOriginAttributes[] = {
               $*{attributeSpecs}
             };
-            JSFunctionSpec sCrossOriginMethods[] = {
+            static const JSFunctionSpec sCrossOriginMethods[] = {
               $*{methodSpecs}
             };
+            $*{chromeOnlyAttributeSpecs}
+            $*{chromeOnlyMethodSpecs}
+            const CrossOriginProperties sCrossOriginProperties = {
+              sCrossOriginAttributes,
+              sCrossOriginMethods,
+              ${chromeOnlyAttributes},
+              ${chromeOnlyMethods}
+            };
             #if defined(__clang__)
             #pragma clang diagnostic pop
             #endif
             """,
             attributeSpecs=",\n".join(self.attributeSpecs),
-            methodSpecs=",\n".join(self.methodSpecs))
+            methodSpecs=",\n".join(self.methodSpecs),
+            chromeOnlyAttributeSpecs=chromeOnlyAttributes[0],
+            chromeOnlyMethodSpecs=chromeOnlyMethods[0],
+            chromeOnlyAttributes=chromeOnlyAttributes[1],
+            chromeOnlyMethods=chromeOnlyMethods[1])
 
 
 class CGCycleCollectionTraverseForOwningUnionMethod(CGAbstractMethod):
     """
     ImplCycleCollectionUnlink for owning union type.
     """
     def __init__(self, type):
         self.type = type
@@ -13584,33 +13631,32 @@ class CGDOMJSProxyHandler_set(ClassMetho
             JS_MarkCrossZoneId(cx, id);
 
             return dom::DOMProxyHandler::set(cx, proxy, id, wrappedValue, wrappedReceiver, result);
             """)
 
 
 class CGDOMJSProxyHandler_EnsureHolder(ClassMethod):
     """
-    Implementation of set().  We only use this for cross-origin objects.
+    Implementation of EnsureHolder().  We only use this for cross-origin objects.
     """
     def __init__(self, descriptor):
         args = [Argument('JSContext*', 'cx'),
                 Argument('JS::Handle<JSObject*>', 'proxy'),
                 Argument('JS::MutableHandle<JSObject*>', 'holder')]
         ClassMethod.__init__(self, "EnsureHolder", "bool", args,
                              virtual=True, override=True, const=True)
         self.descriptor = descriptor
 
     def getBody(self):
         return dedent(
             """
-            // Our holder slot is our last slot.
             return EnsureHolder(cx, proxy,
                                 JSCLASS_RESERVED_SLOTS(js::GetObjectClass(proxy)) - 1,
-                                sCrossOriginAttributes, sCrossOriginMethods, holder);
+                                sCrossOriginProperties, holder);
             """)
 
 
 class CGDOMJSProxyHandler(CGClass):
     def __init__(self, descriptor):
         assert (descriptor.supportsIndexedProperties() or
                 descriptor.supportsNamedProperties() or
                 descriptor.isMaybeCrossOriginObject())
--- a/dom/bindings/RemoteObjectProxy.h
+++ b/dom/bindings/RemoteObjectProxy.h
@@ -129,17 +129,17 @@ class RemoteObjectProxyBase : public js:
  *
  * The properties and methods can be cached on a holder JSObject, stored in a
  * reserved slot on the proxy object.
  *
  * The proxy objects that use a handler derived from this one are stored in a
  * hash map in the JS compartment's private (@see
  * xpc::CompartmentPrivate::GetRemoteProxyMap).
  */
-template <class Native, JSPropertySpec* P, JSFunctionSpec* F>
+template <class Native, const CrossOriginProperties& P>
 class RemoteObjectProxy : public RemoteObjectProxyBase {
  public:
   void finalize(JSFreeOp* aFop, JSObject* aProxy) const final {
     auto native = static_cast<Native*>(GetNative(aProxy));
     RefPtr<Native> self(dont_AddRef(native));
   }
 
   void GetProxyObject(JSContext* aCx, Native* aNative,
@@ -155,17 +155,17 @@ class RemoteObjectProxy : public RemoteO
 
  protected:
   using RemoteObjectProxyBase::RemoteObjectProxyBase;
 
  private:
   bool EnsureHolder(JSContext* aCx, JS::Handle<JSObject*> aProxy,
                     JS::MutableHandle<JSObject*> aHolder) const final {
     return MaybeCrossOriginObjectMixins::EnsureHolder(
-        aCx, aProxy, /* slot = */ 0, P, F, aHolder);
+        aCx, aProxy, /* slot = */ 0, P, aHolder);
   }
 
   static const JSClass sClass;
 };
 
 /**
  * Returns true if aObj is a cross-process proxy object that
  * represents an object implementing the WebIDL interface for