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

[Jacek Caban]

On 12/7/21 9:29 PM, Gabriel Ivăncescu wrote:
> On 07/12/2021 21:18, Jacek Caban wrote:
>> On 12/7/21 7:52 PM, Gabriel Ivăncescu wrote:
>>>> 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.
>>>>
>>>
>>> I did mention the prototype means it is the same function object, I 
>>> was using a hypothetical example with quirks. My point was that *if* 
>>> they end up being different functions (for any reason), using 
>>> func_info_t is more correct because it represents two different 
>>> functions, with possibly two different hooks.
>>>
>>> Don't get me wrong, I did not find yet a case where such a quirk 
>>> exists, that's why I mentioned it's hypothetical. Although I did 
>>> find some duplicated props in some places. Anyway I think it's more 
>>> technically correct, and can be easier handled in the future *if* we 
>>> find such quirks.
>>
>>
>> No, it's not correct. body element hooks from your example are free 
>> to cast 'this' object to HTMLBodyElement. If it did that, you'd end 
>> up casting div element object to HTMLBodyElement and likely crash.
>>
>
> Oh I see what you mean now, but that's not an issue because the hooks 
> behavior needs to change. I have patches for that of course, though I 
> don't know if they'll make it before code freeze.
>
> In short, the hooks will not take any DispatchEx* argument at all (in 
> fact, you can't really know *what* object they are called on), but a 
> generic IDispatch *this_obj arg.
>
> This has nothing to do with the things mentioned above: it is much 
> more general. Think of the following with an actual, real hook:
>
> elem.setAttribute.call(arbitrary_dispatch_object, "A", "B");
>
> As you can see we cannot guarantee that the hook itself is called on 
> the respective element anymore, so casting it is already wrong. The 
> correct thing would be to QueryInterface, or detect it some other ways 
> (but that's not needed with the hooks we have so far).
>
> But the point is: hooks need to operate on arbitrary this_obj 
> IDispatch, and it's a general issue, not related to the proxy interface. 



By the time the code reaches hooks, we should already have the type 
validated. I don't see why you need to change that.


Jacek