Bug 55226

Clarity: When running a 32-bit browser app (via WOW64): Add "WOW64" token after OS version When running a 64-bit browser app (natively): Add "Win64; x64" tokens after OS version I don't know if anyone is building 64-bit WebKit variants yet.

Created attachment 85282 [details] Patch v1

Attachment 85282 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 Source/WebCore/platform/win/SystemInfo.cpp:149: system_info is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/WebCore/platform/win/SystemInfo.cpp:162: An else if statement should be written as an if statement when the prior "if" concludes with a return, break, continue or goto statement. [readability/control_flow] [4] Total errors found: 2 in 3 files If any of these errors are false positives, please file a bug against check-webkit-style.

Comment on attachment 85282 [details] Patch v1 View in context: https://bugs.webkit.org/attachment.cgi?id=85282&action=review > Source/WebCore/platform/win/SystemInfo.cpp:123 > + HMODULE kernel32Module = GetModuleHandle(L"kernel32.dll"); So we always bind to 64-bit version of the API? Shouldn't we be using TEXT macro? > Source/WebCore/platform/win/SystemInfo.cpp:124 > + if (kernel32Module) { It seems like that an early exist will be cleaner. > Source/WebCore/platform/win/SystemInfo.cpp:145 > + if (kernel32Module) { Ditto about early exist. > Source/WebCore/platform/win/SystemInfo.cpp:164 > + else if (processorArchitecture() == PROCESSOR_ARCHITECTURE_AMD64) Nit: no else here.

Created attachment 85284 [details] Patch v2 Fixes style errors and other problems.

Comment on attachment 85284 [details] Patch v2 View in context: https://bugs.webkit.org/attachment.cgi?id=85284&action=review Looks sane to me! > Source/WebCore/platform/win/SystemInfo.cpp:122 > + if (!initialized) { > + initialized = true; Can't we exit early here instead? as in if (initialized) return wow64; But not strongly opinionated. > Source/WebCore/platform/win/SystemInfo.cpp:130 > + wow64 = ((*isWOW64Process)(GetCurrentProcess(), &result) && result); I'd prefer isWOW64Process(GetCurrentProcess(), &result) && result; but there doesn't seem to be an explicit rule stated anywhere.

(In reply to comment #6) > > Source/WebCore/platform/win/SystemInfo.cpp:122 > > + if (!initialized) { > > + initialized = true; > > Can't we exit early here instead? as in > if (initialized) > return wow64; > But not strongly opinionated. After we chatted I'm leaving this alone. > > Source/WebCore/platform/win/SystemInfo.cpp:130 > > + wow64 = ((*isWOW64Process)(GetCurrentProcess(), &result) && result); > > I'd prefer isWOW64Process(GetCurrentProcess(), &result) && result; but there doesn't seem to be an explicit rule stated anywhere. Changed. Also changed function pointers to not use explicit-deref form ("(*ptr)(args)"). Will commit when bots are green.

Fixed in r80693.