Update OS version functions.

Update OS version functions.

BUG=none
TEST=covered

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=226421

[Avi (use Gerrit), 2013-09-30 17:03:12]

Happy 0th anniversary! Here's a gift.

I had to rework the ifdef system a bit; the exponential expansion meant that we
weren't inlining what we could, but that required a different approach.

OTOH, half of those functions are redundant. IsOSSnowLeopard() ==
!IsOSLionOrLater(), IsOSLionOrEarlier() == !IsOSMountainLionOrLater(), etc. Or
at least if we keep them we should inline the wrappers...

[Mark Mentovai, 2013-10-01 16:15:45]

Unless I’m missing something, I think you should remove the #define/#ifdef
rework and go back to how it was before. In complicating things, I don’t think
you’ve actually caught any new cases to inline.

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#newco...
base/mac/mac_util.h:151: // Mavericks is Mac OS X 10.9, Darwin 13.
As a form of civil disobedience, can we do this without using that stupid name?
TenNine, perhaps?

https://codereview.chromium.org/25286002/diff/11001/base/mac/mac_util.h#newco...
base/mac/mac_util.h:176: inline bool IsOSSnowLeopard() { return false; }
This will already have been defined on line 168. If the DT > 10.7, then it
necessarily also >= 10.7.

https://codereview.chromium.org/25286002/diff/11001/chrome/browser/chrome_bro...
File chrome/browser/chrome_browser_main_mac.mm (right):

https://codereview.chromium.org/25286002/diff/11001/chrome/browser/chrome_bro...
chrome/browser/chrome_browser_main_mac.mm:75: // Add new constants here.
Do this?

https://codereview.chromium.org/25286002/diff/11001/content/renderer/renderer...
File content/renderer/renderer_main_platform_delegate_mac.mm (right):

https://codereview.chromium.org/25286002/diff/11001/content/renderer/renderer...
content/renderer/renderer_main_platform_delegate_mac.mm:143: // 10.6, 10.7, and
10.8. It's reportedly fixed on 10.9.
Comment needs revision? We can test this now.

[Avi (use Gerrit), 2013-10-01 16:22:33]

On 2013/10/01 16:15:45, Mark Mentovai wrote:
> go back to how it was before.

Before, as in patch set 1?

[Mark Mentovai, 2013-10-01 16:24:33]

Hard to say, because there’s stuff missing from patch set 1, but it doesn’t have
the rework so it is what I meant by “before” in that sense.

[Avi (use Gerrit), 2013-10-01 16:31:46]

On 2013/10/01 16:24:33, Mark Mentovai wrote:
> Hard to say, because there’s stuff missing from patch set 1, but it doesn’t
have
> the rework so it is what I meant by “before” in that sense.

I'm still not convinced of the need for the inlining anyway. Does it gain us
meaningful speed?

BTW, what about the comment that half our functions are redundant? If I had my
druthers I'd inline those and drop all the "inline for speed" functions.

[Avi (use Gerrit), 2013-10-01 17:07:26]

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#newco...
base/mac/mac_util.h:151: // Mavericks is Mac OS X 10.9, Darwin 13.
On 2013/10/01 16:15:45, Mark Mentovai wrote:
> As a form of civil disobedience, can we do this without using that stupid
name?
> TenNine, perhaps?

As idiotic as it is, that's its name.

https://codereview.chromium.org/25286002/diff/11001/base/mac/mac_util.h#newco...
base/mac/mac_util.h:176: inline bool IsOSSnowLeopard() { return false; }
On 2013/10/01 16:15:45, Mark Mentovai wrote:
> This will already have been defined on line 168. If the DT > 10.7, then it
> necessarily also >= 10.7.

Oh. Right.

https://codereview.chromium.org/25286002/diff/11001/chrome/browser/chrome_bro...
File chrome/browser/chrome_browser_main_mac.mm (right):

