Added a mechanism to detect missing EnumTable entries.

Change-Id: I5b06e37fe7f3029a670f917443c688be5be01da6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1589099
Commit-Queue: John Williams <jrw@chromium.org>
Reviewed-by: mark a. foltz <mfoltz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#659662}

chrome/browser/media/router/providers/cast/cast_internal_message_util.cc

@@ -24,34 +24,41 @@ using media_router::CastInternalMessage;
 
 template <>
 const EnumTable<CastInternalMessage::Type>
-    EnumTable<CastInternalMessage::Type>::instance({
-        {CastInternalMessage::Type::kClientConnect, "client_connect"},
-        {CastInternalMessage::Type::kAppMessage, "app_message"},
-        {CastInternalMessage::Type::kV2Message, "v2_message"},
-        {CastInternalMessage::Type::kLeaveSession, "leave_session"},
-        {CastInternalMessage::Type::kReceiverAction, "receiver_action"},
-        {CastInternalMessage::Type::kNewSession, "new_session"},
-        {CastInternalMessage::Type::kUpdateSession, "update_session"},
-        {CastInternalMessage::Type::kError, "error"},
-        // kOther deliberately omitted
-    });
+    EnumTable<CastInternalMessage::Type>::instance(
+        {
+            {CastInternalMessage::Type::kClientConnect, "client_connect"},
+            {CastInternalMessage::Type::kAppMessage, "app_message"},
+            {CastInternalMessage::Type::kV2Message, "v2_message"},
+            {CastInternalMessage::Type::kLeaveSession, "leave_session"},
+            {CastInternalMessage::Type::kReceiverAction, "receiver_action"},
+            {CastInternalMessage::Type::kNewSession, "new_session"},
+            {CastInternalMessage::Type::kUpdateSession, "update_session"},
+            {CastInternalMessage::Type::kError, "error"},
+            {CastInternalMessage::Type::kOther},
+        },
+        CastInternalMessage::Type::kMaxValue);
 
 template <>
-const EnumTable<CastInternalMessage::ErrorCode> EnumTable<
-    CastInternalMessage::ErrorCode>::instance({
-    {CastInternalMessage::ErrorCode::kInternalError, "internal_error"},
-    {CastInternalMessage::ErrorCode::kCancel, "cancel"},
-    {CastInternalMessage::ErrorCode::kTimeout, "timeout"},
-    {CastInternalMessage::ErrorCode::kApiNotInitialized, "api_not_initialized"},
-    {CastInternalMessage::ErrorCode::kInvalidParameter, "invalid_parameter"},
-    {CastInternalMessage::ErrorCode::kExtensionNotCompatible,
-     "extension_not_compatible"},
-    {CastInternalMessage::ErrorCode::kReceiverUnavailable,
-     "receiver_unavailable"},
-    {CastInternalMessage::ErrorCode::kSessionError, "session_error"},
-    {CastInternalMessage::ErrorCode::kChannelError, "channel_error"},
-    {CastInternalMessage::ErrorCode::kLoadMediaFailed, "load_media_failed"},
-});
+const EnumTable<CastInternalMessage::ErrorCode>
+    EnumTable<CastInternalMessage::ErrorCode>::instance(
+        {
+            {CastInternalMessage::ErrorCode::kInternalError, "internal_error"},
+            {CastInternalMessage::ErrorCode::kCancel, "cancel"},
+            {CastInternalMessage::ErrorCode::kTimeout, "timeout"},
+            {CastInternalMessage::ErrorCode::kApiNotInitialized,
+             "api_not_initialized"},
+            {CastInternalMessage::ErrorCode::kInvalidParameter,
+             "invalid_parameter"},
+            {CastInternalMessage::ErrorCode::kExtensionNotCompatible,
+             "extension_not_compatible"},
+            {CastInternalMessage::ErrorCode::kReceiverUnavailable,
+             "receiver_unavailable"},
+            {CastInternalMessage::ErrorCode::kSessionError, "session_error"},
+            {CastInternalMessage::ErrorCode::kChannelError, "channel_error"},
+            {CastInternalMessage::ErrorCode::kLoadMediaFailed,
+             "load_media_failed"},
+        },
+        CastInternalMessage::ErrorCode::kMaxValue);
 
 }  // namespace cast_util
 

chrome/browser/media/router/providers/cast/cast_internal_message_util.h

@@ -36,8 +36,9 @@ class CastInternalMessage {
     kUpdateSession,   // Message sent by MRP to inform SDK client of updated
                       // session.
     kError,
-    kOther  // All other types of messages which are not considered
-            // part of communication with Cast SDK.
+    kOther,  // All other types of messages which are not considered
+             // part of communication with Cast SDK.
+    kMaxValue = kOther,
   };
 
   // Errors that may be returned by the SDK.
