Fix incorrect IPC_ENUM_TRAITS_MAX_VALUE for NTPLoggingEventType

Fix incorrect IPC_ENUM_TRAITS_MAX_VALUE for NTPLoggingEventType

The comment for IPC_ENUM_TRAITS_MAX_VALUE says you should pass the largest
possible value, inclusive. But NTP_NUM_EVENT_TYPES is one larger than the
largest possible NTPLoggingEventType.

Committed: https://crrev.com/c78637e034c8df88abd12e7c11c1b7f12a1d1eb7
Cr-Commit-Position: refs/heads/master@{#300248}

[johnme, 2014-10-17 14:20:18]

johnme@chromium.org changed reviewers:
+ tsepez@chromium.org

[johnme, 2014-10-17 14:20:18]

In general this seems like an easy mistake to make, as we have very little
consistency in the codebase.

For example, the UMA_HISTOGRAM_ENUMERATION macro requires that you pass the
number of enum values (max+1). So things get confusing when sharing enums
between UMA and IPC.

And naming conventions vary wildly, including _LAST, _MAX, or _MAX_ENUM for an
enum value equal to the last value, and _COUNT, _NUM_TYPES, or NUM_ for an enum
value one greater than the last real value. Meanwhile, some enums have neither
of these, and the last value just gets hardcoded into these macros.

[Mike West, 2014-10-17 14:26:31]

mkwst@chromium.org changed reviewers:
+ mkwst@chromium.org

[Mike West, 2014-10-17 14:26:31]

LGTM.

You're right, this is an easy mistake to make. Given the heterogeneous nature of
our enums, it's not really clear how we'd be able to automagically fix it.

Standardizing on a naming scheme, however, seems like a totally reasonable first
step. To that end, perhaps it makes sense to add another value to this enum like
NTP_EVENT_TYPE_LAST which points to NTP_MOUSEOVER, and use that in the macro
rather than NUM_EVENT_TYPES - 1.

[johnme, 2014-10-17 14:41:55]

johnme@chromium.org changed reviewers:
+ jam@chromium.org

[johnme, 2014-10-17 14:41:56]

> Perhaps it makes sense to add another value to this enum like
> NTP_EVENT_TYPE_LAST which points to NTP_MOUSEOVER, and use that in the macro
> rather than NUM_EVENT_TYPES - 1.

Sure, done.

[jam, 2014-10-17 17:02:49]

lgtm

[johnme, 2014-10-17 17:03:07]

The CQ bit was checked by johnme@chromium.org

[commit-bot: I haz the power, 2014-10-17 17:03:32]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/662823002/20001

[Tom Sepez, 2014-10-17 17:15:40]

LGTM.  Thanks for this fix.

[commit-bot: I haz the power, 2014-10-17 23:04:31]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-10-17 23:04:35]

Exceeded time limit waiting for builds to trigger.

[johnme, 2014-10-20 10:49:49]

The CQ bit was checked by johnme@chromium.org

[commit-bot: I haz the power, 2014-10-20 10:50:34]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/662823002/20001

[commit-bot: I haz the power, 2014-10-20 11:24:52]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2014-10-20 11:25:37]

Patchset 2 (id:??) landed as
https://crrev.com/c78637e034c8df88abd12e7c11c1b7f12a1d1eb7
Cr-Commit-Position: refs/heads/master@{#300248}