https://codereview.chromium.org/25286002/diff/11001/chrome/browser/chrome_bro...
chrome/browser/chrome_browser_main_mac.mm:75: // Add new constants here.
On 2013/10/01 16:15:45, Mark Mentovai wrote:
> Do this?

How? Does FUTURE_CAT become MAVERICKS? Or does FUTURE_CAT keep its meaning of
"unreleased/unannounced OS" and MAVERICKS become its own thing?

https://codereview.chromium.org/25286002/diff/11001/content/renderer/renderer...
File content/renderer/renderer_main_platform_delegate_mac.mm (right):

https://codereview.chromium.org/25286002/diff/11001/content/renderer/renderer...
content/renderer/renderer_main_platform_delegate_mac.mm:143: // 10.6, 10.7, and
10.8. It's reportedly fixed on 10.9.
On 2013/10/01 16:15:45, Mark Mentovai wrote:
> Comment needs revision? We can test this now.

Asked rsesek to verify; I don't have Mavericks running.

[Avi (use Gerrit), 2013-10-01 18:01:02]

https://codereview.chromium.org/25286002/diff/11001/content/renderer/renderer...
File content/renderer/renderer_main_platform_delegate_mac.mm (right):

https://codereview.chromium.org/25286002/diff/11001/content/renderer/renderer...
content/renderer/renderer_main_platform_delegate_mac.mm:143: // 10.6, 10.7, and
10.8. It's reportedly fixed on 10.9.
On 2013/10/01 17:07:27, Avi wrote:
> On 2013/10/01 16:15:45, Mark Mentovai wrote:
> > Comment needs revision? We can test this now.
> 
> Asked rsesek to verify; I don't have Mavericks running.

Verified by rsesek.

[Mark Mentovai, 2013-10-01 18:30:59]

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#newco...
base/mac/mac_util.h:162: inline bool IsOSSnowLeopard() { return
!IsOSLionOrLater(); }
I don’t like the definition of this one. It relies on more than this file
guarantees. This file, and much of base, may be used outside of Chrome, and this
definition would be tricky to find and fix for consumers that don’t mandate a
minimum of 10.6. Can you leave IsOSSnowLeopard defined as it was?

https://codereview.chromium.org/25286002/diff/35001/base/mac/mac_util.h#newco...
base/mac/mac_util.h:163: inline bool IsOSLionOrEarlier() { return
!IsOSMountainLionOrLater(); }
These two are fine.

https://codereview.chromium.org/25286002/diff/35001/chrome/browser/chrome_bro...
File chrome/browser/chrome_browser_main_mac.mm (right):

https://codereview.chromium.org/25286002/diff/35001/chrome/browser/chrome_bro...
chrome/browser/chrome_browser_main_mac.mm:65: FUTURE_CAT_32,  // Unexpected,
it's unlikely Apple will un-obsolete old CPUs.
Since there’s no 10.10 in the wild yet in any substantial number, let’s put
sealion here (and on line 73) and move the futurecats down to the “add new
constants” section. That way, old Chromes reporting based on this enum will
report 10.9, which is the highest futurecat in the wild now. Over time, as there
are fewer and fewer old Chromes, and 10.10 starts showing up, the new Chromes
will report it using the new enum value.

You will also need to edit the internal file that contains the definitions of
these values. I can point you to that file if you don’t know where it is.

https://codereview.chromium.org/25286002/diff/35001/content/renderer/renderer...
File content/renderer/renderer_main_platform_delegate_mac.mm (right):

https://codereview.chromium.org/25286002/diff/35001/content/renderer/renderer...
content/renderer/renderer_main_platform_delegate_mac.mm:144: bool needs_ime_hack
= !base::mac::IsOSMavericksOrLater();
IsOSMountainLionOrEarlier() reads better because it doesn’t require negation.

