Chromium Code Reviews
Help | Chromium Project | Sign in
(25)

Issue 39693002: Introduce isWindowsVistaOrGreater() as replacement of windowsVersion() (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
5 months, 3 weeks ago by Yoshi
Modified:
5 months, 2 weeks ago
Reviewers:
morrita1, tkent, yukawa
CC:
blink-reviews_chromium.org
Visibility:
Public.

Description

Introduce isWindowsVistaOrGreater() as replacement of windowsVersion()

This patch is a part of preparation of migrating Win SDK 8.1.

This patch introduces isWindowsVistaOrGreater() as replacement of windowsVersion() to avoid using deprecated Windows API GetVersionEx(), which is deprecated since Win SDK 8.1+, by using new Windows API IsWindowsVistaOrGreater() for Win SDK 8.1 build or VerifyVersionInfo() before Win SDK 8.1.

BUG=n/a
TEST=n/a; no behavior changes

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=160698

Patch Set 1 #

Total comments: 1

Patch Set 2 : 2013-10-25T11:32:55 #

Patch Set 3 : 2013-10-25T16:43:30 #

Total comments: 4

Patch Set 4 : 2013-10-28T12:54:24 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+36 lines, -83 lines) Lint Patch
M Source/core/platform/ScrollbarThemeWin.cpp View 1 2 3 chunks +11 lines, -11 lines 0 comments 6 errors Download
M Source/core/platform/graphics/chromium/GlyphPageTreeNodeChromiumWin.cpp View 1 2 1 chunk +1 line, -1 line 0 comments 1 errors Download
M Source/core/rendering/RenderThemeChromiumFontProviderWin.cpp View 1 2 1 chunk +1 line, -1 line 0 comments 0 errors Download
M Source/platform/win/SystemInfo.h View 1 2 1 chunk +1 line, -32 lines 0 comments ? errors Download
M Source/platform/win/SystemInfo.cpp View 1 2 3 1 chunk +22 lines, -38 lines 0 comments 3 errors Download
Commit:

Messages

Total messages: 18
Yoshi
Could you review this patch? Thanks in advance.
5 months, 3 weeks ago #1
morrita1
rubberstamp LGTM for make it work. yukawa@ might know what's the best way to handle ...
5 months, 3 weeks ago #2
yukawa
lgtm https://codereview.chromium.org/39693002/diff/1/Source/platform/win/SystemInfo.cpp File Source/platform/win/SystemInfo.cpp (left): https://codereview.chromium.org/39693002/diff/1/Source/platform/win/SystemInfo.cpp#oldcode44 Source/platform/win/SystemInfo.cpp:44: GetVersionEx(reinterpret_cast<OSVERSIONINFO*>(&versionInfo)); Just FYI, you can also suppress warning ...
5 months, 3 weeks ago #3
I haz the power (commit-bot)
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/yosin@chromium.org/39693002/120001
5 months, 3 weeks ago #4
I haz the power (commit-bot)
Retried try job too often on blink_presubmit for step(s) presubmit http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=blink_presubmit&number=9816
5 months, 3 weeks ago #5
Yoshi
+tkent as owner of Source/Platform Could you review this patch? Thanks in advance.
5 months, 3 weeks ago #6
tkent
The usage scenario of windowsVersion() is just "XP or not". How about replacing windowsVersion() with ...
5 months, 3 weeks ago #7
tkent
Wow, I found IsWindowsVistaOrGreater(). http://msdn.microsoft.com/en-us/library/windows/desktop/dn424965(v=vs.85).aspx We can remove SystemInfo.{cpp,h} entirely.
5 months, 3 weeks ago #8
tkent
On 2013/10/25 04:49:04, tkent wrote: > Wow, I found IsWindowsVistaOrGreater(). > http://msdn.microsoft.com/en-us/library/windows/desktop/dn424965%28v=vs.85%29.aspx > > We ...
5 months, 3 weeks ago #9
Yoshi
PTAL
5 months, 3 weeks ago #10
dominicc
On 2013/10/25 08:14:18, Yoshifumi Inoue wrote: > PTAL DBC: You might like to change your ...
5 months, 3 weeks ago #11
Yoshi
On 2013/10/25 08:34:02, dominicc wrote: > On 2013/10/25 08:14:18, Yoshifumi Inoue wrote: > > PTAL ...
5 months, 3 weeks ago #12
tkent
https://codereview.chromium.org/39693002/diff/310007/Source/platform/win/SystemInfo.cpp File Source/platform/win/SystemInfo.cpp (right): https://codereview.chromium.org/39693002/diff/310007/Source/platform/win/SystemInfo.cpp#newcode38 Source/platform/win/SystemInfo.cpp:38: // Taken from VersionHelpers.h in Windows SDK 8.1+. Does ...
5 months, 3 weeks ago #13
yukawa
https://codereview.chromium.org/39693002/diff/310007/Source/platform/win/SystemInfo.cpp File Source/platform/win/SystemInfo.cpp (right): https://codereview.chromium.org/39693002/diff/310007/Source/platform/win/SystemInfo.cpp#newcode64 Source/platform/win/SystemInfo.cpp:64: cachedIsWindowsVistaOrGreater = IsWindowsVistaOrGreater(); IsWindowsVersionOrGreater is just a helper wrapper ...
5 months, 3 weeks ago #14
Yoshi
PTAL Now no copyright issue. https://codereview.chromium.org/39693002/diff/310007/Source/platform/win/SystemInfo.cpp File Source/platform/win/SystemInfo.cpp (right): https://codereview.chromium.org/39693002/diff/310007/Source/platform/win/SystemInfo.cpp#newcode38 Source/platform/win/SystemInfo.cpp:38: // Taken from VersionHelpers.h ...
5 months, 3 weeks ago #15
tkent
lgtm
5 months, 2 weeks ago #16
I haz the power (commit-bot)
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/yosin@chromium.org/39693002/440001
5 months, 2 weeks ago #17
I haz the power (commit-bot)
5 months, 2 weeks ago #18
Message was sent while issue was closed.
Change committed as 160698
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 1275:d14800f88434