Bug 854019 - Continue checking the XBL bit if remote XUL disables XBL scopes. r=bz a=bajaj
authorBobby Holley <bobbyholley@gmail.com>
Wed, 27 Mar 2013 11:40:44 -0700
changeset 128705 c5add7fbc0277437481fadfaa72791778a6f4739
parent 128704 762893c10abfd6569843c3ef2b10bd7b178b644e
child 128706 b8ceed3fed5de2332c8264821d5d786fc2af0526
push id3562
push userbobbyholley@gmail.com
push dateWed, 27 Mar 2013 18:41:22 +0000
treeherdermozilla-aurora@c5add7fbc027 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz, bajaj
bugs854019
milestone21.0a2
Bug 854019 - Continue checking the XBL bit if remote XUL disables XBL scopes. r=bz a=bajaj
content/base/src/nsContentUtils.cpp
js/xpconnect/src/XPCWrappedNativeScope.cpp
js/xpconnect/src/xpcprivate.h
js/xpconnect/src/xpcpublic.h
layout/reftests/bugs/reftest.list
--- a/content/base/src/nsContentUtils.cpp
+++ b/content/base/src/nsContentUtils.cpp
@@ -1749,18 +1749,19 @@ bool
 nsContentUtils::IsCallerXBL()
 {
     JSScript *script;
     JSContext *cx = GetCurrentJSContext();
     if (!cx)
         return false;
 
     // New Hotness.
-    if (XPCJSRuntime::Get()->XBLScopesEnabled())
-        return xpc::IsXBLScope(js::GetContextCompartment(cx));
+    JSCompartment *c = js::GetContextCompartment(cx);
+    if (xpc::AllowXBLScope(c))
+        return xpc::IsXBLScope(c);
 
     // XBL scopes are behind a pref, so check the XBL bit as well.
     if (!JS_DescribeScriptedCaller(cx, &script, nullptr) || !script)
         return false;
     return JS_GetScriptUserBit(script);
 }
 
 
