Bug 59702 - [PATCH] Add missing _WIN32_WINNT and WINVER definitions
Summary: [PATCH] Add missing _WIN32_WINNT and WINVER definitions
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Windows XP
: P2 Normal
Assignee: Nobody
Depends on:
Reported: 2011-04-28 08:34 PDT by Kalev Lember
Modified: 2011-07-08 11:59 PDT (History)
5 users (show)

See Also:

Add missing _WIN32_WINNT and WINVER definitions (1.71 KB, patch)
2011-04-28 08:44 PDT, Kalev Lember
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ec2-cq-02 (1.27 MB, application/zip)
2011-07-08 07:53 PDT, WebKit Review Bot
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Kalev Lember 2011-04-28 08:34:15 PDT
Moved _WIN32_WINNT and WINVER definitions to config.h so that they are
available for all source files.

In particular, wtf/FastMalloc.cpp uses CreateTimerQueueTimer and
DeleteTimerQueueTimer which are both guarded by
#if (_WIN32_WINNT >= 0x0500)
in MinGW headers.

Fixes the following error with MinGW cross compiler in Fedora:
Source/JavaScriptCore/wtf/FastMalloc.cpp: In member function 'void WTF::TCMalloc_PageHeap::scheduleScavenger()':
Source/JavaScriptCore/wtf/FastMalloc.cpp:1575:133: error: 'CreateTimerQueueTimer' was not declared in this scope
Source/JavaScriptCore/wtf/FastMalloc.cpp: In member function 'void WTF::TCMalloc_PageHeap::suspendScavenger()':
Source/JavaScriptCore/wtf/FastMalloc.cpp:1590:51: error: 'DeleteTimerQueueTimer' was not declared in this scope
make[1]: *** [Source/JavaScriptCore/wtf/libJavaScriptCore_la-FastMalloc.lo] Error 1
Comment 1 Kalev Lember 2011-04-28 08:44:11 PDT
Created attachment 91498 [details]
Add missing _WIN32_WINNT and WINVER definitions
Comment 2 Erik van Pienbroek 2011-04-28 10:19:14 PDT
Additional note: the compile error mentioned here only occurs when using the mingw.org toolchain. If the mingw-w64 toolchain is used then this error doesn't happen as _WIN32_WINNT is set to 0x0500 in their headers by default
Comment 3 Eric Seidel (no email) 2011-04-28 11:36:11 PDT
Comment on attachment 91498 [details]
Add missing _WIN32_WINNT and WINVER definitions

I'm not sure this belongs in config.h.
Comment 4 Steve Falkenburg 2011-04-28 11:49:42 PDT
For Apple's Windows port, we define these in a global vsprops file (common.vsprops).
Also, these don't match the versions we're using in that file.
We use:

Comment 5 Kalev Lember 2011-04-28 11:55:19 PDT
For what it's worth, both WebCore/config.h and WebKit2/config.h already contain these definitions, so the patch makes it more similar to these two.
Comment 6 Kalev Lember 2011-05-03 00:43:25 PDT
Any other ideas where these definitions should go instead of config.h?

I think defining it only in the vsprops file isn't enough, as that file is specific to Visual Studio build system, whereas there are ports that use other build systems, for instance autotools. Each individual port might choose to tighten the deps even more (e.g. requiring WINVER=502 as Apple's Windows port is doing), but there should still be _minimal_ WINVER/_WIN32_WINNT definitions somewhere in buildsystem-agnostic code.

I am not entirely sure either where the right place for these definitions should be, but the attached patch at least brings JavaScriptCore in line with the rest of the projects and fixes a build failure with webkitgtk port.
Comment 7 Kalev Lember 2011-07-08 06:56:23 PDT
Any chance this patch could go in?

We definitely need _WIN32_WINNT and WINVER defined also somewhere else than Visual Studio common.vsprops (the GTK+ port uses autotools instead), and in my opinion config.h is a good place for that.
Comment 8 WebKit Review Bot 2011-07-08 07:53:34 PDT
Comment on attachment 91498 [details]
Add missing _WIN32_WINNT and WINVER definitions

Rejecting attachment 91498 [details] from commit-queue.

Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=ec2-cq-02', '--port..." exit_code: 2

Last 500 characters of output:

=> Tests that will only be fixed if they crash (WONTFIX) (8133):
      1 test timed out           ( 0.0%)
    111 text diff mismatch       ( 1.4%)
   7972 skipped                  (98.0%)

Unexpected flakiness: text diff mismatch (3)
  media/media-blocked-by-beforeload.html = TEXT PASS
  media/media-blocked-by-willsendrequest.html = TEXT PASS
  perf/array-reverse.html = TEXT PASS

Regressions: Unexpected text diff mismatch : (1)
  inspector/styles/styles-url-linkify.html = TEXT

Full output: http://queues.webkit.org/results/9003305
Comment 9 WebKit Review Bot 2011-07-08 07:53:39 PDT
Created attachment 100118 [details]
Archive of layout-test-results from ec2-cq-02

The attached test failures were seen while running run-webkit-tests on the commit-queue.
Bot: ec2-cq-02  Port: Chromium  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Comment 10 Kalev Lember 2011-07-08 11:11:45 PDT
Comment on attachment 91498 [details]
Add missing _WIN32_WINNT and WINVER definitions

I don't understand how this can cause a regression in inspector/styles/styles-url-linkify.html; perhaps it was something unrelated? Could you please set the commit-queue flag again to retry?
Comment 11 WebKit Review Bot 2011-07-08 11:59:40 PDT
Comment on attachment 91498 [details]
Add missing _WIN32_WINNT and WINVER definitions

Clearing flags on attachment: 91498

Committed r90649: <http://trac.webkit.org/changeset/90649>
Comment 12 WebKit Review Bot 2011-07-08 11:59:50 PDT
All reviewed patches have been landed.  Closing bug.