@@ -54,6 +55,7 @@ class CastInternalMessage {
     kSessionError,  // A session could not be created, or a session was invalid.
     kChannelError,  // A channel to the receiver is not available.
     kLoadMediaFailed,  // Load media failed.
+    kMaxValue = kLoadMediaFailed,
   };
 
   // Returns a CastInternalMessage for |message|, or nullptr is |message| is not

chrome/common/media_router/providers/cast/cast_media_source.cc

@@ -27,17 +27,21 @@ using media_router::AutoJoinPolicy;
 using media_router::DefaultActionPolicy;
 
 template <>
-const EnumTable<AutoJoinPolicy> EnumTable<AutoJoinPolicy>::instance({
-    {AutoJoinPolicy::kPageScoped, "page_scoped"},
-    {AutoJoinPolicy::kTabAndOriginScoped, "tab_and_origin_scoped"},
-    {AutoJoinPolicy::kOriginScoped, "origin_scoped"},
-});
+const EnumTable<AutoJoinPolicy> EnumTable<AutoJoinPolicy>::instance(
+    {
+        {AutoJoinPolicy::kPageScoped, "page_scoped"},
+        {AutoJoinPolicy::kTabAndOriginScoped, "tab_and_origin_scoped"},
+        {AutoJoinPolicy::kOriginScoped, "origin_scoped"},
+    },
+    AutoJoinPolicy::kMaxValue);
 
 template <>
-const EnumTable<DefaultActionPolicy> EnumTable<DefaultActionPolicy>::instance({
-    {DefaultActionPolicy::kCreateSession, "create_session"},
-    {DefaultActionPolicy::kCastThisTab, "cast_this_tab"},
-});
+const EnumTable<DefaultActionPolicy> EnumTable<DefaultActionPolicy>::instance(
+    {
+        {DefaultActionPolicy::kCreateSession, "create_session"},
+        {DefaultActionPolicy::kCastThisTab, "cast_this_tab"},
+    },
+    DefaultActionPolicy::kMaxValue);
 
 template <>
 const EnumTable<CastDeviceCapability> EnumTable<CastDeviceCapability>::instance(
@@ -50,7 +54,7 @@ const EnumTable<CastDeviceCapability> EnumTable<CastDeviceCapability>::instance(
         {CastDeviceCapability::VIDEO_OUT, "video_out"},
         // NONE deliberately omitted
     },
-    UnsortedEnumTable);
+    NonConsecutiveEnumTable);
 
 }  // namespace cast_util
 

chrome/common/media_router/providers/cast/cast_media_source.h

