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

Issue 25286002: Update OS version functions. (Closed)

Created:
7 years, 2 months ago by Avi (use Gerrit)
Modified:
7 years, 2 months ago
CC:
chromium-reviews, joi+watch-content_chromium.org, darin-cc_chromium.org, erikwright+watch_chromium.org, jam
Visibility:
Public.

Description

Update OS version functions. BUG=none TEST=covered Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=226421

Patch Set 1 #

Patch Set 2 : more? better? #

Patch Set 3 : fix #

Total comments: 9

Patch Set 4 : cleaner #

Patch Set 5 : verified #

Total comments: 7

Patch Set 6 : uma #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+110 lines, -28 lines) Patch
M base/mac/mac_util.h View 1 2 3 4 5 3 chunks +24 lines, -5 lines 0 comments Download
M base/mac/mac_util.mm View 1 2 3 4 5 3 chunks +16 lines, -9 lines 0 comments Download
M base/mac/mac_util_unittest.mm View 1 chunk +24 lines, -4 lines 0 comments Download
M base/process/memory_mac.mm View 1 chunk +1 line, -1 line 0 comments Download
M base/test/expectations/expectation.cc View 1 chunk +2 lines, -0 lines 0 comments Download
M chrome/browser/chrome_browser_main_mac.mm View 1 2 3 4 5 2 chunks +15 lines, -6 lines 2 comments Download
M content/renderer/renderer_main_platform_delegate_mac.mm View 1 2 3 4 5 2 chunks +3 lines, -3 lines 0 comments Download
M tools/metrics/histograms/histograms.xml View 2 chunks +25 lines, -0 lines 0 comments Download

Messages

Total messages: 18 (0 generated)
Avi (use Gerrit)
Happy 0th anniversary! Here's a gift. I had to rework the ifdef system a bit; ...
7 years, 2 months ago (2013-09-30 17:03:12 UTC) #1
Mark Mentovai
Unless I’m missing something, I think you should remove the #define/#ifdef rework and go back ...
7 years, 2 months ago (2013-10-01 16:15:45 UTC) #2
Avi (use Gerrit)
On 2013/10/01 16:15:45, Mark Mentovai wrote: > go back to how it was before. Before, ...
7 years, 2 months ago (2013-10-01 16:22:33 UTC) #3
Mark Mentovai
Hard to say, because there’s stuff missing from patch set 1, but it doesn’t have ...
7 years, 2 months ago (2013-10-01 16:24:33 UTC) #4
Avi (use Gerrit)
On 2013/10/01 16:24:33, Mark Mentovai wrote: > Hard to say, because there’s stuff missing from ...
7 years, 2 months ago (2013-10-01 16:31:46 UTC) #5
Avi (use Gerrit)
https://codereview.chromium.org/25286002/diff/11001/base/mac/mac_util.h File base/mac/mac_util.h (right): https://codereview.chromium.org/25286002/diff/11001/base/mac/mac_util.h#newcode151 base/mac/mac_util.h:151: // Mavericks is Mac OS X 10.9, Darwin 13. ...
7 years, 2 months ago (2013-10-01 17:07:26 UTC) #6
Avi (use Gerrit)
https://codereview.chromium.org/25286002/diff/11001/content/renderer/renderer_main_platform_delegate_mac.mm File content/renderer/renderer_main_platform_delegate_mac.mm (right): https://codereview.chromium.org/25286002/diff/11001/content/renderer/renderer_main_platform_delegate_mac.mm#newcode143 content/renderer/renderer_main_platform_delegate_mac.mm:143: // 10.6, 10.7, and 10.8. It's reportedly fixed on ...
7 years, 2 months ago (2013-10-01 18:01:02 UTC) #7
Mark Mentovai
https://codereview.chromium.org/25286002/diff/35001/base/mac/mac_util.h File base/mac/mac_util.h (right): https://codereview.chromium.org/25286002/diff/35001/base/mac/mac_util.h#newcode162 base/mac/mac_util.h:162: inline bool IsOSSnowLeopard() { return !IsOSLionOrLater(); } I don’t ...
7 years, 2 months ago (2013-10-01 18:30:59 UTC) #8
Avi (use Gerrit)
https://codereview.chromium.org/25286002/diff/35001/base/mac/mac_util.h File base/mac/mac_util.h (right): https://codereview.chromium.org/25286002/diff/35001/base/mac/mac_util.h#newcode162 base/mac/mac_util.h:162: inline bool IsOSSnowLeopard() { return !IsOSLionOrLater(); } On 2013/10/01 ...
7 years, 2 months ago (2013-10-01 19:24:22 UTC) #9
Avi (use Gerrit)
Ilya, this would be the corresponding new location for CatSixtyFour.
7 years, 2 months ago (2013-10-01 19:26:06 UTC) #10
Mark Mentovai
Why is this showing up as a pure addition in tools/metrics/histograms/histograms.xml? We have a definition ...
7 years, 2 months ago (2013-10-01 20:25:37 UTC) #11
Avi (use Gerrit)
On 2013/10/01 20:25:37, Mark Mentovai wrote: > Are there multiple histogram.xmls now, with > this ...
7 years, 2 months ago (2013-10-01 20:27:05 UTC) #12
Mark Mentovai
LGTM
7 years, 2 months ago (2013-10-01 20:28:15 UTC) #13
Avi (use Gerrit)
Nico, for the file in chrome/browser that you own.
7 years, 2 months ago (2013-10-01 20:30:46 UTC) #14
Nico
lgtm https://codereview.chromium.org/25286002/diff/43001/chrome/browser/chrome_browser_main_mac.mm File chrome/browser/chrome_browser_main_mac.mm (right): https://codereview.chromium.org/25286002/diff/43001/chrome/browser/chrome_browser_main_mac.mm#newcode74 chrome/browser/chrome_browser_main_mac.mm:74: FUTURE_CAT_32, // Unexpected, it's unlikely Apple will un-obsolete ...
7 years, 2 months ago (2013-10-01 20:57:45 UTC) #15
Ilya Sherman
histograms.xml lgtm, thanks.
7 years, 2 months ago (2013-10-01 21:04:31 UTC) #16
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/avi@chromium.org/25286002/43001
7 years, 2 months ago (2013-10-01 23:05:36 UTC) #17
commit-bot: I haz the power
7 years, 2 months ago (2013-10-02 07:42:03 UTC) #18
Message was sent while issue was closed.
Change committed as 226421

Powered by Google App Engine
This is Rietveld 408576698