[PATCH v3 0/1] MR3391: mshtml: implement window.MutationObserver with MutationObserver stub

[Yuxuan Shui (＠yshui)]

This get us pass the "Update your browser" blocker in adobe's sign-in page. The page itself doesn't make use of `window.MutationObserver`.

However the sign-in page is still broken.

--
  v3: mshtml: implement window.MutationObserver with MutationObserver stub
 
https://gitlab.winehq.org/wine/wine/-/merge_requests/3391

[PATCH v3 1/1] mshtml: implement window.MutationObserver with MutationObserver stub

[Yuxuan Shui]

From: Yuxuan Shui <yshui(a)codeweavers.com>

---
 dlls/mshtml/htmlwindow.c             |  14 +++
 dlls/mshtml/mshtml_private.h         |   6 +-
 dlls/mshtml/mshtml_private_iface.idl |  13 +++
 dlls/mshtml/mutation.c               | 143 +++++++++++++++++++++++++++
 4 files changed, 175 insertions(+), 1 deletion(-)

diff --git a/dlls/mshtml/htmlwindow.c b/dlls/mshtml/htmlwindow.c
index c6072c7e9fa..324da23ed68 100644
--- a/dlls/mshtml/htmlwindow.c
+++ b/dlls/mshtml/htmlwindow.c
@@ -3330,6 +3330,19 @@ static HRESULT WINAPI window_private_get_console(IWineHTMLWindowPrivate *iface,
     return S_OK;
 }
 
