Bug 1595890 part 3. Remove xpc::IsContentXBLCompartment and its various callsites. r=bholley
authorBoris Zbarsky <bzbarsky@mit.edu>
Thu, 14 Nov 2019 05:20:18 +0000
changeset 501885 810924c606882d4c235729bed6690e713f5bca9e
parent 501884 abee66548e14e53e74efc32050bfea474edce916
child 501886 56ac0284574922f408585caa1b45162421d04c5b
push id114172
push userdluca@mozilla.com
push dateTue, 19 Nov 2019 11:31:10 +0000
treeherdermozilla-inbound@b5c5ba07d3db [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbholley
bugs1595890
milestone72.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 1595890 part 3. Remove xpc::IsContentXBLCompartment and its various callsites. r=bholley It always returns false. Differential Revision: https://phabricator.services.mozilla.com/D52746
dom/base/nsContentUtils.cpp
dom/bindings/BindingUtils.cpp
js/xpconnect/src/XPCJSRuntime.cpp
js/xpconnect/src/XPCWrappedNativeScope.cpp
js/xpconnect/src/nsXPConnect.cpp
js/xpconnect/src/xpcprivate.h
js/xpconnect/src/xpcpublic.h
js/xpconnect/wrappers/WrapperFactory.cpp
--- a/dom/base/nsContentUtils.cpp
+++ b/dom/base/nsContentUtils.cpp
@@ -2110,17 +2110,17 @@ bool nsContentUtils::IsCallerContentXBL(
   // For remote XUL, we run XBL in the XUL scope. Given that we care about
   // compat and not security for remote XUL, just always claim to be XBL.
   if (!xpc::AllowContentXBLScope(realm)) {
     MOZ_ASSERT(
         nsContentUtils::AllowXULXBLForPrincipal(xpc::GetRealmPrincipal(realm)));
     return true;
   }
 
-  return xpc::IsContentXBLScope(realm);
+  return false;
 }
 
 bool nsContentUtils::IsCallerUAWidget() {
   JSContext* cx = GetCurrentJSContext();
   if (!cx) {
     return false;
   }
 
--- a/dom/bindings/BindingUtils.cpp
+++ b/dom/bindings/BindingUtils.cpp
@@ -1555,49 +1555,16 @@ static bool ResolvePrototypeOrConstructo
     desc.setAttributes(attrs);
     desc.setGetter(nullptr);
     desc.setSetter(nullptr);
     desc.value().set(JS::ObjectValue(*protoOrIface));
   }
   return JS_WrapPropertyDescriptor(cx, desc);
 }
 
-#ifdef DEBUG
-
-static void DEBUG_CheckXBLCallable(JSContext* cx, JSObject* obj) {
-  // In general, we shouldn't have cross-compartment wrappers here, because
-  // we should be running in an XBL scope, and the content prototype should
-  // contain wrappers to functions defined in the XBL scope. But if the node
-  // has been adopted into another compartment, those prototypes will now point
-  // to a different XBL scope (which is ok).
-  MOZ_ASSERT_IF(js::IsCrossCompartmentWrapper(obj),
-                xpc::IsInContentXBLScope(js::UncheckedUnwrap(obj)));
-  MOZ_ASSERT(JS::IsCallable(obj));
-}
-
-static void DEBUG_CheckXBLLookup(JSContext* cx, JS::PropertyDescriptor* desc) {
-  if (!desc->obj) return;
-  if (!desc->value.isUndefined()) {
-    MOZ_ASSERT(desc->value.isObject());
-    DEBUG_CheckXBLCallable(cx, &desc->value.toObject());
-  }
-  if (desc->getter) {
-    MOZ_ASSERT(desc->attrs & JSPROP_GETTER);
-    DEBUG_CheckXBLCallable(cx, JS_FUNC_TO_DATA_PTR(JSObject*, desc->getter));
-  }
-  if (desc->setter) {
-    MOZ_ASSERT(desc->attrs & JSPROP_SETTER);
-    DEBUG_CheckXBLCallable(cx, JS_FUNC_TO_DATA_PTR(JSObject*, desc->setter));
-  }
-}
-#else
-#  define DEBUG_CheckXBLLookup(a, b) \
-    {}
-#endif
-
 /* static */ bool XrayResolveOwnProperty(
     JSContext* cx, JS::Handle<JSObject*> wrapper, JS::Handle<JSObject*> obj,
     JS::Handle<jsid> id, JS::MutableHandle<JS::PropertyDescriptor> desc,
     bool& cacheOnHolder) {
   cacheOnHolder = false;
 
   DOMObjectType type;
   const NativePropertyHooks* nativePropertyHooks =
@@ -1645,47 +1612,16 @@ static void DEBUG_CheckXBLLookup(JSConte
       }
 
       if (desc.object()) {
         // None of these should be cached on the holder, since they're dynamic.
         return true;
       }
     }
 
