From 446e7e578b66687a67329523c5276bc2f0d1859f Mon Sep 17 00:00:00 2001 From: Hidehiko Abe Date: Thu, 17 Dec 2020 20:41:11 +0000 Subject: [PATCH] Fix static initializer issue of EnumTable. We do not want to have runtime global static initialization. Currently, EnumTable hits the error, and the any code change for the files including EnumTable::instance causes bot failure, even if it is totally independent. This CL is the short term fix of the problem by changing instance fields to GetInstance methods. For the long term, we're switching to code generation. Thus, this CL tries to minimize the effort of the fix, rather than entier refactoring related constexpr. BUG=1158681 TEST=Ran on bots. Change-Id: Ia3400dd178ba9d5056e6543ccaf8a754559484b0 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2594830 Reviewed-by: mark a. foltz Reviewed-by: John Williams Commit-Queue: Hidehiko Abe Cr-Commit-Position: refs/heads/master@{#838215} --- .../cast/cast_internal_message_util.cc | 74 +++++----- components/cast_channel/cast_message_util.cc | 133 ++++++++++-------- components/cast_channel/enum_table.h | 13 +- .../cast_channel/enum_table_unittest.cc | 12 +- .../providers/cast/cast_media_source.cc | 76 ++++++---- 5 files changed, 170 insertions(+), 138 deletions(-) diff --git a/chrome/browser/media/router/providers/cast/cast_internal_message_util.cc b/chrome/browser/media/router/providers/cast/cast_internal_message_util.cc index 85868ec350b0a..0fdd8c3f11ba6 100644 --- a/chrome/browser/media/router/providers/cast/cast_internal_message_util.cc +++ b/chrome/browser/media/router/providers/cast/cast_internal_message_util.cc @@ -23,42 +23,48 @@ namespace cast_util { using media_router::CastInternalMessage; template <> -const EnumTable - EnumTable::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); +const EnumTable& +EnumTable::GetInstance() { + static const EnumTable kInstance( + { + {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); + return kInstance; +} template <> -const EnumTable - EnumTable::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); +const EnumTable& +EnumTable::GetInstance() { + static const EnumTable kInstance( + { + {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); + return kInstance; +} } // namespace cast_util diff --git a/components/cast_channel/cast_message_util.cc b/components/cast_channel/cast_message_util.cc index afc0d9e37a1eb..8b4649e6149ed 100644 --- a/components/cast_channel/cast_message_util.cc +++ b/components/cast_channel/cast_message_util.cc @@ -28,72 +28,81 @@ using cast_channel::CastMessageType; using cast_channel::GetAppAvailabilityResult; template <> -const EnumTable EnumTable::instance( - { - {CastMessageType::kPing, "PING"}, - {CastMessageType::kPong, "PONG"}, - {CastMessageType::kRpc, "RPC"}, - {CastMessageType::kGetAppAvailability, "GET_APP_AVAILABILITY"}, - {CastMessageType::kGetStatus, "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::kOffer, "OFFER"}, - {CastMessageType::kAnswer, "ANSWER"}, - {CastMessageType::kCapabilitiesResponse, "CAPABILITIES_RESPONSE"}, - {CastMessageType::kStatusResponse, "STATUS_RESPONSE"}, - {CastMessageType::kMultizoneStatus, "MULTIZONE_STATUS"}, - {CastMessageType::kInvalidPlayerState, "INVALID_PLAYER_STATE"}, - {CastMessageType::kLoadFailed, "LOAD_FAILED"}, - {CastMessageType::kLoadCancelled, "LOAD_CANCELLED"}, - {CastMessageType::kInvalidRequest, "INVALID_REQUEST"}, - {CastMessageType::kPresentation, "PRESENTATION"}, - {CastMessageType::kGetCapabilities, "GET_CAPABILITIES"}, - {CastMessageType::kOther}, - }, - CastMessageType::kMaxValue); +const EnumTable& EnumTable::GetInstance() { + static const EnumTable kInstance( + { + {CastMessageType::kPing, "PING"}, + {CastMessageType::kPong, "PONG"}, + {CastMessageType::kRpc, "RPC"}, + {CastMessageType::kGetAppAvailability, "GET_APP_AVAILABILITY"}, + {CastMessageType::kGetStatus, "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::kOffer, "OFFER"}, + {CastMessageType::kAnswer, "ANSWER"}, + {CastMessageType::kCapabilitiesResponse, "CAPABILITIES_RESPONSE"}, + {CastMessageType::kStatusResponse, "STATUS_RESPONSE"}, + {CastMessageType::kMultizoneStatus, "MULTIZONE_STATUS"}, + {CastMessageType::kInvalidPlayerState, "INVALID_PLAYER_STATE"}, + {CastMessageType::kLoadFailed, "LOAD_FAILED"}, + {CastMessageType::kLoadCancelled, "LOAD_CANCELLED"}, + {CastMessageType::kInvalidRequest, "INVALID_REQUEST"}, + {CastMessageType::kPresentation, "PRESENTATION"}, + {CastMessageType::kGetCapabilities, "GET_CAPABILITIES"}, + {CastMessageType::kOther}, + }, + CastMessageType::kMaxValue); + return kInstance; +} template <> -const EnumTable - EnumTable::instance( - { - {cast_channel::V2MessageType::kEditTracksInfo, "EDIT_TRACKS_INFO"}, - {cast_channel::V2MessageType::kGetStatus, "GET_STATUS"}, - {cast_channel::V2MessageType::kLoad, "LOAD"}, - {cast_channel::V2MessageType::kMediaGetStatus, "MEDIA_GET_STATUS"}, - {cast_channel::V2MessageType::kMediaSetVolume, "MEDIA_SET_VOLUME"}, - {cast_channel::V2MessageType::kPause, "PAUSE"}, - {cast_channel::V2MessageType::kPlay, "PLAY"}, - {cast_channel::V2MessageType::kPrecache, "PRECACHE"}, - {cast_channel::V2MessageType::kQueueInsert, "QUEUE_INSERT"}, - {cast_channel::V2MessageType::kQueueLoad, "QUEUE_LOAD"}, - {cast_channel::V2MessageType::kQueueRemove, "QUEUE_REMOVE"}, - {cast_channel::V2MessageType::kQueueReorder, "QUEUE_REORDER"}, - {cast_channel::V2MessageType::kQueueUpdate, "QUEUE_UPDATE"}, - {cast_channel::V2MessageType::kQueueNext, "QUEUE_NEXT"}, - {cast_channel::V2MessageType::kQueuePrev, "QUEUE_PREV"}, - {cast_channel::V2MessageType::kSeek, "SEEK"}, - {cast_channel::V2MessageType::kSetVolume, "SET_VOLUME"}, - {cast_channel::V2MessageType::kStop, "STOP"}, - {cast_channel::V2MessageType::kStopMedia, "STOP_MEDIA"}, - {cast_channel::V2MessageType::kOther}, - }, - cast_channel::V2MessageType::kMaxValue); +const EnumTable& +EnumTable::GetInstance() { + static const EnumTable kInstance( + { + {cast_channel::V2MessageType::kEditTracksInfo, "EDIT_TRACKS_INFO"}, + {cast_channel::V2MessageType::kGetStatus, "GET_STATUS"}, + {cast_channel::V2MessageType::kLoad, "LOAD"}, + {cast_channel::V2MessageType::kMediaGetStatus, "MEDIA_GET_STATUS"}, + {cast_channel::V2MessageType::kMediaSetVolume, "MEDIA_SET_VOLUME"}, + {cast_channel::V2MessageType::kPause, "PAUSE"}, + {cast_channel::V2MessageType::kPlay, "PLAY"}, + {cast_channel::V2MessageType::kPrecache, "PRECACHE"}, + {cast_channel::V2MessageType::kQueueInsert, "QUEUE_INSERT"}, + {cast_channel::V2MessageType::kQueueLoad, "QUEUE_LOAD"}, + {cast_channel::V2MessageType::kQueueRemove, "QUEUE_REMOVE"}, + {cast_channel::V2MessageType::kQueueReorder, "QUEUE_REORDER"}, + {cast_channel::V2MessageType::kQueueUpdate, "QUEUE_UPDATE"}, + {cast_channel::V2MessageType::kQueueNext, "QUEUE_NEXT"}, + {cast_channel::V2MessageType::kQueuePrev, "QUEUE_PREV"}, + {cast_channel::V2MessageType::kSeek, "SEEK"}, + {cast_channel::V2MessageType::kSetVolume, "SET_VOLUME"}, + {cast_channel::V2MessageType::kStop, "STOP"}, + {cast_channel::V2MessageType::kStopMedia, "STOP_MEDIA"}, + {cast_channel::V2MessageType::kOther}, + }, + cast_channel::V2MessageType::kMaxValue); + return kInstance; +} template <> -const EnumTable - EnumTable::instance( - { - {GetAppAvailabilityResult::kAvailable, "APP_AVAILABLE"}, - {GetAppAvailabilityResult::kUnavailable, "APP_UNAVAILABLE"}, - {GetAppAvailabilityResult::kUnknown}, - }, - GetAppAvailabilityResult::kMaxValue); +const EnumTable& +EnumTable::GetInstance() { + static const EnumTable kInstance( + { + {GetAppAvailabilityResult::kAvailable, "APP_AVAILABLE"}, + {GetAppAvailabilityResult::kUnavailable, "APP_UNAVAILABLE"}, + {GetAppAvailabilityResult::kUnknown}, + }, + GetAppAvailabilityResult::kMaxValue); + return kInstance; +} } // namespace cast_util diff --git a/components/cast_channel/enum_table.h b/components/cast_channel/enum_table.h index 85539057d82df..e3130c7b3263f 100644 --- a/components/cast_channel/enum_table.h +++ b/components/cast_channel/enum_table.h @@ -351,7 +351,7 @@ class EnumTable { // instance of this class for a given enum type. Users of this class are // responsible for providing a suitable definition for each enum type if the // EnumToString() or StringToEnum() functions are used. - static const EnumTable instance; + static const EnumTable& GetInstance(); private: #ifdef ARCH_CPU_64_BITS @@ -403,30 +403,31 @@ class EnumTable { // (EnumTable::instance) for the given enum type. template inline base::Optional EnumToString(E value) { - return EnumTable::instance.GetString(value); + return EnumTable::GetInstance().GetString(value); } // Converts a literal enum value to a string at compile time using the default -// table (EnumTable::instance) for the given enum type. +// table (EnumTable::GetInstance()) for the given enum type. // // TODO(jrw): Once C++17 features are allowed, change this function to have only // one template parameter: // // template // inline base::StringPiece EnumToString() { -// return EnumTable::instance.template GetString(); +// return EnumTable::GetInstance().template GetString(); // } // template inline base::StringPiece EnumToString() { - return EnumTable::instance.template GetString(); + return EnumTable::GetInstance().template GetString(); } // Converts a string to an enum value using the default table // (EnumTable::instance) for the given enum type. template inline base::Optional StringToEnum(base::StringPiece str) { - return EnumTable::instance.GetEnum(str); + return EnumTable::GetInstance().GetEnum(str); } } // namespace cast_util diff --git a/components/cast_channel/enum_table_unittest.cc b/components/cast_channel/enum_table_unittest.cc index 77726252af305..5ba3fc88a2fce 100644 --- a/components/cast_channel/enum_table_unittest.cc +++ b/components/cast_channel/enum_table_unittest.cc @@ -37,11 +37,13 @@ const EnumTable kUnsortedMissing({{MyEnum::kOne, "ONE"}, } // namespace template <> -const EnumTable EnumTable::instance( - {{MyEnum::kZero, "ZERO_DEFAULT"}, - {MyEnum::kOne, "ONE_DEFAULT"}, - {MyEnum::kTwo, "TWO_DEFAULT"}}, - MyEnum::kMaxValue); +const EnumTable& EnumTable::GetInstance() { + static const EnumTable kInstance({{MyEnum::kZero, "ZERO_DEFAULT"}, + {MyEnum::kOne, "ONE_DEFAULT"}, + {MyEnum::kTwo, "TWO_DEFAULT"}}, + MyEnum::kMaxValue); + return kInstance; +} namespace { diff --git a/components/media_router/common/providers/cast/cast_media_source.cc b/components/media_router/common/providers/cast/cast_media_source.cc index 89cb2f41b1e4a..acc24584e7a3b 100644 --- a/components/media_router/common/providers/cast/cast_media_source.cc +++ b/components/media_router/common/providers/cast/cast_media_source.cc @@ -30,43 +30,57 @@ using media_router::AutoJoinPolicy; using media_router::DefaultActionPolicy; template <> -const EnumTable EnumTable::instance( - { - {AutoJoinPolicy::kPageScoped, "page_scoped"}, - {AutoJoinPolicy::kTabAndOriginScoped, "tab_and_origin_scoped"}, - {AutoJoinPolicy::kOriginScoped, "origin_scoped"}, - }, - AutoJoinPolicy::kMaxValue); +const EnumTable& EnumTable::GetInstance() { + static const EnumTable kInstance( + { + {AutoJoinPolicy::kPageScoped, "page_scoped"}, + {AutoJoinPolicy::kTabAndOriginScoped, "tab_and_origin_scoped"}, + {AutoJoinPolicy::kOriginScoped, "origin_scoped"}, + }, + AutoJoinPolicy::kMaxValue); + return kInstance; +} template <> -const EnumTable EnumTable::instance( - { - {DefaultActionPolicy::kCreateSession, "create_session"}, - {DefaultActionPolicy::kCastThisTab, "cast_this_tab"}, - }, - DefaultActionPolicy::kMaxValue); +const EnumTable& +EnumTable::GetInstance() { + static const EnumTable kInstance( + { + {DefaultActionPolicy::kCreateSession, "create_session"}, + {DefaultActionPolicy::kCastThisTab, "cast_this_tab"}, + }, + DefaultActionPolicy::kMaxValue); + return kInstance; +} template <> -const EnumTable EnumTable::instance( - { - {CastDeviceCapability::MULTIZONE_GROUP, "multizone_group"}, - {CastDeviceCapability::DEV_MODE, "dev_mode"}, - {CastDeviceCapability::AUDIO_IN, "audio_in"}, - {CastDeviceCapability::AUDIO_OUT, "audio_out"}, - {CastDeviceCapability::VIDEO_IN, "video_in"}, - {CastDeviceCapability::VIDEO_OUT, "video_out"}, - // NONE deliberately omitted - }, - NonConsecutiveEnumTable); +const EnumTable& +EnumTable::GetInstance() { + static const EnumTable kInstance( + { + {CastDeviceCapability::MULTIZONE_GROUP, "multizone_group"}, + {CastDeviceCapability::DEV_MODE, "dev_mode"}, + {CastDeviceCapability::AUDIO_IN, "audio_in"}, + {CastDeviceCapability::AUDIO_OUT, "audio_out"}, + {CastDeviceCapability::VIDEO_IN, "video_in"}, + {CastDeviceCapability::VIDEO_OUT, "video_out"}, + // NONE deliberately omitted + }, + NonConsecutiveEnumTable); + return kInstance; +} template <> -const EnumTable EnumTable::instance( - { - {ReceiverAppType::kOther, "OTHER"}, - {ReceiverAppType::kWeb, "WEB"}, - {ReceiverAppType::kAndroidTv, "ANDROID_TV"}, - }, - ReceiverAppType::kMaxValue); +const EnumTable& EnumTable::GetInstance() { + static const EnumTable kInstance( + { + {ReceiverAppType::kOther, "OTHER"}, + {ReceiverAppType::kWeb, "WEB"}, + {ReceiverAppType::kAndroidTv, "ANDROID_TV"}, + }, + ReceiverAppType::kMaxValue); + return kInstance; +} } // namespace cast_util