[PATCH] user32: Add support for class versions
Nikolay Sivov
bunglehead at gmail.com
Sat Nov 5 09:56:47 CDT 2016
On 05.11.2016 16:36, Ivan Akulinchev wrote:
> Signed-off-by: Ivan Akulinchev <ivan.akulinchev at gmail.com>
> ---
> dlls/comctl32/comctl32.manifest | 1 -
> dlls/comctl32/theming.c | 15 +++++++--
> dlls/user32/class.c | 73 +++++++++++++++++++++++++++++++++++++----
> dlls/user32/user_private.h | 2 ++
> dlls/user32/win.c | 7 ++--
> 5 files changed, 86 insertions(+), 12 deletions(-)
>
Hi, thanks for looking into this.
> diff --git a/dlls/comctl32/comctl32.manifest b/dlls/comctl32/comctl32.manifest
> index 4c86137..5aa55a3 100644
> --- a/dlls/comctl32/comctl32.manifest
> +++ b/dlls/comctl32/comctl32.manifest
> @@ -12,7 +12,6 @@
> <windowClass>NativeFontCtl</windowClass>
> <windowClass>ReBarWindow32</windowClass>
> <windowClass>ScrollBar</windowClass>
> - <windowClass>Static</windowClass>
> <windowClass>SysAnimate32</windowClass>
> <windowClass>SysDateTimePick32</windowClass>
> <windowClass>SysHeader32</windowClass>
Something tells me we'll need opposite logic here,
if versioned classes support are to be added one by one.
> diff --git a/dlls/comctl32/theming.c b/dlls/comctl32/theming.c
> index 93d6fe6..b78ecc9 100644
> --- a/dlls/comctl32/theming.c
> +++ b/dlls/comctl32/theming.c
> @@ -28,6 +28,7 @@
> #include "comctl32.h"
> #include "uxtheme.h"
> #include "wine/debug.h"
> +#include "wine/unicode.h"
>
> WINE_DEFAULT_DEBUG_CHANNEL(theming);
>
> @@ -124,8 +125,6 @@ void THEMING_Initialize (void)
> static const WCHAR refDataPropName[] =
> { 'C','C','3','2','T','h','e','m','i','n','g','D','a','t','a',0 };
>
> - if (!IsThemeActive()) return;
> -
> atSubclassProp = GlobalAddAtomW (subclassPropName);
> atRefDataProp = GlobalAddAtomW (refDataPropName);
>
> @@ -142,6 +141,18 @@ void THEMING_Initialize (void)
> }
> originalProcs[i] = class.lpfnWndProc;
> class.lpfnWndProc = subclassProcs[i];
> +
> + /*
> + * FIXME: The Dialog class ('#32770') should NOT be overridden here.
> + *
> + * Temporarily ignore this issue using the hack below...
> + */
> + if (strcmpiW(subclasses[i].className, dialogClass) != 0)
> + {
> + /* If not a dialog (see above), make the class global */
> + class.style |= CS_GLOBALCLASS;
> + class.hInstance = NULL;
> + }
>
> if (!class.lpfnWndProc)
> {
All theming.c should go when transition is complete, because once you're
able to create 6.0.0.0!Static it will support theme drawing API
regardless if theming is enabled or not.
> diff --git a/dlls/user32/class.c b/dlls/user32/class.c
> index 0b3582e..5b16008 100644
> --- a/dlls/user32/class.c
> +++ b/dlls/user32/class.c
> @@ -411,6 +411,42 @@ static CLASS *CLASS_RegisterClass( LPCWSTR name, HINSTANCE hInstance, BOOL local
> return classPtr;
> }
>
> +/***********************************************************************
> + * CLASS_GetVersionedName
> + *
> + * Return a versioned class name, like "6.0.2600.2982!Button".
> + */
> +LPCWSTR CLASS_GetVersionedName( LPCWSTR name )
> +{
> + ACTCTX_SECTION_KEYED_DATA data = { sizeof(data) };
> +
> + static const WCHAR staticW[] = { 'S','t','a','t','i','c',0 };
> +
> + if (!name)
> + return NULL;
> +
> + if (IS_INTRESOURCE( name ))
> + return name;
> +
> + /*
> + * FIXME: I disabled the Static class in comctl32.manifest (because it is
> + * not implemented there yet), however I still get "6.0.2600.2982!Static".
> + *
> + * Is it a FindActCtxSectionString bug?
> + */
> + if (strcmpiW( name, staticW ) == 0)
> + return name;
This needs separate investigation if it's truly happens without Static
present in manifest.
> +
> + if (FindActCtxSectionStringW( 0, NULL, 3, name, &data ))
> + {
> + BYTE *res = (BYTE *)data.lpData;
> + ULONG offset = *(ULONG *)(res + sizeof(ULONG) * 2 + sizeof(DWORD));
> + return (LPCWSTR)(res + offset);
> + }
> +
> + return name;
This makes sense, but please use types from ntdll/actctx.c (just
duplicate what you need in user32, same as it's done in ole32 and
kernel32 already). Also 3 here stands for
ACTIVATION_CONTEXT_SECTION_WINDOW_CLASS_REDIRECTION.
> +}
> +
>
> /***********************************************************************
> * register_builtin
> @@ -576,10 +612,12 @@ ATOM WINAPI RegisterClassExA( const WNDCLASSEXA* wc )
>
> if (!IS_INTRESOURCE(wc->lpszClassName))
> {
> + LPCWSTR versioned_name;
> WCHAR name[MAX_ATOM_LEN + 1];
>
> if (!MultiByteToWideChar( CP_ACP, 0, wc->lpszClassName, -1, name, MAX_ATOM_LEN + 1 )) return 0;
> - classPtr = CLASS_RegisterClass( name, instance, !(wc->style & CS_GLOBALCLASS),
> + versioned_name = CLASS_GetVersionedName( name );
> + classPtr = CLASS_RegisterClass( versioned_name, instance, !(wc->style & CS_GLOBALCLASS),
> wc->style, wc->cbClsExtra, wc->cbWndExtra );
> }
Why do you need this in -A call too?
> else
> @@ -619,6 +657,7 @@ ATOM WINAPI RegisterClassExW( const WNDCLASSEXW* wc )
> ATOM atom;
> CLASS *classPtr;
> HINSTANCE instance;
> + LPCWSTR versioned_name;
>
> GetDesktopWindow(); /* create the desktop window to trigger builtin class registration */
>
> @@ -630,7 +669,9 @@ ATOM WINAPI RegisterClassExW( const WNDCLASSEXW* wc )
> }
> if (!(instance = wc->hInstance)) instance = GetModuleHandleW( NULL );
>
> - if (!(classPtr = CLASS_RegisterClass( wc->lpszClassName, instance, !(wc->style & CS_GLOBALCLASS),
> + versioned_name = CLASS_GetVersionedName( wc->lpszClassName );
> +
> + if (!(classPtr = CLASS_RegisterClass( versioned_name, instance, !(wc->style & CS_GLOBALCLASS),
> wc->style, wc->cbClsExtra, wc->cbWndExtra )))
> return 0;
>
This part I looks wrong. Why would it register something that is not
what user asked for? My understanding is that comctl32 itself should
register its versioned classes (and in ideal v5 vs v6 separation only v6
module should be doing that). So comctl32 is loaded, it registers
regular classes first, like Listview, then it registered versioned names
like 6.0.0.0!Listview.
What RegisterClassExW should probably check is if target class in
current context is already registered, so if application tries to
register ClassName, and context has redirection set as
6.0.0.0!ClassName, registration should fail. I think it should be
possible to add a test for that:
- create temporary context with window class redirection section;
- manually register corresponding versioned class name;
- activate context;
- see what happens if you try to register non-version class.
I vaguely remember I tried something like that on Windows years ago,
and as expected it breaks this whole redirection scheme, because if
6.x.x.x!Button is already registered by the time context is activated
comctl32 won't be able to register it again.
> @@ -678,6 +719,7 @@ BOOL WINAPI UnregisterClassA( LPCSTR className, HINSTANCE hInstance )
> BOOL WINAPI UnregisterClassW( LPCWSTR className, HINSTANCE hInstance )
> {
> CLASS *classPtr = NULL;
> + LPCWSTR versioned_name = CLASS_GetVersionedName( className );
>
> GetDesktopWindow(); /* create the desktop window to trigger builtin class registration */
>
> @@ -685,7 +727,7 @@ BOOL WINAPI UnregisterClassW( LPCWSTR className, HINSTANCE hInstance )
> {
> req->instance = wine_server_client_ptr( hInstance );
> if (!(req->atom = get_int_atom_value(className)) && className)
> - wine_server_add_data( req, className, strlenW(className) * sizeof(WCHAR) );
> + wine_server_add_data( req, versioned_name, strlenW(versioned_name) * sizeof(WCHAR) );
> if (!wine_server_call_err( req )) classPtr = wine_server_get_ptr( reply->client_ptr );
> }
> SERVER_END_REQ;
> @@ -1192,10 +1234,15 @@ BOOL WINAPI GetClassInfoExA( HINSTANCE hInstance, LPCSTR name, WNDCLASSEXA *wc )
>
> if (!IS_INTRESOURCE(name))
> {
> + LPCWSTR versioned_name;
> WCHAR nameW[MAX_ATOM_LEN + 1];
> if (!MultiByteToWideChar( CP_ACP, 0, name, -1, nameW, sizeof(nameW)/sizeof(WCHAR) ))
> return FALSE;
> - classPtr = CLASS_FindClass( nameW, hInstance );
> + versioned_name = CLASS_GetVersionedName( nameW );
> + classPtr = CLASS_FindClass( versioned_name, hInstance );
> + /* FIXME: I believe we shouldn't fall back to the normal class. Fill
> + * free to remove it if necessary. */
> + if (!classPtr) classPtr = CLASS_FindClass( nameW, hInstance );
> }
> else classPtr = CLASS_FindClass( (LPCWSTR)name, hInstance );
>
> @@ -1230,6 +1277,7 @@ BOOL WINAPI GetClassInfoExW( HINSTANCE hInstance, LPCWSTR name, WNDCLASSEXW *wc
> {
> ATOM atom;
> CLASS *classPtr;
> + LPCWSTR versioned_name;
>
> TRACE("%p %s %p\n", hInstance, debugstr_w(name), wc);
>
> @@ -1241,10 +1289,21 @@ BOOL WINAPI GetClassInfoExW( HINSTANCE hInstance, LPCWSTR name, WNDCLASSEXW *wc
>
> if (!hInstance) hInstance = user32_module;
>
> - if (!(classPtr = CLASS_FindClass( name, hInstance )))
> + versioned_name = CLASS_GetVersionedName( name );
> +
> + classPtr = CLASS_FindClass( versioned_name, hInstance );
> + if (!classPtr)
> {
> - SetLastError( ERROR_CLASS_DOES_NOT_EXIST );
> - return FALSE;
> + /*
> + * FIXME: It's not correct to fall back to the normal class, but our
> + * comctl32 implementation depends on it...
> + */
> + classPtr = CLASS_FindClass( name, hInstance );
> + if (!classPtr)
> + {
> + SetLastError( ERROR_CLASS_DOES_NOT_EXIST );
> + return FALSE;
> + }
> }
> wc->style = classPtr->style;
> wc->lpfnWndProc = WINPROC_GetProc( classPtr->winproc, TRUE );
GetClassInfo part looks okay. It shouldn't break anything even if we had
it today, except that it adds bogus context lookup that is a waste of time.
> diff --git a/dlls/user32/user_private.h b/dlls/user32/user_private.h
> index 0b5b2ac..0c212ce 100644
> --- a/dlls/user32/user_private.h
> +++ b/dlls/user32/user_private.h
> @@ -281,6 +281,8 @@ extern void SPY_EnterMessage( INT iFlag, HWND hwnd, UINT msg, WPARAM wParam, LPA
> extern void SPY_ExitMessage( INT iFlag, HWND hwnd, UINT msg,
> LRESULT lReturn, WPARAM wParam, LPARAM lParam ) DECLSPEC_HIDDEN;
>
> +extern LPCWSTR CLASS_GetVersionedName( LPCWSTR name ) DECLSPEC_HIDDEN;
> +
> #include "pshpack1.h"
>
> typedef struct
> diff --git a/dlls/user32/win.c b/dlls/user32/win.c
> index a7be4a3..42676b5 100644
> --- a/dlls/user32/win.c
> +++ b/dlls/user32/win.c
> @@ -1740,9 +1740,11 @@ HWND WINAPI DECLSPEC_HOTPATCH CreateWindowExA( DWORD exStyle, LPCSTR className,
> if (!IS_INTRESOURCE(className))
> {
> WCHAR bufferW[256];
> + LPCWSTR versioned_name;
> if (!MultiByteToWideChar( CP_ACP, 0, className, -1, bufferW, sizeof(bufferW)/sizeof(WCHAR) ))
> return 0;
> - return wow_handlers.create_window( (CREATESTRUCTW *)&cs, bufferW, instance, FALSE );
> + versioned_name = CLASS_GetVersionedName( bufferW );
> + return wow_handlers.create_window( (CREATESTRUCTW *)&cs, versioned_name, instance, FALSE );
> }
> /* Note: we rely on the fact that CREATESTRUCTA and */
> /* CREATESTRUCTW have the same layout. */
> @@ -1760,6 +1762,7 @@ HWND WINAPI DECLSPEC_HOTPATCH CreateWindowExW( DWORD exStyle, LPCWSTR className,
> HINSTANCE instance, LPVOID data )
> {
> CREATESTRUCTW cs;
> + LPCWSTR versioned_name = CLASS_GetVersionedName( className );
>
> cs.lpCreateParams = data;
> cs.hInstance = instance;
> @@ -1774,7 +1777,7 @@ HWND WINAPI DECLSPEC_HOTPATCH CreateWindowExW( DWORD exStyle, LPCWSTR className,
> cs.lpszClass = className;
> cs.dwExStyle = exStyle;
>
> - return wow_handlers.create_window( &cs, className, instance, TRUE );
> + return wow_handlers.create_window( &cs, versioned_name, instance, TRUE );
> }
>
Looks fine, but it could go to WIN_CreateWindowEx().
There's one potentially very important thing about all this, according
to docs every winproc call is guaranteed to happen under correct
context. So basically if you send a message while particular context was
activated, if this window belongs to different thread docs imply that
context is temporary activated around winproc call on that receiving
thread. I was unable to make this happen, maybe a fresh pair of eyes
will be helpful.
More information about the wine-devel
mailing list