+static HRESULT WINAPI window_private_MutationObserver(IWineHTMLWindowPrivate *iface, IDispatch *callback, IDispatch **mutation_observer)
+{
+    HTMLWindow *This = impl_from_IWineHTMLWindowPrivateVtbl(iface);
+    IWineMSHTMLMutationObserver *ret = NULL;
+
+    FIXME("iface %p, mutation_observer %p, stub.\n", iface, mutation_observer);
+
+    create_mutation_observer(dispex_compat_mode(&This->inner_window->event_target.dispex), callback, &ret);
+    *mutation_observer = (IDispatch *)ret;
+
+    return S_OK;
+}
+
 static const IWineHTMLWindowPrivateVtbl WineHTMLWindowPrivateVtbl = {
     window_private_QueryInterface,
     window_private_AddRef,
@@ -3343,6 +3356,7 @@ static const IWineHTMLWindowPrivateVtbl WineHTMLWindowPrivateVtbl = {
     window_private_get_console,
     window_private_matchMedia,
     window_private_postMessage,
+    window_private_MutationObserver
 };
 
 static inline HTMLWindow *impl_from_IWineHTMLWindowCompatPrivateVtbl(IWineHTMLWindowCompatPrivate *iface)
diff --git a/dlls/mshtml/mshtml_private.h b/dlls/mshtml/mshtml_private.h
index 59d652a828b..dc6b38dbc6a 100644
--- a/dlls/mshtml/mshtml_private.h
+++ b/dlls/mshtml/mshtml_private.h
@@ -294,7 +294,8 @@ typedef struct EventTarget EventTarget;
     XIID(IWinePageTransitionEvent) \
     XIID(IWineXMLHttpRequestPrivate) \
     XIID(IWineMSHTMLConsole) \
-    XIID(IWineMSHTMLMediaQueryList)
+    XIID(IWineMSHTMLMediaQueryList) \
+    XIID(IWineMSHTMLMutationObserver)
 
 typedef enum {
 #define XIID(iface) iface ## _tid,
@@ -1479,3 +1480,6 @@ IInternetSecurityManager *get_security_manager(void);
 extern HINSTANCE hInst;
 void create_console(compat_mode_t compat_mode, IWineMSHTMLConsole **ret);
 HRESULT create_media_query_list(HTMLWindow *window, BSTR media_query, IDispatch **ret);
+
+void create_mutation_observer(compat_mode_t compat_mode, IDispatch *callback,
+                              IWineMSHTMLMutationObserver **ret);
diff --git a/dlls/mshtml/mshtml_private_iface.idl b/dlls/mshtml/mshtml_private_iface.idl
index c0bb30fbbc8..90e8846adb1 100644
--- a/dlls/mshtml/mshtml_private_iface.idl
+++ b/dlls/mshtml/mshtml_private_iface.idl
@@ -76,6 +76,17 @@ interface IWineMSHTMLConsole : IDispatch
     HRESULT warn([in, optional] VARIANT *varargStart);
 }
 
+[
+    odl,
+    oleautomation,
+    dual,
+    hidden,
+    uuid(6ac5491e-1758-4b82-98a2-83e31a7c8871)
+]
+interface IWineMSHTMLMutationObserver : IDispatch
+{
+}
+
 [
     odl,
     oleautomation,
@@ -114,6 +125,8 @@ interface IWineHTMLWindowPrivate : IDispatch
     HRESULT matchMedia([in] BSTR media_query, [retval, out] IDispatch **media_query_list);
     [id(54)]
     HRESULT postMessage([in] VARIANT msg, [in] BSTR targetOrigin, [in, optional] VARIANT transfer);
+    [id(55)]
+    HRESULT MutationObserver([in] IDispatch *callback, [retval, out] IDispatch **observer);
 }
 
 [
diff --git a/dlls/mshtml/mutation.c b/dlls/mshtml/mutation.c
index 83ab623aea8..af019d9c056 100644
--- a/dlls/mshtml/mutation.c
+++ b/dlls/mshtml/mutation.c
@@ -1076,3 +1076,146 @@ void init_mutation(nsIComponentManager *component_manager)
     if(NS_FAILED(nsres))
         ERR("Could not create nsIContentUtils instance: %08lx\n", nsres);
 }
+
+struct mutation_observer {
+    IWineMSHTMLMutationObserver IWineMSHTMLMutationObserver_iface;
+
+    LONG ref;
+    DispatchEx dispex;
+    IDispatch *callback;
+
+    HTMLDOMNode *node;
+};
+
+static inline struct mutation_observer *impl_from_IWineMSHTMLMutationObserver(IWineMSHTMLMutationObserver *iface)
+{
+    return CONTAINING_RECORD(iface, struct mutation_observer, IWineMSHTMLMutationObserver_iface);
+}
+
+static HRESULT WINAPI MutationObserver_QueryInterface(IWineMSHTMLMutationObserver *iface, REFIID riid, void **ppv)
+{
+    struct mutation_observer *This = impl_from_IWineMSHTMLMutationObserver(iface);
+
+    TRACE("(%p)->(%s %p)\n", This, debugstr_mshtml_guid(riid), ppv);
+
+    if(IsEqualGUID(&IID_IUnknown, riid) || IsEqualGUID(&IID_IWineMSHTMLMutationObserver, riid)) {
+        *ppv = &This->IWineMSHTMLMutationObserver_iface;
+    }else {
+        WARN("(%p)->(%s %p)\n", This, debugstr_mshtml_guid(riid), ppv);
+        *ppv = NULL;
+        return E_NOINTERFACE;
+    }
+
+    IUnknown_AddRef((IUnknown*)*ppv);
+    return S_OK;
+}
+
+static ULONG WINAPI MutationObserver_AddRef(IWineMSHTMLMutationObserver *iface)
+{
+    struct mutation_observer *This = impl_from_IWineMSHTMLMutationObserver(iface);
+    LONG ref = InterlockedIncrement(&This->ref);
+
+    TRACE("(%p) ref=%ld\n", This, ref);
+
+    return ref;
+}
+
+static ULONG WINAPI MutationObserver_Release(IWineMSHTMLMutationObserver *iface)
+{
+    struct mutation_observer *This = impl_from_IWineMSHTMLMutationObserver(iface);
+    LONG ref = InterlockedDecrement(&This->ref);
+
+    TRACE("(%p) ref=%ld\n", This, ref);
+
+    if(!ref) {
+        if(This->node) {
+            IHTMLDOMNode_Release(&This->node->IHTMLDOMNode_iface);
+            This->node = NULL;
+        }
+        release_dispex(&This->dispex);
+        IDispatch_Release(This->callback);
+        This->callback = NULL;
+        free(This);
+    }
+
+    return ref;
+}
+
+static HRESULT WINAPI MutationObserver_GetTypeInfoCount(IWineMSHTMLMutationObserver *iface, UINT *pctinfo)
+{
+    struct mutation_observer *This = impl_from_IWineMSHTMLMutationObserver(iface);
+    FIXME("(%p)->(%p)\n", This, pctinfo);
+    return E_NOTIMPL;
+}
+
+static HRESULT WINAPI MutationObserver_GetTypeInfo(IWineMSHTMLMutationObserver *iface, UINT iTInfo,
+                                                   LCID lcid, ITypeInfo **ppTInfo)
+{
+    struct mutation_observer *This = impl_from_IWineMSHTMLMutationObserver(iface);
+
+    return IDispatchEx_GetTypeInfo(&This->dispex.IDispatchEx_iface, iTInfo, lcid, ppTInfo);
+}
+
+static HRESULT WINAPI MutationObserver_GetIDsOfNames(IWineMSHTMLMutationObserver *iface, REFIID riid,
+                                                     LPOLESTR *rgszNames, UINT cNames, LCID lcid,
+                                                     DISPID *rgDispId)
+{
+    struct mutation_observer *This = impl_from_IWineMSHTMLMutationObserver(iface);
+
+    return IDispatchEx_GetIDsOfNames(&This->dispex.IDispatchEx_iface, riid, rgszNames, cNames, lcid,
+            rgDispId);
+}
+
+static HRESULT WINAPI MutationObserver_Invoke(IWineMSHTMLMutationObserver *iface, DISPID dispIdMember,
+                                              REFIID riid, LCID lcid, WORD wFlags, DISPPARAMS *pDispParams,
+                                              VARIANT *pVarResult, EXCEPINFO *pExcepInfo, UINT *puArgErr)
+{
+    struct mutation_observer *This = impl_from_IWineMSHTMLMutationObserver(iface);
+
+    return IDispatchEx_Invoke(&This->dispex.IDispatchEx_iface, dispIdMember, riid, lcid, wFlags,
+            pDispParams, pVarResult, pExcepInfo, puArgErr);
+}
+
+static const IWineMSHTMLMutationObserverVtbl WineMSHTMLMutationObserverVtbl = {
+    MutationObserver_QueryInterface,
+    MutationObserver_AddRef,
+    MutationObserver_Release,
+    MutationObserver_GetTypeInfoCount,
+    MutationObserver_GetTypeInfo,
+    MutationObserver_GetIDsOfNames,
+    MutationObserver_Invoke,
+};
+
+static const tid_t mutation_observer_iface_tids[] = {
+    IWineMSHTMLMutationObserver_tid,
+    0
+};
+static dispex_static_data_t console_dispex = {
+    L"MutationObserver",
+    NULL,
+    IWineMSHTMLMutationObserver_tid,
+    mutation_observer_iface_tids
+};
+
+void create_mutation_observer(compat_mode_t compat_mode, IDispatch *callback,
+                              IWineMSHTMLMutationObserver **ret) {
+    struct mutation_observer *obj;
+
+    TRACE("(compat_mode = %d, callback = %p, ret = %p)\n", compat_mode, callback, ret);
+
+    obj = calloc(1, sizeof(*obj));
+    if(!obj)
+    {
+        ERR("No memory.\n");
+        return;
+    }
+
+    obj->IWineMSHTMLMutationObserver_iface.lpVtbl = &WineMSHTMLMutationObserverVtbl;
+    obj->ref = 1;
+    init_dispatch(&obj->dispex, (IUnknown*)&obj->IWineMSHTMLMutationObserver_iface.lpVtbl,
+                  &console_dispex, compat_mode);
+
+    IDispatch_AddRef(callback);
+    obj->callback = callback;
+    *ret = &obj->IWineMSHTMLMutationObserver_iface;
+}
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/3391

[Yuxuan Shui (＠yshui)]

On Mon Jul 24 09:22:56 2023 +0000, Jactry Zeng wrote:
> I think adding a trace in QI() will be useful when debugging?
> added!

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/3391#note_39921

[Gabriel Ivăncescu]

Gabriel Ivăncescu (@insn) commented about dlls/mshtml/htmlwindow.c:
> return S_OK;
>  }
>
> +static HRESULT WINAPI window_private_MutationObserver(IWineHTMLWindowPrivate *iface, IDispatch *callback, IDispatch **mutation_observer)
> +{
> +    HTMLWindow *This = impl_from_IWineHTMLWindowPrivateVtbl(iface);
> +    IWineMSHTMLMutationObserver *ret = NULL;
> +
> +    FIXME("iface %p, mutation_observer %p, stub.\n", iface, mutation_observer);
> +
> +    create_mutation_observer(dispex_compat_mode(&This->inner_window->event_target.dispex), callback, &ret);
> +    *mutation_observer = (IDispatch *)ret;

You should handle allocation failure here (probably by having it return HRESULT and E_OUTOFMEMORY, but you could also check for NULL and return it here).

Note that the result should likely be cached in the inner window, so that you return the same constructor everytime, and you create it if the window has the cached one NULL. Then you'd have to AddRef before returning it from this method, because the created ref is held by the window (and released in `release_inner_window`).

BTW I suggest you add a simple test in documentmode.js "window_props" test to check how it's exposed, probably in IE11 only.

Also note that this is a constructor, and as such you should add a getter in the private idl that returns the constructor itself (and not an instance created from it).

Since I expect this to be IE11 specific (I haven't tested yet but I looked at caniuse.com), and the `IWineHTMLWindowPrivate` interface is IE10+, you can probably add a hook to remove this from being exposed in IE10 rather than creating a new interface.

First give it some named DISPID like:

```idl
const long DISPID_IWINEHTMLWINDOWPRIVATE_MUTATIONOBS = 55;
```

above the interface definition then use it in the interface (note that it's a getter now, since it "gets" the constructor):

```idl
    [propget, id(DISPID_IWINEHTMLWINDOWPRIVATE_MUTATIONOBS)]
    HRESULT MutationObserver([retval, out] IDispatch **observer);
```

next add a hook in `HTMLWindow_init_dispex_info` like:

```c
    static const dispex_hook_t private_ie10_hooks[] = {
        {DISPID_IWINEHTMLWINDOWPRIVATE_MUTATIONOBS},
        {DISPID_UNKNOWN}
    };
```

and use it:

```diff
     if(compat_mode >= COMPAT_MODE_IE10)
-        dispex_info_add_interface(info, IWineHTMLWindowPrivate_tid, NULL);
+        dispex_info_add_interface(info, IWineHTMLWindowPrivate_tid, compat_mode <= COMPAT_MODE_IE10 ? private_ie10_hooks : NULL);
```

this hook itself is set to NULL so basically it removes it in IE10 from being exposed (since it's IE11 only).

Note that if it is also exposed in IE10 then you don't need to do this of course. I did something similar for msCrypto which is IE11 in Proton.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/3391#note_39938

[Gabriel Ivăncescu]

Gabriel Ivăncescu (@insn) commented about dlls/mshtml/mshtml_private.h:
> XIID(IWinePageTransitionEvent) \
>      XIID(IWineXMLHttpRequestPrivate) \
>      XIID(IWineMSHTMLConsole) \
> -    XIID(IWineMSHTMLMediaQueryList)
> +    XIID(IWineMSHTMLMediaQueryList) \
> +    XIID(IWineMSHTMLMutationObserver)

I don't think this is needed here? Since you only add the constructor? This (empty) interface is for the _instance_ created from the constructor. So you don't need `IWineMSHTMLMutationObserver`.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/3391#note_39939

[Gabriel Ivăncescu]

Gabriel Ivăncescu (@insn) commented about dlls/mshtml/mshtml_private.h:
> extern HINSTANCE hInst;
>  void create_console(compat_mode_t compat_mode, IWineMSHTMLConsole **ret);
>  HRESULT create_media_query_list(HTMLWindow *window, BSTR media_query, IDispatch **ret);
> +
> +void create_mutation_observer(compat_mode_t compat_mode, IDispatch *callback,
> +                              IWineMSHTMLMutationObserver **ret);
> Likewise to prevent confusion I'd rename this to make it known it's a constructor so maybe `create_mutation_observer_ctor`? Also in this case you don't pass the callback when creating the constructor, you pass it when instantiating something from the constructor (but since you make it a stub, you don't need that now).

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/3391#note_39940

[Gabriel Ivăncescu]

Gabriel Ivăncescu (@insn) commented about dlls/mshtml/htmlwindow.c:
> window_private_get_console,
>      window_private_matchMedia,
>      window_private_postMessage,
> +    window_private_MutationObserver

Since it's a getter now that gets the constructor, name it `window_private_get_MutationObserver`.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/3391#note_39941

[Gabriel Ivăncescu]

Gabriel Ivăncescu (@insn) commented about dlls/mshtml/mshtml_private_iface.idl:
> HRESULT warn([in, optional] VARIANT *varargStart);
>  }
>
> +[
> +    odl,
> +    oleautomation,
> +    dual,
> +    hidden,
> +    uuid(6ac5491e-1758-4b82-98a2-83e31a7c8871)
> +]
> +interface IWineMSHTMLMutationObserver : IDispatch
> +{
> +}
> +

As above, this is not needed for the constructor.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/3391#note_39942

[Gabriel Ivăncescu]

Gabriel Ivăncescu (@insn) commented about dlls/mshtml/mshtml_private_iface.idl:
> HRESULT matchMedia([in] BSTR media_query, [retval, out] IDispatch **media_query_list);
>      [id(54)]
>      HRESULT postMessage([in] VARIANT msg, [in] BSTR targetOrigin, [in, optional] VARIANT transfer);
> +    [id(55)]
> +    HRESULT MutationObserver([in] IDispatch *callback, [retval, out] IDispatch **observer);

See comment above to turn it into a getter for the constructor (with named DISPID for the hook maybe).

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/3391#note_39943

[Gabriel Ivăncescu]

Gabriel Ivăncescu (@insn) commented about dlls/mshtml/mutation.c:
> if(NS_FAILED(nsres))
>          ERR("Could not create nsIContentUtils instance: %08lx\n", nsres);
>  }
> +
> +struct mutation_observer {
> +    IWineMSHTMLMutationObserver IWineMSHTMLMutationObserver_iface;
> +
> +    LONG ref;
> +    DispatchEx dispex;
> +    IDispatch *callback;
> +
> +    HTMLDOMNode *node;
> +};

Ok so I will comment in general here for the entire thing. First name it `mutation_observer_ctor` or something like that to make it known it's a constructor and **not** the instance created from it.

Secondly, this is not a DOM node (and anyway what you did was wrong and you didn't even make use of it), so you can simply remove the entire node thing.

For an example of constructors you can look at e.g. `HTMLXMLHttpRequestFactory`. You just need the dispex, a ref and probably an IUnknown iface to implement it (and the dispex qi). IUnknown because I don't think this constructor exposes any props or methods… (IUnknown instead of your IWineMSHTMLMutationObserver basically, since that's for the _instance_ not the constructor).

If you make it IUnknown you'll have to return the dispex's IDispatchEx iface when returning the constructor as an IDispatch, obviously. This will avoid having to add forwarding methods like GetIDsOfNames and so on.

If you want to actually implement construction you'll have to use the dispex vtbl's value (see the other constructors like XMLHttpRequestFactory) but you can set that aside for now since you just expose it as stub.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/3391#note_39944

[Gabriel Ivăncescu]

Gabriel Ivăncescu (@insn) commented about dlls/mshtml/mutation.c:
> +    obj = calloc(1, sizeof(*obj));
> +    if(!obj)
> +    {
> +        ERR("No memory.\n");
> +        return;
> +    }
> +
> +    obj->IWineMSHTMLMutationObserver_iface.lpVtbl = &WineMSHTMLMutationObserverVtbl;
> +    obj->ref = 1;
> +    init_dispatch(&obj->dispex, (IUnknown*)&obj->IWineMSHTMLMutationObserver_iface.lpVtbl,
> +                  &console_dispex, compat_mode);
> +
> +    IDispatch_AddRef(callback);
> +    obj->callback = callback;
> +    *ret = &obj->IWineMSHTMLMutationObserver_iface;
> +}
> Again this should probably return HRESULT and you handle it on caller (E_OUTOFMEMORY likely), and no need for "callback" because that's for the construction of the instance, here you just want to give it the constructor.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/3391#note_39946

[Gabriel Ivăncescu]

Gabriel Ivăncescu (@insn) commented about dlls/mshtml/mutation.c:
> +
> +static const IWineMSHTMLMutationObserverVtbl WineMSHTMLMutationObserverVtbl = {
> +    MutationObserver_QueryInterface,
> +    MutationObserver_AddRef,
> +    MutationObserver_Release,
> +    MutationObserver_GetTypeInfoCount,
> +    MutationObserver_GetTypeInfo,
> +    MutationObserver_GetIDsOfNames,
> +    MutationObserver_Invoke,
> +};
> +
> +static const tid_t mutation_observer_iface_tids[] = {
> +    IWineMSHTMLMutationObserver_tid,
> +    0
> +};
> +static dispex_static_data_t console_dispex = {
> Copy-paste mistake :)  ("console")

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/3391#note_39945