Cast: Let the extension control if DSCP is on or off.

Cast: Let the extension control if DSCP is on or off.

Extension now has to call udptransport->setOptions(":DSCP") to get dscp set.

This framework should also let us experiment with different wifi settings in windows and different pacing settings.

Committed: https://crrev.com/857fde275f2b0e0ef555e83535d0dd3375a70123
Cr-Commit-Position: refs/heads/master@{#294211}

[hubbe, 2014-09-08 23:54:40]

hubbe@chromium.org changed reviewers:
+ hclam@chromium.org

[Alpha Left Google, 2014-09-08 23:55:44]

You work too fast man. :)

[Alpha Left Google, 2014-09-09 00:30:46]

https://codereview.chromium.org/551883004/diff/1/chrome/common/extensions/api...
File chrome/common/extensions/api/cast_streaming_udp_transport.idl (right):

https://codereview.chromium.org/551883004/diff/1/chrome/common/extensions/api...
chrome/common/extensions/api/cast_streaming_udp_transport.idl:34: [nocompile]
static void setOptions(DOMString options);
I think this should be an object instead of a string. Being an object javascript
code can easily add / remove entries and don't have to join when calling this
method. It's also a proper key value data structure.

e.g.
[nocompile] static void setOptions(object options);

v8 object can easily be converted to a base::DictionaryValue.

https://codereview.chromium.org/551883004/diff/1/chrome/renderer/extensions/c...
File chrome/renderer/extensions/cast_streaming_native_handler.cc (right):

https://codereview.chromium.org/551883004/diff/1/chrome/renderer/extensions/c...
chrome/renderer/extensions/cast_streaming_native_handler.cc:476: std::string
option_data;
If the option is using the type base::DictionaryValue you can use |converter| to
convert it into base::DictionaryValue easily. Note that IPC system supports
base::DictionaryValue already.

https://codereview.chromium.org/551883004/diff/1/chrome/renderer/media/cast_s...
File chrome/renderer/media/cast_session.h (right):

https://codereview.chromium.org/551883004/diff/1/chrome/renderer/media/cast_s...
chrome/renderer/media/cast_session.h:74: const std::string& options);
I suggest this be base::DictionaryValue which is a key value pair data
structure.

https://codereview.chromium.org/551883004/diff/1/chrome/renderer/media/cast_s...
File chrome/renderer/media/cast_session_delegate.h (right):

https://codereview.chromium.org/551883004/diff/1/chrome/renderer/media/cast_s...
chrome/renderer/media/cast_session_delegate.h:63: const std::string& options);
I suggest this be base::DictionaryValue which is a key value pair data
structure.

https://codereview.chromium.org/551883004/diff/1/chrome/renderer/media/cast_t...
File chrome/renderer/media/cast_transport_sender_ipc.cc (right):

https://codereview.chromium.org/551883004/diff/1/chrome/renderer/media/cast_t...
chrome/renderer/media/cast_transport_sender_ipc.cc:26: Send(new
CastHostMsg_New(channel_id_, remote_end_point, options));
If |options| is base::DictionaryValue the serialization / deserialization will
work nicely.

https://codereview.chromium.org/551883004/diff/1/chrome/renderer/media/cast_u...
File chrome/renderer/media/cast_udp_transport.h (right):

https://codereview.chromium.org/551883004/diff/1/chrome/renderer/media/cast_u...
chrome/renderer/media/cast_udp_transport.h:27: void SetOptions(const
std::string& options);
I suggest this be base::DictionaryValue which is a key value pair data
structure.

https://codereview.chromium.org/551883004/diff/1/media/cast/net/cast_transpor...
File media/cast/net/cast_transport_sender.h (right):

https://codereview.chromium.org/551883004/diff/1/media/cast/net/cast_transpor...
media/cast/net/cast_transport_sender.h:56: const std::string& options,
I suggest this be base::DictionaryValue which is a key value pair data
structure.

[hubbe, 2014-09-09 20:10:01]

https://codereview.chromium.org/551883004/diff/1/chrome/common/extensions/api...
File chrome/common/extensions/api/cast_streaming_udp_transport.idl (right):

https://codereview.chromium.org/551883004/diff/1/chrome/common/extensions/api...
chrome/common/extensions/api/cast_streaming_udp_transport.idl:34: [nocompile]
static void setOptions(DOMString options);
On 2014/09/09 00:30:45, Alpha wrote:
> I think this should be an object instead of a string. Being an object
javascript
> code can easily add / remove entries and don't have to join when calling this
> method. It's also a proper key value data structure.
> 
> e.g.
> [nocompile] static void setOptions(object options);
> 
> v8 object can easily be converted to a base::DictionaryValue.

Done.

https://codereview.chromium.org/551883004/diff/1/chrome/renderer/extensions/c...
File chrome/renderer/extensions/cast_streaming_native_handler.cc (right):

https://codereview.chromium.org/551883004/diff/1/chrome/renderer/extensions/c...
chrome/renderer/extensions/cast_streaming_native_handler.cc:476: std::string
option_data;
On 2014/09/09 00:30:45, Alpha wrote:
> If the option is using the type base::DictionaryValue you can use |converter|
to
> convert it into base::DictionaryValue easily. Note that IPC system supports
> base::DictionaryValue already.

Done.

https://codereview.chromium.org/551883004/diff/1/chrome/renderer/media/cast_s...
File chrome/renderer/media/cast_session.h (right):

https://codereview.chromium.org/551883004/diff/1/chrome/renderer/media/cast_s...
chrome/renderer/media/cast_session.h:74: const std::string& options);
On 2014/09/09 00:30:45, Alpha wrote:
> I suggest this be base::DictionaryValue which is a key value pair data
> structure.