--- a/js/xpconnect/src/XPCWrappedNativeScope.cpp
+++ b/js/xpconnect/src/XPCWrappedNativeScope.cpp
@@ -89,16 +89,39 @@ XPCWrappedNativeScope::GetNewOrUsed(JSCo
 {
     XPCWrappedNativeScope* scope = GetObjectScope(aGlobal);
     if (!scope) {
         scope = new XPCWrappedNativeScope(cx, aGlobal);
     }
     return scope;
 }
 
+static bool
+RemoteXULForbidsXBLScope(nsIPrincipal *aPrincipal)
+{
+  // We end up getting called during SSM bootstrapping to create the
+  // SafeJSContext. In that case, nsContentUtils isn't ready for us.
+  //
+  // Also check for random JSD scopes that don't have a principal.
+  if (!nsContentUtils::IsInitialized() || !aPrincipal)
+      return false;
+
+  // AllowXULXBLForPrincipal will return true for system principal, but we
+  // don't want that here.
+  if (nsContentUtils::IsSystemPrincipal(aPrincipal))
+      return false;
+
+  // If this domain isn't whitelisted, we're done.
+  if (!nsContentUtils::AllowXULXBLForPrincipal(aPrincipal))
+      return false;
+
+  // Check the pref to determine how we should behave.
+  return !Preferences::GetBool("dom.use_xbl_scopes_for_remote_xul", false);
+}
+
 XPCWrappedNativeScope::XPCWrappedNativeScope(JSContext *cx,
                                              JSObject* aGlobal)
       : mWrappedNativeMap(Native2WrappedNativeMap::newMap(XPC_NATIVE_MAP_SIZE)),
         mWrappedNativeProtoMap(ClassInfo2WrappedNativeProtoMap::newMap(XPC_NATIVE_PROTO_MAP_SIZE)),
         mMainThreadWrappedNativeProtoMap(ClassInfo2WrappedNativeProtoMap::newMap(XPC_NATIVE_PROTO_MAP_SIZE)),
         mComponents(nullptr),
         mNext(nullptr),
         mGlobalJSObject(aGlobal),
@@ -125,33 +148,34 @@ XPCWrappedNativeScope::XPCWrappedNativeS
 
     DEBUG_TrackNewScope(this);
     MOZ_COUNT_CTOR(XPCWrappedNativeScope);
 
     // Attach ourselves to the compartment private.
     CompartmentPrivate *priv = EnsureCompartmentPrivate(aGlobal);
     priv->scope = this;
 
-    // Determine whether to use an XBL scope or not.
+    // Determine whether we would allow an XBL scope in this situation.
+    // In addition to being pref-controlled, we also disable XBL scopes for
+    // remote XUL domains, _except_ if we have an additional pref override set.
     nsIPrincipal *principal = GetPrincipal();
-    mUseXBLScope = XPCJSRuntime::Get()->XBLScopesEnabled();
+    mAllowXBLScope = XPCJSRuntime::Get()->XBLScopesEnabled() &&
+                     !RemoteXULForbidsXBLScope(principal);
+
+    // Determine whether to use an XBL scope.
+    mUseXBLScope = mAllowXBLScope;
     if (mUseXBLScope) {
       js::Class *clasp = js::GetObjectClass(mGlobalJSObject);
       mUseXBLScope = !strcmp(clasp->name, "Window") ||
                      !strcmp(clasp->name, "ChromeWindow") ||
                      !strcmp(clasp->name, "ModalContentWindow");
     }
     if (mUseXBLScope) {
       mUseXBLScope = principal && !nsContentUtils::IsSystemPrincipal(principal);
     }
-    if (mUseXBLScope) {
-      mUseXBLScope = !nsContentUtils::AllowXULXBLForPrincipal(principal) ||
-                      Preferences::GetBool("dom.use_xbl_scopes_for_remote_xul",
-                                           false);
-    }
 }
 
 // static
 JSBool
 XPCWrappedNativeScope::IsDyingScope(XPCWrappedNativeScope *scope)
 {
     for (XPCWrappedNativeScope *cur = gDyingScopes; cur; cur = cur->mNext) {
         if (scope == cur)
@@ -243,16 +267,22 @@ namespace xpc {
 JSObject *GetXBLScope(JSContext *cx, JSObject *contentScope)
 {
     JSAutoCompartment ac(cx, contentScope);
     JSObject *scope = EnsureCompartmentPrivate(contentScope)->scope->EnsureXBLScope(cx);
     scope = js::UnwrapObject(scope);
     xpc_UnmarkGrayObject(scope);
     return scope;
 }
+
+bool AllowXBLScope(JSCompartment *c)
+{
+  XPCWrappedNativeScope *scope = EnsureCompartmentPrivate(c)->scope;
+  return scope && scope->AllowXBLScope();
+}
 } /* namespace xpc */
 
 // Dummy JS class to let wrappers w/o an xpc prototype share
 // scopes. By doing this we avoid allocating a new scope for every
 // wrapper on creation of the wrapper, and most wrappers won't need
 // their own scope at all for the lifetime of the wrapper.
 // WRAPPER_SLOTS is key here (even though there's never anything
 // in the private data slot in these prototypes), as the number of
--- a/js/xpconnect/src/xpcprivate.h
+++ b/js/xpconnect/src/xpcprivate.h
@@ -1717,16 +1717,17 @@ public:
     // object is wrapped into the compartment of the global.
     JSObject *EnsureXBLScope(JSContext *cx);
 
     XPCWrappedNativeScope(JSContext *cx, JSObject* aGlobal);
 
     nsAutoPtr<JSObject2JSObjectMap> mWaiverWrapperMap;
 
     bool IsXBLScope() { return mIsXBLScope; }
+    bool AllowXBLScope() { return mAllowXBLScope; }
 
 protected:
     virtual ~XPCWrappedNativeScope();
 
     static void KillDyingScopes();
 
     XPCWrappedNativeScope(); // not implemented
 
@@ -1755,16 +1756,32 @@ private:
     JSObject*                        mPrototypeNoHelper;
 
     XPCContext*                      mContext;
 
     nsAutoPtr<DOMExpandoMap> mDOMExpandoMap;
 
     JSBool mExperimentalBindingsEnabled;
     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
+    // 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.
+    bool mAllowXBLScope;
     bool mUseXBLScope;
 };
 
 /***************************************************************************/
 // XPCNativeMember represents a single idl declared method, attribute or
 // constant.
 
 // Tight. No virtual methods. Can be bitwise copied (until any resolution done).
--- a/js/xpconnect/src/xpcpublic.h
+++ b/js/xpconnect/src/xpcpublic.h
@@ -47,16 +47,21 @@ TransplantObjectWithWrapper(JSContext *c
 // 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.
 JSObject *
 GetXBLScope(JSContext *cx, JSObject *contentScope);
 
+// Returns whether XBL scopes have been explicitly disabled for code running
+// in this compartment. See the comment around mAllowXBLScope.
+bool
+AllowXBLScope(JSCompartment *c);
+
 } /* namespace xpc */
 
 #define XPCONNECT_GLOBAL_FLAGS                                                \
     JSCLASS_DOM_GLOBAL | JSCLASS_HAS_PRIVATE |                                \
     JSCLASS_PRIVATE_IS_NSISUPPORTS | JSCLASS_IMPLEMENTS_BARRIERS |            \
     JSCLASS_GLOBAL_FLAGS_WITH_SLOTS(2)
 
 void
--- a/layout/reftests/bugs/reftest.list
+++ b/layout/reftests/bugs/reftest.list
@@ -1166,16 +1166,19 @@ skip-if(B2G) == 446100-1c.html about:bla
 == 446100-1e.html about:blank
 == 446100-1f.html about:blank
 skip-if(B2G) == 446100-1g.html about:blank
 == 446100-1h.html about:blank
 fuzzy(127,2) == 448193.html 448193-ref.html
 # == 448987.html 448987-ref.html  # Disabled for now - it needs privileges
 != 449149-1a.html about:blank
 != 449149-1b.html about:blank
+# Retry the above with our default remote XUL behavior
+test-pref(dom.use_xbl_scopes_for_remote_xul,false) != 449149-1a.html about:blank
+test-pref(dom.use_xbl_scopes_for_remote_xul,false) != 449149-1b.html about:blank
 == 449149-2.html 449149-2-ref.html
 == 449171-1.html 449171-ref.html
 == 449362-1.html 449362-1-ref.html
 == 449519-1.html 449519-1-ref.html
 # == 449653-1.html 449653-1-ref.html # Disabled for now - it needs privileges
 == 450670-1.html 450670-1-ref.html
 == 451168-1.html 451168-1-ref.html
 == 451876-1.html 451876-1-ref.html