Bug 911864 - Install XBL members in the safe scope, then clone them into content (rather than the reverse). r=smaug, a=bajaj
authorBobby Holley <bobbyholley@gmail.com>
Fri, 01 Nov 2013 15:31:57 +0100
changeset 161301 68fdfda73db11b9e4354545b8992aa0099534d84
parent 161300 ba7bbbeadf7b5660bac8b488e8d1b0498f788e88
child 161302 72ff90fd470bf68b332f2c590a98febf91242cea
push id4600
push userryanvm@gmail.com
push dateTue, 12 Nov 2013 17:09:20 +0000
treeherdermozilla-aurora@9e1c21d8987c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug, bajaj
bugs911864
milestone27.0a2
Bug 911864 - Install XBL members in the safe scope, then clone them into content (rather than the reverse). r=smaug, a=bajaj
content/xbl/src/nsXBLProtoImpl.cpp
content/xbl/src/nsXBLProtoImplMethod.cpp
content/xbl/src/nsXBLProtoImplProperty.cpp
js/xpconnect/src/XPCJSRuntime.cpp
js/xpconnect/src/xpcpublic.h
--- a/content/xbl/src/nsXBLProtoImpl.cpp
+++ b/content/xbl/src/nsXBLProtoImpl.cpp
@@ -17,16 +17,18 @@
 #include "nsIDOMNode.h"
 #include "nsXBLPrototypeBinding.h"
 #include "nsXBLProtoImplProperty.h"
 #include "nsIURI.h"
 #include "mozilla/dom/XULElementBinding.h"
 #include "xpcpublic.h"
 
 using namespace mozilla;
+using js::GetGlobalForObjectCrossCompartment;
+using js::AssertSameCompartment;
 
 nsresult
 nsXBLProtoImpl::InstallImplementation(nsXBLPrototypeBinding* aPrototypeBinding,
                                       nsXBLBinding* aBinding)
 {
   // This function is called to install a concrete implementation on a bound element using
   // this prototype implementation as a guide.  The prototype implementation is compiled lazily,
   // so for the first bound element that needs a concrete implementation, we also build the
@@ -64,59 +66,68 @@ nsXBLProtoImpl::InstallImplementation(ns
   aBinding->SetJSClass(nsXBLJSClass::fromJSClass(JS_GetClass(targetClassObject)));
 
   // If the prototype already existed, we don't need to install anything. return early.
   if (!targetObjectIsNew)
     return NS_OK;
 
   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);
+
+  // From here on out, work in the scope of the bound element.
+  JSAutoCompartment ac2(cx, targetClassObject);
 
-    // Content shouldn't have any way to touch this object, but freeze it just
-    // to be safe.
-    ok = JS_FreezeObject(cx, shadowProto);
+  // 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);
   }
 
   // Install all of our field accessors.
   for (nsXBLProtoImplField* curr = mFields;
        curr;
        curr = curr->GetNext())
     curr->InstallAccessors(cx, targetClassObject);
--- a/content/xbl/src/nsXBLProtoImplMethod.cpp
+++ b/content/xbl/src/nsXBLProtoImplMethod.cpp
@@ -99,35 +99,25 @@ nsresult
 nsXBLProtoImplMethod::InstallMember(JSContext* aCx,
                                     JS::Handle<JSObject*> aTargetClassObject)
 {
   NS_PRECONDITION(IsCompiled(),
                   "Should not be installing an uncompiled method");
   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,
                                static_cast<const jschar*>(mName),
                                name.Length(), value,
                                nullptr, nullptr, JSPROP_ENUMERATE)) {
       return NS_ERROR_OUT_OF_MEMORY;
     }
--- a/content/xbl/src/nsXBLProtoImplProperty.cpp
+++ b/content/xbl/src/nsXBLProtoImplProperty.cpp
@@ -124,42 +124,34 @@ nsresult
 nsXBLProtoImplProperty::InstallMember(JSContext *aCx,
                                       JS::Handle<JSObject*> aTargetClassObject)
 {
   NS_PRECONDITION(mIsCompiled,
                   "Should not be installing an uncompiled property");
   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_DATA_TO_FUNC_PTR(JSStrictPropertyOp, setter.get()),
                                mJSAttributes))
       return NS_ERROR_OUT_OF_MEMORY;
   }
   return NS_OK;
--- a/js/xpconnect/src/XPCJSRuntime.cpp
+++ b/js/xpconnect/src/XPCJSRuntime.cpp
@@ -402,16 +402,22 @@ IsXBLScope(JSCompartment *compartment)
     // We always eagerly create compartment privates for XBL scopes.
     CompartmentPrivate *priv = GetCompartmentPrivate(compartment);
     if (!priv || !priv->scope)
         return false;
     return priv->scope->IsXBLScope();
 }
 
 bool
+IsInXBLScope(JSObject *obj)
+{
+    return IsXBLScope(js::GetObjectCompartment(obj));
+}
+
+bool
 IsUniversalXPConnectEnabled(JSCompartment *compartment)
 {
     CompartmentPrivate *priv = GetCompartmentPrivate(compartment);
     if (!priv)
         return false;
     return priv->universalXPConnectEnabled;
 }
 
--- a/js/xpconnect/src/xpcpublic.h
+++ b/js/xpconnect/src/xpcpublic.h
@@ -261,16 +261,17 @@ bool StringToJsval(JSContext* cx, mozill
         return true;
     }
     return NonVoidStringToJsval(cx, str, rval);
 }
 
 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);
 
 // ReportJSRuntimeExplicitTreeStats will expect this in the |extra| member
 // of JS::ZoneStats.
 class ZoneStatsExtras {
 public: