From f7b64929d9aae58e8588779e1594d7f379880235 Mon Sep 17 00:00:00 2001 From: "dbradley%netscape.com" Date: Fri, 4 Apr 2003 15:32:30 +0000 Subject: [PATCH] bug 199122 - IDispatch logic should not depend on order of properties. r=adamlock, sr=alecf (Not part of the build) --- js/src/xpconnect/src/XPCDispInlines.h | 53 ++++-- js/src/xpconnect/src/XPCDispInterface.cpp | 196 +++++++++++++++------- js/src/xpconnect/src/XPCDispParams.cpp | 4 +- js/src/xpconnect/src/XPCDispPrivate.h | 83 ++++++++- js/src/xpconnect/src/XPCDispTearOff.cpp | 2 - 5 files changed, 250 insertions(+), 88 deletions(-) diff --git a/js/src/xpconnect/src/XPCDispInlines.h b/js/src/xpconnect/src/XPCDispInlines.h index 818cd54fa404..5548c85dff7f 100644 --- a/js/src/xpconnect/src/XPCDispInlines.h +++ b/js/src/xpconnect/src/XPCDispInlines.h @@ -108,8 +108,8 @@ VARTYPE XPCDispInterface::Member::ParamInfo::GetType() const inline XPCDispInterface::Member::Member() : - mType(UNINITIALIZED), mFuncDesc(0), mGetterFuncDesc(0), - mTypeInfo(0) + mType(UNINITIALIZED), mFuncDesc(nsnull), mGetterFuncDesc(nsnull), + mTypeInfo(nsnull) { } @@ -118,9 +118,12 @@ XPCDispInterface::Member::~Member() { if(mTypeInfo) { - if (mFuncDesc) + // Test to see if we have a separate getter. If we only have a getter they can + // be the same + PRBool releaseGetter = mGetterFuncDesc != nsnull && mFuncDesc != mGetterFuncDesc; + if(mFuncDesc) mTypeInfo->ReleaseFuncDesc(mFuncDesc); - if (mGetterFuncDesc) + if(releaseGetter) mTypeInfo->ReleaseFuncDesc(mGetterFuncDesc); } } @@ -198,7 +201,7 @@ inline PRBool XPCDispInterface::Member::IsParameterizedGetter() const { return IsGetter() && (GetParamCount(PR_TRUE) > 1 || - (GetParamCount(PR_TRUE) == 1 && GetParamInfo(0, PR_TRUE).IsRetVal())); + (GetParamCount(PR_TRUE) == 1 && !GetParamInfo(0, PR_TRUE).IsRetVal())); } inline @@ -259,6 +262,38 @@ PRUint16 XPCDispInterface::Member::GetParamType(PRUint32 index) const return mFuncDesc->lprgelemdescParam[index].paramdesc.wParamFlags; } +inline +void XPCDispInterface::Member::SetMemID(DISPID memID) +{ + mMemID = memID; +} + +inline +DISPID XPCDispInterface::Member::GetMemID() const +{ + return mMemID; +} + +//============================================================================= +// XPCDispInterface::Allocator + +inline +XPCDispInterface* XPCDispInterface::Allocator::Allocate() +{ + return Valid() ? new (Count()) XPCDispInterface(mCX, mTypeInfo, mIDispatchMembers) : nsnull; +} + +inline +XPCDispInterface::Allocator::~Allocator() +{ + delete [] mMemIDs; +} + +PRBool XPCDispInterface::Allocator::Valid() const +{ + return mMemIDs ? PR_TRUE : PR_FALSE; +} + //============================================================================= // XPCDispInterface inlines @@ -561,14 +596,6 @@ void XPCDispParams::SetNamedPropID() mDispParams.rgdispidNamedArgs = &mPropID; mDispParams.cNamedArgs = 1; } -/* -inline -const VARIANT & XPCDispParams::GetParamRef(PRUint32 index) const -{ - NS_ASSERTION(index < mDispParams.cArgs, "XPCDispParams::GetParam bounds error"); - return mDispParams.rgvarg[mDispParams.cArgs - index - 1]; -} -*/ inline VARIANT & XPCDispParams::GetParamRef(PRUint32 index) diff --git a/js/src/xpconnect/src/XPCDispInterface.cpp b/js/src/xpconnect/src/XPCDispInterface.cpp index 821b0d9e0054..1eece89b4496 100644 --- a/js/src/xpconnect/src/XPCDispInterface.cpp +++ b/js/src/xpconnect/src/XPCDispInterface.cpp @@ -54,32 +54,61 @@ PRBool IsReflectable(FUNCDESC * pFuncDesc) pFuncDesc->funckind == FUNC_DISPATCH; } -/** - * Counts the reflectable methods - */ -PRUint32 GetReflectableCount(ITypeInfo * pTypeInfo, PRUint32 members) +XPCDispInterface::Allocator::Allocator(JSContext * cx, ITypeInfo * pTypeInfo) : + mMemIDs(nsnull), mCount(0), mIDispatchMembers(0), mCX(cx), + mTypeInfo(pTypeInfo) { - DISPID lastDispID = 0; - PRUint32 result = 0; - for(UINT iMethod = 0; iMethod < members; iMethod++ ) + TYPEATTR * attr; + HRESULT hr = pTypeInfo->GetTypeAttr(&attr); + if(SUCCEEDED(hr)) + { + mIDispatchMembers = attr->cFuncs; + mMemIDs = new DISPID[mIDispatchMembers]; + pTypeInfo->ReleaseTypeAttr(attr); + // Bail if we couldn't create the buffer + if(!mMemIDs) + return; + } + for(UINT iMethod = 0; iMethod < mIDispatchMembers; iMethod++ ) { FUNCDESC* pFuncDesc; if(SUCCEEDED(pTypeInfo->GetFuncDesc(iMethod, &pFuncDesc))) { // Only add the function to our list if it is at least at nesting level // 2 (i.e. defined in an interface derived from IDispatch). - if(lastDispID != pFuncDesc->memid && IsReflectable(pFuncDesc)) - { - ++result; - lastDispID = pFuncDesc->memid; - } + if(IsReflectable(pFuncDesc)) + Add(pFuncDesc->memid); pTypeInfo->ReleaseFuncDesc(pFuncDesc); } } - return result; } -XPCDispInterface* XPCDispInterface::NewInstance(JSContext* cx, nsISupports * pIface) +void XPCDispInterface::Allocator::Add(DISPID memID) +{ + NS_ASSERTION(Valid(), "Add should never be called if out of memory"); + // Start from the end and work backwards, the last item is the most + // likely to match + PRUint32 index = mCount; + while(index > 0) + { + if(mMemIDs[--index] == memID) + return; + }; + NS_ASSERTION(Count() < mIDispatchMembers, "mCount should always be less " + "than the IDispatch member count " + "here"); + mMemIDs[mCount++] = memID; + return; +} + +inline +PRUint32 XPCDispInterface::Allocator::Count() const +{ + return mCount; +} + +XPCDispInterface* +XPCDispInterface::NewInstance(JSContext* cx, nsISupports * pIface) { CComQIPtr pDispatch(NS_REINTERPRET_CAST(IUnknown*,pIface)); @@ -93,15 +122,8 @@ XPCDispInterface* XPCDispInterface::NewInstance(JSContext* cx, nsISupports * pIf hr = pDispatch->GetTypeInfo(0,LOCALE_SYSTEM_DEFAULT, &pTypeInfo); if(SUCCEEDED(hr)) { - TYPEATTR * attr; - hr = pTypeInfo->GetTypeAttr(&attr); - if(SUCCEEDED(hr)) - { - UINT funcs = attr->cFuncs; - pTypeInfo->ReleaseTypeAttr(attr); - PRUint32 memberCount = GetReflectableCount(pTypeInfo, funcs); - return new (memberCount) XPCDispInterface(cx, pTypeInfo, funcs); - } + Allocator allocator(cx, pTypeInfo); + return allocator.Allocate(); } } } @@ -140,10 +162,60 @@ void ConvertInvokeKind(INVOKEKIND invokeKind, XPCDispInterface::Member & member) } } +static +PRBool InitializeMember(JSContext * cx, ITypeInfo * pTypeInfo, + FUNCDESC * pFuncDesc, + XPCDispInterface::Member * pInfo) +{ + pInfo->SetMemID(pFuncDesc->memid); + BSTR name; + UINT nameCount; + if(FAILED(pTypeInfo->GetNames( + pFuncDesc->memid, + &name, + 1, + &nameCount))) + return PR_FALSE; + if(nameCount != 1) + return PR_FALSE; + JSString* str = JS_InternUCStringN(cx, name, ::SysStringLen(name)); + ::SysFreeString(name); + if(!str) + return PR_FALSE; + // Initialize + pInfo = new (pInfo) XPCDispInterface::Member; + if(!pInfo) + return PR_FALSE; + pInfo->SetName(STRING_TO_JSVAL(str)); + pInfo->ResetType(); + ConvertInvokeKind(pFuncDesc->invkind, *pInfo); + pInfo->SetTypeInfo(pFuncDesc->memid, pTypeInfo, pFuncDesc); + return PR_TRUE; +} + +static +XPCDispInterface::Member * FindExistingMember(XPCDispInterface::Member * first, + XPCDispInterface::Member * last, + MEMBERID memberID) +{ + // Iterate backward since the last one in is the most likely match + XPCDispInterface::Member * cur = last; + if (cur != first) + { + do + { + --cur; + if(cur->GetMemID() == memberID) + return cur; + } while(cur != first); + } + // no existing property, return the new one + return last; +} + PRBool XPCDispInterface::InspectIDispatch(JSContext * cx, ITypeInfo * pTypeInfo, PRUint32 members) { HRESULT hResult; - DISPID lastDispID = 0; XPCDispInterface::Member * pInfo = mMembers; mMemberCount = 0; @@ -151,55 +223,53 @@ PRBool XPCDispInterface::InspectIDispatch(JSContext * cx, ITypeInfo * pTypeInfo, { FUNCDESC* pFuncDesc; hResult = pTypeInfo->GetFuncDesc(index, &pFuncDesc ); - if(SUCCEEDED(hResult)) + if(FAILED(hResult)) + continue; + if(IsReflectable(pFuncDesc)) { - PRBool release = PR_TRUE; - if(IsReflectable(pFuncDesc)) + switch(pFuncDesc->invkind) { - // Check and see if the previous dispid was the same - if(lastDispID != pFuncDesc->memid) + case INVOKE_PROPERTYPUT: + case INVOKE_PROPERTYPUTREF: + case INVOKE_PROPERTYGET: { - BSTR name; - UINT nameCount; - if(SUCCEEDED(pTypeInfo->GetNames( - pFuncDesc->memid, - &name, - 1, - &nameCount))) + XPCDispInterface::Member * pExisting = FindExistingMember(mMembers, pInfo, pFuncDesc->memid); + if(pExisting == pInfo) + { + if(InitializeMember(cx, pTypeInfo, pFuncDesc, pInfo)) + { + ++pInfo; + ++mMemberCount; + } + } + else + { + ConvertInvokeKind(pFuncDesc->invkind, *pExisting); + } + if(pFuncDesc->invkind == INVOKE_PROPERTYGET) + { + pExisting->SetGetterFuncDesc(pFuncDesc); + } + } + break; + case INVOKE_FUNC: + { + if(InitializeMember(cx, pTypeInfo, pFuncDesc, pInfo)) { - JSString* str = JS_InternUCStringN(cx, name, ::SysStringLen(name)); - ::SysFreeString(name); - if(!str) - return PR_FALSE; - // Initialize - pInfo = new (pInfo) Member; - if(!pInfo) - return PR_FALSE; - pInfo->SetName(STRING_TO_JSVAL(str)); - lastDispID = pFuncDesc->memid; - pInfo->ResetType(); - ConvertInvokeKind(pFuncDesc->invkind, *pInfo); - pInfo->SetTypeInfo(pFuncDesc->memid, pTypeInfo, pFuncDesc); - release = PR_FALSE; ++pInfo; ++mMemberCount; } } - // if it was then we're on the second part of the - // property - else - { - XPCDispInterface::Member * lastInfo = pInfo - 1; - ConvertInvokeKind(pFuncDesc->invkind, *lastInfo); - lastInfo->SetGetterFuncDesc(pFuncDesc); - release = PR_FALSE; - } - } - if(release) - { - pTypeInfo->ReleaseFuncDesc(pFuncDesc); + break; + default: + pTypeInfo->ReleaseFuncDesc(pFuncDesc); + break; } } + else + { + pTypeInfo->ReleaseFuncDesc(pFuncDesc); + } } return PR_TRUE; } diff --git a/js/src/xpconnect/src/XPCDispParams.cpp b/js/src/xpconnect/src/XPCDispParams.cpp index 32e8aafbdddc..4767edb20a13 100644 --- a/js/src/xpconnect/src/XPCDispParams.cpp +++ b/js/src/xpconnect/src/XPCDispParams.cpp @@ -60,7 +60,9 @@ XPCDispParams::XPCDispParams(PRUint32 args) : memset(mRefBuffer - sizeof(VARIANT), 0, RefBufferSize(args)); // Initialize the IDispatch parameters mDispParams.cArgs = args; - if(args <= DEFAULT_ARG_ARRAY_SIZE) + if(args == 0) + mDispParams.rgvarg = nsnull; + else if (args <= DEFAULT_ARG_ARRAY_SIZE) mDispParams.rgvarg = mStackArgs + 1; else { diff --git a/js/src/xpconnect/src/XPCDispPrivate.h b/js/src/xpconnect/src/XPCDispPrivate.h index d0c6286f8b26..d15e1ab45e88 100644 --- a/js/src/xpconnect/src/XPCDispPrivate.h +++ b/js/src/xpconnect/src/XPCDispPrivate.h @@ -860,8 +860,19 @@ public: * @param funcdesc function description */ void SetGetterFuncDesc(FUNCDESC* funcdesc); + /** + * Sets the member ID + * @param memID the IDispatch ID of the member + */ + void SetMemID(DISPID memID); + /** + * Returns the IDispatch ID of the member + * @return the IDispatch ID of the member + */ + DISPID GetMemID() const; private: + DISPID mMemID; /** * Our internal flags identify the type of member * A member can be both getter/setter @@ -878,9 +889,8 @@ public: jsval mVal; // Mutable jsval mName; // Mutable CComPtr mTypeInfo; - FUNCDESC* mFuncDesc; // We own this, and must release it - FUNCDESC* mGetterFuncDesc; // We own this, and must release it - // This may be the same as mFuncDesc + FUNCDESC* mFuncDesc; // We own this + FUNCDESC* mGetterFuncDesc; // We own this /** * Helper function to return the parameter type * @param index index of the parameter to return the type of @@ -982,6 +992,67 @@ private: */ PRBool InspectIDispatch(JSContext * cx, ITypeInfo * pTypeInfo, PRUint32 members); + + /** + * Small utility to count members needed for XPConnect + * XPConnect has one entry for a property while IDispatch can have two + * Generally interfaces are small enough, that linear searching should + * be ok + */ + class Allocator + { + public: + /** + * Constructor, creates the initial buffer + * @param cx a JS context + * @param pTypeInfo pointer to IDispatch type info, our caller holds + * the reference we don't need to + */ + Allocator(JSContext * cx, ITypeInfo * pTypeInfo); + /** + * Destructor, frees the buffer we allocated + */ + inline + ~Allocator(); + /** + * Returns the allocated XPCDispInterface object + * @return the allocated XPCDispInterface object + */ + inline + XPCDispInterface* Allocate(); + private: + DISPID * mMemIDs; + PRUint32 mCount; // Total unique ID's found + PRUint32 mIDispatchMembers; // Total entries reported by ITypeInfo + JSContext* mCX; + ITypeInfo* mTypeInfo; + + /** + * Returns the number of members found + * @return The number of members found + */ + inline + PRUint32 Count() const; + /** + * Adds the member ID to the list + * @param memID The member ID to test + */ + void Add(DISPID memID); + /** + * Allows our caller to handle unexpected problems like out of memory + * @return PR_TRUE if the buffer was allocated + */ + inline + PRBool Valid() const; + + // No copying or assigning allowed + Allocator(const Allocator&); + Allocator& operator =(const Allocator&); + }; + /** + * Friendship need to gain access to private operator new + */ + friend class Allocator; }; /** @@ -1130,12 +1201,6 @@ public: * @param index index of the parameter * @return a reference to the parameter at index */ -// const VARIANT & GetParamRef(PRUint32 index) const; - /** - * Returns a reference to a parameter - * @param index index of the parameter - * @return a reference to the parameter at index - */ VARIANT & GetParamRef(PRUint32 index); /** * Returns the parameter by value diff --git a/js/src/xpconnect/src/XPCDispTearOff.cpp b/js/src/xpconnect/src/XPCDispTearOff.cpp index 04590ad4c3b4..3c860d790f03 100644 --- a/js/src/xpconnect/src/XPCDispTearOff.cpp +++ b/js/src/xpconnect/src/XPCDispTearOff.cpp @@ -310,14 +310,12 @@ STDMETHODIMP XPCDispatchTearOff::Invoke(DISPID dispIdMember, REFIID riid, uint8 paramCount=0; nsresult retval = NS_ERROR_FAILURE; nsresult pending_result = NS_OK; - JSErrorReporter older; JSBool success; JSBool readyToDoTheCall = JS_FALSE; uint8 outConversionFailedIndex; JSObject* obj; jsval fval; nsCOMPtr xpc_exception; - jsval js_exception; void* mark; JSBool foundDependentParam; JSObject* thisObj;