[PATCH v2 1/8] jscript: Implement proxy objects along with a private interface.

[Jacek Caban]

Hi Gabriel,

The main idea behind this patch looks good, but I think some more 
consideration would be nice to make it fit better the bigger picture.

Right now, we use IDispatch[Ex] all over jscript (that's how it's stored 
in jsval_t, for example) and we often needs three code paths: for 
IDispatch, IDispatchEx and jsdisp_t. With your patches, that we'd have 
four code paths, even if it's often hidden behind helpers. While we 
ultimately need separated code paths, I feel like we could do it 
architecturally better and it would be good to consider that. One way to 
achieve that is to abstract JS objects enough so that we'd operate on a 
single type everywhere outside the code dealing with the difference 
(dispex.c, more or less).

Note that pure external IDispatch and IDispatchEx could be represented 
by jsdisp_t restricted to have only proxy properties. IDispatch -> 
jsdisp_t mapping would be a bit tricky in this case, but some sort of rb 
tree should do. If we had that, we could replace all IDispatch usage 
with jsdisp_t, including jsval_t. jsdisp_t (or a new jsobj_t type) could 
then internally sort of differences between those, maybe with an 
internal vtbl.


Another thing that we will need to support is proper prototype chain 
support for MSHTML objects. Eg. those functions should not really be 
properties of objects, but rather their prototypes. I think that it will 
be possible to express by extending something like this extension, but 
that's something to keep in mind. I'm not expecting you to implement all 
of that, I'm not even sure about details of it without trying it, but 
let's make sure we won't make it harder in the future.


On 10/13/21 4:11 PM, Gabriel Ivăncescu wrote:
> Native mshtml objects act like JS objects in IE9 modes and up (the JS engine
> is implemented in mshtml). This implements proxy JS objects for certain
> dispatch objects that export a new private (internal to Wine) interface in
> versions ES5 and up, to achieve the same behavior. It delegates as much as
> possible to mshtml to implement all quirks there. Props are queried by the
> underlying dispatch and accesses to them forwarded to the dispatch object.
>
> Since it's purely internal, the private interface is not part of any typelib,
> and for simplicity, it extends (inherits from) IDispatchEx, so we can just
> cast it as needed.
>
> Given certain conditions, we query IDispatch objects on demand for the
> interface. If it's available, we either create a JS proxy object that is
> associated with the dispatch object exposing the interface, or re-acquire
> another one (possibly dangling). It is also associated on the other end, so
> that we keep a 1:1 mapping between the JS proxy object and the actual dispatch
> object, to avoid having multiple JS proxy objects for the same mshtml object
> (which would result in different dispids on them, and subtle bugs).
>
> This keeps them both in sync at all times, on demand, without having to
> pre-create any objects, and deals with dynamic props and such automatically.
>
> Signed-off-by: Gabriel Ivăncescu <gabrielopcode at gmail.com>
> ---
>
> There's only one caveat here: since the JS proxy object references the
> dispatch object, while the dispatch object is also associated to the JS
> proxy object, the JS proxy object must be kept alive even when its refcount
> is zero. We can't hold a reference from the dispatch object, because that
> would create a circular reference.
>
> In effect, we *need* to know *when* the JS proxy object has no references
> from jscript part, to release the dispatch object. But the dispatch object
> itself might still remain alive (if kept alive by mshtml refs), and the
> JS proxy object must also remain associated in this case, hence we keep it
> alive even when refcount == 0, but remove the proxy association on its side.
>
> This is slightly ugly, but it is the best approach I could think of, to
> avoid bugs or having to rely on more complicated hash table lookups.


While I'd hope for a bit cleaner solution, there is one more problem 
with moving all properties to jscript: cycle collector will no longer be 
able to deal with them. See dispex_traverse. Without cycle collector 
knowing about object properties, we will leak on cycles (we already 
don't traverse through builtin jscript objects, so there are leaks 
already, but this will make it worse).

> +typedef struct {
> +    FunctionInstance function;
> +    IDispatchEx *proxy;
> +    DISPID id;
> +} ProxyFunction;


Binding this to one exact object doesn't seem right. For example 
document.body.click.call(div_elem) should probably work fine and call on 
div_elem. Maybe it's better to store such functions as a pair of 
(IID,DISPID)?


Thanks,

Jacek