Bug 825392 - Add xpc::GetXBLScopeOrGlobal and Sprinkle some calls to it where necessary. r=bz
authorBobby Holley <bobbyholley@gmail.com>
Wed, 19 Mar 2014 13:35:45 -0300
changeset 186361 3626a613190ec9570f760a81ff3a3f6fe3061a30
parent 186360 7462463054e3146815c1aba1ee7e2e823a3629ef
child 186362 7c1d075bb7f67b4e4004058ba0bd1789a65e178c
push idunknown
push userunknown
push dateunknown
reviewersbz
bugs825392
milestone31.0a1
Bug 825392 - Add xpc::GetXBLScopeOrGlobal and Sprinkle some calls to it where necessary. r=bz
dom/xbl/nsBindingManager.cpp
dom/xbl/nsXBLBinding.cpp
dom/xbl/nsXBLProtoImpl.cpp
dom/xbl/nsXBLProtoImplField.cpp
dom/xbl/nsXBLProtoImplMethod.cpp
dom/xbl/nsXBLPrototypeHandler.cpp
js/xpconnect/src/xpcpublic.h
--- a/dom/xbl/nsBindingManager.cpp
+++ b/dom/xbl/nsBindingManager.cpp
@@ -740,19 +740,20 @@ nsBindingManager::GetBindingImplementati
       JS::Rooted<JSObject*> jsobj(cx, aContent->GetWrapper());
       NS_ENSURE_TRUE(jsobj, NS_NOINTERFACE);
 
       // If we're using an XBL scope, we need to use the Xray view to the bound
       // content in order to view the full array of methods defined in the
       // binding, some of which may not be exposed on the prototype of
       // untrusted content.
       //
-      // If there's no separate XBL scope, we'll end up with the global of the
-      // reflector, and this will all be a no-op.
-      JS::Rooted<JSObject*> xblScope(cx, xpc::GetXBLScope(cx, jsobj));
+      // If there's no separate XBL scope, or if the reflector itself lives in
+      // the XBL scope, we'll end up with the global of the reflector, and this
+      // will all be a no-op.
+      JS::Rooted<JSObject*> xblScope(cx, xpc::GetXBLScopeOrGlobal(cx, jsobj));
       JSAutoCompartment ac(cx, xblScope);
       bool ok = JS_WrapObject(cx, &jsobj);
       NS_ENSURE_TRUE(ok, NS_ERROR_OUT_OF_MEMORY);
       MOZ_ASSERT_IF(js::IsWrapper(jsobj), xpc::IsXrayWrapper(jsobj));
 
       nsresult rv = xpConnect->WrapJSAggregatedToNative(aContent, cx,
                                                         jsobj, aIID, aResult);
       if (NS_FAILED(rv))
