Cat taste and bitsiness

Add a histogram enum to determine the cat's flavor and how many bits there are
in it.

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

[Nico, 2012-11-07 21:48:54]

lgtm

https://codereview.chromium.org/11391002/diff/4001/chrome/browser/chrome_brow...
File chrome/browser/chrome_browser_main_mac.mm (right):

https://codereview.chromium.org/11391002/diff/4001/chrome/browser/chrome_brow...
chrome/browser/chrome_browser_main_mac.mm:54: enum CatSixtyFour {
"// This is one enum instead of two so that it can be used in a histogram."

https://codereview.chromium.org/11391002/diff/4001/chrome/browser/chrome_brow...
chrome/browser/chrome_browser_main_mac.mm:61: LION_32,  // Unexpected, Lion
requires a 64-bit CPU
.

https://codereview.chromium.org/11391002/diff/4001/chrome/browser/chrome_brow...
chrome/browser/chrome_browser_main_mac.mm:63: MOUNTAIN_LION_32,  // Unexpected,
Mountain Lion requires a 64-bit CPU
.

https://codereview.chromium.org/11391002/diff/4001/chrome/browser/chrome_brow...
chrome/browser/chrome_browser_main_mac.mm:67: FUTURE_CAT_32,  // Unexpected,
it's unlikely Apple will un-obsolete old CPUs
.

https://codereview.chromium.org/11391002/diff/4001/chrome/browser/chrome_brow...
chrome/browser/chrome_browser_main_mac.mm:71: SABER_TOOTHED_CAT_DUNNO,
Do you want to start these at, dunno, 32, or something, so that if we add more
cats they don't collide with the DUNNO values?

https://codereview.chromium.org/11391002/diff/4001/chrome/browser/chrome_brow...
chrome/browser/chrome_browser_main_mac.mm:82: bool cpu64 = true;
s/bool/int/ for consistency

https://codereview.chromium.org/11391002/diff/4001/chrome/browser/chrome_brow...
chrome/browser/chrome_browser_main_mac.mm:116:
IsOSDangerouslyLaterThanMountainLionForUseByCFAllocatorReplacement()) {
If future cat is deemed to be later, but not dangerously so, this will misreport
futurecat as mountain goat. I suppose we really only care about 64bittiness, so
it's probably fine.

https://codereview.chromium.org/11391002/diff/4001/chrome/browser/chrome_brow...
chrome/browser/chrome_browser_main_mac.mm:130: const int kMaxCatsAndSixtyFours =
32;
Add a "latest" element to the end of the enum, CHECK that kMaxCatsAndSixtyFours
>= kNumCatCrossBittiness

[Avi (use Gerrit), 2012-11-07 21:50:39]

https://codereview.chromium.org/11391002/diff/4001/chrome/browser/chrome_brow...
File chrome/browser/chrome_browser_main_mac.mm (right):

https://codereview.chromium.org/11391002/diff/4001/chrome/browser/chrome_brow...
chrome/browser/chrome_browser_main_mac.mm:116:
IsOSDangerouslyLaterThanMountainLionForUseByCFAllocatorReplacement()) {
Does this function need a new name, then?

[Mark Mentovai, 2012-11-07 22:08:12]

https://codereview.chromium.org/11391002/diff/4001/chrome/browser/chrome_brow...
File chrome/browser/chrome_browser_main_mac.mm (right):

https://codereview.chromium.org/11391002/diff/4001/chrome/browser/chrome_brow...
chrome/browser/chrome_browser_main_mac.mm:71: SABER_TOOTHED_CAT_DUNNO,
Nico wrote:
> Do you want to start these at, dunno, 32, or something, so that if we add more
> cats they don't collide with the DUNNO values?

The new values would have to be added at the end of the list. There isn’t really
anything wrong with that, though.

If we really wanted to keep things in order, I’d have to start with
FUTURE_CAT_32 at some higher value to leave room for whatever cat follows
Mountain Lion.

Does it matter? Doubt it. But if you feel strongly, I’ll change it.

https://codereview.chromium.org/11391002/diff/4001/chrome/browser/chrome_brow...
chrome/browser/chrome_browser_main_mac.mm:116:
IsOSDangerouslyLaterThanMountainLionForUseByCFAllocatorReplacement()) {
I was just trying to avoid touching the header. Silly.

https://codereview.chromium.org/11391002/diff/4001/chrome/browser/chrome_brow...
chrome/browser/chrome_browser_main_mac.mm:130: const int kMaxCatsAndSixtyFours =
32;
On 2012/11/07 21:48:54, Nico wrote:
> Add a "latest" element to the end of the enum, CHECK that
kMaxCatsAndSixtyFours
> >= kNumCatCrossBittiness

I hate doing that because it adds a valid (as far as the compiler is concerned)
element to the enum that shouldn’t actually be valid, but nothing enforces that
it’s unused. There isn’t a good solution to this problem. numeric_limits doesn’t
work on enums. You can’t extend enums.

[Nico, 2012-11-07 22:11:45]

(still lgtm, as is too)

https://codereview.chromium.org/11391002/diff/4001/chrome/browser/chrome_brow...
File chrome/browser/chrome_browser_main_mac.mm (right):

https://codereview.chromium.org/11391002/diff/4001/chrome/browser/chrome_brow...
chrome/browser/chrome_browser_main_mac.mm:71: SABER_TOOTHED_CAT_DUNNO,
On 2012/11/07 22:08:12, Mark Mentovai wrote:
> Nico wrote:
> > Do you want to start these at, dunno, 32, or something, so that if we add
more
> > cats they don't collide with the DUNNO values?
> 
> The new values would have to be added at the end of the list. There isn’t
really
> anything wrong with that, though.
> 
> If we really wanted to keep things in order, I’d have to start with
> FUTURE_CAT_32 at some higher value to leave room for whatever cat follows
> Mountain Lion.
> 
> Does it matter? Doubt it. But if you feel strongly, I’ll change it.

No, it's fine. Maybe add a "// Add new stuff down here" at the bottom or after
future_cat_32, but that's optional too.

https://codereview.chromium.org/11391002/diff/4001/chrome/browser/chrome_brow...
chrome/browser/chrome_browser_main_mac.mm:130: const int kMaxCatsAndSixtyFours =
32;
On 2012/11/07 22:08:12, Mark Mentovai wrote:
> On 2012/11/07 21:48:54, Nico wrote:
> > Add a "latest" element to the end of the enum, CHECK that
> kMaxCatsAndSixtyFours
> > >= kNumCatCrossBittiness
> 
> I hate doing that because it adds a valid (as far as the compiler is
concerned)
> element to the enum that shouldn’t actually be valid, but nothing enforces
that
> it’s unused.

True, but you don't switch over this enum and it likely will never be used for
anything but the histogram, so it doesn't matter here.

[Avi (use Gerrit), 2012-11-07 22:15:49]

lgtm

[Mark Mentovai, 2012-11-07 22:20:28]

OK, updated.