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

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 year, 9 months ago by Yosi_UTC9
Modified:
1 year, 9 months 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 #

Messages

Total messages: 18 (0 generated)
Yosi_UTC9
Could you review this patch? Thanks in advance.
1 year, 9 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 ...
1 year, 9 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 ...
1 year, 9 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
1 year, 9 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
1 year, 9 months ago (2013-10-25 04:02:28 UTC) #5
Yosi_UTC9
+tkent as owner of Source/Platform Could you review this patch? Thanks in advance.
1 year, 9 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 ...
1 year, 9 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.
1 year, 9 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 ...
1 year, 9 months ago (2013-10-25 04:50:39 UTC) #9
Yosi_UTC9
PTAL
1 year, 9 months ago (2013-10-25 08:14:18 UTC) #10
dominicc
On 2013/10/25 08:14:18, Yoshifumi Inoue wrote: > PTAL DBC: You might like to change your ...
1 year, 9 months ago (2013-10-25 08:34:02 UTC) #11
Yosi_UTC9
On 2013/10/25 08:34:02, dominicc wrote: > On 2013/10/25 08:14:18, Yoshifumi Inoue wrote: > > PTAL ...
1 year, 9 months 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 ...
1 year, 9 months 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 ...
1 year, 9 months ago (2013-10-28 02:21:19 UTC) #14
Yosi_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 ...
1 year, 9 months ago (2013-10-28 04:30:59 UTC) #15
tkent
lgtm
1 year, 9 months 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
1 year, 9 months ago (2013-10-28 05:18:11 UTC) #17
commit-bot: I haz the power
1 year, 9 months ago (2013-10-28 07:00:51 UTC) #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 5fa3ca5