Refactoring: Use enum for RtpPayloadType.

chrome/renderer/extensions/cast_streaming_native_handler.cc

Index: chrome/renderer/extensions/cast_streaming_native_handler.cc
diff --git a/chrome/renderer/extensions/cast_streaming_native_handler.cc b/chrome/renderer/extensions/cast_streaming_native_handler.cc
index c03f63fc5607ae012e854405569efef0f9a5d0d0..1193d5d4313277adc5c68a9d3de76230bb26a783 100644
--- a/chrome/renderer/extensions/cast_streaming_native_handler.cc
+++ b/chrome/renderer/extensions/cast_streaming_native_handler.cc
@@ -64,6 +64,7 @@ const char kInvalidRtpParams[] = "Invalid value for RTP params";
 const char kInvalidLatency[] = "Invalid value for max_latency. (0-1000)";
 const char kInvalidRtpTimebase[] = "Invalid rtp_timebase. (1000-1000000)";
 const char kInvalidStreamArgs[] = "Invalid stream arguments";
+const char kInvalidCodec[] = "Invalid codec";
 const char kRtpStreamNotFound[] = "The RTP stream cannot be found";
 const char kUdpTransportNotFound[] = "The UDP transport cannot be found";
 const char kUnableToConvertArgs[] = "Unable to convert arguments";
@@ -96,7 +97,6 @@ bool HexDecode(const std::string& input, std::string* output) {
 bool ToCastRtpPayloadParamsOrThrow(v8::Isolate* isolate,
                                    const RtpPayloadParams& ext_params,
                                    CastRtpPayloadParams* cast_params) {
-  cast_params->payload_type = ext_params.payload_type;
   cast_params->max_latency_ms = ext_params.max_latency;
   cast_params->min_latency_ms =
       ext_params.min_latency ? *ext_params.min_latency : ext_params.max_latency;
@@ -104,6 +104,21 @@ bool ToCastRtpPayloadParamsOrThrow(v8::Isolate* isolate,
                                          ? *ext_params.animated_latency
                                          : ext_params.max_latency;
   cast_params->codec_name = ext_params.codec_name;
+  if (cast_params->codec_name == "OPUS") {
+    cast_params->payload_type = media::cast::RtpPayloadType::AUDIO_OPUS;
+  } else if (cast_params->codec_name == "PCM16") {
+    cast_params->payload_type = media::cast::RtpPayloadType::AUDIO_PCM16;
+  } else if (cast_params->codec_name == "AAC") {
+    cast_params->payload_type = media::cast::RtpPayloadType::AUDIO_AAC;
+  } else if (cast_params->codec_name == "VP8") {
+    cast_params->payload_type = media::cast::RtpPayloadType::VIDEO_VP8;
+  } else if (cast_params->codec_name == "H264") {
+    cast_params->payload_type = media::cast::RtpPayloadType::VIDEO_H264;
+  } else {
+    isolate->ThrowException(

[Marijn Kruisselbrink, 2016/06/24 16:54:57]

One of the now failing tests seems to be a test to exactly test the behavior
when an invalid codec name was specified. Where the test expects the error
callback to be called, but now you changed that to throw an exception instead. I
don't know if making such breaking API changes is a compatibility issue. And if
"codec_name" has a limited set of hard-coded values that is acceptable, would it
make sense for it to be an enum in the idl files, rather than just an arbitrary
string?

[miu, 2016/06/27 21:00:36]

FWIW, I have a change WIP to clean-up and simplify all these Cast Streaming
APIs. We won't have to worry about backwards-compatibility because they are
private and only used by a few whitelisted extensions our team owns.

I talked about this with xjz@ face-to-face and, for this change, we'll just go
back to the old behavior for now (i.e., the onError event handler will trigger
on a bad codecName, as the API test expects). In my API clean-up change, I will
be using enums instead of free-form string fields for this property; and will do
strict, immediate input validation (rather than delayed via an error handler).

[xjz, 2016/06/28 16:46:26]

Thanks Yuri for the reply. Now revert the CL to Patch #2, which does not handle
the invalid |codec_name| here.

+        v8::Exception::Error(v8::String::NewFromUtf8(isolate, kInvalidCodec)));
+    return false;
+  }
   cast_params->ssrc = ext_params.ssrc;
   cast_params->feedback_ssrc = ext_params.feedback_ssrc;
   cast_params->clock_rate = ext_params.clock_rate ? *ext_params.clock_rate : 0;
@@ -137,7 +152,7 @@ bool ToCastRtpPayloadParamsOrThrow(v8::Isolate* isolate,
 
 void FromCastRtpPayloadParams(const CastRtpPayloadParams& cast_params,
                               RtpPayloadParams* ext_params) {
-  ext_params->payload_type = cast_params.payload_type;
+  ext_params->payload_type = static_cast<int>(cast_params.payload_type);
   ext_params->max_latency = cast_params.max_latency_ms;
   ext_params->min_latency.reset(new int(cast_params.min_latency_ms));
   ext_params->animated_latency.reset(new int(cast_params.animated_latency_ms));
@@ -676,23 +691,27 @@ bool CastStreamingNativeHandler::FrameReceiverConfigFromArg(
   if (params->codec_name == "OPUS") {
     config->codec = media::cast::CODEC_AUDIO_OPUS;
     config->rtp_timebase = 48000;
-    config->rtp_payload_type = media::cast::kDefaultRtpAudioPayloadType;
+    config->rtp_payload_type = media::cast::RtpPayloadType::AUDIO_OPUS;
   } else if (params->codec_name == "PCM16") {
     config->codec = media::cast::CODEC_AUDIO_PCM16;
     config->rtp_timebase = 48000;
-    config->rtp_payload_type = media::cast::kDefaultRtpAudioPayloadType;
+    config->rtp_payload_type = media::cast::RtpPayloadType::AUDIO_PCM16;
   } else if (params->codec_name == "AAC") {
     config->codec = media::cast::CODEC_AUDIO_AAC;
     config->rtp_timebase = 48000;
-    config->rtp_payload_type = media::cast::kDefaultRtpAudioPayloadType;
+    config->rtp_payload_type = media::cast::RtpPayloadType::AUDIO_AAC;
   } else if (params->codec_name == "VP8") {
     config->codec = media::cast::CODEC_VIDEO_VP8;
     config->rtp_timebase = 90000;
-    config->rtp_payload_type = media::cast::kDefaultRtpVideoPayloadType;
+    config->rtp_payload_type = media::cast::RtpPayloadType::VIDEO_VP8;
   } else if (params->codec_name == "H264") {
     config->codec = media::cast::CODEC_VIDEO_H264;
     config->rtp_timebase = 90000;
-    config->rtp_payload_type = media::cast::kDefaultRtpVideoPayloadType;
+    config->rtp_payload_type = media::cast::RtpPayloadType::VIDEO_H264;
+  } else {
+    isolate->ThrowException(
+        v8::Exception::Error(v8::String::NewFromUtf8(isolate, kInvalidCodec)));
+    return false;
   }
   if (params->rtp_timebase) {
     config->rtp_timebase = *params->rtp_timebase;