--- a/dom/xbl/nsXBLBinding.cpp
+++ b/dom/xbl/nsXBLBinding.cpp
@@ -1120,18 +1120,25 @@ nsXBLBinding::LookupMember(JSContext* aC
 
   // We have a weak reference to our bound element, so make sure it's alive.
   if (!mBoundElement || !mBoundElement->GetWrapper()) {
     return false;
   }
 
   // Get the scope of mBoundElement and the associated XBL scope. We should only
   // be calling into this machinery if we're running in a separate XBL scope.
+  //
+  // Note that we only end up in LookupMember for XrayWrappers from XBL scopes
+  // into content. So for NAC reflectors that live in the XBL scope, we should
+  // never get here. But on the off-chance that someone adds new callsites to
+  // LookupMember, we do a release-mode assertion as belt-and-braces.
+  // We do a release-mode assertion here to be extra safe.
   JS::Rooted<JSObject*> boundScope(aCx,
     js::GetGlobalForObjectCrossCompartment(mBoundElement->GetWrapper()));
+  MOZ_RELEASE_ASSERT(!xpc::IsInXBLScope(boundScope));
   JS::Rooted<JSObject*> xblScope(aCx, xpc::GetXBLScope(aCx, boundScope));
   NS_ENSURE_TRUE(xblScope, false);
   MOZ_ASSERT(boundScope != xblScope);
 
   // Enter the xbl scope and invoke the internal version.
   {
     JSAutoCompartment ac(aCx, xblScope);
     JS::Rooted<jsid> id(aCx, aId);
--- a/dom/xbl/nsXBLProtoImpl.cpp
+++ b/dom/xbl/nsXBLProtoImpl.cpp
@@ -75,17 +75,17 @@ nsXBLProtoImpl::InstallImplementation(ns
   // 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.
 
   // 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,
     GetGlobalForObjectCrossCompartment(targetClassObject));
-  JS::Rooted<JSObject*> scopeObject(cx, xpc::GetXBLScope(cx, globalObject));
+  JS::Rooted<JSObject*> scopeObject(cx, xpc::GetXBLScopeOrGlobal(cx, globalObject));
   NS_ENSURE_TRUE(scopeObject, NS_ERROR_OUT_OF_MEMORY);
   JSAutoCompartment ac(cx, scopeObject);
 
   // If they're different, create our safe holder object in the XBL scope.
   JS::Rooted<JSObject*> propertyHolder(cx);
   if (scopeObject != globalObject) {
 
     // This is just a property holder, so it doesn't need any special JSClass.
--- a/dom/xbl/nsXBLProtoImplField.cpp
+++ b/dom/xbl/nsXBLProtoImplField.cpp
@@ -307,17 +307,17 @@ FieldSetter(JSContext *cx, unsigned argc
 }
 
 nsresult
 nsXBLProtoImplField::InstallAccessors(JSContext* aCx,
                                       JS::Handle<JSObject*> aTargetClassObject)
 {
   MOZ_ASSERT(js::IsObjectInContextCompartment(aTargetClassObject, aCx));
   JS::Rooted<JSObject*> globalObject(aCx, JS_GetGlobalForObject(aCx, aTargetClassObject));
-  JS::Rooted<JSObject*> scopeObject(aCx, xpc::GetXBLScope(aCx, globalObject));
+  JS::Rooted<JSObject*> scopeObject(aCx, xpc::GetXBLScopeOrGlobal(aCx, globalObject));
   NS_ENSURE_TRUE(scopeObject, NS_ERROR_OUT_OF_MEMORY);
 
   // Don't install it if the field is empty; see also InstallField which also must
   // implement the not-empty requirement.
   if (IsEmpty()) {
     return NS_OK;
   }
 
@@ -414,17 +414,17 @@ nsXBLProtoImplField::InstallField(nsIScr
   aBindingDocURI->GetSpec(uriSpec);
 
   AutoPushJSContext cx(aContext->GetNativeContext());
   NS_ASSERTION(!::JS_IsExceptionPending(cx),
                "Shouldn't get here when an exception is pending!");
 
   // First, enter the xbl scope, wrap the node, and use that as the scope for
   // the evaluation.
-  JS::Rooted<JSObject*> scopeObject(cx, xpc::GetXBLScope(cx, aBoundNode));
+  JS::Rooted<JSObject*> scopeObject(cx, xpc::GetXBLScopeOrGlobal(cx, aBoundNode));
   NS_ENSURE_TRUE(scopeObject, NS_ERROR_OUT_OF_MEMORY);
   JSAutoCompartment ac(cx, scopeObject);
 
   JS::Rooted<JSObject*> wrappedNode(cx, aBoundNode);
   if (!JS_WrapObject(cx, &wrappedNode))
       return NS_ERROR_OUT_OF_MEMORY;
 
   JS::Rooted<JS::Value> result(cx);
--- a/dom/xbl/nsXBLProtoImplMethod.cpp
+++ b/dom/xbl/nsXBLProtoImplMethod.cpp
@@ -305,17 +305,17 @@ nsXBLProtoImplAnonymousMethod::Execute(n
   // Make sure to do this before entering the compartment, since pushing Push()
   // may call JS_SaveFrameChain(), which puts us back in an unentered state.
   nsCxPusher pusher;
   if (!pusher.Push(aBoundElement))
     return NS_ERROR_UNEXPECTED;
   MOZ_ASSERT(cx == nsContentUtils::GetCurrentJSContext());
 
   JS::Rooted<JSObject*> thisObject(cx, &v.toObject());
-  JS::Rooted<JSObject*> scopeObject(cx, xpc::GetXBLScope(cx, globalObject));
+  JS::Rooted<JSObject*> scopeObject(cx, xpc::GetXBLScopeOrGlobal(cx, globalObject));
   NS_ENSURE_TRUE(scopeObject, NS_ERROR_OUT_OF_MEMORY);
 
   JSAutoCompartment ac(cx, scopeObject);
   if (!JS_WrapObject(cx, &thisObject))
       return NS_ERROR_OUT_OF_MEMORY;
 
   // Clone the function object, using thisObject as the parent so "this" is in
   // the scope chain of the resulting function (for backwards compat to the
--- a/dom/xbl/nsXBLPrototypeHandler.cpp
+++ b/dom/xbl/nsXBLPrototypeHandler.cpp
@@ -281,17 +281,17 @@ nsXBLPrototypeHandler::ExecuteHandler(Ev
 
   AutoPushJSContext cx(boundContext->GetNativeContext());
   JS::Rooted<JSObject*> handler(cx);
 
   rv = EnsureEventHandler(boundGlobal, boundContext, onEventAtom, &handler);
   NS_ENSURE_SUCCESS(rv, rv);
 
   JS::Rooted<JSObject*> globalObject(cx, boundGlobal->GetGlobalJSObject());
-  JS::Rooted<JSObject*> scopeObject(cx, xpc::GetXBLScope(cx, globalObject));
+  JS::Rooted<JSObject*> scopeObject(cx, xpc::GetXBLScopeOrGlobal(cx, globalObject));
   NS_ENSURE_TRUE(scopeObject, NS_ERROR_OUT_OF_MEMORY);
 
   // Bind it to the bound element. Note that if we're using a separate XBL scope,
   // we'll actually be binding the event handler to a cross-compartment wrapper
   // to the bound element's reflector.
 
   // First, enter our XBL scope. This is where the generic handler should have
   // been compiled, above.
@@ -358,17 +358,17 @@ nsXBLPrototypeHandler::EnsureEventHandle
     }
   }
 
   // Ensure that we have something to compile
   nsDependentString handlerText(mHandlerText);
   NS_ENSURE_TRUE(!handlerText.IsEmpty(), NS_ERROR_FAILURE);
 
   JS::Rooted<JSObject*> globalObject(cx, aGlobal->GetGlobalJSObject());
-  JS::Rooted<JSObject*> scopeObject(cx, xpc::GetXBLScope(cx, globalObject));
+  JS::Rooted<JSObject*> scopeObject(cx, xpc::GetXBLScopeOrGlobal(cx, globalObject));
   NS_ENSURE_TRUE(scopeObject, NS_ERROR_OUT_OF_MEMORY);
 
   nsAutoCString bindingURI;
   mPrototypeBinding->DocURI()->GetSpec(bindingURI);
 
   uint32_t argCount;
   const char **argNames;
   nsContentUtils::GetEventArgNames(kNameSpaceID_XBL, aName, &argCount,
--- a/js/xpconnect/src/xpcpublic.h
+++ b/js/xpconnect/src/xpcpublic.h
@@ -65,28 +65,42 @@ private:
     // Whether the new-style domain policy when this compartment was created
     // forbids script execution.
     bool mScriptBlockedByPolicy;
 };
 
 JSObject *
 TransplantObject(JSContext *cx, JS::HandleObject origobj, JS::HandleObject target);
 
+bool IsXBLScope(JSCompartment *compartment);
+bool IsInXBLScope(JSObject *obj);
+
 // Return a raw XBL scope object corresponding to contentScope, which must
 // be an object whose global is a DOM window.
 //
 // The return value is not wrapped into cx->compartment, so be sure to enter
 // its compartment before doing anything meaningful.
 //
 // Also note that XBL scopes are lazily created, so the return-value should be
 // null-checked unless the caller can ensure that the scope must already
 // exist.
+//
+// This function asserts if |contentScope| is itself in an XBL scope to catch
+// sloppy consumers. Conversely, GetXBLScopeOrGlobal will handle objects that
+// are in XBL scope (by just returning the global).
 JSObject *
 GetXBLScope(JSContext *cx, JSObject *contentScope);
 
+inline JSObject *
+GetXBLScopeOrGlobal(JSContext *cx, JSObject *obj) {
+    if (IsInXBLScope(obj))
+        return js::GetGlobalForObjectCrossCompartment(obj);
+    return GetXBLScope(cx, obj);
+}
+
 // Returns whether XBL scopes have been explicitly disabled for code running
 // in this compartment. See the comment around mAllowXBLScope.
 bool
 AllowXBLScope(JSCompartment *c);
 
 // Returns whether we will use an XBL scope for this compartment. This is
 // semantically equivalent to comparing global != GetXBLScope(global), but it
 // does not have the side-effect of eagerly creating the XBL scope if it does
@@ -340,19 +354,16 @@ bool StringToJsval(JSContext* cx, mozill
         rval.setNull();
         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:
     ZoneStatsExtras()