mshtml #2: Added HTMLWindow's IDispatch methods implementation.

Jacek Caban jacek at codeweavers.com
Wed Aug 1 13:01:07 CDT 2007


Hi Robert,

Thanks for your review.

Robert Shearman wrote:
> Jacek Caban wrote:
>> static HRESULT WINAPI HTMLWindow2_item(IHTMLWindow2 *iface, VARIANT 
>> *pvarIndex, VARIANT *pvarResult)
>> diff --git a/dlls/mshtml/main.c b/dlls/mshtml/main.c
>> index 3f8131c..41f32eb 100644
>> --- a/dlls/mshtml/main.c
>> +++ b/dlls/mshtml/main.c
>> @@ -51,10 +51,52 @@ DWORD mshtml_tls = 0;
>>  
>>  static HINSTANCE shdoclc = NULL;
>>  
>> +static ITypeLib *typelib;
>> +static ITypeInfo *typeinfos[LAST_tid];
>> +
>> +static const REFIID tid_ids[] = {
>> +    &IID_IHTMLWindow2
>> +};
>> +
>> +HRESULT get_typeinfo(enum tid_t tid, ITypeInfo **typeinfo)
>> +{
>> +    HRESULT hres;
>> +
>> +    if(!typelib) {
>> +        hres = LoadRegTypeLib(&LIBID_MSHTML, 4, 0, 
>> LOCALE_SYSTEM_DEFAULT, &typelib);
>>   
>
> This isn't thread-safe.

I don't consider it really important here ATM. I haven't seen any app 
that would benefit from thread safe MSHTML. Anyway I will fix it.

>> +        if(FAILED(hres)) {
>> +            ERR("LoadRegTypeLib failed: %08x\n", hres);
>> +            return hres;
>> +        }
>> +    }
>> +
>> +    if(!typeinfos[tid]) {
>> +        hres = ITypeLib_GetTypeInfoOfGuid(typelib, tid_ids[tid], 
>> typeinfos+tid);
>> +        if(FAILED(hres)) {
>> +            ERR("GetTypeInfoOfGuid failed: %08x\n", hres);
>> +            return hres;
>> +        }
>> +    }
>>   
>
> Why not just use the IID instead of an enumeration here? I know you're 
> trying to cache the typeinfo here, but the current implementation of 
> typelibs makes ITypeLib_GetTypeInfoOfGuid very cheap.

It's surely not cheaper than a simple table element check. It's a part 
of code that is extremely profiled in Gecko (that is its counterpart in 
Gecko). We don't have to take that much care of it, but I think such 
simple caching is wort the efforts.

>> +
>> +    *typeinfo = typeinfos[tid];
>> +    return S_OK;
>> +}
>> +
>>  static void thread_detach(void)
>>  {
>> -    thread_data_t *thread_data = get_thread_data(FALSE);
>> +    thread_data_t *thread_data;
>> +
>> +    if(typelib) {
>> +        unsigned i;
>> +
>> +        for(i=0; i < sizeof(typeinfos)/sizeof(*typeinfos); i++)
>> +            if(typeinfos[i])
>> +                ITypeInfo_Release(typeinfos[i]);
>> +
>> +        ITypeLib_Release(typelib);
>> +    }
>>   
>
> Why are you doing this from DLL_THREAD_DETACH and not DLL_PROCESS_DETACH?

Ops, that's my fatal mistake.


Thanks,
    Jacek




More information about the wine-devel mailing list