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

[Gabriel Ivăncescu]

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.

> 
>>>
>>> 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.
>>>
>>
>> Well, they're opaque, so not exposed at all. It's pretty common to 
>> pass "context" or "user" pointers around in callbacks (which is what 
>> this is). Do you think renaming it to "context" is better then? 
>> jscript won't really care what it is, just has to pass it back to the 
>> callback. 
> 
> 
> I think that it needs to fit the rest of the design, so I would need to 
> see more of it to answer properly. It's not really something I expected. 
> I imagined that the interface exposed by MSHTML object could look like this:
> 
> 
> interface IJSDispatchHost
> 
> {
> 
>      HRESULT getJSDispatch(IJSDispatch **p);
> 
>      HRESULT setJSDispatch(IJSDispatch *jsdisp); // or maybe expose a 
> constructor from the script engine itself and call it from MSHTML instead
> 
>      HRESULT getOwnProperty( ... );
> 
>      HRESULT call(DISPID dispid, unsigned int argc, VARIANT *argv, 
> VARIANT *r);
> 
>      // probably more
> 
> }
> 
> 
> On top of that, you'd need a constructor for such wrapped objects, so 
> another interface exposed by script engine itself will likely be needed. 
> I don't see why you need anything else.
> 

Hmm. I have it quite different in that I create it on demand. Mshtml 
objects expose a specific interface that jscript queries, then jscript 
uses it for retrieving prop flags, invoking props, prop information 
(i.e. the callbacks mentioned), etc.

Basically, I use a similar model to how the PROP_BUILTIN props are 
implement in jscript right now, except that instead of using jscript 
builtin vtables, I use the interface exposed by mshtml, and mshtml does 
the rest of the job.

Sometimes mshtml calls back into jscript for specific things (mostly 
notifications), using a different interface but one that is always 
present in jscript (so mshtml knows once it's set up).

I got to the point where everything relevant, I believe, works like 
native. All constructors are the same, prototypes are similar (there's 
some undocumented stuff that I didn't bother with), prototype chains are 
the same (e.g. HTMLBodyElement->HTMLElement->Element->Node->Object) with 
same props exposed and all that in each prototype. I mentioned it 
because at least I know it works and isn't some dead end.

Anyway this is for later. :-)