[PATCH v2 0/1] MR2068: Draft: comdlg32: Enable visual styles when showing IFileDialog.

[Vladislav Timonin (＠vt)]

Required for !1857 to not break when comctl32 version 6 isn't requested by application (in particular, found this in [qapitrace](https://github.com/apitrace/apitrace)).

Manifest resource id and assembly identity name match with Windows.

For isolation purposes, activation context is disabled while emitting events.

--
  v2: comdlg32: Enable visual styles when showing IFileDialog.

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

[PATCH v2 1/1] comdlg32: Enable visual styles when showing IFileDialog.

[Vladislav Timonin]

From: Vladislav Timonin

---
 dlls/comdlg32/cdlg.h            |  1 +
 dlls/comdlg32/cdlg32.c          | 16 ++++++-
 dlls/comdlg32/comdlg32.manifest | 16 +++++++
 dlls/comdlg32/comdlg32.rc       |  3 ++
 dlls/comdlg32/itemdlg.c         | 84 +++++++++++++++++++++++++++++++++
 5 files changed, 119 insertions(+), 1 deletion(-)
 create mode 100644 dlls/comdlg32/comdlg32.manifest

diff --git a/dlls/comdlg32/cdlg.h b/dlls/comdlg32/cdlg.h
index 1ddd157b880..e47d685fac3 100644
--- a/dlls/comdlg32/cdlg.h
+++ b/dlls/comdlg32/cdlg.h
@@ -27,6 +27,7 @@
 #define COMDLG32_Atom   MAKEINTATOM(0xa000)     /* MS uses this one to identify props */

extern HINSTANCE	COMDLG32_hInstance DECLSPEC_HIDDEN;
+extern HANDLE	COMDLG32_hActCtx DECLSPEC_HIDDEN;

void	COMDLG32_SetCommDlgExtendedError(DWORD err) DECLSPEC_HIDDEN;
 LPVOID	COMDLG32_AllocMem(int size) __WINE_ALLOC_SIZE(1) DECLSPEC_HIDDEN;
diff --git a/dlls/comdlg32/cdlg32.c b/dlls/comdlg32/cdlg32.c
index f9619cc6a49..aebb3475674 100644
--- a/dlls/comdlg32/cdlg32.c
+++ b/dlls/comdlg32/cdlg32.c
@@ -40,6 +40,7 @@ WINE_DEFAULT_DEBUG_CHANNEL(commdlg);

DECLSPEC_HIDDEN HINSTANCE	COMDLG32_hInstance = 0;
+HANDLE	COMDLG32_hActCtx = INVALID_HANDLE_VALUE;

static DWORD COMDLG32_TlsIndex = TLS_OUT_OF_INDEXES;

@@ -72,6 +73,9 @@ BOOL WINAPI DllMain(HINSTANCE hInstance, DWORD Reason, LPVOID Reserved)
    switch(Reason)
    {
    case DLL_PROCESS_ATTACH:
+	{
+		ACTCTXW actctx = {0};
+
    	COMDLG32_hInstance = hInstance;
    	DisableThreadLibraryCalls(hInstance);

@@ -79,11 +83,21 @@ BOOL WINAPI DllMain(HINSTANCE hInstance, DWORD Reason, LPVOID Reserved)

/* SHELL */
    	GPA(COMDLG32_SHSimpleIDListFromPathAW, SHELL32_hInstance, (LPCSTR)162);
-		break;

+		actctx.cbSize = sizeof(actctx);
+		actctx.hModule = COMDLG32_hInstance;
+		actctx.lpResourceName = MAKEINTRESOURCEW(123);
+		actctx.dwFlags = ACTCTX_FLAG_HMODULE_VALID | ACTCTX_FLAG_RESOURCE_NAME_VALID;
+		COMDLG32_hActCtx = CreateActCtxW(&actctx);
+                if (COMDLG32_hActCtx == INVALID_HANDLE_VALUE)
+                    ERR("failed to create activation context, last error %lu\n", GetLastError());
+
+		break;
+	}
    case DLL_PROCESS_DETACH:
             if (Reserved) break;
             if (COMDLG32_TlsIndex != TLS_OUT_OF_INDEXES) TlsFree(COMDLG32_TlsIndex);
+            ReleaseActCtx(&COMDLG32_hActCtx);
             break;
    }
    return TRUE;
