[webkit-reviews] review denied: [Bug 93275] Windows 64 bit compliance : [Attachment 156716] Patch

[bugzilla-daemon at webkit.org]

Brent Fulgham <bfulgham at webkit.org> has denied Alex Christensen
<alex.christensen at flexsim.com>'s request for review:
Bug 93275: Windows 64 bit compliance
https://bugs.webkit.org/show_bug.cgi?id=93275

Attachment 156716: Patch
https://bugs.webkit.org/attachment.cgi?id=156716&action=review

------- Additional Comments from Brent Fulgham <bfulgham at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=156716&action=review


This is a good start, but needs a few adjustments before landing.  I think
Patrick's idea to use the WindowsExtras.h code is good, but I think you will
need to add a static value to represent the token you want to get from
GetWindowLongPtr.

>> Tools/WinLauncher/WinLauncher.cpp:232
>> +	::SetWindowLongPtr(hMainWnd, GWLP_WNDPROC,
reinterpret_cast<LONG_PTR>(WndProc));
> 
> i'd like to see usage of
http://trac.webkit.org/browser/trunk/Source/WebCore/platform/win/WindowsExtras.
h instead, which adds additional abstraction required for OS(WINCE).

How would using that help with GWL_WNDPROC versus GWLP_WNDPROC?  GWL_WNDPROC
resolves to -4 for win32 (and to nothing in 64-bit), while GWLP_WNDPROC is -4
for 64-bit, and undefined for 32-bit.

I guess we could add a static const "s_wndProc" member to the WindowsExtras.h
that would be defined as -4.

>> Tools/win/DLLLauncher/DLLLauncherMain.cpp:198
>> +#endif
> 
> they look very similar

Maybe this should be "#ifdef _M_AMD64 || _WIN64"?