From e97495d7b3b1be49c4e2e432e73e4fbb95f297d6 Mon Sep 17 00:00:00 2001 From: Nika Layzell Date: Mon, 23 Apr 2018 13:18:39 -0400 Subject: [PATCH] Bug 1457972 - Part 5: Use modern JS APIs to root jsval temporaries in XPConnect, r=mccr8 When a jsval passed from JS code it needs to be stored in a nsXPTCVariant object. This object is not rooted by default, as it is stored in some C++-allocated memory. Currently, we root the values by adding a custom root using the `js::AddRawValueRoot` API, which is deprecated, and only used by this code and ErrorResult. This also has the unfortunate effect that we cannot support XPCOM arrays of jsvals, as we cannot root all of the values in the array using this API. Fortunately, the JS engine has a better rooting API which we can use here instead. I make the call context a custom rooter, like the SequenceRooter type from WebIDL, and make sure to note every jsval when tracing, both in arrays and as direct values. This should allow us to avoid some hashtable operations with roots when performing XPConnect calls, and remove a consumer of this gross legacy API. In addition it allows us to support arrays. This will be even more useful in the future when I add support for sequence (which is a nsTArray) to xpidl and xpconnect. --- js/xpconnect/src/XPCWrappedNative.cpp | 81 +++++++++++++++++---------- 1 file changed, 50 insertions(+), 31 deletions(-) diff --git a/js/xpconnect/src/XPCWrappedNative.cpp b/js/xpconnect/src/XPCWrappedNative.cpp index d5f6080afe65..564e56317ea6 100644 --- a/js/xpconnect/src/XPCWrappedNative.cpp +++ b/js/xpconnect/src/XPCWrappedNative.cpp @@ -1133,7 +1133,7 @@ static bool Throw(nsresult errNum, XPCCallContext& ccx) /***************************************************************************/ -class MOZ_STACK_CLASS CallMethodHelper +class MOZ_STACK_CLASS CallMethodHelper final { XPCCallContext& mCallContext; nsresult mInvokeResult; @@ -1187,8 +1187,6 @@ class MOZ_STACK_CLASS CallMethodHelper MOZ_ALWAYS_INLINE bool ConvertDependentParams(); MOZ_ALWAYS_INLINE bool ConvertDependentParam(uint8_t i); - MOZ_ALWAYS_INLINE void CleanupParam(nsXPTCMiniVariant& param, const nsXPTType& type); - MOZ_ALWAYS_INLINE nsresult Invoke(); public: @@ -1215,6 +1213,9 @@ public: MOZ_ALWAYS_INLINE bool Call(); + // Trace implementation so we can put our CallMethodHelper in a Rooted. + void trace(JSTracer* aTrc); + }; // static @@ -1227,7 +1228,8 @@ XPCWrappedNative::CallMethod(XPCCallContext& ccx, return Throw(rv, ccx); } - return CallMethodHelper(ccx).Call(); + JS::Rooted helper(ccx, CallMethodHelper(ccx)); + return helper.get().Call(); } bool @@ -1281,7 +1283,11 @@ CallMethodHelper::~CallMethodHelper() if (!param.DoesValNeedCleanup()) continue; - CleanupParam(param, param.type); + uint32_t arraylen = 0; + if (!GetArraySizeFromParam(param.type, UndefinedHandleValue, &arraylen)) + continue; + + xpc::CleanupValue(param.type, ¶m.val, arraylen); } } @@ -1596,16 +1602,10 @@ CallMethodHelper::ConvertIndependentParam(uint8_t i) // indirectly, regardless of in/out-ness. These types are stored in the // nsXPTCVariant's extended value. switch (type.Tag()) { - // Ensure that the jsval has a valid value and root it so we can trace - // through it. + // Ensure that the jsval has a valid value. case nsXPTType::T_JSVAL: new (&dp->Ext().jsval) JS::Value(); MOZ_ASSERT(dp->Ext().jsval.isUndefined()); - if (!js::AddRawValueRoot(mCallContext, &dp->Ext().jsval, - "XPCWrappedNative::CallMethod param")) - { - return false; - } break; // Initialize our temporary string class values so they can be assigned @@ -1787,25 +1787,6 @@ CallMethodHelper::ConvertDependentParam(uint8_t i) return true; } -// Performs all necessary teardown on a parameter after method invocation. -void -CallMethodHelper::CleanupParam(nsXPTCMiniVariant& param, const nsXPTType& type) -{ - uint32_t arraylen = 0; - switch (type.Tag()) { - // JSValues at the toplevel need to be unrooted. - case nsXPTType::T_JSVAL: - js::RemoveRawValueRoot(mCallContext, (Value*)¶m.val); - return; - - default: - MOZ_ALWAYS_TRUE(GetArraySizeFromParam(type, UndefinedHandleValue, &arraylen)); - xpc::CleanupValue(type, ¶m.val, arraylen); - return; - } - -} - nsresult CallMethodHelper::Invoke() { @@ -1815,6 +1796,44 @@ CallMethodHelper::Invoke() return NS_InvokeByIndex(mCallee, mVTableIndex, argc, argv); } +static void +TraceParam(JSTracer* aTrc, void* aVal, const nsXPTType& aType, + uint32_t aArrayLen = 0) +{ + if (aType.Tag() == nsXPTType::T_JSVAL) { + JS::UnsafeTraceRoot(aTrc, (JS::Value*)aVal, + "XPCWrappedNative::CallMethod param"); + } else if (aType.Tag() == nsXPTType::T_ARRAY && *(void**)aVal) { + const nsXPTType& elty = aType.ArrayElementType(); + if (elty.Tag() != nsXPTType::T_JSVAL) { + return; + } + + for (uint32_t i = 0; i < aArrayLen; ++i) { + TraceParam(aTrc, elty.ElementPtr(aVal, i), elty); + } + } +} + +void +CallMethodHelper::trace(JSTracer* aTrc) +{ + // We need to note each of our initialized parameters which contain jsvals. + for (nsXPTCVariant& param : mDispatchParams) { + if (!param.DoesValNeedCleanup()) { + MOZ_ASSERT(param.type.Tag() != nsXPTType::T_JSVAL, + "JSVals are marked as needing cleanup (even though they don't)"); + continue; + } + + uint32_t arrayLen = 0; + if (!GetArraySizeFromParam(param.type, UndefinedHandleValue, &arrayLen)) + continue; + + TraceParam(aTrc, ¶m.val, param.type, arrayLen); + } +} + /***************************************************************************/ // interface methods