-    // If we're a special scope for in-content XBL, our script expects to see
-    // the bound XBL methods and attributes when accessing content. However,
-    // these members are implemented in content via custom-spliced prototypes,
-    // and thus aren't visible through Xray wrappers unless we handle them
-    // explicitly. So we check if we're running in such a scope, and if so,
-    // whether the wrappee is a bound element. If it is, we do a lookup via
-    // specialized XBL machinery.
-    //
-    // While we have to do some sketchy walking through content land, we should
-    // be protected by read-only/non-configurable properties, and any functions
-    // we end up with should _always_ be living in our own scope (the XBL
-    // scope). Make sure to assert that.
-    JS::Rooted<JSObject*> maybeElement(cx, obj);
-    Element* element;
-    if (xpc::IsInContentXBLScope(wrapper) &&
-        NS_SUCCEEDED(UNWRAP_OBJECT(Element, &maybeElement, element))) {
-      if (!nsContentUtils::LookupBindingMember(cx, element, id, desc)) {
-        return false;
-      }
-
-      DEBUG_CheckXBLLookup(cx, desc.address());
-
-      if (desc.object()) {
-        // XBL properties shouldn't be cached on the holder, as they might be
-        // shadowed by own properties returned from mResolveOwnProperty.
-        desc.object().set(wrapper);
-
-        return true;
-      }
-    }
-
     // For non-global instance Xrays there are no other properties, so return
     // here for them.
     if (type != eGlobalInstance) {
       return true;
     }
   } else if (type == eInterface) {
     if (id.get() == GetJSIDByIndex(cx, XPCJSContext::IDX_PROTOTYPE)) {
       return nativePropertyHooks->mPrototypeID == prototypes::id::_ID_Count ||
--- a/js/xpconnect/src/XPCJSRuntime.cpp
+++ b/js/xpconnect/src/XPCJSRuntime.cpp
@@ -492,26 +492,16 @@ void Scriptability::SetDocShellAllowsScr
   mDocShellAllowsScript = aAllowed || mImmuneToScriptPolicy;
 }
 
 /* static */
 Scriptability& Scriptability::Get(JSObject* aScope) {
   return RealmPrivate::Get(aScope)->scriptability;
 }
 
-bool IsContentXBLCompartment(JS::Compartment* compartment) { return false; }
-
-bool IsContentXBLScope(JS::Realm* realm) {
-  return IsContentXBLCompartment(JS::GetCompartmentForRealm(realm));
-}
-
-bool IsInContentXBLScope(JSObject* obj) {
-  return IsContentXBLCompartment(js::GetObjectCompartment(obj));
-}
-
 bool IsUAWidgetCompartment(JS::Compartment* compartment) {
   // We always eagerly create compartment privates for UA Widget compartments.
   CompartmentPrivate* priv = CompartmentPrivate::Get(compartment);
   return priv && priv->isUAWidgetCompartment;
 }
 
 bool IsUAWidgetScope(JS::Realm* realm) {
   return IsUAWidgetCompartment(JS::GetCompartmentForRealm(realm));
--- a/js/xpconnect/src/XPCWrappedNativeScope.cpp
+++ b/js/xpconnect/src/XPCWrappedNativeScope.cpp
@@ -186,17 +186,16 @@ bool XPCWrappedNativeScope::AttachCompon
 #undef DEFINE_SUBCOMPONENT_PROPERTY
 
   return true;
 }
 
 JSObject* XPCWrappedNativeScope::EnsureContentXBLScope(JSContext* cx) {
   JS::RootedObject global(cx, CurrentGlobalOrNull(cx));
   MOZ_ASSERT(js::IsObjectInContextCompartment(global, cx));
-  MOZ_ASSERT(!IsContentXBLScope());
   MOZ_ASSERT(strcmp(js::GetObjectClass(global)->name,
                     "nsXBLPrototypeScript compilation scope"));
 
   // We can probably remove EnsureContentXBLScope and clean up all its callers,
   // but a bunch (all?) of those callers will just go away when we remove XBL
   // support, so it's simpler to just leave it here as a no-op.
   return global;
 }
--- a/js/xpconnect/src/nsXPConnect.cpp
+++ b/js/xpconnect/src/nsXPConnect.cpp
@@ -1153,18 +1153,17 @@ bool IsChromeOrXBL(JSContext* cx, JSObje
   JS::Compartment* c = JS::GetCompartmentForRealm(realm);
 
   // For remote XUL, we run XBL in the XUL scope. Given that we care about
   // compat and not security for remote XUL, we just always claim to be XBL.
   //
   // Note that, for performance, we don't check AllowXULXBLForPrincipal here,
   // and instead rely on the fact that AllowContentXBLScope() only returns false
   // in remote XUL situations.
-  return AccessCheck::isChrome(c) || IsContentXBLCompartment(c) ||
-         !AllowContentXBLScope(realm);
+  return AccessCheck::isChrome(c) || !AllowContentXBLScope(realm);
 }
 
 bool IsNotUAWidget(JSContext* cx, JSObject* /* unused */) {
   MOZ_ASSERT(NS_IsMainThread());
   JS::Realm* realm = JS::GetCurrentRealmOrNull(cx);
   MOZ_ASSERT(realm);
   JS::Compartment* c = JS::GetCompartmentForRealm(realm);
 
--- a/js/xpconnect/src/xpcprivate.h
+++ b/js/xpconnect/src/xpcprivate.h
@@ -900,19 +900,16 @@ class XPCWrappedNativeScope final
 
   // Returns the global to use for new WrappedNative objects allocated in this
   // compartment. This is better than using arbitrary globals we happen to be in
   // because it prevents leaks (objects keep their globals alive).
   JSObject* GetGlobalForWrappedNatives() {
     return js::GetFirstGlobalInCompartment(Compartment());
   }
 
-  bool IsContentXBLScope() {
-    return xpc::IsContentXBLCompartment(Compartment());
-  }
   bool AllowContentXBLScope(JS::Realm* aRealm);
 
   // ID Object prototype caches.
   JS::Heap<JSObject*> mIDProto;
   JS::Heap<JSObject*> mIIDProto;
   JS::Heap<JSObject*> mCIDProto;
 
  protected:
--- a/js/xpconnect/src/xpcpublic.h
+++ b/js/xpconnect/src/xpcpublic.h
@@ -86,20 +86,16 @@ JSObject* TransplantObjectRetainingXrayE
                                                 JS::HandleObject origobj,
                                                 JS::HandleObject target);
 
 // If origObj has an xray waiver, nuke it before transplant.
 JSObject* TransplantObjectNukingXrayWaiver(JSContext* cx,
                                            JS::HandleObject origObj,
                                            JS::HandleObject target);
 
-bool IsContentXBLCompartment(JS::Compartment* compartment);
-bool IsContentXBLScope(JS::Realm* realm);
-bool IsInContentXBLScope(JSObject* obj);
-
 bool IsUAWidgetCompartment(JS::Compartment* compartment);
 bool IsUAWidgetScope(JS::Realm* realm);
 bool IsInUAWidgetScope(JSObject* obj);
 
 bool MightBeWebContentCompartment(JS::Compartment* compartment);
 
 void SetCompartmentChangedDocumentDomain(JS::Compartment* compartment);
 
@@ -119,19 +115,16 @@ void SetCompartmentChangedDocumentDomain
 JSObject* GetXBLScope(JSContext* cx, JSObject* contentScope);
 
 JSObject* GetUAWidgetScope(JSContext* cx, nsIPrincipal* principal);
 
 JSObject* GetUAWidgetScope(JSContext* cx, JSObject* contentScope);
 
 inline JSObject* GetXBLScopeOrGlobal(JSContext* cx, JSObject* obj) {
   MOZ_ASSERT(!js::IsCrossCompartmentWrapper(obj));
-  if (IsInContentXBLScope(obj)) {
-    return JS::GetNonCCWObjectGlobal(obj);
-  }
   return GetXBLScope(cx, obj);
 }
 
 // Returns whether XBL scopes have been explicitly disabled for code running
 // in this compartment. See the comment around mAllowContentXBLScope.
 bool AllowContentXBLScope(JS::Realm* realm);
 
 // Get the scope for creating reflectors for native anonymous content
--- a/js/xpconnect/wrappers/WrapperFactory.cpp
+++ b/js/xpconnect/wrappers/WrapperFactory.cpp
@@ -482,26 +482,16 @@ static const Wrapper* SelectWrapper(bool
       return &PermissiveXrayJS::singleton;
     }
     MOZ_ASSERT(xrayType == XrayForOpaqueObject);
     return &PermissiveXrayOpaque::singleton;
   }
 
   // There's never any reason to expose other objects to non-subsuming actors.
   // Just use an opaque wrapper in these cases.
-  //
-  // In general, we don't want opaque function wrappers to be callable.
-  // But in the case of XBL, we rely on content being able to invoke
-  // functions exposed from the XBL scope. We could remove this exception,
-  // if needed, by using ExportFunction to generate the content-side
-  // representations of XBL methods.
-  if (xrayType == XrayForJSObject && IsInContentXBLScope(obj)) {
-    return &FilteringWrapper<CrossCompartmentSecurityWrapper,
-                             OpaqueWithCall>::singleton;
-  }
   return &FilteringWrapper<CrossCompartmentSecurityWrapper, Opaque>::singleton;
 }
 
 JSObject* WrapperFactory::Rewrap(JSContext* cx, HandleObject existing,
                                  HandleObject obj) {
   MOZ_ASSERT(!IsWrapper(obj) || GetProxyHandler(obj) == &XrayWaiver ||
                  js::IsWindowProxy(obj),
              "wrapped object passed to rewrap");