Bug 1164011 - Shim optimisation. r=billm

js/xpconnect/idl/nsIAddonInterposition.idl

@@ -20,7 +20,7 @@
  * property, it should return a replacement property descriptor for it. If not,
  * it should return null.
  */
-[scriptable,uuid(215353cb-6e77-462f-a791-6891f42e593f)]
+[scriptable,uuid(d05cc5fd-ad88-41a6-854c-36fd94d69ddb)]
 interface nsIAddonInterposition : nsISupports
 {
     /**
@@ -54,4 +54,14 @@ interface nsIAddonInterposition : nsISupports
                         in jsval originalFunc,
                         in jsval originalThis,
                         in jsval args);
+
+    /**
+     * For the first time when the interposition is registered the engine
+     * calls getWhitelist and expects an array of strings. The strings are
+     * the name of properties the interposition wants interposeProperty
+     * to be called. It can be an empty array.
+     * Note: for CPOWs interposeProperty is always called regardless if
+     * the name of the property is on the whitelist or not.
+     */
+    jsval getWhitelist();
 };

js/xpconnect/src/XPCComponents.cpp

@@ -32,6 +32,7 @@
 #include "nsWindowMemoryReporter.h"
 #include "nsDOMClassInfo.h"
 #include "ShimInterfaceInfo.h"
+#include "nsIAddonInterposition.h"
 
 using namespace mozilla;
 using namespace JS;
@@ -3490,8 +3491,9 @@ nsXPCComponents_Utils::SetAddonInterposition(const nsACString& addonIdStr,
     JSAddonId* addonId = xpc::NewAddonId(cx, addonIdStr);
     if (!addonId)
         return NS_ERROR_FAILURE;
-    if (!XPCWrappedNativeScope::SetAddonInterposition(addonId, interposition))
+    if (!XPCWrappedNativeScope::SetAddonInterposition(cx, addonId, interposition))
         return NS_ERROR_FAILURE;
+
     return NS_OK;
 }
 

js/xpconnect/src/XPCWrappedNativeScope.cpp

@@ -27,6 +27,7 @@ using namespace JS;
 XPCWrappedNativeScope* XPCWrappedNativeScope::gScopes = nullptr;
 XPCWrappedNativeScope* XPCWrappedNativeScope::gDyingScopes = nullptr;
 XPCWrappedNativeScope::InterpositionMap* XPCWrappedNativeScope::gInterpositionMap = nullptr;
+InterpositionWhitelistArray* XPCWrappedNativeScope::gInterpositionWhitelists = nullptr;
 
 NS_IMPL_ISUPPORTS(XPCWrappedNativeScope::ClearInterpositionsObserver, nsIObserver)
 
@@ -46,6 +47,11 @@ XPCWrappedNativeScope::ClearInterpositionsObserver::Observe(nsISupports* subject
         gInterpositionMap = nullptr;
     }
 
+    if (gInterpositionWhitelists) {
+        delete gInterpositionWhitelists;
+        gInterpositionWhitelists = nullptr;
+    }
+
     nsContentUtils::UnregisterShutdownObserver(this);
     return NS_OK;
 }
@@ -143,6 +149,8 @@ XPCWrappedNativeScope::XPCWrappedNativeScope(JSContext* cx,
             "extensions.interposition.enabled", false);
           if (interpositionEnabled) {
             mInterposition = do_GetService("@mozilla.org/addons/default-addon-shims;1");
+            MOZ_ASSERT(mInterposition);
+            UpdateInterpositionWhitelist(cx, mInterposition);
           }
         }
     }
@@ -756,7 +764,8 @@ XPCWrappedNativeScope::SetExpandoChain(JSContext* cx, HandleObject target,
 }
 
 /* static */ bool
