Summary: | Add appropriate UA string tags for running on Windows x64 | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Peter Kasting <pkasting> | ||||||
Component: | Platform | Assignee: | Peter Kasting <pkasting> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | aroben, dbates, webkit.review.bot | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | PC | ||||||||
OS: | Windows XP | ||||||||
Bug Depends on: | 55979 | ||||||||
Bug Blocks: | 54556 | ||||||||
Attachments: |
|
Description
Peter Kasting
2011-02-25 09:35:35 PST
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. |