Bug 1032457 - Separate out cloning and non-cloning function forwarders. r=gabor
authorBobby Holley <bobbyholley@gmail.com>
Thu, 03 Jul 2014 11:00:54 -0700
changeset 213036 704be3e8cd10d49c3ada27938ef2665cdf63d657
parent 213035 cd5ec2d1435d2640274c8618b91dba882d84a6cc
child 213037 2afd5db295eef0afff559d2c7423009ae3720cbc
push id3857
push userraliiev@mozilla.com
push dateTue, 02 Sep 2014 16:39:23 +0000
treeherdermozilla-beta@5638b907b505 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgabor
bugs1032457
milestone33.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1032457 - Separate out cloning and non-cloning function forwarders. r=gabor We're going to add functionality to the cloning version, and the non-cloning version is going away.
js/xpconnect/src/ExportHelpers.cpp
js/xpconnect/src/XPCComponents.cpp
js/xpconnect/src/XPCJSRuntime.cpp
js/xpconnect/src/xpcprivate.h
--- a/js/xpconnect/src/ExportHelpers.cpp
+++ b/js/xpconnect/src/ExportHelpers.cpp
@@ -74,17 +74,17 @@ StackScopedCloneRead(JSContext *cx, JSSt
       MOZ_ASSERT(data < cloneData->mFunctions.length());
 
       RootedValue functionValue(cx);
       RootedObject obj(cx, cloneData->mFunctions[data]);
 
       if (!JS_WrapObject(cx, &obj))
           return nullptr;
 
-      if (!xpc::NewFunctionForwarder(cx, obj, true, &functionValue))
+      if (!xpc::NewFunctionForwarder(cx, JSID_VOIDHANDLE, obj, &functionValue))
           return nullptr;
 
       return &functionValue.toObject();
     }
 
     MOZ_ASSERT_UNREACHABLE("Encountered garbage in the clone stream!");
     return nullptr;
 }
@@ -238,42 +238,47 @@ NonCloningFunctionForwarder(JSContext *c
 
     RootedObject obj(cx, JS_THIS_OBJECT(cx, vp));
     if (!obj) {
         return false;
     }
     return JS_CallFunctionValue(cx, obj, v, args, args.rval());
 }
 bool
-NewFunctionForwarder(JSContext *cx, HandleId id, HandleObject callable, bool doclone,
-                          MutableHandleValue vp)
+NewFunctionForwarder(JSContext *cx, HandleId idArg, HandleObject callable,
+                     MutableHandleValue vp)
 {
-    JSFunction *fun = js::NewFunctionByIdWithReserved(cx, doclone ? CloningFunctionForwarder :
-                                                                    NonCloningFunctionForwarder,
-                                                                    0,0, JS::CurrentGlobalOrNull(cx), id);
+    RootedId id(cx, idArg);
+    if (id == JSID_VOIDHANDLE)
+        id = GetRTIdByIndex(cx, XPCJSRuntime::IDX_EMPTYSTRING);
 
+    JSFunction *fun = js::NewFunctionByIdWithReserved(cx, CloningFunctionForwarder, 0,0,
+                                                      JS::CurrentGlobalOrNull(cx), id);
     if (!fun)
         return false;
 
     JSObject *funobj = JS_GetFunctionObject(fun);
     js::SetFunctionNativeReserved(funobj, 0, ObjectValue(*callable));
     vp.setObject(*funobj);
     return true;
 }
 
 bool