-XPCWrappedNativeScope::SetAddonInterposition(JSAddonId* addonId,
+XPCWrappedNativeScope::SetAddonInterposition(JSContext* cx,
+                                             JSAddonId* addonId,
                                              nsIAddonInterposition* interp)
 {
     if (!gInterpositionMap) {
@@ -764,14 +773,17 @@ XPCWrappedNativeScope::SetAddonInterposition(JSAddonId* addonId,
         gInterpositionMap->init();
 
         // Make sure to clear the map at shutdown.
+        // Note: this will take care of gInterpositionWhitelists too.
         nsContentUtils::RegisterShutdownObserver(new ClearInterpositionsObserver());
     }
     if (interp) {
-        return gInterpositionMap->put(addonId, interp);
+        bool ok = gInterpositionMap->put(addonId, interp);
+        NS_ENSURE_TRUE(ok, false);
+        UpdateInterpositionWhitelist(cx, interp);
     } else {
         gInterpositionMap->remove(addonId);
-        return true;
     }
+    return true;
 }
 
 nsCOMPtr<nsIAddonInterposition>
@@ -780,6 +792,103 @@ XPCWrappedNativeScope::GetInterposition()
     return mInterposition;
 }
 
+/* static */ InterpositionWhitelist*
+XPCWrappedNativeScope::GetInterpositionWhitelist(nsIAddonInterposition* interposition)
+{
+    if (!gInterpositionWhitelists)
+        return nullptr;
+
+    InterpositionWhitelistArray& wls = *gInterpositionWhitelists;
+    for (size_t i = 0; i < wls.Length(); i++) {
+        if (wls[i].interposition == interposition)
+            return &wls[i].whitelist;
+    }
+
+    return nullptr;
+}
+
+/* static */ bool
+XPCWrappedNativeScope::UpdateInterpositionWhitelist(JSContext* cx,
+                                                    nsIAddonInterposition* interposition)
+{
+    // We want to set the interpostion whitelist only once.
+    InterpositionWhitelist* whitelist = GetInterpositionWhitelist(interposition);
+    if (whitelist)
+        return true;
+
+    // The hashsets in gInterpositionWhitelists do not have a copy constructor so
+    // a reallocation for the array will lead to a memory corruption. If you
+    // need more interpositions, change the capacity of the array please.
+    static const size_t MAX_INTERPOSITION = 8;
+    if (!gInterpositionWhitelists)
+        gInterpositionWhitelists = new InterpositionWhitelistArray(MAX_INTERPOSITION);
+
+    MOZ_RELEASE_ASSERT(MAX_INTERPOSITION > gInterpositionWhitelists->Length() + 1);
+    InterpositionWhitelistPair* newPair = gInterpositionWhitelists->AppendElement();
+    newPair->interposition = interposition;
+    newPair->whitelist.init();
+    whitelist = &newPair->whitelist;
+
+    RootedValue whitelistVal(cx);
+    nsresult rv = interposition->GetWhitelist(&whitelistVal);
+    if (NS_FAILED(rv)) {
+        JS_ReportError(cx, "Could not get the whitelist from the interposition.");
+        return false;
+    }
+
+    if (!whitelistVal.isObject()) {
+        JS_ReportError(cx, "Whitelist must be an array.");
+        return false;
+    }
+
+    // We want to enter the whitelist's compartment to avoid any wrappers.
+    // To be on the safe side let's make sure that it's a system compartment
+    // and we don't accidentally trigger some content function here by parsing
+    // the whitelist object.
+    RootedObject whitelistObj(cx, &whitelistVal.toObject());
+    whitelistObj = js::UncheckedUnwrap(whitelistObj);
+    if (!AccessCheck::isChrome(whitelistObj)) {
+        JS_ReportError(cx, "Whitelist must be from system scope.");
+        return false;
+    }
+
+    {
+        JSAutoCompartment ac(cx, whitelistObj);
+
+        uint32_t length;
+        if (!JS_IsArrayObject(cx, whitelistObj) ||
+            !JS_GetArrayLength(cx, whitelistObj, &length)) {
+            JS_ReportError(cx, "Whitelist must be an array.");
+            return false;
+        }
+
+        for (uint32_t i = 0; i < length; i++) {
+            RootedValue idval(cx);
+            if (!JS_GetElement(cx, whitelistObj, i, &idval))
+                return false;
+
+            if (!idval.isString()) {
+                JS_ReportError(cx, "Whitelist must contain strings only.");
+                return false;
+            }
+
+            RootedString str(cx, idval.toString());
+            str = JS_InternJSString(cx, str);
+            if (!str) {
+                JS_ReportError(cx, "String internization failed.");
+                return false;
+            }
+
+            // By internizing the id's we ensure that they won't get
+            // GCed so we can use them as hash keys.
+            jsid id = INTERNED_STRING_TO_JSID(cx, str);
+            whitelist->put(JSID_BITS(id));
+        }
+    }
+
+    return true;
+}
+
 /***************************************************************************/
 
 // static

js/xpconnect/src/nsXPConnect.cpp

@@ -1360,17 +1360,14 @@ bool
 SetAddonInterposition(const nsACString& addonIdStr, nsIAddonInterposition* interposition)
 {
     JSAddonId* addonId;
-    {
-        // We enter the junk scope just to allocate a string, which actually will go
-        // in the system zone.
-        AutoJSAPI jsapi;
-        jsapi.Init(xpc::PrivilegedJunkScope());
-        addonId = NewAddonId(jsapi.cx(), addonIdStr);
-        if (!addonId)
-            return false;
-    }
-
-    return XPCWrappedNativeScope::SetAddonInterposition(addonId, interposition);
+    // We enter the junk scope just to allocate a string, which actually will go
+    // in the system zone.
+    AutoJSAPI jsapi;
+    jsapi.Init(xpc::PrivilegedJunkScope());
+    addonId = NewAddonId(jsapi.cx(), addonIdStr);
+    if (!addonId)
+        return false;
+    return XPCWrappedNativeScope::SetAddonInterposition(jsapi.cx(), addonId, interposition);
 }
 
 } // namespace xpc

