[Mac] Add the OS version check functions for 10.8.

[Mac] Add the OS version check functions for 10.8.

BUG=none
TEST=Careful review by Mark. And a unittest.


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=126709

[Mark Mentovai, 2012-03-14 03:30:38]

http://codereview.chromium.org/9706001/diff/1/base/mac/mac_util.h
File base/mac/mac_util.h (right):

http://codereview.chromium.org/9706001/diff/1/base/mac/mac_util.h#newcode146
base/mac/mac_util.h:146: BASE_EXPORT bool IsOSMountainLionOrEarlier();
This should not be present. Instead, provide IsOSLaterThanMountainLion, and
remove IsOSLaterThanLion.

[Robert Sesek, 2012-03-14 14:46:22]

http://codereview.chromium.org/9706001/diff/1/base/mac/mac_util.h
File base/mac/mac_util.h (right):

http://codereview.chromium.org/9706001/diff/1/base/mac/mac_util.h#newcode146
base/mac/mac_util.h:146: BASE_EXPORT bool IsOSMountainLionOrEarlier();
On 2012/03/14 03:30:38, Mark Mentovai wrote:
> This should not be present. Instead, provide IsOSLaterThanMountainLion, and
> remove IsOSLaterThanLion.

I think it should be present. It will just have to be added later, so might as
well future proof.

IsOSLaterThanLion is currently used by a few (1) things, which is why it's not
changed here. But I think that the function should be renamed to IsOSFutureCat()
because that's what all the callsites are expressing with this function. And
then we don't have to rename it with each new OS release.

[Mark Mentovai, 2012-03-14 15:24:52]

If we need to keep IsOSLaterThanLion to handle that one call site for now,
OK, keep it.

We shouldn't add IsOSMountainLionOrEarlier, though. Logically, it has the
same meaning as not-IsOSLaterThanMountainLion, but semantically, it's
nowhere near as clear for the one expected use. Providing
IsOSMountainLionOrEarlier will just encourage people to call it, but
there's really no reason for anyone to call it because nobody has any idea
what might follow Mountain Lion.
On Mar 14, 2012 10:46 AM, <rsesek@chromium.org> wrote:

>
>
http://codereview.chromium.**org/9706001/diff/1/base/mac/**mac_util.h<http://...
> File base/mac/mac_util.h (right):
>
> http://codereview.chromium.**org/9706001/diff/1/base/mac/**
>
mac_util.h#newcode146<http://codereview.chromium.org/9706001/diff/1/base/mac/mac_util.h#newcode146>
> base/mac/mac_util.h:146: BASE_EXPORT bool IsOSMountainLionOrEarlier();
> On 2012/03/14 03:30:38, Mark Mentovai wrote:
>
>> This should not be present. Instead, provide
>>
> IsOSLaterThanMountainLion, and
>
>> remove IsOSLaterThanLion.
>>
>
> I think it should be present. It will just have to be added later, so
> might as well future proof.
>
> IsOSLaterThanLion is currently used by a few (1) things, which is why
> it's not changed here. But I think that the function should be renamed
> to IsOSFutureCat() because that's what all the callsites are expressing
> with this function. And then we don't have to rename it with each new OS
> release.
>
>
http://codereview.chromium.**org/9706001/<http://codereview.chromium.org/9706...
>

[Robert Sesek, 2012-03-14 15:35:20]

On 2012/03/14 15:24:52, Mark Mentovai wrote:
> If we need to keep IsOSLaterThanLion to handle that one call site for now,
> OK, keep it.
> 
> We shouldn't add IsOSMountainLionOrEarlier, though. Logically, it has the
> same meaning as not-IsOSLaterThanMountainLion, but semantically, it's
> nowhere near as clear for the one expected use. Providing
> IsOSMountainLionOrEarlier will just encourage people to call it, but
> there's really no reason for anyone to call it because nobody has any idea
> what might follow Mountain Lion.
> On Mar 14, 2012 10:46 AM, <mailto:rsesek@chromium.org> wrote:
> 
> >
> >
>
http://codereview.chromium.**org/9706001/diff/1/base/mac/**mac_util.h%3Chttp:...>
> > File base/mac/mac_util.h (right):
> >
> > http://codereview.chromium.**org/9706001/diff/1/base/mac/**
> >
>
mac_util.h#newcode146<http://codereview.chromium.org/9706001/diff/1/base/mac/mac_util.h#newcode146>
> > base/mac/mac_util.h:146: BASE_EXPORT bool IsOSMountainLionOrEarlier();
> > On 2012/03/14 03:30:38, Mark Mentovai wrote:
> >
> >> This should not be present. Instead, provide
> >>
> > IsOSLaterThanMountainLion, and
> >
> >> remove IsOSLaterThanLion.
> >>
> >
> > I think it should be present. It will just have to be added later, so
> > might as well future proof.
> >
> > IsOSLaterThanLion is currently used by a few (1) things, which is why
> > it's not changed here. But I think that the function should be renamed
> > to IsOSFutureCat() because that's what all the callsites are expressing
> > with this function. And then we don't have to rename it with each new OS
> > release.
> >
> >
>
http://codereview.chromium.**org/9706001/%3Chttp://codereview.chromium.org/97...>
> >

