An OLE_HANDLE is a UINT, which is always a 32-bit integer. A HWND is a HWND__*, which is 32-bits for Win32 and 64-bits for Win64. Casting between the two seems to be ok in most cases, and they're often used interchangeably without a problem. There are several functions in WebKit that take an OLE_HANDLE* to put something into, and we're giving it a casted pointer to a HWND. This does not cause a problem on Win32 because they're equivalent, but it puts a 32-bit integer at the beginning of a 64-bit integer with my Win64 builds, which causes some crashes sometimes. This fixes those crashes.
Created attachment 223170 [details] Patch
Attachment 223170 [details] did not pass style-queue: ERROR: Tools/DumpRenderTree/win/TestRunnerWin.cpp:717: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/WebKit/win/WebCoreSupport/WebContextMenuClient.cpp:75: Wrong number of spaces before statement. (expected: 8) [whitespace/indent] [4] ERROR: Source/WebKit/win/WebCoreSupport/WebContextMenuClient.cpp:76: Wrong number of spaces before statement. (expected: 8) [whitespace/indent] [4] ERROR: Source/WebKit/win/WebCoreSupport/WebContextMenuClient.cpp:76: Wrong number of spaces before statement. (expected: 8) [whitespace/indent] [4] ERROR: Source/WebKit/win/WebDropSource.cpp:102: Wrong number of spaces before statement. (expected: 8) [whitespace/indent] [4] Total errors found: 5 in 11 files If any of these errors are false positives, please file a bug against check-webkit-style.
I'll fix the spacing, but is it ok to use (HWND) style casting like everything else in this code, or should I change it to reinterpret_cast?
(In reply to comment #3) > I'll fix the spacing, but is it ok to use (HWND) style casting like everything else in this code, or should I change it to reinterpret_cast? You should use static_cast/reinterpret_cast/const_cast, etc., as necessary to be clear about what's going on. Thanks for working on this!
Would you be ok with me using helpers like PtrToUInt in windows-specific code? http://msdn.microsoft.com/en-us/library/windows/desktop/aa384267(v=vs.85).aspx
Created attachment 223891 [details] patch
Attachment 223891 [details] did not pass style-queue: ERROR: Tools/DumpRenderTree/win/TestRunnerWin.cpp:717: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/WebKit/win/WebCoreSupport/WebContextMenuClient.cpp:75: Wrong number of spaces before statement. (expected: 8) [whitespace/indent] [4] ERROR: Source/WebKit/win/WebCoreSupport/WebContextMenuClient.cpp:76: Wrong number of spaces before statement. (expected: 8) [whitespace/indent] [4] ERROR: Source/WebKit/win/WebCoreSupport/WebContextMenuClient.cpp:76: Wrong number of spaces before statement. (expected: 8) [whitespace/indent] [4] ERROR: Source/WebKit/win/WebDropSource.cpp:102: Wrong number of spaces before statement. (expected: 8) [whitespace/indent] [4] Total errors found: 5 in 13 files If any of these errors are false positives, please file a bug against check-webkit-style.
I think the style failure is a bug, and I think reinterpret_cast is better than PtrToUInt because it does not assume that a OLE_HANDLE is an unsigned int.
Comment on attachment 223891 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=223891&action=review Looks good, but I'd like the one C-cast to be changed to a C++ cast, please! :-) > Source/WebKit/win/WebCoreSupport/WebContextMenuClient.cpp:66 > + OLE_HANDLE nativeMenu = (OLE_HANDLE)menu->platformContextMenu(); reinterpret_cast<OLE_HANDLE>
http://trac.webkit.org/changeset/163903 with fixed c-cast and removed duplicate change log entry