[PATCH 0/1] MR6664: mshtml: Make sure disp_invoke is called before locking the document mode.

[Gabriel Ivăncescu]

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

[PATCH 1/1] mshtml: Make sure disp_invoke is called before locking the document mode.

[Gabriel Ivăncescu]

From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com>

ieframe's get_doc_ready_state retrieves the document's readystate, but
this happens way too early before meta elements are parsed (and thus
compat mode is not set yet). Prior to the regression, it was handling
DISPID_READYSTATE directly without going through DispatchEx's InvokeEx,
which calls ensure_real_info and locks the document mode.

Fixes a regression introduced by 88a06f18b467a31d96ab5b9c816cf3ee3b2bb2f2.

Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com>
---
 dlls/mshtml/dispex.c       |  6 +++---
 dlls/mshtml/tests/script.c | 23 +++++++++++++++++++++++
 2 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/dlls/mshtml/dispex.c b/dlls/mshtml/dispex.c
index 9d69f413d44..e194024607d 100644
--- a/dlls/mshtml/dispex.c
+++ b/dlls/mshtml/dispex.c
@@ -2259,9 +2259,6 @@ static HRESULT WINAPI DispatchEx_InvokeEx(IWineJSDispatchHost *iface, DISPID id,
 
     TRACE("%s (%p)->(%lx %lx %x %p %p %p %p)\n", This->info->name, This, id, lcid, wFlags, pdp, pvarRes, pei, pspCaller);
 
-    if(!ensure_real_info(This))
-        return E_OUTOFMEMORY;
-
     if(wFlags == (DISPATCH_PROPERTYPUT|DISPATCH_PROPERTYPUTREF))
         wFlags = DISPATCH_PROPERTYPUT;
 
@@ -2271,6 +2268,9 @@ static HRESULT WINAPI DispatchEx_InvokeEx(IWineJSDispatchHost *iface, DISPID id,
             return hres;
     }
 
+    if(!ensure_real_info(This))
+        return E_OUTOFMEMORY;
+
     if(This->jsdisp)
         return IWineJSDispatch_InvokeEx(This->jsdisp, id, lcid, wFlags, pdp, pvarRes, pei, pspCaller);
 
diff --git a/dlls/mshtml/tests/script.c b/dlls/mshtml/tests/script.c
index 88831bc66c1..f454de86711 100644
--- a/dlls/mshtml/tests/script.c
+++ b/dlls/mshtml/tests/script.c
@@ -185,6 +185,7 @@ static const GUID CLSID_TestScript[] = {
 static const GUID CLSID_TestActiveX =
     {0x178fc163,0xf585,0x4e24,{0x9c,0x13,0x4b,0xb7,0xfa,0xf8,0x06,0x46}};
 
+static DWORD main_thread_id;
 static BOOL is_ie9plus, is_english;
 static IHTMLDocument2 *notif_doc;
 static IOleDocumentView *view;
@@ -4487,6 +4488,27 @@ static HRESULT WINAPI Protocol_Read(IInternetProtocolEx *iface, void *pv, ULONG
     ULONG read;
     HRESULT hres;
 
+    if(GetCurrentThreadId() == main_thread_id) {
+        IHTMLDocument2 *doc_node;
+        DISPPARAMS dp = { 0 };
+        IHTMLWindow2 *window;
+        VARIANT v;
+
+        hres = IHTMLDocument2_get_parentWindow(notif_doc, &window);
+        ok(hres == S_OK, "get_parentWindow failed: %08lx\n", hres);
+
+        hres = IHTMLWindow2_get_document(window, &doc_node);
+        IHTMLWindow2_Release(window);
+        ok(hres == S_OK, "get_document failed: %08lx\n", hres);
+
+        V_VT(&v) = VT_EMPTY;
+        V_I4(&v) = 1234;
+        hres = IHTMLDocument2_Invoke(doc_node, DISPID_READYSTATE, &IID_NULL, 0, DISPATCH_PROPERTYGET, &dp, &v, NULL, NULL);
+        ok(hres == S_OK, "Invoke(DISPID_READYSTATE) failed: %08lx\n", hres);
+        ok(V_VT(&v) == VT_I4, "V_VT(v) = %d\n", V_VT(&v));
+        IHTMLDocument2_Release(doc_node);
+    }
+
     if(This->stream) {
         hres = IStream_Read(This->stream, pv, cb, &read);
         ok(SUCCEEDED(hres), "Read failed: %08lx\n", hres);
@@ -5247,6 +5269,7 @@ START_TEST(script)
 
     argc = winetest_get_mainargs(&argv);
     CoInitialize(NULL);
+    main_thread_id = GetCurrentThreadId();
     container_hwnd = create_container_window();
 
     detect_locale();
-- 
GitLab

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

[Jacek Caban (＠jacek)]

Jacek Caban (@jacek) commented about dlls/mshtml/tests/script.c:
> +        VARIANT v;
> +
> +        hres = IHTMLDocument2_get_parentWindow(notif_doc, &window);
> +        ok(hres == S_OK, "get_parentWindow failed: %08lx\n", hres);
> +
> +        hres = IHTMLWindow2_get_document(window, &doc_node);
> +        IHTMLWindow2_Release(window);
> +        ok(hres == S_OK, "get_document failed: %08lx\n", hres);
> +
> +        V_VT(&v) = VT_EMPTY;
> +        V_I4(&v) = 1234;
> +        hres = IHTMLDocument2_Invoke(doc_node, DISPID_READYSTATE, &IID_NULL, 0, DISPATCH_PROPERTYGET, &dp, &v, NULL, NULL);
> +        ok(hres == S_OK, "Invoke(DISPID_READYSTATE) failed: %08lx\n", hres);
> +        ok(V_VT(&v) == VT_I4, "V_VT(v) = %d\n", V_VT(&v));
> +        IHTMLDocument2_Release(doc_node);
> +    }
> This place seems a bit random for the test, maybe something like `PropertyNotifySink_OnChanged` would work?

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/6664#note_84943

[Gabriel Ivăncescu]

On Mon Oct 14 18:00:10 2024 +0000, Jacek Caban wrote:
> This place seems a bit random for the test, maybe something like
> `PropertyNotifySink_OnChanged` would work?
> I tried it, and it doesn't work, it's too "late" (after the meta
> elements are parsed and compat mode is set correctly). My understanding
> is that this is triggered by some binding implementation (perhaps even
> ours, I didn't dig enough), and I don't think reconstructing a custom
> one just for the test is worth it… so I hijacked the protocol instead
> since we already use a custom one in the test.

What I mean by "it doens't work" is that it passes even without the fix.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/6664#note_84944

[PATCH 0/1] MR6664: mshtml: Make sure disp_invoke is called before locking the document mode. - approved

[Jacek Caban (＠jacek)]

This merge request was approved by Jacek Caban.
-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/6664