[Robert Sesek, 2012-03-14 15:37:45]

On 2012/03/14 15:24:52, Mark Mentovai wrote:
> If we need to keep IsOSLaterThanLion to handle that one call site for now,
> OK, keep it.
> 
> We shouldn't add IsOSMountainLionOrEarlier, though. Logically, it has the
> same meaning as not-IsOSLaterThanMountainLion, but semantically, it's
> nowhere near as clear for the one expected use. Providing
> IsOSMountainLionOrEarlier will just encourage people to call it, but
> there's really no reason for anyone to call it because nobody has any idea
> what might follow Mountain Lion.

It has the same meaning *now* but it won't in the future. It doesn't matter if
we have an idea as to what might follow Mountain Lion, just that something will,
and we have no indication that 10.8 will be the last release.

I also dislike the name IsOSLaterThanMountainLion() for the reason I wrote
above. The intent of this function is to test if we're running on an
unknown/unreleased OS.

[Mark Mentovai, 2012-03-14 15:49:10]

The sole intent of the function is to test whether we know what the
internals of CFAllocator are. The answer is flexible according to what
we know about CFAllocator. It’s distinct from whether an OS is known
or released.

[Robert Sesek, 2012-03-14 16:11:12]

On 2012/03/14 15:49:10, Mark Mentovai wrote:
> The sole intent of the function is to test whether we know what the
> internals of CFAllocator are. The answer is flexible according to what
> we know about CFAllocator. It’s distinct from whether an OS is known
> or released.

Since it is just that callsite, why don't we just remove the function altogether
and just let the allocator internals check the Darwin/OS minor version manually?

[Mark Mentovai, 2012-03-14 16:16:53]

Because we intentionally don’t expose MacOSXMinorVersion.

[Robert Sesek, 2012-03-14 16:48:45]

IsOSDangerouslyLaterThanMountainLionForUseByCFAllocatorReplacement()

[Mark Mentovai, 2012-03-14 16:55:40]

http://codereview.chromium.org/9706001/diff/5003/base/mac/mac_util.h
File base/mac/mac_util.h (right):

http://codereview.chromium.org/9706001/diff/5003/base/mac/mac_util.h#newcode146
base/mac/mac_util.h:146: BASE_EXPORT bool IsOSMountainLionOrEarlier();
xxx

http://codereview.chromium.org/9706001/diff/5003/base/mac/mac_util.h#newcode193
base/mac/mac_util.h:193: inline bool IsOSMountainLionOrEarlier() { return false;
}
xxx

http://codereview.chromium.org/9706001/diff/5003/base/mac/mac_util.mm
File base/mac/mac_util.mm (right):

http://codereview.chromium.org/9706001/diff/5003/base/mac/mac_util.mm#newcode635
base/mac/mac_util.mm:635: bool IsOSMountainLionOrEarlier() {
xxx

http://codereview.chromium.org/9706001/diff/5003/base/process_util_mac.mm
File base/process_util_mac.mm (right):

http://codereview.chromium.org/9706001/diff/5003/base/process_util_mac.mm#new...
base/process_util_mac.mm:721: return
!base::mac::IsOSDangerouslyLaterThanMountainLionForUseByCFAllocatorReplacement();
This is not right (yet) because we haven’t guessed what the internals look like
(until Avi finishes what he’s doing).

Leave IsOSLaterThanLion in place for now, and then let Avi remove it when he
starts using this really awesomely-named function here.

[Avi (use Gerrit), 2012-03-14 16:59:22]

On 2012/03/14 16:55:40, Mark Mentovai wrote:
> Leave IsOSLaterThanLion in place for now, and then let Avi remove it when he
> starts using this really awesomely-named function here.

Can I please just get IsOSLaterThanMountainLion()? That's all I want for
Christmas.

[Avi (use Gerrit), 2012-03-14 17:00:52]

http://codereview.chromium.org/9706001/diff/2003/base/mac/mac_util.h
File base/mac/mac_util.h (right):

http://codereview.chromium.org/9706001/diff/2003/base/mac/mac_util.h#newcode152
base/mac/mac_util.h:152: bool
IsOSDangerouslyLaterThanMountainLionForUseByCFAllocatorReplacement();
Alternatively, drop this and give me IsOSMountainLionOrEarlier().

[Robert Sesek, 2012-03-14 17:03:24]

http://codereview.chromium.org/9706001/diff/5003/base/process_util_mac.mm
File base/process_util_mac.mm (right):

http://codereview.chromium.org/9706001/diff/5003/base/process_util_mac.mm#new...
base/process_util_mac.mm:721: return
!base::mac::IsOSDangerouslyLaterThanMountainLionForUseByCFAllocatorReplacement();
On 2012/03/14 16:55:41, Mark Mentovai wrote:
> This is not right (yet) because we haven’t guessed what the internals look
like
> (until Avi finishes what he’s doing).
> 
> Leave IsOSLaterThanLion in place for now, and then let Avi remove it when he
> starts using this really awesomely-named function here.

Done.

[Mark Mentovai, 2012-03-14 17:37:54]

lgtm

[commit-bot: I haz the power, 2012-03-14 17:41:07]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rsesek@chromium.org/9706001/4002

[commit-bot: I haz the power, 2012-03-14 19:58:23]

Change committed as 126709