[tor-commits] [tor-browser/esr24] Bug 912322 - Update semantics of IsChromeOrXBL to return true for remote XUL. r=bz

[mikeperry at torproject.org]

commit c13c8f0e40eaeed7a7f31f1f7fdbe48001ce3671
Author: Bobby Holley <bobbyholley at 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;
 };