[Avi (use Gerrit), 2013-10-01 19:24:22]

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#newco...
base/mac/mac_util.h:162: inline bool IsOSSnowLeopard() { return
!IsOSLionOrLater(); }
On 2013/10/01 18:30:59, Mark Mentovai wrote:
> I don’t like the definition of this one. It relies on more than this file
> guarantees. This file, and much of base, may be used outside of Chrome, and
this
> definition would be tricky to find and fix for consumers that don’t mandate a
> minimum of 10.6. Can you leave IsOSSnowLeopard defined as it was?

Done.

https://codereview.chromium.org/25286002/diff/35001/chrome/browser/chrome_bro...
File chrome/browser/chrome_browser_main_mac.mm (right):

https://codereview.chromium.org/25286002/diff/35001/chrome/browser/chrome_bro...
chrome/browser/chrome_browser_main_mac.mm:65: FUTURE_CAT_32,  // Unexpected,
it's unlikely Apple will un-obsolete old CPUs.
On 2013/10/01 18:30:59, Mark Mentovai wrote:
> Since there’s no 10.10 in the wild yet in any substantial number, let’s put
> sealion here (and on line 73) and move the futurecats down to the “add new
> constants” section. That way, old Chromes reporting based on this enum
will
> report 10.9, which is the highest futurecat in the wild now. Over time, as
there
> are fewer and fewer old Chromes, and 10.10 starts showing up, the new Chromes
> will report it using the new enum value.
> 
> You will also need to edit the internal file that contains the definitions of
> these values. I can point you to that file if you don’t know where it is.

Done.

https://codereview.chromium.org/25286002/diff/35001/content/renderer/renderer...
File content/renderer/renderer_main_platform_delegate_mac.mm (right):

https://codereview.chromium.org/25286002/diff/35001/content/renderer/renderer...
content/renderer/renderer_main_platform_delegate_mac.mm:144: bool needs_ime_hack
= !base::mac::IsOSMavericksOrLater();
On 2013/10/01 18:30:59, Mark Mentovai wrote:
> IsOSMountainLionOrEarlier() reads better because it doesn’t require negation.

Done.

[Avi (use Gerrit), 2013-10-01 19:26:06]

Ilya, this would be the corresponding new location for CatSixtyFour.

[Mark Mentovai, 2013-10-01 20:25:37]

Why is this showing up as a pure addition in
tools/metrics/histograms/histograms.xml? We have a definition of this enum in
histograms.xml already somewhere. Are there multiple histogram.xmls now, with
this being a move from one file to another?

https://codereview.chromium.org/25286002/diff/43001/chrome/browser/chrome_bro...
File chrome/browser/chrome_browser_main_mac.mm (right):

https://codereview.chromium.org/25286002/diff/43001/chrome/browser/chrome_bro...
chrome/browser/chrome_browser_main_mac.mm:78: // As new versions of Mac OS X are
released with sillier and sillier names,
:)

[Avi (use Gerrit), 2013-10-01 20:27:05]

On 2013/10/01 20:25:37, Mark Mentovai wrote:
> Are there multiple histogram.xmls now, with
> this being a move from one file to another?

Yes. This is the open-source one.

[Mark Mentovai, 2013-10-01 20:28:15]

LGTM

[Avi (use Gerrit), 2013-10-01 20:30:46]

Nico, for the file in chrome/browser that you own.

[Nico, 2013-10-01 20:57:45]

lgtm

https://codereview.chromium.org/25286002/diff/43001/chrome/browser/chrome_bro...
File chrome/browser/chrome_browser_main_mac.mm (right):

https://codereview.chromium.org/25286002/diff/43001/chrome/browser/chrome_bro...
chrome/browser/chrome_browser_main_mac.mm:74: FUTURE_CAT_32,  // Unexpected,
it's unlikely Apple will un-obsolete old CPUs.
Should this be FUTURE_CALI now? :-P

[Ilya Sherman, 2013-10-01 21:04:31]

histograms.xml lgtm, thanks.

[commit-bot: I haz the power, 2013-10-01 23:05:36]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/avi@chromium.org/25286002/43001

[commit-bot: I haz the power, 2013-10-02 07:42:03]

Change committed as 226421