diff --git a/dlls/comdlg32/comdlg32.manifest b/dlls/comdlg32/comdlg32.manifest
new file mode 100644
index 00000000000..f9e7b7cc115
--- /dev/null
+++ b/dlls/comdlg32/comdlg32.manifest
@@ -0,0 +1,16 @@
+<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
+<assembly xmlns="urn:schemas-microsoft-com:asm.v1" manifestVersion="1.0">
+<assemblyIdentity type="win32" name="Microsoft.Windows.Shell.comdlg32" version="0.0.0.0"/>
+<dependency>
+    <dependentAssembly>
+        <assemblyIdentity
+            type="win32"
+            name="Microsoft.Windows.Common-Controls"
+            version="6.0.0.0"
+            processorArchitecture="*"
+            publicKeyToken="6595b64144ccf1df"
+            language="*"
+        />
+    </dependentAssembly>
+</dependency>
+</assembly>
diff --git a/dlls/comdlg32/comdlg32.rc b/dlls/comdlg32/comdlg32.rc
index a5d09faa563..474bafd8760 100644
--- a/dlls/comdlg32/comdlg32.rc
+++ b/dlls/comdlg32/comdlg32.rc
@@ -602,3 +602,6 @@ NETWORK ICON network.ico

/* @makedep: fontpics.bmp */
 38 BITMAP fontpics.bmp
+
+/* @makedep: comdlg32.manifest */
+123 RT_MANIFEST comdlg32.manifest
diff --git a/dlls/comdlg32/itemdlg.c b/dlls/comdlg32/itemdlg.c
index 95778c2f358..1bb77fa430c 100644
--- a/dlls/comdlg32/itemdlg.c
+++ b/dlls/comdlg32/itemdlg.c
@@ -145,6 +145,8 @@ typedef struct FileDialogImpl {
     DWORD opendropdown_selection;

GUID client_guid;
+
+    HANDLE user_actctx;
 } FileDialogImpl;

