commit c13c8f0e40eaeed7a7f31f1f7fdbe48001ce3671 Author: Bobby Holley bobbyholley@gmail.com Date: Mon Feb 10 14:07:27 2014 -0800

Bug 912322 - Update semantics of IsChromeOrXBL to return true for remote XUL. r=bz

This brings us into alignment with nsContentUtils::IsCallerXBL(). We also take the opportunity to clean up some comments and invariants that changed with the removal of the XBL bit. --- js/xpconnect/src/XPCWrappedNativeScope.cpp | 9 +++++++++ js/xpconnect/src/nsXPConnect.cpp | 13 ++++++++++--- js/xpconnect/src/xpcprivate.h | 17 +++++++---------- 3 files changed, 26 insertions(+), 13 deletions(-)

diff --git a/js/xpconnect/src/XPCWrappedNativeScope.cpp b/js/xpconnect/src/XPCWrappedNativeScope.cpp index c3a0780..223fcc4 100644 --- a/js/xpconnect/src/XPCWrappedNativeScope.cpp +++ b/js/xpconnect/src/XPCWrappedNativeScope.cpp @@ -266,6 +266,15 @@ XPCWrappedNativeScope::EnsureXBLScope(JSContext *cx) return mXBLScope; }

+bool +XPCWrappedNativeScope::AllowXBLScope() +{ + // We only disallow XBL scopes in remote XUL situations. + MOZ_ASSERT_IF(!mAllowXBLScope, + nsContentUtils::AllowXULXBLForPrincipal(GetPrincipal())); + return mAllowXBLScope; +} + namespace xpc { JSObject *GetXBLScope(JSContext *cx, JSObject *contentScopeArg) { diff --git a/js/xpconnect/src/nsXPConnect.cpp b/js/xpconnect/src/nsXPConnect.cpp index 12a8fd0..67d4fb1 100644 --- a/js/xpconnect/src/nsXPConnect.cpp +++ b/js/xpconnect/src/nsXPConnect.cpp @@ -1713,9 +1713,16 @@ namespace dom { bool IsChromeOrXBL(JSContext* cx, JSObject* /* unused */) { - JSCompartment* compartment = js::GetContextCompartment(cx); - return AccessCheck::isChrome(compartment) || - IsXBLScope(compartment); + MOZ_ASSERT(NS_IsMainThread()); + JSCompartment* c = js::GetContextCompartment(cx); + + // 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 AllowXBLScope() only returns false in + // remote XUL situations. + return AccessCheck::isChrome(c) || IsXBLScope(c) || !AllowXBLScope(c); }

} // namespace dom diff --git a/js/xpconnect/src/xpcprivate.h b/js/xpconnect/src/xpcprivate.h index b63c634..af1cf89 100644 --- a/js/xpconnect/src/xpcprivate.h +++ b/js/xpconnect/src/xpcprivate.h @@ -1435,7 +1435,7 @@ public: nsAutoPtr<JSObject2JSObjectMap> mWaiverWrapperMap;

bool IsXBLScope() { return mIsXBLScope; } - bool AllowXBLScope() { return mAllowXBLScope; } + bool AllowXBLScope(); bool UseXBLScope() { return mUseXBLScope; }

protected: @@ -1472,20 +1472,17 @@ private:

bool mIsXBLScope;

- // There are certain cases where we explicitly disallow XBL scopes: they - // can be prefed off, or we might be running in a remote XUL domain where - // we want to run all XBL in content to maintain compat. We separately + // For remote XUL domains, we run all XBL in the content scope for compat + // reasons (though we sometimes pref this off for automation). We separately // track the result of this decision (mAllowXBLScope), from the decision // of whether to actually _use_ an XBL scope (mUseXBLScope), which depends // on the type of global and whether the compartment is system principal // or not. // - // This distinction is useful primarily because it tells us whether we - // can infer the XBL-ness of a caller by checking that the caller is - // running in an XBL scope, or whether we need to check the XBL bit on the - // script. The XBL bit is nasty, so we want to consult it only if we - // absolutely have to, which should generally happen only in unsupported - // pref configurations. + // This distinction is useful primarily because, if true, we know that we + // have no way of distinguishing XBL script from content script for the + // given scope. In these (unsupported) situations, we just always claim to + // be XBL. bool mAllowXBLScope; bool mUseXBLScope; };