js/xpconnect/src/xpcprivate.h

@@ -1018,6 +1018,17 @@ static inline bool IS_PROTO_CLASS(const js::Class* clazz)
            clazz == &XPC_WN_ModsAllowed_NoCall_Proto_JSClass;
 }
 
+typedef js::HashSet<uint64_t,
+                    js::DefaultHasher<uint64_t>,
+                    js::SystemAllocPolicy> InterpositionWhitelist;
+
+struct InterpositionWhitelistPair {
+    nsIAddonInterposition* interposition;
+    InterpositionWhitelist whitelist;
+};
+
+typedef nsTArray<InterpositionWhitelistPair> InterpositionWhitelistArray;
+
 /***************************************************************************/
 // XPCWrappedNativeScope is one-to-one with a JS global object.
 
@@ -1187,9 +1198,14 @@ public:
     bool HasInterposition() { return mInterposition; }
     nsCOMPtr<nsIAddonInterposition> GetInterposition();
 
-    static bool SetAddonInterposition(JSAddonId* addonId,
+    static bool SetAddonInterposition(JSContext* cx,
+                                      JSAddonId* addonId,
                                       nsIAddonInterposition* interp);
 
+    static InterpositionWhitelist* GetInterpositionWhitelist(nsIAddonInterposition* interposition);
+    static bool UpdateInterpositionWhitelist(JSContext* cx,
+                                             nsIAddonInterposition* interposition);
+
     void SetAddonCallInterposition() { mHasCallInterpositions = true; }
     bool HasCallInterposition() { return mHasCallInterpositions; };
 
@@ -1212,6 +1228,8 @@ private:
 
     static InterpositionMap*         gInterpositionMap;
 
+    static InterpositionWhitelistArray* gInterpositionWhitelists;
+
     XPCJSRuntime*                    mRuntime;
     Native2WrappedNativeMap*         mWrappedNativeMap;
     ClassInfo2WrappedNativeProtoMap* mWrappedNativeProtoMap;

js/xpconnect/tests/unit/test_interposition.js

@@ -34,6 +34,11 @@ let TestInterposition = {
   QueryInterface: XPCOMUtils.generateQI([Ci.nsIAddonInterposition,
                                          Ci.nsISupportsWeakReference]),
 
+  getWhitelist: function() {
+    return ["abcxyz", "utils", "dataprop", "getterprop", "setterprop",
+            "objprop", "defineprop", "configurableprop"];
+  },
+
   interposeProperty: function(addonId, target, iid, prop) {
     do_check_eq(addonId, ADDONID);
     do_check_eq(gExpectedProp, prop);

js/xpconnect/wrappers/AddonWrapper.cpp

@@ -29,11 +29,12 @@ InterposeProperty(JSContext* cx, HandleObject target, const nsIID* iid, HandleId
     // wrapped natives.
     RootedObject unwrapped(cx, UncheckedUnwrap(target));
     const js::Class* clasp = js::GetObjectClass(unwrapped);
+    bool isCPOW = jsipc::IsWrappedCPOW(unwrapped);
     if (!mozilla::dom::IsDOMClass(clasp) &&
         !IS_WN_CLASS(clasp) &&
         !IS_PROTO_CLASS(clasp) &&
         clasp != &OuterWindowProxyClass &&
-        !jsipc::IsWrappedCPOW(unwrapped)) {
+        !isCPOW) {
         return true;
     }
 
@@ -41,6 +42,13 @@ InterposeProperty(JSContext* cx, HandleObject target, const nsIID* iid, HandleId
     MOZ_ASSERT(scope->HasInterposition());
 
     nsCOMPtr<nsIAddonInterposition> interp = scope->GetInterposition();
+    InterpositionWhitelist* wl = XPCWrappedNativeScope::GetInterpositionWhitelist(interp);
+    MOZ_ASSERT(wl);
+    // We do InterposeProperty only if the id is on the whitelist of the interpostion
+    // or if the target is a CPOW.
+    if (!wl->has(JSID_BITS(id.get())) && !isCPOW)
+        return true;
+
     JSAddonId* addonId = AddonIdOfObject(target);
     RootedValue addonIdValue(cx, StringValue(StringOfAddonId(addonId)));
     RootedValue prop(cx, IdToValue(id));

toolkit/components/addoncompat/defaultShims.js

@@ -22,6 +22,10 @@ DefaultInterpositionService.prototype = {
   classID: Components.ID("{50bc93ce-602a-4bef-bf3a-61fc749c4caf}"),
   QueryInterface: XPCOMUtils.generateQI([Ci.nsIAddonInterposition, Ci.nsISupportsWeakReference]),
 
+  getWhitelist: function() {
+    return [];
+  },
+
   interposeProperty: function(addon, target, iid, prop) {
     return null;
   },

toolkit/components/addoncompat/multiprocessShims.js

@@ -70,12 +70,42 @@ function AddonInterpositionService()
   // kinds of objects.
   this._interfaceInterpositions = RemoteAddonsParent.getInterfaceInterpositions();
   this._taggedInterpositions = RemoteAddonsParent.getTaggedInterpositions();
+
+  let wl = [];
+  for (let v in this._interfaceInterpositions) {
+    let interp = this._interfaceInterpositions[v];
+    wl.push(...Object.getOwnPropertyNames(interp.methods));
+    wl.push(...Object.getOwnPropertyNames(interp.getters));
+    wl.push(...Object.getOwnPropertyNames(interp.setters));
+  }
+
+  for (let v in this._taggedInterpositions) {
+    let interp = this._taggedInterpositions[v];
+    wl.push(...Object.getOwnPropertyNames(interp.methods));
+    wl.push(...Object.getOwnPropertyNames(interp.getters));
+    wl.push(...Object.getOwnPropertyNames(interp.setters));
+  }
+
+  let nameSet = new Set();
+  wl = wl.filter(function(item) {
+    if (nameSet.has(item))
+      return true;
+
+    nameSet.add(item);
+    return true;
+  });
+
+  this._whitelist = wl;
 }
 
 AddonInterpositionService.prototype = {
   classID: Components.ID("{1363d5f0-d95e-11e3-9c1a-0800200c9a66}"),
   QueryInterface: XPCOMUtils.generateQI([Ci.nsIAddonInterposition, Ci.nsISupportsWeakReference]),
 
+  getWhitelist: function() {
+    return this._whitelist;
+  },
+
   // When the interface is not known for a method call, this code
   // determines the type of the target object.
   getObjectTag: function(target) {