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

@@ -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)
 {

js/xpconnect/src/nsXPConnect.cpp

@@ -1710,9 +1710,15 @@ bool
 IsChromeOrXBL(JSContext* cx, JSObject* /* unused */)
 {
     MOZ_ASSERT(NS_IsMainThread());
-    JSCompartment* compartment = js::GetContextCompartment(cx);
+    JSCompartment* c = js::GetContextCompartment(cx);
-    return AccessCheck::isChrome(compartment) ||
+
-           IsXBLScope(compartment);
+    // 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

js/xpconnect/src/xpcprivate.h

@@ -1413,7 +1413,7 @@ public:
     nsAutoPtr<JSObject2JSObjectMap> mWaiverWrapperMap;
 
     bool IsXBLScope() { return mIsXBLScope; }
-    bool AllowXBLScope() { return mAllowXBLScope; }
+    bool AllowXBLScope();
 
 protected:
     virtual ~XPCWrappedNativeScope();
@@ -1449,20 +1449,17 @@ private:
 
     bool mIsXBLScope;
 
-    // There are certain cases where we explicitly disallow XBL scopes: they
+    // For remote XUL domains, we run all XBL in the content scope for compat
-    // can be prefed off, or we might be running in a remote XUL domain where
+    // reasons (though we sometimes pref this off for automation). We separately
-    // we want to run all XBL in content to maintain compat. 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
+    // This distinction is useful primarily because, if true, we know that we
-    // can infer the XBL-ness of a caller by checking that the caller is
+    // have no way of distinguishing XBL script from content script for the
-    // running in an XBL scope, or whether we need to check the XBL bit on the
+    // given scope. In these (unsupported) situations, we just always claim to
-    // script. The XBL bit is nasty, so we want to consult it only if we
+    // be XBL.
-    // absolutely have to, which should generally happen only in unsupported
-    // pref configurations.
     bool mAllowXBLScope;
     bool mUseXBLScope;
 };