-NewFunctionForwarder(JSContext *cx, HandleObject callable, bool doclone,
-                          MutableHandleValue vp)
+NewNonCloningFunctionForwarder(JSContext *cx, HandleId id, HandleObject callable,
+                               MutableHandleValue vp)
 {
-    RootedId emptyId(cx);
-    RootedValue emptyStringValue(cx, JS_GetEmptyStringValue(cx));
-    if (!JS_ValueToId(cx, emptyStringValue, &emptyId))
+    JSFunction *fun = js::NewFunctionByIdWithReserved(cx, NonCloningFunctionForwarder,
+                                                      0,0, JS::CurrentGlobalOrNull(cx), id);
+    if (!fun)
         return false;
 
-    return NewFunctionForwarder(cx, emptyId, callable, doclone, vp);
+    JSObject *funobj = JS_GetFunctionObject(fun);
+    js::SetFunctionNativeReserved(funobj, 0, ObjectValue(*callable));
+    vp.setObject(*funobj);
+    return true;
 }
 
 bool
 ExportFunction(JSContext *cx, HandleValue vfunction, HandleValue vscope, HandleValue voptions,
                MutableHandleValue rval)
 {
     bool hasOptions = !voptions.isUndefined();
     if (!vscope.isObject() || !vfunction.isObject() || (hasOptions && !voptions.isObject())) {
@@ -329,17 +334,17 @@ ExportFunction(JSContext *cx, HandleValu
         // The function forwarder will live in the target compartment. Since
         // this function will be referenced from its private slot, to avoid a
         // GC hazard, we must wrap it to the same compartment.
         if (!JS_WrapObject(cx, &funObj))
             return false;
 
         // And now, let's create the forwarder function in the target compartment
         // for the function the be exported.
-        if (!NewFunctionForwarder(cx, id, funObj, /* doclone = */ true, rval)) {
+        if (!NewFunctionForwarder(cx, id, funObj, rval)) {
             JS_ReportError(cx, "Exporting function failed");
             return false;
         }
 
         // We have the forwarder function in the target compartment. If
         // defineAs was set, we also need to define it as a property on
         // the target.
         if (!JSID_IS_VOID(options.defineAs)) {
--- a/js/xpconnect/src/XPCComponents.cpp
+++ b/js/xpconnect/src/XPCComponents.cpp
@@ -3079,17 +3079,17 @@ nsXPCComponents_Utils::MakeObjectPropsNo
         if (v.isPrimitive())
             continue;
 
         RootedObject propobj(cx, &v.toObject());
         // TODO Deal with non-functions.
         if (!js::IsWrapper(propobj) || !JS_ObjectIsCallable(cx, propobj))
             continue;
 
-        if (!NewFunctionForwarder(cx, id, propobj, /* doclone = */ false, &v) ||
+        if (!NewNonCloningFunctionForwarder(cx, id, propobj, &v) ||
             !JS_SetPropertyById(cx, obj, id, v))
             return NS_ERROR_FAILURE;
     }
 
     return NS_OK;
 }
 
 NS_IMETHODIMP
--- a/js/xpconnect/src/XPCJSRuntime.cpp
+++ b/js/xpconnect/src/XPCJSRuntime.cpp
@@ -80,16 +80,17 @@ const char* const XPCJSRuntime::mStrings
     "__iterator__",         // IDX_ITERATOR
     "__exposedProps__",     // IDX_EXPOSEDPROPS
     "eval",                 // IDX_EVAL
     "controllers",          // IDX_CONTROLLERS
     "realFrameElement",     // IDX_REALFRAMEELEMENT
     "length",               // IDX_LENGTH
     "name",                 // IDX_NAME
     "undefined",            // IDX_UNDEFINED
+    "",                     // IDX_EMPTYSTRING
 };
 
 /***************************************************************************/
 
 static mozilla::Atomic<bool> sDiscardSystemSource(false);
 
 bool
 xpc::ShouldDiscardSystemSource() { return sDiscardSystemSource; }
--- a/js/xpconnect/src/xpcprivate.h
+++ b/js/xpconnect/src/xpcprivate.h
@@ -476,16 +476,17 @@ public:
         IDX_ITERATOR                ,
         IDX_EXPOSEDPROPS            ,
         IDX_EVAL                    ,
         IDX_CONTROLLERS             ,
         IDX_REALFRAMEELEMENT        ,
         IDX_LENGTH                  ,
         IDX_NAME                    ,
         IDX_UNDEFINED               ,
+        IDX_EMPTYSTRING             ,
         IDX_TOTAL_COUNT // just a count of the above
     };
 
     JS::HandleId GetStringID(unsigned index) const
     {
         MOZ_ASSERT(index < IDX_TOTAL_COUNT, "index out of range");
         // fromMarkedLocation() is safe because the string is interned.
         return JS::HandleId::fromMarkedLocation(&mStrIDs[index]);
@@ -3277,25 +3278,27 @@ namespace xpc {
 bool
 Atob(JSContext *cx, unsigned argc, jsval *vp);
 
 bool
 Btoa(JSContext *cx, unsigned argc, jsval *vp);
 
 
 // Helper function that creates a JSFunction that wraps a native function that
-// forwards the call to the original 'callable'. If the 'doclone' argument is
-// set, it also structure clones non-native arguments for extra security.
+// forwards the call to the original 'callable'. Any object-valued arguments are
+// cloned at call time for improved security.
 bool
 NewFunctionForwarder(JSContext *cx, JS::HandleId id, JS::HandleObject callable,
-                     bool doclone, JS::MutableHandleValue vp);
-
+                     JS::MutableHandleValue vp);
+
+// Old-style function forwarding without structured-cloning for arguments. This
+// is deprecated.
 bool
-NewFunctionForwarder(JSContext *cx, JS::HandleObject callable,
-                     bool doclone, JS::MutableHandleValue vp);
+NewNonCloningFunctionForwarder(JSContext *cx, JS::HandleId id,
+                               JS::HandleObject callable, JS::MutableHandleValue vp);
 
 // Old fashioned xpc error reporter. Try to use JS_ReportError instead.
 nsresult
 ThrowAndFail(nsresult errNum, JSContext *cx, bool *retval);
 
 struct GlobalProperties {
     GlobalProperties(bool aPromise) {
       mozilla::PodZero(this);