Done.

https://codereview.chromium.org/551883004/diff/1/chrome/renderer/media/cast_s...
File chrome/renderer/media/cast_session_delegate.h (right):

https://codereview.chromium.org/551883004/diff/1/chrome/renderer/media/cast_s...
chrome/renderer/media/cast_session_delegate.h:63: const std::string& options);
On 2014/09/09 00:30:45, Alpha wrote:
> I suggest this be base::DictionaryValue which is a key value pair data
> structure.

Done.

https://codereview.chromium.org/551883004/diff/1/chrome/renderer/media/cast_t...
File chrome/renderer/media/cast_transport_sender_ipc.cc (right):

https://codereview.chromium.org/551883004/diff/1/chrome/renderer/media/cast_t...
chrome/renderer/media/cast_transport_sender_ipc.cc:26: Send(new
CastHostMsg_New(channel_id_, remote_end_point, options));
On 2014/09/09 00:30:45, Alpha wrote:
> If |options| is base::DictionaryValue the serialization / deserialization will
> work nicely.
> 
> 

Done.

https://codereview.chromium.org/551883004/diff/1/chrome/renderer/media/cast_u...
File chrome/renderer/media/cast_udp_transport.h (right):

https://codereview.chromium.org/551883004/diff/1/chrome/renderer/media/cast_u...
chrome/renderer/media/cast_udp_transport.h:27: void SetOptions(const
std::string& options);
On 2014/09/09 00:30:46, Alpha wrote:
> I suggest this be base::DictionaryValue which is a key value pair data
> structure.

Done.

https://codereview.chromium.org/551883004/diff/1/media/cast/net/cast_transpor...
File media/cast/net/cast_transport_sender.h (right):

https://codereview.chromium.org/551883004/diff/1/media/cast/net/cast_transpor...
media/cast/net/cast_transport_sender.h:56: const std::string& options,
On 2014/09/09 00:30:46, Alpha wrote:
> I suggest this be base::DictionaryValue which is a key value pair data
> structure.

Done.

[Alpha Left Google, 2014-09-09 20:15:12]

Sweet. Thanks! Only have two comments.

https://codereview.chromium.org/551883004/diff/20001/chrome/common/cast_messa...
File chrome/common/cast_messages.h (right):

https://codereview.chromium.org/551883004/diff/20001/chrome/common/cast_messa...
chrome/common/cast_messages.h:8: #include <string>
Is this still needed?

https://codereview.chromium.org/551883004/diff/20001/media/cast/net/cast_tran...
File media/cast/net/cast_transport_sender_impl.cc (right):

https://codereview.chromium.org/551883004/diff/20001/media/cast/net/cast_tran...
media/cast/net/cast_transport_sender_impl.cc:81: if (options->HasKey("DSCP")) {
I was thinking that we can apply DSCP value from the dictinoary. But this is
good for now. Might consider having this lower case to be consistent with future
option keys.

[hubbe, 2014-09-09 20:45:14]

https://codereview.chromium.org/551883004/diff/20001/chrome/common/cast_messa...
File chrome/common/cast_messages.h (right):

https://codereview.chromium.org/551883004/diff/20001/chrome/common/cast_messa...
chrome/common/cast_messages.h:8: #include <string>
On 2014/09/09 20:15:12, Alpha wrote:
> Is this still needed?

no

https://codereview.chromium.org/551883004/diff/20001/media/cast/net/cast_tran...
File media/cast/net/cast_transport_sender_impl.cc (right):

https://codereview.chromium.org/551883004/diff/20001/media/cast/net/cast_tran...
media/cast/net/cast_transport_sender_impl.cc:81: if (options->HasKey("DSCP")) {
On 2014/09/09 20:15:12, Alpha wrote:
> I was thinking that we can apply DSCP value from the dictinoary. But this is
> good for now. Might consider having this lower case to be consistent with
future
> option keys.
> 

Since DSCP is an abbreviation, I think it should be upper case.

[hubbe, 2014-09-09 20:45:55]

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

[hubbe, 2014-09-09 20:45:55]

+tsepez for *messages* security review.

[Alpha Left Google, 2014-09-09 20:46:50]

LGTM.

[Tom Sepez, 2014-09-09 23:49:26]

Messages LGTM

[hubbe, 2014-09-10 00:36:51]

The CQ bit was checked by hubbe@chromium.org

[commit-bot: I haz the power, 2014-09-10 00:43:51]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/hubbe@chromium.org/551883004/40001

[commit-bot: I haz the power, 2014-09-10 01:48:13]

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

[commit-bot: I haz the power, 2014-09-10 01:48:14]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[hubbe, 2014-09-10 18:29:23]

hubbe@chromium.org changed reviewers:
+ yoz@chromium.org

[hubbe, 2014-09-10 18:29:24]

+ yoz for chrome/{common,renderer}/extensions/* ownership.
(Also adding kalman, but I think he is OOO)

[Yoyo Zhou, 2014-09-10 19:15:08]

LGTM for extensions

[hubbe, 2014-09-10 19:19:18]

The CQ bit was checked by hubbe@chromium.org

[commit-bot: I haz the power, 2014-09-10 19:20:30]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/hubbe@chromium.org/551883004/40001

[commit-bot: I haz the power, 2014-09-10 19:55:50]

Committed patchset #3 (id:40001) as 2cf96d19b00c351ab8c971759a8552213f3598c1

[commit-bot: I haz the power, 2014-09-10 20:18:45]

Patchset 3 (id:??) landed as
https://crrev.com/857fde275f2b0e0ef555e83535d0dd3375a70123
Cr-Commit-Position: refs/heads/master@{#294211}