Here are a few fixes from my Windows 64-bit build. I'll post most of the others as separate bugs. https://github.com/achristensen07/webkit64prep
Created attachment 156707 [details] Patch
Created attachment 156709 [details] patch patch
Created attachment 156716 [details] Patch
Comment on attachment 156716 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=156716&action=review > Tools/WinLauncher/WinLauncher.cpp:232 > - DefWebKitProc = reinterpret_cast<WNDPROC>(::GetWindowLongPtr(hMainWnd, GWL_WNDPROC)); > - ::SetWindowLongPtr(hMainWnd, GWL_WNDPROC, reinterpret_cast<LONG_PTR>(WndProc)); > + DefWebKitProc = reinterpret_cast<WNDPROC>(::GetWindowLongPtr(hMainWnd, GWLP_WNDPROC)); > + ::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). > Tools/win/DLLLauncher/DLLLauncherMain.cpp:191 > +#ifdef _M_AMD64 > + const char* entryPointName = "_dllLauncherEntryPoint"; > +#else > const char* entryPointName = "_dllLauncherEntryPoint@8"; > +#endif isn't there a possibility to export them as extern C without any additional decoration? > Tools/win/DLLLauncher/DLLLauncherMain.cpp:198 > +#ifdef _M_AMD64 > const char* entryPointName = "_dllLauncherEntryPoint@16"; > +#else > + const char* entryPointName = "_dllLauncherEntryPoint@16"; > +#endif they look very similar
(In reply to comment #4) > 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). That's a good idea, but it would still need to use GWLP_WNDPROC instead of GWL_WNDPROC. > isn't there a possibility to export them as extern C without any additional decoration? I don't know why the symbol names are in the source code, but they don't work with the 64-bit dlls, hence the additional 64-bit symbols. > they look very similar They are the same. The first shouldn't have @16. My bad.
> 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 does it know what #if OS(WINCE) means? Whatever is done in WebKit to define that isn't done in WinLauncher. > isn't there a possibility to export them as extern C without any additional decoration? This isn't exporting, it's the parameter given to GetProcAddress to link to an already exported symbol.
Comment on attachment 156716 [details] Patch 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"?
(In reply to comment #7) > 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. Try adding #include "WindowsExtras.h" to WinLauncher.cpp. It doesn't know what to do with the #if OS(WINCE) directive. > I guess we could add a static const "s_wndProc" member to the WindowsExtras.h that would be defined as -4. GWLP_WNDPROC is defined for 32-bit and 64-bit. GWL_WNDPROC is not defined for 64-bit. > Maybe this should be "#ifdef _M_AMD64 || _WIN64"? Good idea. #if defined _M_AMD64 || defined _WIN64
(In reply to comment #8) > (In reply to comment #7) > > > 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. > > Try adding #include "WindowsExtras.h" to WinLauncher.cpp. It doesn't know what to do with the #if OS(WINCE) directive. Ah, right. That's internal to WebCore. Why not do an #if/def around it using the _M_AMD64 and _WIN64. WINCE won't define these, and can stick to the old way. Then you don't need to touch WindowsExtras at all, and it should build fine on all systems. -Brent
Created attachment 158031 [details] Patch
Comment on attachment 158031 [details] Patch Thanks for your patience revising the patch. These look great.
Comment on attachment 158031 [details] Patch Clearing flags on attachment: 158031 Committed r125505: <http://trac.webkit.org/changeset/125505>
All reviewed patches have been landed. Closing bug.