@@ -75,12 +75,16 @@ struct CastAppInfo {
 enum class AutoJoinPolicy {
   // No automatic connection.  This is the default when no policy is specified.
   kPageScoped,
+
   // Automatically connects when the session was started with the same app ID,
   // in the same tab and page origin.
   kTabAndOriginScoped,
+
   // Automatically connects when the session was started with the same app ID
   // and the same page origin (regardless of tab).
   kOriginScoped,
+
+  kMaxValue = kOriginScoped,
 };
 
 // Default action policy determines when the SDK will automatically create a
@@ -96,6 +100,8 @@ enum class DefaultActionPolicy {
   // being cast.  The Cast dialog prompts the user to mirror the tab (mirror,
   // not cast, despite the name).
   kCastThisTab,
+
+  kMaxValue = kCastThisTab,
 };
 
 // Tests whether a sender specified by (origin1, tab_id1) is allowed by |policy|

components/cast_channel/cast_message_util.cc

@@ -24,48 +24,57 @@ namespace cast_util {
 using namespace cast_channel;
 
 template <>
-const EnumTable<CastMessageType> EnumTable<CastMessageType>::instance({
-    {CastMessageType::kPing, "PING"},
-    {CastMessageType::kPong, "PONG"},
-    {CastMessageType::kGetAppAvailability, "GET_APP_AVAILABILITY"},
-    {CastMessageType::kReceiverStatusRequest, "GET_STATUS"},
-    {CastMessageType::kConnect, "CONNECT"},
-    {CastMessageType::kCloseConnection, "CLOSE"},
-    {CastMessageType::kBroadcast, "APPLICATION_BROADCAST"},
-    {CastMessageType::kLaunch, "LAUNCH"},
-    {CastMessageType::kStop, "STOP"},
-    {CastMessageType::kReceiverStatus, "RECEIVER_STATUS"},
-    {CastMessageType::kMediaStatus, "MEDIA_STATUS"},
-    {CastMessageType::kLaunchError, "LAUNCH_ERROR"},
-});
+const EnumTable<CastMessageType> EnumTable<CastMessageType>::instance(
+    {
+        {CastMessageType::kPing, "PING"},
+        {CastMessageType::kPong, "PONG"},
+        {CastMessageType::kGetAppAvailability, "GET_APP_AVAILABILITY"},
+        {CastMessageType::kReceiverStatusRequest, "GET_STATUS"},
+        {CastMessageType::kConnect, "CONNECT"},
+        {CastMessageType::kCloseConnection, "CLOSE"},
+        {CastMessageType::kBroadcast, "APPLICATION_BROADCAST"},
+        {CastMessageType::kLaunch, "LAUNCH"},
+        {CastMessageType::kStop, "STOP"},
+        {CastMessageType::kReceiverStatus, "RECEIVER_STATUS"},
+        {CastMessageType::kMediaStatus, "MEDIA_STATUS"},
+        {CastMessageType::kLaunchError, "LAUNCH_ERROR"},
+        {CastMessageType::kOther},
+    },
+    CastMessageType::kMaxValue);
 
 template <>
-const EnumTable<V2MessageType> EnumTable<V2MessageType>::instance({
-    {V2MessageType::kEditTracksInfo, "EDIT_TRACKS_INFO"},
-    {V2MessageType::kGetStatus, "GET_STATUS"},
-    {V2MessageType::kLoad, "LOAD"},
-    {V2MessageType::kMediaGetStatus, "MEDIA_GET_STATUS"},
-    {V2MessageType::kMediaSetVolume, "MEDIA_SET_VOLUME"},
-    {V2MessageType::kPause, "PAUSE"},
-    {V2MessageType::kPlay, "PLAY"},
-    {V2MessageType::kPrecache, "PRECACHE"},
-    {V2MessageType::kQueueInsert, "QUEUE_INSERT"},
-    {V2MessageType::kQueueLoad, "QUEUE_LOAD"},
-    {V2MessageType::kQueueRemove, "QUEUE_REMOVE"},
-    {V2MessageType::kQueueReorder, "QUEUE_REORDER"},
-    {V2MessageType::kQueueUpdate, "QUEUE_UPDATE"},
-    {V2MessageType::kSeek, "SEEK"},
-    {V2MessageType::kSetVolume, "SET_VOLUME"},
-    {V2MessageType::kStop, "STOP"},
-    {V2MessageType::kStopMedia, "STOP_MEDIA"},
-});
+const EnumTable<V2MessageType> EnumTable<V2MessageType>::instance(
+    {
+        {V2MessageType::kEditTracksInfo, "EDIT_TRACKS_INFO"},
+        {V2MessageType::kGetStatus, "GET_STATUS"},
+        {V2MessageType::kLoad, "LOAD"},
+        {V2MessageType::kMediaGetStatus, "MEDIA_GET_STATUS"},
+        {V2MessageType::kMediaSetVolume, "MEDIA_SET_VOLUME"},
+        {V2MessageType::kPause, "PAUSE"},
+        {V2MessageType::kPlay, "PLAY"},
+        {V2MessageType::kPrecache, "PRECACHE"},
+        {V2MessageType::kQueueInsert, "QUEUE_INSERT"},
+        {V2MessageType::kQueueLoad, "QUEUE_LOAD"},
+        {V2MessageType::kQueueRemove, "QUEUE_REMOVE"},
+        {V2MessageType::kQueueReorder, "QUEUE_REORDER"},
+        {V2MessageType::kQueueUpdate, "QUEUE_UPDATE"},
+        {V2MessageType::kSeek, "SEEK"},
+        {V2MessageType::kSetVolume, "SET_VOLUME"},
+        {V2MessageType::kStop, "STOP"},
+        {V2MessageType::kStopMedia, "STOP_MEDIA"},
+        {V2MessageType::kOther},
+    },
+    V2MessageType::kMaxValue);
 
 template <>
 const EnumTable<GetAppAvailabilityResult>
-    EnumTable<GetAppAvailabilityResult>::instance({
-        {GetAppAvailabilityResult::kAvailable, "APP_AVAILABLE"},
-        {GetAppAvailabilityResult::kUnavailable, "APP_UNAVAILABLE"},
-    });
+    EnumTable<GetAppAvailabilityResult>::instance(
+        {
+            {GetAppAvailabilityResult::kAvailable, "APP_AVAILABLE"},
+            {GetAppAvailabilityResult::kUnavailable, "APP_UNAVAILABLE"},
+            {GetAppAvailabilityResult::kUnknown},
+        },
+        GetAppAvailabilityResult::kMaxValue);
 
 }  // namespace cast_util
 
@@ -485,7 +494,7 @@ LaunchSessionResponse GetLaunchSessionResponse(const base::Value& payload) {
   if (!type_value)
     return LaunchSessionResponse();
 
-  CastMessageType type = CastMessageTypeFromString(type_value->GetString());
+  const auto type = CastMessageTypeFromString(type_value->GetString());
   if (type != CastMessageType::kReceiverStatus &&
       type != CastMessageType::kLaunchError)
     return LaunchSessionResponse();

components/cast_channel/cast_message_util.h

@@ -48,7 +48,8 @@ enum class CastMessageType {
   kReceiverStatus,
   kMediaStatus,
   kLaunchError,
-  kOther  // Add new types above |kOther|.
+  kOther,  // Add new types above |kOther|.
+  kMaxValue = kOther,
 };
 
 enum class V2MessageType {
@@ -69,7 +70,8 @@ enum class V2MessageType {
   kSetVolume,
   kStop,
   kStopMedia,
-  kOther  // Add new types above |kOther|.
+  kOther,  // Add new types above |kOther|.
+  kMaxValue = kOther,
 };
 
 // Checks if the contents of |message_proto| are valid.
@@ -198,6 +200,7 @@ enum class GetAppAvailabilityResult {
   kAvailable,
   kUnavailable,
   kUnknown,
+  kMaxValue = kUnknown,
 };
 
 const char* ToString(GetAppAvailabilityResult result);
@@ -213,7 +216,7 @@ GetAppAvailabilityResult GetAppAvailabilityResultFromResponse(
 
 // Result of a session launch.
 struct LaunchSessionResponse {
-  enum Result { kOk, kError, kTimedOut, kUnknown };
+  enum Result { kOk, kError, kTimedOut, kUnknown, kMaxValue = kUnknown };
 
   LaunchSessionResponse();
   LaunchSessionResponse(LaunchSessionResponse&& other);

components/cast_channel/enum_table.cc

@@ -35,7 +35,7 @@ base::Optional<base::StringPiece> GenericEnumTableEntry::FindByValue(
     std::size_t size,
     int value) {
   for (std::size_t i = 0; i < size; i++) {
-    if (data[i].value == value)
+    if (data[i].value == value && data[i].has_str())
       return data[i].str();
   }
   return base::nullopt;

components/cast_channel/enum_table.h

@@ -6,6 +6,7 @@
 #define COMPONENTS_CAST_CHANNEL_ENUM_TABLE_H_
 
 #include <cstdint>
+#include <cstring>
 
 #include "base/logging.h"
 #include "base/optional.h"
@@ -25,17 +26,18 @@
 // a table of enum values and their corresponding strings:
 //
 //     // In .h file:
-//     enum class MyEnum { kFoo, kBar, ..., kOther };
+//     enum class MyEnum { kFoo, kBar, ..., kOther, kMaxValue = kOther };
 //
 //     // In .cc file:
 //
 //     template <>
 //     constexpr EnumTable<MyEnum> EnumTable<MyEnum>::instance({
-//       {MyEnum::kFoo, "FOO"},
-//       {MyEnum::kBar, "BAR"},
-//       ...
-//       // No entry for kOther because it has no string representation.
-//     });
+//         {MyEnum::kFoo, "FOO"},
+//         {MyEnum::kBar, "BAR"},
+//         ...
+//         {MyEnum::kOther},  // Not all values need strings.
+//       },
+//       MyEnum::kMaxValue);  // Needed to detect missing table entries.
 //
 // The functions EnumToString() and StringToEnum() are used to look up values
 // in the table.  In some cases, it may be useful to wrap these functions for
@@ -65,6 +67,40 @@
 // string literals or define each enum string as a named constant.
 //
 //
+// Consecutive Enum Tables
+// -----------------------
+//
+// When using an EnumTable, it's generally best for the following conditions
+// to be true:
+//
+// - The enum is defined with "enum class" syntax.
+//
+// - The members have the default values assigned by the compiler.
+//
+// - There is an extra member named kMaxValue which is set equal to the highest
+//   ordinary value.  (The Chromium style checker will verify that kMaxValue
+//   really is the maximum value.)
+//
+// - The values in the EnumTable constructor appear in sorted order.
+//
+// - Every member of the enum (other than kMaxValue) is included in the table.
+//
+// When these conditions are met, the enum's |kMaxValue| member should be passed
+// as the second argument of the EnumTable.  This will create a table where enum
+// values can be converted to strings in constant time.  It will also reliably
+// detect an incomplete enum table during startup of a debug build.
+//
+//
+// Non-Consecutive Enum Tables
+// ---------------------------
+//
+// When the conditions in the previous section cannot be satisfied, the second
+// argument of the EnumTable constructor should be the special constant
+// |NonConsecutiveEnumTable|.  Doing so has some unfortunate side-effects: there
+// is no automatic way to detect when an enum value is missing from the table,
+// and looking up a non-constant enum value requires a linear search.
+//
+//
 // Why use an EnumTable?
 // ---------------------
 //
@@ -146,6 +182,7 @@ class
 #endif
         GenericEnumTableEntry {
  public:
+  inline constexpr GenericEnumTableEntry(int32_t value);
   inline constexpr GenericEnumTableEntry(int32_t value, base::StringPiece str);
 
  private:
@@ -156,7 +193,12 @@ class
   static base::Optional<base::StringPiece>
   FindByValue(const GenericEnumTableEntry data[], std::size_t size, int value);
 
-  constexpr base::StringPiece str() const { return {chars, length}; }
+  constexpr base::StringPiece str() const {
+    DCHECK(has_str());
+    return {chars, length};
+  }
+
+  constexpr bool has_str() const { return chars != nullptr; }
 
   // Instead of storing a base::StringPiece, it's broken apart and stored as a
   // pointer and an unsigned int (rather than a std::size_t) so that table
@@ -171,9 +213,13 @@ class
   DISALLOW_COPY_AND_ASSIGN(GenericEnumTableEntry);
 };
 
-// Yes, this really needs to be inlined.  Even though it looks "complex" to the
-// style checker, everything is executed at compile time, so an EnumTable
-// instance can be fully initialized without executing any code.
+// Yes, these constructors really needs to be inlined.  Even though they look
+// "complex" to the style checker, everything is executed at compile time, so an
+// EnumTable instance can be fully initialized without executing any code.
+
+inline constexpr GenericEnumTableEntry::GenericEnumTableEntry(int32_t value)
+    : chars(nullptr), length(UINT32_MAX), value(value) {}
+
 inline constexpr GenericEnumTableEntry::GenericEnumTableEntry(
     int32_t value,
     base::StringPiece str)
@@ -181,8 +227,8 @@ inline constexpr GenericEnumTableEntry::GenericEnumTableEntry(
       length(static_cast<uint32_t>(str.length())),
       value(value) {}
 
-struct UnsortedEnumTable_t {};
-constexpr UnsortedEnumTable_t UnsortedEnumTable;
+struct NonConsecutiveEnumTable_t {};
+constexpr NonConsecutiveEnumTable_t NonConsecutiveEnumTable;
 
 // A table for associating enum values with string literals.  This class is
 // designed for use as an initialized global variable, so it has a trivial
@@ -190,11 +236,16 @@ constexpr UnsortedEnumTable_t UnsortedEnumTable;
 template <typename E>
 class EnumTable {
  public:
-  // DO NOT add any members to this class, or it will break the
+  // DO NOT add any data members to this class, or it will break the
   // reinterpret_casts below.  If necessary, add new members to
   // GenericEnumTableEntry instead.
   class Entry : public GenericEnumTableEntry {
    public:
+    // Constructor for placeholder entries with no string value.
+    constexpr Entry(E value)
+        : GenericEnumTableEntry(static_cast<int32_t>(value)) {}
+
+    // Constructor for regular entries.
     constexpr Entry(E value, base::StringPiece str)
         : GenericEnumTableEntry(static_cast<int32_t>(value), str) {}
 
@@ -209,7 +260,14 @@ class EnumTable {
   // Creates an EnumTable where data[i].value == i for all i.  When a table is
   // created with this constructor, the GetString() method is a simple array
   // lookup that runs in constant time.
-  constexpr EnumTable(std::initializer_list<Entry> data)
+  //
+  // If |max_value| is specified, all enum values in |data| must be less than or
+  // equal to |max_value|.  This feature is intended to help catch errors cause
+  // by a new value being added to an enum without the new value being added to
+  // the corresponding table.  For best results, use an enum class and create a
+  // constant named kMaxValue.  For more details, see
+  // https://www.chromium.org/developers/coding-style/chromium-style-checker-errors#TOC-Enumerator-max-values
+  constexpr EnumTable(std::initializer_list<Entry> data, E max_value)
       : EnumTable(data, true) {
 #ifndef NDEBUG
     // NOTE(jrw): This is compiled out when NDEBUG is defined, even if DCHECKS
@@ -217,20 +275,25 @@ class EnumTable {
     // this is that if DCHECKs are enabled, global EnumTable instances will have
     // a nontrivial constructor, which is flagged as a style violation by the
     // linux-rel trybot.
-    auto found = FindUnsortedEntry(data);
+    auto found = FindNonConsecutiveEntry(data);
     DCHECK(!found) << "Entries' numerical values must be consecutive "
-                   << "integers starting at 0; found problem at index "
+                   << "integers starting from 0; found problem at index "
                    << *found;
+
+    const auto int_max_value = static_cast<int32_t>(max_value);
+    DCHECK(data.end()[-1].value == int_max_value)
+        << "Missing entry for enum value " << int_max_value;
 #endif  // NDEBUG
   }
 
   // Creates an EnumTable where data[i].value != i for some values of i.  When
   // a table is created with this constructor, the GetString() method must
   // perform a linear search to find the correct string.
-  constexpr EnumTable(std::initializer_list<Entry> data, UnsortedEnumTable_t)
+  constexpr EnumTable(std::initializer_list<Entry> data,
+                      NonConsecutiveEnumTable_t)
       : EnumTable(data, false) {
 #ifndef NDEBUG
-    DCHECK(FindUnsortedEntry(data))
+    DCHECK(FindNonConsecutiveEntry(data))
         << "Don't use this constructor for sorted entries.";
 #endif  // NDEBUG
   }
@@ -242,9 +305,10 @@ class EnumTable {
       const std::size_t index = static_cast<std::size_t>(value);
       if (ANALYZER_ASSUME_TRUE(index < data_.size())) {
         const auto& entry = data_.begin()[index];
-        return entry.str();
+        if (ANALYZER_ASSUME_TRUE(entry.has_str()))
+          return entry.str();
       }
-      return {};
+      return base::nullopt;
     }
     return GenericEnumTableEntry::FindByValue(
         reinterpret_cast<const GenericEnumTableEntry*>(data_.begin()),
@@ -262,7 +326,7 @@ class EnumTable {
   template <E Value>
   constexpr base::StringPiece GetString() const {
     for (const auto& entry : data_) {
-      if (entry.value == static_cast<int32_t>(Value))
+      if (entry.value == static_cast<int32_t>(Value) && entry.has_str())
         return entry.str();
     }
 
@@ -308,7 +372,7 @@ class EnumTable {
         const Entry& ej = data.begin()[j];
         DCHECK(ei.value != ej.value)
             << "Found duplicate enum values at indices " << i << " and " << j;
-        DCHECK(ei.str() != ej.str())
+        DCHECK(!(ei.has_str() && ej.has_str() && ei.str() == ej.str()))
             << "Found duplicate strings at indices " << i << " and " << j;
       }
     }
@@ -317,11 +381,11 @@ class EnumTable {
 
 #ifndef NDEBUG
   // Finds and returns the first i for which data[i].value != i;
-  constexpr static base::Optional<std::size_t> FindUnsortedEntry(
+  constexpr static base::Optional<std::size_t> FindNonConsecutiveEntry(
       std::initializer_list<Entry> data) {
-    std::size_t counter = 0;
+    int32_t counter = 0;
     for (const auto& entry : data) {
-      if (static_cast<std::size_t>(entry.value) != counter) {
+      if (entry.value != counter) {
         return counter;
       }
       ++counter;

components/cast_channel/enum_table_unittest.cc

@@ -5,22 +5,34 @@
 #include "components/cast_channel/enum_table.h"
 
 #include "base/macros.h"
+#include "base/optional.h"
 #include "base/test/gtest_util.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace cast_util {
 namespace {
 
-enum class MyEnum { kZero, kOne, kTwo, kOther };
+enum class MyEnum { kZero, kOne, kTwo, kMaxValue = kTwo };
 
 const EnumTable<MyEnum> kSorted({{MyEnum::kZero, "ZERO"},
                                  {MyEnum::kOne, "ONE"},
-                                 {MyEnum::kTwo, "TWO"}});
+                                 {MyEnum::kTwo, "TWO"}},
+                                MyEnum::kMaxValue);
 
 const EnumTable<MyEnum> kUnsorted({{MyEnum::kOne, "ONE"},
                                    {MyEnum::kZero, "ZERO"},
                                    {MyEnum::kTwo, "TWO"}},
-                                  UnsortedEnumTable);
+                                  NonConsecutiveEnumTable);
+
+const EnumTable<MyEnum> kSortedMissing({{MyEnum::kZero, "ZERO"},
+                                        {MyEnum::kOne},
+                                        {MyEnum::kTwo, "TWO"}},
+                                       MyEnum::kMaxValue);
+
+const EnumTable<MyEnum> kUnsortedMissing({{MyEnum::kOne, "ONE"},
+                                          {MyEnum::kZero},
+                                          {MyEnum::kTwo, "TWO"}},
+                                         NonConsecutiveEnumTable);
 
 }  // namespace
 
@@ -28,7 +40,8 @@ template <>
 const EnumTable<MyEnum> EnumTable<MyEnum>::instance(
     {{MyEnum::kZero, "ZERO_DEFAULT"},
      {MyEnum::kOne, "ONE_DEFAULT"},
-     {MyEnum::kTwo, "TWO_DEFAULT"}});
+     {MyEnum::kTwo, "TWO_DEFAULT"}},
+    MyEnum::kMaxValue);
 
 namespace {
 
@@ -36,21 +49,30 @@ TEST(EnumTableTest, TestGetString) {
   EXPECT_EQ("ZERO", kSorted.GetString(MyEnum::kZero));
   EXPECT_EQ("ONE", kSorted.GetString(MyEnum::kOne));
   EXPECT_EQ("TWO", kSorted.GetString(MyEnum::kTwo));
-  EXPECT_EQ(base::nullopt, kSorted.GetString(MyEnum::kOther));
 }
 
 TEST(EnumTableTest, TestGetStringUnsorted) {
   EXPECT_EQ("ZERO", kUnsorted.GetString(MyEnum::kZero));
   EXPECT_EQ("ONE", kUnsorted.GetString(MyEnum::kOne));
   EXPECT_EQ("TWO", kUnsorted.GetString(MyEnum::kTwo));
-  EXPECT_EQ(base::nullopt, kUnsorted.GetString(MyEnum::kOther));
+}
+
+TEST(EnumTableTest, TestGetMissingString) {
+  EXPECT_EQ("ZERO", kSortedMissing.GetString(MyEnum::kZero));
+  EXPECT_EQ(base::nullopt, kSortedMissing.GetString(MyEnum::kOne));
+  EXPECT_EQ("TWO", kSortedMissing.GetString(MyEnum::kTwo));
+}
+
+TEST(EnumTableTest, TestGetMissingStringUnsorted) {
+  EXPECT_EQ(base::nullopt, kUnsortedMissing.GetString(MyEnum::kZero));
+  EXPECT_EQ("ONE", kUnsortedMissing.GetString(MyEnum::kOne));
+  EXPECT_EQ("TWO", kUnsortedMissing.GetString(MyEnum::kTwo));
 }
 
 TEST(EnumTableTest, TestEnumToStringGlobal) {
   EXPECT_EQ("ZERO_DEFAULT", EnumToString(MyEnum::kZero));
   EXPECT_EQ("ONE_DEFAULT", EnumToString(MyEnum::kOne));
   EXPECT_EQ("TWO_DEFAULT", EnumToString(MyEnum::kTwo));
-  EXPECT_EQ(base::nullopt, EnumToString(MyEnum::kOther));
 }
 
 TEST(EnumTableTest, TestStaticGetString) {
@@ -86,15 +108,27 @@ TEST(EnumTableTest, TestStringToEnumGlobal) {
 // out when NDEBUG is defined.
 #ifndef NDEBUG
 
+TEST(EnumTableDeathTest, MaxValueTooSmall) {
+  EXPECT_DCHECK_DEATH(EnumTable<MyEnum>(
+      {{MyEnum::kZero, "ZERO"}, {MyEnum::kOne, "ONE"}, {MyEnum::kTwo, "TWO"}},
+      MyEnum::kOne));
+}
+
+TEST(EnumTableDeathTest, MaxValueTooLarge) {
+  EXPECT_DCHECK_DEATH(EnumTable<MyEnum>(
+      {{MyEnum::kZero, "ZERO"}, {MyEnum::kOne, "ONE"}}, MyEnum::kTwo));
+}
+
 TEST(EnumTableDeathTest, Sorted) {
   EXPECT_DCHECK_DEATH(EnumTable<MyEnum>(
-      {{MyEnum::kZero, "ZERO"}, {MyEnum::kTwo, "TWO"}, {MyEnum::kOne, "ONE"}}));
+      {{MyEnum::kZero, "ZERO"}, {MyEnum::kTwo, "TWO"}, {MyEnum::kOne, "ONE"}},
+      MyEnum::kMaxValue));
 }
 
 TEST(EnumTableDeathTest, Unsorted) {
   EXPECT_DCHECK_DEATH(EnumTable<MyEnum>(
       {{MyEnum::kZero, "ZERO"}, {MyEnum::kOne, "ONE"}, {MyEnum::kTwo, "TWO"}},
-      UnsortedEnumTable));
+      NonConsecutiveEnumTable));
 }
 
 TEST(EnumTableDeathTest, DuplicateEnums) {
@@ -102,17 +136,24 @@ TEST(EnumTableDeathTest, DuplicateEnums) {
                                          {MyEnum::kTwo, "TWO"},
                                          {MyEnum::kOne, "ONE"},
                                          {MyEnum::kZero, "ZERO"}},
-                                        UnsortedEnumTable));
+                                        NonConsecutiveEnumTable));
 }
 
 TEST(EnumTableDeathTest, DuplicateStrings) {
   EXPECT_DCHECK_DEATH(EnumTable<MyEnum>(
-      {{MyEnum::kZero, "FOO"}, {MyEnum::kOne, "ONE"}, {MyEnum::kTwo, "FOO"}}));
+      {{MyEnum::kZero, "FOO"}, {MyEnum::kOne, "ONE"}, {MyEnum::kTwo, "FOO"}},
+      MyEnum::kMaxValue));
+}
+
+constexpr MyEnum kInvalid =
+    static_cast<MyEnum>(static_cast<int>(MyEnum::kMaxValue) + 1);
+
+TEST(EnumTableDeathTest, EnumToString) {
+  EXPECT_DCHECK_DEATH(kSorted.GetString<kInvalid>());
 }
 
 TEST(EnumTableDeathTest, StaticEnumToString) {
-  EXPECT_DCHECK_DEATH(kSorted.GetString<MyEnum::kOther>());
-  EXPECT_DCHECK_DEATH((EnumToString<MyEnum, MyEnum::kOther>()));
+  EXPECT_DCHECK_DEATH((EnumToString<MyEnum, kInvalid>()));
 }
 
 enum class HugeEnum {
@@ -149,22 +190,31 @@ enum class HugeEnum {
   k30,
   k31,
   k32,
+  kMaxValue = k32,
 };
 
 TEST(EnumTableDeathTest, HugeEnum) {
-  EXPECT_DCHECK_DEATH(EnumTable<HugeEnum>({
-      {HugeEnum::k0, "k0"},   {HugeEnum::k1, "k1"},   {HugeEnum::k2, "k2"},
-      {HugeEnum::k3, "k3"},   {HugeEnum::k4, "k4"},   {HugeEnum::k5, "k5"},
-      {HugeEnum::k6, "k6"},   {HugeEnum::k7, "k7"},   {HugeEnum::k8, "k8"},
-      {HugeEnum::k9, "k9"},   {HugeEnum::k10, "k10"}, {HugeEnum::k11, "k11"},
-      {HugeEnum::k12, "k12"}, {HugeEnum::k13, "k13"}, {HugeEnum::k14, "k14"},
-      {HugeEnum::k15, "k15"}, {HugeEnum::k16, "k16"}, {HugeEnum::k17, "k17"},
-      {HugeEnum::k18, "k18"}, {HugeEnum::k19, "k19"}, {HugeEnum::k20, "k20"},
-      {HugeEnum::k21, "k21"}, {HugeEnum::k22, "k22"}, {HugeEnum::k23, "k23"},
-      {HugeEnum::k24, "k24"}, {HugeEnum::k25, "k25"}, {HugeEnum::k26, "k26"},
-      {HugeEnum::k27, "k27"}, {HugeEnum::k28, "k28"}, {HugeEnum::k29, "k29"},
-      {HugeEnum::k30, "k30"}, {HugeEnum::k31, "k31"}, {HugeEnum::k32, "k32"},
-  }));
+  EXPECT_DCHECK_DEATH(EnumTable<HugeEnum>(
+      {
+          {HugeEnum::k0, "k0"},   {HugeEnum::k1, "k1"},
+          {HugeEnum::k2, "k2"},   {HugeEnum::k3, "k3"},
+          {HugeEnum::k4, "k4"},   {HugeEnum::k5, "k5"},
+          {HugeEnum::k6, "k6"},   {HugeEnum::k7, "k7"},
+          {HugeEnum::k8, "k8"},   {HugeEnum::k9, "k9"},
+          {HugeEnum::k10, "k10"}, {HugeEnum::k11, "k11"},
+          {HugeEnum::k12, "k12"}, {HugeEnum::k13, "k13"},
+          {HugeEnum::k14, "k14"}, {HugeEnum::k15, "k15"},
+          {HugeEnum::k16, "k16"}, {HugeEnum::k17, "k17"},
+          {HugeEnum::k18, "k18"}, {HugeEnum::k19, "k19"},
+          {HugeEnum::k20, "k20"}, {HugeEnum::k21, "k21"},
+          {HugeEnum::k22, "k22"}, {HugeEnum::k23, "k23"},
+          {HugeEnum::k24, "k24"}, {HugeEnum::k25, "k25"},
+          {HugeEnum::k26, "k26"}, {HugeEnum::k27, "k27"},
+          {HugeEnum::k28, "k28"}, {HugeEnum::k29, "k29"},
+          {HugeEnum::k30, "k30"}, {HugeEnum::k31, "k31"},
+          {HugeEnum::k32, "k32"},
+      },
+      HugeEnum::kMaxValue));
 }
 
 #endif  // NDEBUG