On Thu Apr 11 15:05:09 2024 +0000, Gabriel Ivăncescu wrote:
Well yes, in a way it does require interop, but that's not the point I'm trying to make. As in, this specific part of the code—dealing with "async" props—has to be done regardless. Imagine that localStorage was a jscript object, no mshtml at all. So, we write to the local file, add some keys and values. Now *every* localStorage object needs to have that key as a prop. So, my point was only about hooks in this case for stuff like prop_get, delete_prop, definining new props (to see if an old one now exists from the file) and looking up props (by name or DISPID). Hooks that we would need *even if the localStorage object was in jscript*. That's because we have to look at them regardless of whether we have a jscript prop or not. Now for the other topic, for mshtml interop, so far I have it like this, and btw it works so it's not a dead end:
interface IWineJSDispatchHost : IDispatchEx { IWineJSDispatch** GetProxyFieldRef(); HRESULT PropFixOverride([out] struct proxy_prop_info *info); HRESULT PropOverride([in] const WCHAR *name, [out, optional] VARIANT *value); HRESULT PropDefineOverride([out] struct proxy_prop_info *info); HRESULT PropGetInfo([in] const WCHAR *name, [in] BOOL case_insens, [retval, out] struct proxy_prop_info *info); HRESULT PropInvoke([in] IDispatch *this_obj, [in] DISPID id, [in] LCID lcid, [in] DWORD flags, [in] DISPPARAMS *dp, [out, optional] VARIANT *ret, [out] EXCEPINFO *ei, [in] IServiceProvider *caller); HRESULT PropDelete([in] DISPID id); HRESULT PropEnum(); HRESULT ToString([retval, out] BSTR *string); }Most of those after `PropGetInfo` are self-explanatory and out of scope for this. The ones dealing with the async hooks are `PropOverride` and `PropFixOverride`. `PropOverride` is the one dealing with the hooks I outlined above; right now, of course without this MR, in prop_get for instance I have this at the top:
if((proxy_target = get_proxy_target(prop_obj))) { hres = IWineJSDispatchHost_PropOverride(proxy_target, prop->name, &var); if(hres != S_FALSE) { if(SUCCEEDED(hres)) { hres = variant_to_jsval(This->ctx, &var, r); VariantClear(&var); } goto done; } }As you can see, it redirects the get to check if an async prop exists now first. **Even if** this was a jscript object, something similar would be required (I mean, a hook/check here for such object). In my idea, along with this MR, it would be to simply have the prop_get method in the vtbl (now in the per-prop vtbl) handle this without any special checks. That is, for jscript proxy objects, we'd have something like:
static HRESULT proxy_prop_get(struct prop_desc *prop, IDispatch *jsthis, jsval_t *r) { /* Do checks here if it's dispex prop etc */ ... hres = IWineJSDispatchHost_PropOverride(proxy_target, p->name, &var); if(hres != S_FALSE) { if(SUCCEEDED(hres)) { hres = variant_to_jsval(ctx, &var, r); VariantClear(&var); } return hres; } /* Forward to normal dispex method */ return dispex_prop_get(prop, jsthis, r); }And the rest of the code that uses prop.vtbl->get is completely unchanged, no special casing needed. Similar is for stuff like delete_prop or defining new props. Now `PropFixOverride` is different. It "fixes" them up similar to fix_protref_prop. It will need new stuff in the vtbl, because it's used when looking up the prop (get_prop and find_prop_name). Obviously I'm not going to include that, and dead code, in this MR yet. It's not needed by Typed Arrays either. And maybe there's a better way with it, too. Anyway, I should probably start to rewrite this so it uses mandatory methods in the object vtbl, right? It's probably hard to see how it would look, but one thing I want to avoid is to add special cases and checks, since the whole point of the vtbl approach was to get rid of them…
Generally, I think we should emphasize exposing properties as the intent of the interface, not "hooking" or "proxing". In this spirit:
``` IWineJSDispatch** GetProxyFieldRef(); ```
It will need a better solution, but I guess we may discuss it separately and keep this thread for properties.
``` HRESULT PropOverride([in] const WCHAR *name, [out, optional] VARIANT *value); ```
This is essentially a property getter mixed with some "override" logic, right? Why not just use something like `PropGetInfo` paired with property getter call when appropriate?
``` HRESULT PropDefineOverride([out] struct proxy_prop_info *info); ```
I don't know what it's for, hopefully it's redundant.
``` HRESULT PropInvoke([in] IDispatch *this_obj, [in] DISPID id, [in] LCID lcid, [in] DWORD flags, [in] DISPPARAMS *dp, [out, optional] VARIANT *ret, [out] EXCEPINFO *ei, [in] IServiceProvider *caller); ```
Do we really need `this_obj` param? This really makes sense only on "call on the object" (aka. DISPID_VALUE). But I guess that we will want MSHTML functions to be represented by jscript function objects, right? In that case it shouldn't be needed to pass that to MSHTML.
Calling a member like `InvokeEx` does is not really the way JavaScript calls work (at least not abstractly, as in spec). Member calls are really `func = propget(obj, name); call(func, this=obj, args...);`, this could be handled on jscript side with little MSHTML involvement. Calling a member in `IDispatch` style is really just an optimization to avoid allocating all functions objects that are not used as objects. With MSHTML using prototypes, it's less of the concern, so maybe we don't need to worry about that and just allocate them on demand? Or if we still want the optimization, we could provide a way for property getter to return something like (iid, dispid) pair that jscript would treat like a function or something along those lines.
I'm also don't feel like `flags` argument is needed. I'd expect getters and setters to fit better in separate functions.