/**************************************************************************
@@ -152,10 +154,14 @@ typedef struct FileDialogImpl {
  */
 static HRESULT events_OnFileOk(FileDialogImpl *This)
 {
+    ULONG_PTR ctx_cookie;
     events_client *cursor;
     HRESULT hr = S_OK;
     TRACE("%p\n", This);

+    if (This->user_actctx != INVALID_HANDLE_VALUE)
+        ActivateActCtx(This->user_actctx, &ctx_cookie);
+
     LIST_FOR_EACH_ENTRY(cursor, &This->events_clients, events_client, entry)
     {
         TRACE("Notifying %p\n", cursor);
@@ -164,6 +170,9 @@ static HRESULT events_OnFileOk(FileDialogImpl *This)
             break;
     }

+    if (This->user_actctx != INVALID_HANDLE_VALUE)
+        DeactivateActCtx(0, ctx_cookie);
+
     if(hr == E_NOTIMPL)
         hr = S_OK;

@@ -172,10 +181,14 @@ static HRESULT events_OnFileOk(FileDialogImpl *This)

static HRESULT events_OnFolderChanging(FileDialogImpl *This, IShellItem *folder)
 {
+    ULONG_PTR ctx_cookie;
     events_client *cursor;
     HRESULT hr = S_OK;
     TRACE("%p (%p)\n", This, folder);

+    if (This->user_actctx != INVALID_HANDLE_VALUE)
+        ActivateActCtx(This->user_actctx, &ctx_cookie);
+
     LIST_FOR_EACH_ENTRY(cursor, &This->events_clients, events_client, entry)
     {
         TRACE("Notifying %p\n", cursor);
@@ -184,6 +197,9 @@ static HRESULT events_OnFolderChanging(FileDialogImpl *This, IShellItem *folder)
             break;
     }

+    if (This->user_actctx != INVALID_HANDLE_VALUE)
+        DeactivateActCtx(0, ctx_cookie);
+
     if(hr == E_NOTIMPL)
         hr = S_OK;

@@ -192,47 +208,72 @@ static HRESULT events_OnFolderChanging(FileDialogImpl *This, IShellItem *folder)

static void events_OnFolderChange(FileDialogImpl *This)
 {
+    ULONG_PTR ctx_cookie;
     events_client *cursor;
     TRACE("%p\n", This);

+    if (This->user_actctx != INVALID_HANDLE_VALUE)
+        ActivateActCtx(This->user_actctx, &ctx_cookie);
+
     LIST_FOR_EACH_ENTRY(cursor, &This->events_clients, events_client, entry)
     {
         TRACE("Notifying %p\n", cursor);
         IFileDialogEvents_OnFolderChange(cursor->pfde, (IFileDialog*)&This->IFileDialog2_iface);
     }
+
+    if (This->user_actctx != INVALID_HANDLE_VALUE)
+        DeactivateActCtx(0, ctx_cookie);
 }

static void events_OnSelectionChange(FileDialogImpl *This)
 {
+    ULONG_PTR ctx_cookie;
     events_client *cursor;
     TRACE("%p\n", This);

+    if (This->user_actctx != INVALID_HANDLE_VALUE)
+        ActivateActCtx(This->user_actctx, &ctx_cookie);
+
     LIST_FOR_EACH_ENTRY(cursor, &This->events_clients, events_client, entry)
     {
         TRACE("Notifying %p\n", cursor);
         IFileDialogEvents_OnSelectionChange(cursor->pfde, (IFileDialog*)&This->IFileDialog2_iface);
     }
+
+    if (This->user_actctx != INVALID_HANDLE_VALUE)
+        DeactivateActCtx(0, ctx_cookie);
 }

static void events_OnTypeChange(FileDialogImpl *This)
 {
+    ULONG_PTR ctx_cookie;
     events_client *cursor;
     TRACE("%p\n", This);

+    if (This->user_actctx != INVALID_HANDLE_VALUE)
+        ActivateActCtx(This->user_actctx, &ctx_cookie);
+
     LIST_FOR_EACH_ENTRY(cursor, &This->events_clients, events_client, entry)
     {
         TRACE("Notifying %p\n", cursor);
         IFileDialogEvents_OnTypeChange(cursor->pfde, (IFileDialog*)&This->IFileDialog2_iface);
     }
+
+    if (This->user_actctx != INVALID_HANDLE_VALUE)
+        DeactivateActCtx(0, ctx_cookie);
 }

static HRESULT events_OnOverwrite(FileDialogImpl *This, IShellItem *shellitem)
 {
+    ULONG_PTR ctx_cookie;
     events_client *cursor;
     HRESULT hr = S_OK;
     FDE_OVERWRITE_RESPONSE response = FDEOR_DEFAULT;
     TRACE("%p %p\n", This, shellitem);

+    if (This->user_actctx != INVALID_HANDLE_VALUE)
+        ActivateActCtx(This->user_actctx, &ctx_cookie);
+
     LIST_FOR_EACH_ENTRY(cursor, &This->events_clients, events_client, entry)
     {
         TRACE("Notifying %p\n", cursor);
@@ -242,6 +283,9 @@ static HRESULT events_OnOverwrite(FileDialogImpl *This, IShellItem *shellitem)
             break;
     }

+    if (This->user_actctx != INVALID_HANDLE_VALUE)
+        DeactivateActCtx(0, ctx_cookie);
+
     if(hr == E_NOTIMPL)
         hr = S_OK;

@@ -274,9 +318,13 @@ static inline HRESULT get_cctrl_event(IFileDialogEvents *pfde, IFileDialogContro

static HRESULT cctrl_event_OnButtonClicked(FileDialogImpl *This, DWORD ctl_id)
 {
+    ULONG_PTR ctx_cookie;
     events_client *cursor;
     TRACE("%p\n", This);

+    if (This->user_actctx != INVALID_HANDLE_VALUE)
+        ActivateActCtx(This->user_actctx, &ctx_cookie);
+
     LIST_FOR_EACH_ENTRY(cursor, &This->events_clients, events_client, entry)
     {
         IFileDialogControlEvents *pfdce;
@@ -288,14 +336,21 @@ static HRESULT cctrl_event_OnButtonClicked(FileDialogImpl *This, DWORD ctl_id)
         }
     }

+    if (This->user_actctx != INVALID_HANDLE_VALUE)
+        DeactivateActCtx(0, ctx_cookie);
+
     return S_OK;
 }

static HRESULT cctrl_event_OnItemSelected(FileDialogImpl *This, DWORD ctl_id, DWORD item_id)
 {
+    ULONG_PTR ctx_cookie;
     events_client *cursor;
     TRACE("%p %li %li\n", This, ctl_id, item_id);

+    if (This->user_actctx != INVALID_HANDLE_VALUE)
+        ActivateActCtx(This->user_actctx, &ctx_cookie);
+
     LIST_FOR_EACH_ENTRY(cursor, &This->events_clients, events_client, entry)
     {
         IFileDialogControlEvents *pfdce;
@@ -307,14 +362,21 @@ static HRESULT cctrl_event_OnItemSelected(FileDialogImpl *This, DWORD ctl_id, DW
         }
     }

+    if (This->user_actctx != INVALID_HANDLE_VALUE)
+        DeactivateActCtx(0, ctx_cookie);
+
     return S_OK;
 }

static HRESULT cctrl_event_OnCheckButtonToggled(FileDialogImpl *This, DWORD ctl_id, BOOL checked)
 {
+    ULONG_PTR ctx_cookie;
     events_client *cursor;
     TRACE("%p\n", This);

+    if (This->user_actctx != INVALID_HANDLE_VALUE)
+        ActivateActCtx(This->user_actctx, &ctx_cookie);
+
     LIST_FOR_EACH_ENTRY(cursor, &This->events_clients, events_client, entry)
     {
         IFileDialogControlEvents *pfdce;
@@ -326,15 +388,22 @@ static HRESULT cctrl_event_OnCheckButtonToggled(FileDialogImpl *This, DWORD ctl_
         }
     }

+    if (This->user_actctx != INVALID_HANDLE_VALUE)
+        DeactivateActCtx(0, ctx_cookie);
+
     return S_OK;
 }

static HRESULT cctrl_event_OnControlActivating(FileDialogImpl *This,
                                                   DWORD ctl_id)
 {
+    ULONG_PTR ctx_cookie;
     events_client *cursor;
     TRACE("%p\n", This);

+    if (This->user_actctx != INVALID_HANDLE_VALUE)
+        ActivateActCtx(This->user_actctx, &ctx_cookie);
+
     LIST_FOR_EACH_ENTRY(cursor, &This->events_clients, events_client, entry)
     {
         IFileDialogControlEvents *pfdce;
@@ -346,6 +415,9 @@ static HRESULT cctrl_event_OnControlActivating(FileDialogImpl *This,
         }
     }

+    if (This->user_actctx != INVALID_HANDLE_VALUE)
+        DeactivateActCtx(0, ctx_cookie);
+
     return S_OK;
 }

@@ -2243,12 +2315,22 @@ static INT_PTR CALLBACK itemdlg_dlgproc(HWND hwnd, UINT umessage, WPARAM wparam,
 static HRESULT create_dialog(FileDialogImpl *This, HWND parent)
 {
     INT_PTR res;
+    ULONG_PTR actctx_cookie = 0;
+
+    GetCurrentActCtx(&This->user_actctx);
+    if (!ActivateActCtx(COMDLG32_hActCtx, &actctx_cookie))
+        ERR("failed to activate activation context, last error %lx\n", GetLastError());

SetLastError(0);
     res = DialogBoxParamW(COMDLG32_hInstance,
                           MAKEINTRESOURCEW(NEWFILEOPENV3ORD),
                           parent, itemdlg_dlgproc, (LPARAM)This);
     This->dlg_hwnd = NULL;
+
+    if (actctx_cookie != 0)
+        if (!DeactivateActCtx(0, actctx_cookie))
+            ERR("failed to deactivate activation context, last error %lx\n", GetLastError());
+
     if(res == -1)
     {
         ERR("Failed to show dialog (LastError: %ld)\n", GetLastError());
@@ -4642,6 +4724,8 @@ static HRESULT FileDialog_constructor(IUnknown *pUnkOuter, REFIID riid, void **p
         return E_FAIL;
     }

+    fdimpl->user_actctx = INVALID_HANDLE_VALUE;
+
     hr = IFileDialog2_QueryInterface(&fdimpl->IFileDialog2_iface, riid, ppv);
     IFileDialog2_Release(&fdimpl->IFileDialog2_iface);
     return hr;

[Jinoh Kang (＠iamahuman)]

Jinoh Kang (@iamahuman) commented about dlls/comdlg32/itemdlg.c:
> static HRESULT create_dialog(FileDialogImpl *This, HWND parent)
>  {
>      INT_PTR res;
> -
>
> ULONG_PTR actctx_cookie = 0;
> -
> -
>
> GetCurrentActCtx(&This->user_actctx);
> -
>
> if (!ActivateActCtx(COMDLG32_hActCtx, &actctx_cookie))
> -
>    ERR("failed to activate activation context, last error %lx\n", GetLastError());
>
> SetLastError(0);
>    res = DialogBoxParamW(COMDLG32_hInstance,
>                          MAKEINTRESOURCEW(NEWFILEOPENV3ORD),
>                          parent, itemdlg_dlgproc, (LPARAM)This);
>    This->dlg_hwnd = NULL;
> -
> -
>
> if (actctx_cookie != 0)

You shouldn't rely on `0` being an invalid cookie value.

[Jinoh Kang (＠iamahuman)]

Jinoh Kang (@iamahuman) commented about dlls/comdlg32/itemdlg.c:
> static HRESULT create_dialog(FileDialogImpl *This, HWND parent)
>  {
>      INT_PTR res;
> - ULONG_PTR actctx_cookie = 0;
> -
> - GetCurrentActCtx(&This->user_actctx);

In case of nested `Show()` calls, you should either:

1. Fail with `E_UNEXPECTED`, or
2. Backup the previous `user_actctx` value before calling `GetCurrentActCtx`.

[Jinoh Kang (＠iamahuman)]

Jinoh Kang (@iamahuman) commented about dlls/comdlg32/itemdlg.c:
>          break;
>  }
>
> - if (This->user_actctx != INVALID_HANDLE_VALUE)
> -
>    DeactivateActCtx(0, ctx_cookie);

You should only deactivate if `ActivateActCtx` succeeded.

I suggest writing activation/deactivation helpers.