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

[Jacek Caban]

On 12/7/21 4:32 PM, Gabriel Ivăncescu wrote:
> On 07/12/2021 14:28, Jacek Caban wrote:
>> 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.
>>
>
> I don't really use an object at all. Here's a rough sketch of how it is:
>
> On mshtml side:
>
> static HRESULT proxy_func_invoke(IDispatch *this_obj, void *func_info, 
> DISPPARAMS *dp, VARIANT *res, EXCEPINFO *ei, IServiceProvider *caller)
> {
>     func_info_t *func = func_info;


So why not get func_info_t from DISPID here? Of course, you also need to 
validate that we're called on the right type somewhere as well.


> return invoke_builtin_function(this_obj, func, DISPATCH_METHOD, dp, 
> res, ei, caller);
> }
>
> then in the proxy interface, we have a method that returns...
>
> 1) Address of proxy_func_invoke
> 2) The func_info_t
>
> ...back to jscript. Jscript uses it when creating a ProxyFunction and 
> stores it inside, then during ProxyFunction_call:
>
> ProxyFunction_call(...):
> {
>     ...
>     function->func.invoke(this_disp, function->func.info, ...);
>     ...
> }
>
> Where 'invoke' is the function pointer from mshtml (that points to 
> proxy_func_invoke) and 'info' is the func_into_t from mshtml. jscript 
> doesn't have to know anything about them.


That sounds like a very ad-hoc way to expose it. MSHTML objects will 
need to expose some new interface anyway, right? Why not make it a part 
of that interface?


>
> The catch is that, of course, I need the *same* underlying mechanism 
> as invoking a function object without proxies, so that's why I'm 
> sending it now. It doesn't mean I actually use the function object, 
> though, but since the function object needs the same mechanism more or 
> less (and invoke_builtin_function fixed to call hooks and do 
> everything on arbitrary "this" dispatches), it's an opportunity to fix 
> it as well.
>
> Of course, I also have to do this for accessors (which can be 
> retrieved with getOwnPropertyDescriptor), but it's the same idea, 
> nothing special.
>
>>
>> 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.
>>
>
> Isn't that technically correct? Imagine, for a hypothetical example, 
> that body element's setAttribute has a quirk on native, where div 
> element's does not; i.e. it does not have the stringifying hook, where 
> all other elements do. In this case, doing something like:
>
> document.body.setAttribute.call(divElem, "attr", obj);
>
> Needs to *not* stringify the obj value, because we are using body's 
> setAttribute (which has the quirk) rather than divElem's, so in fact 
> using func_info_t is more correct than IID/DIPSID pair, which would 
> look it up on divElem and use the wrong setAttribute with the wrong hook.
>
> This is of course, not a problem in practice, also because 
> setAttribute is part of the prototype, so it's the same thing that's 
> called.
>
> That said there's some special cases (for example, style's prototype 
> having a separate setAttribute, not using the element prototype 
> chain), so I believe should we encounter such quirks in the future, 
> it's still more correct anyway.


That just doesn't make sense at all. Once we support prototypes 
document.body.setAttribute === 
document.createElement("div").setAttribute === 
document.head.__proto__.setAttribute === Element.setAttribute. It's one 
and the same function object, not separated quirk holders. Hooks are an 
implementation details of such abstract functions in context of a 
specific object type and there is nothing special about that. They are 
not a part of function object itself.


That all said, I'm not attached to representing those as DISPIDs. It 
just feels safe to continue using them here, but it's possible that 
something separated will be better at some point. Your current solution 
just exposes some MSHTML internals for no real need.


Jacek