[PATCH] user32: Add support for class versions
Ivan Akulinchev
ivan.akulinchev at gmail.com
Sat Nov 5 11:31:54 CDT 2016
Sorry, forgot to CC wine-devel at winehq.org...
-------------
Hi, thanks for a reply!
On Saturday, 5 November 2016 17:56:47 CET you wrote:
> 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.
>
This line ("<windowClass>Static</windowClass>") tells Windows to use the
"6.0.0.0!Static" class instead of "Static". However this class does not
exist and all applications crash.
If you remove this line, the standard Static class from user32 is used.
> > 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.
>
The problem here is that it is impossible to create a versioned class
for #32770, and therefore we can not create a global class - only a
local one. I believe this stuff is a part of the undocumented Theme
Hooks API, and not a part of comctl32...
theming.c is needed because nobody wants to copy & paste the
corresponding code from user32, as Microsoft did. Maybe we can create a
private library containing the base classes, used by both libraries?
> > 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.
>
Yes, I was very surprised by this behaviour.
> > +
> > + 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?
>
I didn't test the ASCII functions on a real Windows instance, but why do
you think this shouldn't have the class redirections 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.
>
Well, I had to disassemble this chunk of code, but I can suggest you to
do the following test, which doesn't violate the Clean Room design.
1. On Windows XP or higher, try to register a "Button" class with
CS_GLOBALCLASS. This should fail.
2. Add a manifest file like:
<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
<assembly xmlns="urn:schemas-microsoft-com:asm.v1"
manifestVersion="1.0">
<assemblyIdentity type="win32"
name="Wine.TestMe"
version="1.0.0.0" />
<file name="yourapp.exe">
<windowClass>Button</windowClass>
</file>
</assembly>
3. Launch your application again. Now the "Button" class is registered,
and it is actually "1.0.0.0!Button". You can create a new window with
both names.
This test shows, that RegisterClassExW takes care about the prefix
itself, and comctl32 doesn't.
> 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.
>
See above.
> > @@ -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.
>
CLASS_GetVersionedName should return the original class name, if no
activation context exist. The only reason I wrote a fall-back routine,
because comctl32 needs to get a pointer to a windowproc from user32.
I believe Windows doesn't has this "fall-back" mode.
> > 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.
I tried something like this, but FindActCtxSectionStringW fails if you
call it too early, what is the case for the internal functions.
Therefore I decided to place it higher.
More information about the wine-devel
mailing list