Bug 76781

Created attachment 123445 [details] Patch

Comment on attachment 123445 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=123445&action=review > Source/WebCore/platform/win/RunLoopWin.cpp:44 > +#if OS(WINCE) > + LONG longPtr = ::GetWindowLong(hWnd, 0); > +#else > LONG_PTR longPtr = ::GetWindowLongPtr(hWnd, 0); > - > +#endif Maybe we should take a different approach here. wtf/MathExtras.h and wtf/StringExtras.h have shim functions that provide compatibility between various compilers/runtimes. Maybe we should have a WindowsExtras.h (maybe in WTF, maybe in WebCore/platform) that does the same thing to bridge the gap between WinCE and other versions of Windows? For example, WindowsExtras.h could have an inline GetWIndowLongPtr function that is defined only on WinCE that just calls through to GetWindowLong. We could do the same thing for SetWindowLong[Ptr]. And we could even have a WinCE-only HWND_MESSAGE that is just 0. What do you think? Seems like that would make it easier to fix these kinds of issues in the future (by just including WindowsExtras.h).

(In reply to comment #2) > (From update of attachment 123445 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=123445&action=review > > > Source/WebCore/platform/win/RunLoopWin.cpp:44 > > +#if OS(WINCE) > > + LONG longPtr = ::GetWindowLong(hWnd, 0); > > +#else > > LONG_PTR longPtr = ::GetWindowLongPtr(hWnd, 0); > > - > > +#endif > > Maybe we should take a different approach here. wtf/MathExtras.h and wtf/StringExtras.h have shim functions that provide compatibility between various compilers/runtimes. Maybe we should have a WindowsExtras.h (maybe in WTF, maybe in WebCore/platform) that does the same thing to bridge the gap between WinCE and other versions of Windows? For example, WindowsExtras.h could have an inline GetWIndowLongPtr function that is defined only on WinCE that just calls through to GetWindowLong. We could do the same thing for SetWindowLong[Ptr]. And we could even have a WinCE-only HWND_MESSAGE that is just 0. What do you think? Seems like that would make it easier to fix these kinds of issues in the future (by just including WindowsExtras.h). I'd prefere a solution with WindowsExtras.h too, but I'm not a big fan of "implementing" a function with the same name on WinCE. IMHO a wrapper function witch does the correct stuff for both platform would be better. This function can do the cast to LONG_PTR on Win32 then too. Defining HWND_MESSAGE to 0 should be ok and can be done via an #ifndef HWND_MESSAGE.

Comment on attachment 123445 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=123445&action=review >>> Source/WebCore/platform/win/RunLoopWin.cpp:44 >>> +#endif >> >> Maybe we should take a different approach here. wtf/MathExtras.h and wtf/StringExtras.h have shim functions that provide compatibility between various compilers/runtimes. Maybe we should have a WindowsExtras.h (maybe in WTF, maybe in WebCore/platform) that does the same thing to bridge the gap between WinCE and other versions of Windows? For example, WindowsExtras.h could have an inline GetWIndowLongPtr function that is defined only on WinCE that just calls through to GetWindowLong. We could do the same thing for SetWindowLong[Ptr]. And we could even have a WinCE-only HWND_MESSAGE that is just 0. What do you think? Seems like that would make it easier to fix these kinds of issues in the future (by just including WindowsExtras.h). > > I'd prefere a solution with WindowsExtras.h too, but I'm not a big fan of "implementing" a function with the same name on WinCE. IMHO a wrapper function witch does the correct stuff for both platform would be better. This function can do the cast to LONG_PTR on Win32 then too. Defining HWND_MESSAGE to 0 should be ok and can be done via an #ifndef HWND_MESSAGE. What specifically concerns you about implementing GetWindowLongPtr on WinCE? A differently-named-and-typed wrapper function could be good if it made the code clearer; what do you suggest as a name and signature? We tend not to use macros for constants, so I'd recommend a "const HWND HWND_MESSAGE = 0;", similar to constants like piFloat in MathExtras.h.

Created attachment 126485 [details] Patch

Comment on attachment 126485 [details] Patch Clearing flags on attachment: 126485 Committed r107585: <http://trac.webkit.org/changeset/107585>

All reviewed patches have been landed. Closing bug.