[PATCH 4/9] mshtml: Use builtin hooks even when calling function dispatch objects.

[Jacek Caban]

On 12/6/21 4:40 PM, Gabriel Ivăncescu wrote:
> On 06/12/2021 16:48, Jacek Caban wrote:
>> On 12/5/21 5:54 PM, Gabriel Ivăncescu wrote:
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> This seems to be a good place to call the hook, but could you 
>>>>>>>> just keep typeinfo_invoke call here and don't expose 
>>>>>>>> invoke_builtin_function?
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>> But I need to expose it for hooks so they can forward to it. 
>>>>>>> It's either that or I expose typeinfo_invoke. Keep in mind that, 
>>>>>>> for hooks, the Dispatch object itself may not exist at all, we 
>>>>>>> can't use it to look up the function, that's incorrect.
>>>>>>>
>>>>>>> That's not the case right now, no, but it will be when I'll 
>>>>>>> either implement function.apply/call (for all modes) or the 
>>>>>>> proxy jscript implementation, in further patches.
>>>>>>>
>>>>>>> Here's a future example:
>>>>>>>
>>>>>>> f = elem.setAttribute;
>>>>>>> f.call(some_other_obj, "a", blah);
>>>>>>>
>>>>>>> The hook needs to run on some_other_obj and stringify blah 
>>>>>>> appropriately before calling the builtin function on 
>>>>>>> some_other_obj. In fact, we won't have a DispatchEx at all, just 
>>>>>>> a generic IDispatch with "some_other_obj". 
>>>>>>
>>>>>>
>>>>>> I hoped that the result of your 'proxy' patches will be that 
>>>>>> those functions will be true JavaScript objects. It means that 
>>>>>> MSHTML's function objects will not be relevant in ES5 mode. Why 
>>>>>> do you still need them?
>>>>>>  >
>>>>>
>>>>> Yes, that's true, but I need a way to encapsulate a builtin 
>>>>> function (including its hook) into a jscript function. Currently, 
>>>>> I create a ProxyFunction that holds the func_info opaque pointer 
>>>>> and an internal interface pointer that it uses to call into mshtml.
>>>>>
>>>>> mshtml then uses this entry point to call the hook first, passing 
>>>>> it the func_info and the 'this' dispatch object, and if no hook 
>>>>> reverts back to invoke_builtin_function. Of course the hook also 
>>>>> has to use invoke_builtin_function at some point, after massaging 
>>>>> the args.
>>>>>
>>>>> Either way, the point is that the hook *has* to be called on an 
>>>>> arbitrary object, not the original dispatch it's from, so that's 
>>>>> why I'm passing func_info_t to it, which is needed to be forwarded 
>>>>> into invoke_builtin_function (or another wrapper).
>>>>
>>>>
>>>> I don't think we need that internal interface pointer.  Did you 
>>>> consider something similar to storing a builtin function as 
>>>> (IID,DISPID) like I suggested earlier? This would not need any 
>>>> object representing functions on MSHTML side.
>>>>
>>>
>>> Yes, I had it that way, but I had to change it to handle hooks. I 
>>> don't actually use any object though, so it's not a problem. I just 
>>> give the func_info_t as an opaque pointer to jscript, and a function 
>>> pointer so that it knows what to call (in ProxyFunction_call) and 
>>> pass it there.
>>>
>>> The function pointer points to a function in mshtml that simply does 
>>> the necessary things (typically, just invokes 
>>> invoke_builtin_function, or for accessors, builtin_propget/put and 
>>> the hooks). No actual object is used, but I still need the hooks 
>>> patch of course. 
>>
>>
>> I still don't see why you need that. IID is enough to make sure that 
>> we're trying to call the function on the right object type. Once we 
>> know that the object type is right, DISPID already uniquely 
>> identifies func_info_t, which MSHTML may get as needed when the 
>> function is called.
>>
>>
>
> Well it's simpler to pass an opaque pointer that includes all the 
> information rather than store two separate pieces and then have to 
> look it up. The lookup would have to be done on mshtml side anyway 
> (since it has func_info_t and hooks), so there's no advantage.


As long as you need a function object in mshtml, even if you don't call 
it an actual object, I don't see how it's simpler.


Also note that (at least currently, prototypes support may change it, 
but likely not) func_info_t, separated object types will use separated 
function infos. For example div element's function info for setAttribute 
is not the same as body element's. It happens to work if you use the 
other one, but that's now how it's meant to be used.


Jacek