msi: Add full JScript/VBScript support and partial expandable OLE automation support. [PATCH 3/3]

[Robert Shearman]

Misha Koshelev wrote:
> +/* Macros to get pointer to AutomationObject) from the other VTables. */
>   

Typo with extra ")" here.

> +HRESULT WINAPI SessionImpl_Invoke(
> +        AutomationObject* This,
> +        DISPID dispIdMember,
> +        REFIID riid,
> +        LCID lcid,
> +        WORD wFlags,
> +        DISPPARAMS* pDispParams,
> +        VARIANT* pVarResult,
> +        EXCEPINFO* pExcepInfo,
> +        UINT* puArgErr)
> +{
> +    WCHAR szString[MAX_MSI_STRING];
> +    DWORD dwLen = MAX_MSI_STRING;
> +    IDispatch *iDispatch = NULL;
> +    MSIHANDLE msiHandle;
> +    LANGID langId;
> +    UINT ret;
> +    INSTALLSTATE iInstalled, iAction;
>   

Some error checking here would be good, like you do in 
AutomationObject_Invoke.

> +
> +    switch (dispIdMember)
> +    {
>   
...
> +/*
> + * AutomationObject - "base" class for all automation objects so we don't have to repeat functions. Just
> + *                    need to implement Invoke function for each dispinterface and pass the new function
> + *                    to create_automation_object.
> + */
> +
> +typedef struct {
> +    /*
> +     * VTables - We provide IDispatch, IProvideClassInfo, IProvideClassInfo2, IProvideMultipleClassInfo
> +     */
> +    const IDispatchVtbl *lpVtbl;
> +    const IProvideClassInfoVtbl *lpvtblIProvideClassInfo;
> +    const IProvideClassInfo2Vtbl *lpvtblIProvideClassInfo2;
> +    const IProvideMultipleClassInfoVtbl *lpvtblIProvideMultipleClassInfo;
> +    
> +    /* Object reference count */
> +    LONG ref;
> +
> +    /* Clsid for this class and it's appropriate ITypeInfo object */
> +    LPCLSID clsid;
> +    ITypeInfo *iTypeInfo;
> +
> +    /* The MSI handle of the current object */
> +    MSIHANDLE msiHandle;;
> +
> +    /* A function that is called from IDispatch::Invoke, specific to this type of object */
> +    LPVOID funcInvoke;
> +} AutomationObject;
>   

You shouldn't need to expose the implementation details of 
AutomationObject outside of automation.c.

> +
> +/* This is the function that one needs to call to create an automation object. */
> +extern HRESULT create_automation_object(MSIHANDLE msiHandle, IUnknown *pUnkOuter, LPVOID *ppObj, REFIID clsid, LPVOID funcInvoke);
>   

It is dangerous to use LPVOID as the type to pass a function in.

> +
> +/* We need to expose these functions because our IActiveScriptSite calls it */
> +extern HRESULT WINAPI LoadTypeInfo(IDispatch *iface, ITypeInfo **pptinfo, REFIID clsid, LCID lcid);
> +extern HRESULT WINAPI SessionImpl_Invoke(AutomationObject*,DISPID,REFIID,LCID,WORD,DISPPARAMS*,VARIANT*,EXCEPINFO*,UINT*);
> +
> +/* Disp Ids 
> + * (not complete, look in msiserver.idl to add more) */
> +typedef enum {
> +    RecordDispId_StringData=1,
> +    RecordDispId_IntegerData,
> +    RecordDispId_SetStream,
> +    RecordDispId_ReadStream,
> +    RecordDispId_FieldCount,
> +    RecordDispId_IsNull,
> +    RecordDispId_DataSize,
> +    RecordDispId_ClearData,
> +    RecordDispId_FormatText
> +} RecordDispId;
> +
> +typedef enum {
> +    ViewDispId_Execute=1,
> +    ViewDispId_Fetch,
> +    ViewDispId_Modify,
> +    ViewDispId_Close,
> +    ViewDispId_ColumnInfo,
> +    ViewDispId_GetError
> +} ViewDispId;
> +
> +typedef enum {
> +    DatabaseDispId_DatabaseState=1,
> +    DatabaseDispId_SummaryInformation,
> +    DatabaseDispId_OpenView,
> +    DatabaseDispId_Commit,
> +    DatabaseDispId_PrimaryKeys,
> +    DatabaseDispId_Import,
> +    DatabaseDispId_Export,
> +    DatabaseDispId_Merge,
> +    DatabaseDispId_GenerateTransform,
> +    DatabaseDispId_ApplyTransform,
> +    DatabaseDispId_EnableUIPreview,
> +    DatabaseDispId_TablePersistent,
> +    DatabaseDispId_CreateTransformSummaryInfo
> +} DatabaseDispId;
> +
> +typedef enum {
> +    SessionDispId_Installer=1,
> +    SessionDispId_Property,
> +    SessionDispId_Language,
> +    SessionDispId_Mode,
> +    SessionDispId_Database,
> +    SessionDispId_SourcePath,
> +    SessionDispId_TargetPath,
> +    SessionDispId_DoAction,
> +    SessionDispId_Sequence,
> +    SessionDispId_EvaluateCondition,
> +    SessionDispId_FormatRecord,
> +    SessionDispId_Message,
> +    SessionDispId_FeatureCurrentState,
> +    SessionDispId_FeatureRequestState,
> +    SessionDispId_FeatureValidStates,
> +    SessionDispId_FeatureCost,
> +    SessionDispId_ComponentCurrentState,
> +    SessionDispId_ComponentRequestState,
> +    SessionDispId_SetInstallLevel,
> +    SessionDispId_VerifyDiskSpace,
> +    SessionDispId_ProductProperty,
> +    SessionDispId_FeatureInfo,
> +    SessionDispId_ComponentCost
> +} SessionDispId;
>   

See dlls/oleaut32/tests/tmarshal_dispids.h and 
dlls/oleaut32/tests/tmarshal.idl for how to use DISPIDs properly.

> +typedef struct {
> +    IActiveScriptSite lpVtbl;
> +    AutomationObject *session;
>   

You don't actually access this as anything other than an IDispatch 
object, so you might as well change to "IDispatch *session".

> +LPCWSTR read_script_from_file(LPCWSTR szFile, INT type)
>   

Should be static.

> +/* JScript or VBScript? */
> +LPCWSTR progid_from_type(INT type)
>   

Should also be static.

> +
> +/*
> + * Call a script. This is our meat and potatoes. 
> + *     - Currently, since the function is relatively new, it will always end up returning S_OK.
> + *       Think of it like a bonus feature, we can run the script - great. If we have a problem,
> + *       we are no worse off than if this function had not been called.
> + */
> +DWORD call_script(MSIHANDLE hPackage, INT type, LPCWSTR filename, LPCWSTR function, LPCWSTR action)
> +{
> +    LPCWSTR script = NULL, progId = NULL;
> +    HRESULT hr;
> +    IActiveScript *iActiveScript = NULL;
> +    IActiveScriptParse *iActiveScriptParse = NULL;
>   

I haven't seen this variable notation style before. In Hungarian 
notation "p" is used instead of "i" to show a pointer to an object 
exposing the interface declared.

-- 
Rob Shearman