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@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