[webkit-reviews] review denied: [Bug 35607] Allow building smoothly on win32 and win64 using GCC : [Attachment 52433] patch with fixes required by QtWebkit for mingw64 -WebCore

[bugzilla-daemon at webkit.org]

Adam Roben (aroben) <aroben at apple.com> has denied vanboxem.ruben at gmail.com's
request for review:
Bug 35607: Allow building smoothly on win32 and win64 using GCC
https://bugs.webkit.org/show_bug.cgi?id=35607

Attachment 52433: patch with fixes required by QtWebkit for mingw64 -WebCore
https://bugs.webkit.org/attachment.cgi?id=52433&action=review

------- Additional Comments from Adam Roben (aroben) <aroben at apple.com>
> +++ WebCore/ChangeLog (working copy)
> @@ -23807,6 +23807,26 @@
>	   (WebCore::GraphicsContext::fillRect): Ditto
>	   (WebCore::GraphicsContext::strokeRect): Ditto
>  
> +2010-03-25  Ruben Van Boxem	<vanboxem.ruben at gmail.com>
> +
> +	   Reviewed by NOBODY (OOPS!).
> +
> +	   Mingw-w64 fixes for WebCore
> +	   https://bugs.webkit.org/show_bug.cgi?id=35607
> +
> +	   No new tests.
> +
> +	   * platform/Arena.h: fix pointer precision loss error with mingw-w64
> +	   * platform/graphics/transforms/TransformationMatrix.h: let mingw64
use msvc codepath
> +	   * platform/text/TextStream.cpp: let mingw-w64 use msvc codepath
> +	   * platform/text/TextStream.h: let mingw-w64 use msvc codepath
> +	   * plugins/PluginView.cpp: fix pointer precision loss error with
mingw-w64
> +	   (WebCore::PluginView::stop):
> +	   * plugins/win/PluginViewWin.cpp: let mingw-w64 use msvc codepath
> +	   (WebCore::PluginView::paintIntoTransformedContext):
> +	   (WebCore::PluginView::setNPWindowRect):
> +	   (WebCore::PluginView::platformStart):
> +

You should put your entry at the top of the ChangeLog file, and update the
date.

> +++ WebCore/platform/Arena.h	(working copy)
> @@ -44,7 +44,7 @@
>  
>  namespace WebCore {
>  
> -typedef unsigned long uword;
> +typedef uintptr_t uword;

I know that some uses of uword are meant to hold pointers, but what about
ArenaPool::mask?

> -#if OS(WINDOWS) && PLATFORM(X86_64) && COMPILER(MSVC)
> +#if OS(WINDOWS) && CPU(X86_64)
>  TextStream& TextStream::operator<<(__int64 i)
>  {
>      char buffer[printBufferSize];
> Index: WebCore/platform/text/TextStream.h
> ===================================================================
> --- WebCore/platform/text/TextStream.h	(revision 56567)
> +++ WebCore/platform/text/TextStream.h	(working copy)
> @@ -45,7 +45,7 @@ public:
>      TextStream& operator<<(const char*);
>      TextStream& operator<<(void*);
>      TextStream& operator<<(const String&);
> -#if OS(WINDOWS) && PLATFORM(X86_64) && COMPILER(MSVC)
> +#if OS(WINDOWS) && CPU(X86_64)
>      TextStream& operator<<(unsigned __int64);
>      TextStream& operator<<(__int64);
>  #endif

Will this screw up the 64-bit Qt Windows build? (Is there such a thing?)

> @@ -544,7 +544,7 @@ void PluginView::paintIntoTransformedCon
>  
>      NPEvent npEvent;
>      npEvent.event = WM_WINDOWPOSCHANGED;
> -    npEvent.lParam = reinterpret_cast<uint32>(&windowpos);
> +    npEvent.lParam = reinterpret_cast<uintptr_t>(&windowpos);
>      npEvent.wParam = 0;
>  
>      dispatchNPEvent(npEvent);
> @@ -552,7 +552,7 @@ void PluginView::paintIntoTransformedCon
>      setNPWindowRect(frameRect());
>  
>      npEvent.event = WM_PAINT;
> -    npEvent.wParam = reinterpret_cast<uint32>(hdc);
> +    npEvent.wParam = reinterpret_cast<uintptr_t>(hdc);

These fields are declared to be of type uint32, so these casts look wrong.

> @@ -829,7 +829,7 @@ void PluginView::setNPWindowRect(const I
>  #else
>	   WNDPROC currentWndProc =
(WNDPROC)GetWindowLongPtr(platformPluginWidget(), GWLP_WNDPROC);
>	   if (currentWndProc != PluginViewWndProc)
> -	       m_pluginWndProc =
(WNDPROC)SetWindowLongPtr(platformPluginWidget(), GWLP_WNDPROC,
(LONG)PluginViewWndProc);
> +	       m_pluginWndProc =
(WNDPROC)SetWindowLongPtr(platformPluginWidget(), GWLP_WNDPROC,
(intptr_t)PluginViewWndProc);

That should be LONG_PTR, not intptr_t.