Bug 911864 - Install XBL members in the safe scope, then clone them into content (rather than the reverse). r=smaug

content/xbl/src/nsXBLProtoImpl.cpp

@@ -22,6 +22,8 @@
 #include "xpcpublic.h"
 
 using namespace mozilla;
+using js::GetGlobalForObjectCrossCompartment;
+using js::AssertSameCompartment;
 
 nsresult
 nsXBLProtoImpl::InstallImplementation(nsXBLPrototypeBinding* aPrototypeBinding,
@@ -69,49 +71,58 @@ nsXBLProtoImpl::InstallImplementation(nsXBLPrototypeBinding* aPrototypeBinding,
 
   JS::Rooted<JSObject*> targetScriptObject(cx, holder->GetJSObject());
 
-  JSAutoCompartment ac(cx, targetClassObject);
+  // We want to define the canonical set of members in a safe place. If we're
+  // using a separate XBL scope, we want to define them there first (so that
+  // they'll be available for Xray lookups, among other things), and then copy
+  // the properties to the content-side prototype as needed. We don't need to
+  // bother about the field accessors here, since we don't use/support those
+  // for in-content bindings.
 
-  // Walk our member list and install each one in turn.
-  for (nsXBLProtoImplMember* curr = mMembers;
-       curr;
-       curr = curr->GetNext())
-    curr->InstallMember(cx, targetClassObject);
-
-  // If we're using a separate XBL scope, make a safe copy of the target class
-  // object in the XBL scope that we can use for Xray lookups. We don't need
-  // the field accessors, so do this before installing them.
+  // First, start by entering the compartment of the XBL scope. This may or may
+  // not be the same compartment as globalObject.
   JS::Rooted<JSObject*> globalObject(cx,
-    JS_GetGlobalForObject(cx, targetClassObject));
+    GetGlobalForObjectCrossCompartment(targetClassObject));
   JS::Rooted<JSObject*> scopeObject(cx, xpc::GetXBLScope(cx, globalObject));
   NS_ENSURE_TRUE(scopeObject, NS_ERROR_OUT_OF_MEMORY);
-  if (scopeObject != globalObject) {
-    JSAutoCompartment ac2(cx, scopeObject);
+  JSAutoCompartment ac(cx, scopeObject);
 
-    // Create the object. This is just a property holder, so it doesn't need
-    // any special JSClass.
-    JS::Rooted<JSObject*> shadowProto(cx,
-      JS_NewObjectWithGivenProto(cx, nullptr, nullptr, scopeObject));
-    NS_ENSURE_TRUE(shadowProto, NS_ERROR_OUT_OF_MEMORY);
+  // If they're different, create our safe holder object in the XBL scope.
+  JS::RootedObject propertyHolder(cx);
+  if (scopeObject != globalObject) {
+
+    // This is just a property holder, so it doesn't need any special JSClass.
+    propertyHolder = JS_NewObjectWithGivenProto(cx, nullptr, nullptr, scopeObject);
+    NS_ENSURE_TRUE(propertyHolder, NS_ERROR_OUT_OF_MEMORY);
 
     // Define it as a property on the scopeObject, using the same name used on
     // the content side.
     bool ok = JS_DefineProperty(cx, scopeObject,
                                 js::GetObjectClass(targetClassObject)->name,
-                                JS::ObjectValue(*shadowProto), JS_PropertyStub,
+                                JS::ObjectValue(*propertyHolder), JS_PropertyStub,
                                 JS_StrictPropertyStub,
                                 JSPROP_PERMANENT | JSPROP_READONLY);
     NS_ENSURE_TRUE(ok, NS_ERROR_UNEXPECTED);
+  } else {
+    propertyHolder = targetClassObject;
+  }
 
-    // Copy all the properties from the content-visible prototype to the shadow
-    // object. This rewraps them appropriately, which should result in vanilla
-    // functions, since the properties on the content prototype were cross-
-    // compartment wrappers.
-    ok = JS_CopyPropertiesFrom(cx, shadowProto, targetClassObject);
-    NS_ENSURE_TRUE(ok, NS_ERROR_UNEXPECTED);
+  // Walk our member list and install each one in turn on the XBL scope object.
+  for (nsXBLProtoImplMember* curr = mMembers;
+       curr;
+       curr = curr->GetNext())
+    curr->InstallMember(cx, propertyHolder);
 
-    // Content shouldn't have any way to touch this object, but freeze it just
-    // to be safe.
-    ok = JS_FreezeObject(cx, shadowProto);
+  // From here on out, work in the scope of the bound element.
+  JSAutoCompartment ac2(cx, targetClassObject);
+
+  // Now, if we're using a separate XBL scope, enter the compartment of the
+  // bound node and copy the properties to the prototype there. This rewraps
+  // them appropriately, which should result in cross-compartment function
+  // wrappers.
+  if (propertyHolder != targetClassObject) {
+    AssertSameCompartment(propertyHolder, scopeObject);
+    AssertSameCompartment(targetClassObject, globalObject);
+    bool ok = JS_CopyPropertiesFrom(cx, targetClassObject, propertyHolder);
     NS_ENSURE_TRUE(ok, NS_ERROR_UNEXPECTED);
   }
 

content/xbl/src/nsXBLProtoImplMethod.cpp

@@ -104,25 +104,15 @@ nsXBLProtoImplMethod::InstallMember(JSContext* aCx,
   MOZ_ASSERT(js::IsObjectInContextCompartment(aTargetClassObject, aCx));
 
   JS::Rooted<JSObject*> globalObject(aCx, JS_GetGlobalForObject(aCx, aTargetClassObject));
-  JS::Rooted<JSObject*> scopeObject(aCx, xpc::GetXBLScope(aCx, globalObject));
-  NS_ENSURE_TRUE(scopeObject, NS_ERROR_OUT_OF_MEMORY);
+  MOZ_ASSERT(xpc::IsInXBLScope(globalObject) ||
+             globalObject == xpc::GetXBLScope(aCx, globalObject));
 
   JS::Rooted<JSObject*> jsMethodObject(aCx, GetCompiledMethod());
   if (jsMethodObject) {
     nsDependentString name(mName);
 
-    // First, make the function in the compartment of the scope object.
-    JSAutoCompartment ac(aCx, scopeObject);
-    JS::Rooted<JSObject*> method(aCx, ::JS_CloneFunctionObject(aCx, jsMethodObject, scopeObject));
-    if (!method) {
-      return NS_ERROR_OUT_OF_MEMORY;
-    }
-
-    // Then, enter the content compartment, wrap the method pointer, and define
-    // the wrapped version on the class object.
-    JSAutoCompartment ac2(aCx, aTargetClassObject);
-    if (!JS_WrapObject(aCx, &method))
-      return NS_ERROR_OUT_OF_MEMORY;
+    JS::Rooted<JSObject*> method(aCx, JS_CloneFunctionObject(aCx, jsMethodObject, globalObject));
+    NS_ENSURE_TRUE(method, NS_ERROR_OUT_OF_MEMORY);
 
     JS::Rooted<JS::Value> value(aCx, JS::ObjectValue(*method));
     if (!::JS_DefineUCProperty(aCx, aTargetClassObject,

content/xbl/src/nsXBLProtoImplProperty.cpp

@@ -129,32 +129,24 @@ nsXBLProtoImplProperty::InstallMember(JSContext *aCx,
   MOZ_ASSERT(mGetter.IsCompiled() && mSetter.IsCompiled());
   MOZ_ASSERT(js::IsObjectInContextCompartment(aTargetClassObject, aCx));
   JS::Rooted<JSObject*> globalObject(aCx, JS_GetGlobalForObject(aCx, aTargetClassObject));
-  JS::Rooted<JSObject*> scopeObject(aCx, xpc::GetXBLScope(aCx, globalObject));
-  NS_ENSURE_TRUE(scopeObject, NS_ERROR_OUT_OF_MEMORY);
+  MOZ_ASSERT(xpc::IsInXBLScope(globalObject) ||
+             globalObject == xpc::GetXBLScope(aCx, globalObject));
 
   if (mGetter.GetJSFunction() || mSetter.GetJSFunction()) {
-    // First, enter the compartment of the scope object and clone the functions.
-    JSAutoCompartment ac(aCx, scopeObject);
-
     JS::Rooted<JSObject*> getter(aCx, nullptr);
     if (mGetter.GetJSFunction()) {
-      if (!(getter = ::JS_CloneFunctionObject(aCx, mGetter.GetJSFunction(), scopeObject)))
+      if (!(getter = ::JS_CloneFunctionObject(aCx, mGetter.GetJSFunction(), globalObject)))
         return NS_ERROR_OUT_OF_MEMORY;
     }
 
     JS::Rooted<JSObject*> setter(aCx, nullptr);
     if (mSetter.GetJSFunction()) {
-      if (!(setter = ::JS_CloneFunctionObject(aCx, mSetter.GetJSFunction(), scopeObject)))
+      if (!(setter = ::JS_CloneFunctionObject(aCx, mSetter.GetJSFunction(), globalObject)))
         return NS_ERROR_OUT_OF_MEMORY;
     }
 
-    // Now, enter the content compartment, wrap the getter/setter, and define
-    // them on the class object.
-    JSAutoCompartment ac2(aCx, aTargetClassObject);
     nsDependentString name(mName);
-    if (!JS_WrapObject(aCx, &getter) ||
-        !JS_WrapObject(aCx, &setter) ||
-        !::JS_DefineUCProperty(aCx, aTargetClassObject,
+    if (!::JS_DefineUCProperty(aCx, aTargetClassObject,
                                static_cast<const jschar*>(mName),
                                name.Length(), JSVAL_VOID,
                                JS_DATA_TO_FUNC_PTR(JSPropertyOp, getter.get()),

js/xpconnect/src/XPCJSRuntime.cpp

@@ -420,6 +420,12 @@ IsXBLScope(JSCompartment *compartment)
     return priv->scope->IsXBLScope();
 }
 
+bool
+IsInXBLScope(JSObject *obj)
+{
+    return IsXBLScope(js::GetObjectCompartment(obj));
+}
+
 bool
 IsUniversalXPConnectEnabled(JSCompartment *compartment)
 {

js/xpconnect/src/xpcpublic.h

@@ -266,6 +266,7 @@ bool StringToJsval(JSContext* cx, mozilla::dom::DOMString& str,
 nsIPrincipal *GetCompartmentPrincipal(JSCompartment *compartment);
 
 bool IsXBLScope(JSCompartment *compartment);
+bool IsInXBLScope(JSObject *obj);
 
 void SetLocationForGlobal(JSObject *global, const nsACString& location);
 void SetLocationForGlobal(JSObject *global, nsIURI *locationURI);