Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(157)

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

Created:
7 years, 2 months ago by yosin_UTC9
Modified:
7 years, 1 month ago
CC:
blink-reviews
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) Patch
M Source/core/platform/ScrollbarThemeWin.cpp View 1 2 3 chunks +11 lines, -11 lines 0 comments Download
M Source/core/platform/graphics/chromium/GlyphPageTreeNodeChromiumWin.cpp View 1 2 1 chunk +1 line, -1 line 0 comments Download
M Source/core/rendering/RenderThemeChromiumFontProviderWin.cpp View 1 2 1 chunk +1 line, -1 line 0 comments Download
M Source/platform/win/SystemInfo.h View 1 2 1 chunk +1 line, -32 lines 0 comments Download
M Source/platform/win/SystemInfo.cpp View 1 2 3 1 chunk +22 lines, -38 lines 0 comments Download

Messages

Total messages: 18 (0 generated)
yosin_UTC9
Could you review this patch? Thanks in advance.
7 years, 2 months ago (2013-10-24 09:52:47 UTC) #1
Hajime Morrita
rubberstamp LGTM for make it work. yukawa@ might know what's the best way to handle ...
7 years, 2 months ago (2013-10-24 10:29:38 UTC) #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 ...
7 years, 2 months ago (2013-10-24 11:35:38 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/yosin@chromium.org/39693002/120001
7 years, 2 months ago (2013-10-25 03:29:57 UTC) #4
commit-bot: I haz the power
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
7 years, 2 months ago (2013-10-25 04:02:28 UTC) #5
yosin_UTC9
+tkent as owner of Source/Platform Could you review this patch? Thanks in advance.
7 years, 2 months ago (2013-10-25 04:23:24 UTC) #6
tkent
The usage scenario of windowsVersion() is just "XP or not". How about replacing windowsVersion() with ...
7 years, 2 months ago (2013-10-25 04:42:46 UTC) #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.
7 years, 2 months ago (2013-10-25 04:49:04 UTC) #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 ...
7 years, 2 months ago (2013-10-25 04:50:39 UTC) #9
yosin_UTC9
PTAL
7 years, 1 month ago (2013-10-25 08:14:18 UTC) #10
dominicc (has gone to gerrit)
On 2013/10/25 08:14:18, Yoshifumi Inoue wrote: > PTAL DBC: You might like to change your ...
7 years, 1 month ago (2013-10-25 08:34:02 UTC) #11
yosin_UTC9
On 2013/10/25 08:34:02, dominicc wrote: > On 2013/10/25 08:14:18, Yoshifumi Inoue wrote: > > PTAL ...
7 years, 1 month ago (2013-10-25 08:36:28 UTC) #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 ...
7 years, 1 month ago (2013-10-27 21:22:28 UTC) #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 ...
7 years, 1 month ago (2013-10-28 02:21:19 UTC) #14
yosin_UTC9
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 ...
7 years, 1 month ago (2013-10-28 04:30:59 UTC) #15
tkent
lgtm
7 years, 1 month ago (2013-10-28 05:17:56 UTC) #16
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/yosin@chromium.org/39693002/440001
7 years, 1 month ago (2013-10-28 05:18:11 UTC) #17
commit-bot: I haz the power
7 years, 1 month ago (2013-10-28 07:00:51 UTC) #18
Message was sent while issue was closed.
Change committed as 160698

Powered by Google App Engine
This is Rietveld 408576698