Cast: Add JS API to get raw events logs from cast extension.

Cast: Add JS API to get raw events logs from cast extension.

- 3 new APIs will be added to cast.streaming.rtpStream: startLogging, getRawEvents and getStats.
-- startLogging will install a event subscriber and start listening to events for a stream.
-- getRawEvents will return a string of serialized raw events logs for that stream.
-- getStats will return a Blob of stats for that stream in serialized format.
- Currently, only getRawEvents is implemented. This will finish the plumbing of raw event logs from cast library to extension to Js.
- CastSessionDelegate keeps a map from stream id to EncodingEventSubscribers instead of just having one subscriber for audio and one for video. Subscribers for a stream are only created when startLogging() for that stream is called.
- CastRtpStream keeps track of stream id so it can be passed to CastSessionDelegate. The ID will be set when it is assigned by CastStreamingNativeHandler.

BUG=301920, 338574
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=253686

[Alpha Left Google, 2014-02-24 20:18:56]

https://codereview.chromium.org/170063006/diff/160001/chrome/common/extension...
File chrome/common/extensions/api/cast_streaming_rtp_stream.idl (right):

https://codereview.chromium.org/170063006/diff/160001/chrome/common/extension...
chrome/common/extensions/api/cast_streaming_rtp_stream.idl:98: [nocompile]
static void startLogging(long streamId);
This should be enableRawEventsLogging(long streamId, bool enabled);

Having just a startXXX() but no stopXXX() doesn't feel right. A simple
enableXXX() is probably the best.

https://codereview.chromium.org/170063006/diff/160001/chrome/renderer/extensi...
File chrome/renderer/extensions/cast_streaming_native_handler.cc (right):

https://codereview.chromium.org/170063006/diff/160001/chrome/renderer/extensi...
chrome/renderer/extensions/cast_streaming_native_handler.cc:448:
get_raw_events_callbacks_.insert(std::make_pair(transport_id, callback));
This is odd. Why not just pass the callback and v8::function directly to
CallGetRawEventsCallback?

[imcheng, 2014-02-24 21:28:25]

https://codereview.chromium.org/170063006/diff/160001/chrome/common/extension...
File chrome/common/extensions/api/cast_streaming_rtp_stream.idl (right):

https://codereview.chromium.org/170063006/diff/160001/chrome/common/extension...
chrome/common/extensions/api/cast_streaming_rtp_stream.idl:98: [nocompile]
static void startLogging(long streamId);
On 2014/02/24 20:18:56, Alpha wrote:
> This should be enableRawEventsLogging(long streamId, bool enabled);
> 
> Having just a startXXX() but no stopXXX() doesn't feel right. A simple
> enableXXX() is probably the best.

Per discussion, we can have a way to stop logging and free up memory. But I want
to name it toggleLogging or similar - passing false to a function named
"enableXXX" in order to disable something seems odd.

https://codereview.chromium.org/170063006/diff/160001/chrome/renderer/extensi...
File chrome/renderer/extensions/cast_streaming_native_handler.cc (right):

https://codereview.chromium.org/170063006/diff/160001/chrome/renderer/extensi...
chrome/renderer/extensions/cast_streaming_native_handler.cc:448:
get_raw_events_callbacks_.insert(std::make_pair(transport_id, callback));
On 2014/02/24 20:18:56, Alpha wrote:
> This is odd. Why not just pass the callback and v8::function directly to
> CallGetRawEventsCallback?

This is similar to CreateCastSession / CallCreateCallback. Per discussion it was
because the function needs to be persistent across different scopes. The args
only return Locals.

[Alpha Left Google, 2014-02-24 23:25:51]

https://codereview.chromium.org/170063006/diff/220001/chrome/renderer/extensi...
File chrome/renderer/extensions/cast_streaming_native_handler.cc (right):

https://codereview.chromium.org/170063006/diff/220001/chrome/renderer/extensi...
chrome/renderer/extensions/cast_streaming_native_handler.cc:230:
stream1->SetStreamId(stream1_id);
stream id is a extensions API thing. It shouldn't be handed to CastRtpStream.
Can you hide that information in a callback?

https://codereview.chromium.org/170063006/diff/220001/chrome/renderer/media/c...
File chrome/renderer/media/cast_rtp_stream.h (right):

https://codereview.chromium.org/170063006/diff/220001/chrome/renderer/media/c...
chrome/renderer/media/cast_rtp_stream.h:120: void SetStreamId(const int
stream_id);
Stream ID doesn't belong to CastRtpStream.

https://codereview.chromium.org/170063006/diff/220001/chrome/renderer/media/c...
chrome/renderer/media/cast_rtp_stream.h:123: void ToggleLogging(const bool
enable);
nit: there's no need to make this const.

https://codereview.chromium.org/170063006/diff/220001/chrome/renderer/media/c...
File chrome/renderer/media/cast_session.h (right):

https://codereview.chromium.org/170063006/diff/220001/chrome/renderer/media/c...
chrome/renderer/media/cast_session.h:64: void ToggleLogging(const int stream_id,
You already have |is_audio|. Is |stream_id| useful?

https://codereview.chromium.org/170063006/diff/220001/chrome/renderer/media/c...
File chrome/renderer/media/cast_session_delegate.h (right):

https://codereview.chromium.org/170063006/diff/220001/chrome/renderer/media/c...
chrome/renderer/media/cast_session_delegate.h:63: void ToggleLogging(const int
stream_id,
Is |stream_id| useful given there's already |is_audio|? Also there's no need to
make int/bool as const.

[imcheng, 2014-02-25 08:10:17]

https://codereview.chromium.org/170063006/diff/220001/chrome/renderer/extensi...
File chrome/renderer/extensions/cast_streaming_native_handler.cc (right):

https://codereview.chromium.org/170063006/diff/220001/chrome/renderer/extensi...
chrome/renderer/extensions/cast_streaming_native_handler.cc:230:
stream1->SetStreamId(stream1_id);
On 2014/02/24 23:25:51, Alpha wrote:
> stream id is a extensions API thing. It shouldn't be handed to CastRtpStream.
> Can you hide that information in a callback?
> 

Per offline discussion, I am removing stream_id from CastRtpStream and down, and
stick with a bool to indicate video/audio for now.

https://codereview.chromium.org/170063006/diff/220001/chrome/renderer/media/c...
File chrome/renderer/media/cast_rtp_stream.h (right):

https://codereview.chromium.org/170063006/diff/220001/chrome/renderer/media/c...
chrome/renderer/media/cast_rtp_stream.h:120: void SetStreamId(const int
stream_id);
On 2014/02/24 23:25:51, Alpha wrote:
> Stream ID doesn't belong to CastRtpStream.

See other reply

https://codereview.chromium.org/170063006/diff/220001/chrome/renderer/media/c...
chrome/renderer/media/cast_rtp_stream.h:123: void ToggleLogging(const bool
enable);
On 2014/02/24 23:25:51, Alpha wrote:
> nit: there's no need to make this const.

Done.

https://codereview.chromium.org/170063006/diff/220001/chrome/renderer/media/c...
File chrome/renderer/media/cast_session.h (right):

https://codereview.chromium.org/170063006/diff/220001/chrome/renderer/media/c...
chrome/renderer/media/cast_session.h:64: void ToggleLogging(const int stream_id,
On 2014/02/24 23:25:51, Alpha wrote:
> You already have |is_audio|. Is |stream_id| useful?

See other reply.

https://codereview.chromium.org/170063006/diff/220001/chrome/renderer/media/c...
File chrome/renderer/media/cast_session_delegate.h (right):

https://codereview.chromium.org/170063006/diff/220001/chrome/renderer/media/c...
chrome/renderer/media/cast_session_delegate.h:63: void ToggleLogging(const int
stream_id,
On 2014/02/24 23:25:51, Alpha wrote:
> Is |stream_id| useful given there's already |is_audio|? Also there's no need
to
> make int/bool as const.

Done and see other reply.

[imcheng, 2014-02-25 19:31:47]

+kalman for extensions code.
+scherkus for chrome/renderer/media code.

[not at google - send to devlin, 2014-02-25 19:34:36]

https://codereview.chromium.org/170063006/diff/260001/chrome/common/extension...
File chrome/common/extensions/api/cast_streaming_rtp_stream.idl (right):

https://codereview.chromium.org/170063006/diff/260001/chrome/common/extension...
chrome/common/extensions/api/cast_streaming_rtp_stream.idl:64: // |rawEvents|:
events encoded in protocol buffer format along with other
encoded in protobuf? like the raw bytes? or are you unwrapping it into its real
key/values?

https://codereview.chromium.org/170063006/diff/260001/chrome/common/extension...
chrome/common/extensions/api/cast_streaming_rtp_stream.idl:105: [nocompile]
static void getRawEvents(
how big could this be? can you limit it?

[not at google - send to devlin, 2014-02-25 19:35:44]

also: reminder that when this API goes stable you'll need to have the API
re-viewed so consider getting early feedback about it in that case.

[scherkus (not reviewing), 2014-02-25 19:40:11]

lgtm

FYI as of http://crrev.com/253213 hclam@ should be able to do OWNERS reviews for
chrome/*/media/cast_* code

[imcheng, 2014-02-25 21:00:50]

https://codereview.chromium.org/170063006/diff/260001/chrome/common/extension...
File chrome/common/extensions/api/cast_streaming_rtp_stream.idl (right):

https://codereview.chromium.org/170063006/diff/260001/chrome/common/extension...
chrome/common/extensions/api/cast_streaming_rtp_stream.idl:64: // |rawEvents|:
events encoded in protocol buffer format along with other
On 2014/02/25 19:34:36, kalman wrote:
> encoded in protobuf? like the raw bytes? or are you unwrapping it into its
real
> key/values?

The events will be in serialized format as specified by protobuf library, so it
will be raw bytes. The exact format of the overall string is TBD because there
will be additional metadata in addition to just the events. I changed the
comments a bit. The specification of the format can be found at the cc file
mentioned here.

https://codereview.chromium.org/170063006/diff/260001/chrome/common/extension...
chrome/common/extensions/api/cast_streaming_rtp_stream.idl:105: [nocompile]
static void getRawEvents(
On 2014/02/25 19:34:36, kalman wrote:
> how big could this be? can you limit it?

Currently it is limited is 20MB. This will be smaller when we start to compress
the string before returning it to JS (~7 MB). The limit is subject to change but
won't be too large.

[not at google - send to devlin, 2014-02-25 21:03:45]

https://codereview.chromium.org/170063006/diff/260001/chrome/common/extension...
File chrome/common/extensions/api/cast_streaming_rtp_stream.idl (right):

https://codereview.chromium.org/170063006/diff/260001/chrome/common/extension...
chrome/common/extensions/api/cast_streaming_rtp_stream.idl:64: // |rawEvents|:
events encoded in protocol buffer format along with other
On 2014/02/25 21:00:51, imcheng1 wrote:
> On 2014/02/25 19:34:36, kalman wrote:
> > encoded in protobuf? like the raw bytes? or are you unwrapping it into its
> real
> > key/values?
> 
> The events will be in serialized format as specified by protobuf library, so
it
> will be raw bytes. The exact format of the overall string is TBD because there
> will be additional metadata in addition to just the events. I changed the
> comments a bit. The specification of the format can be found at the cc file
> mentioned here.

It's a bit crazy to expect extensions in the wild to include a protobuf parsing
library in order to use this API. It's reasonable for the wire format to be
protos but as a developer-facing API could you unwrap it into key-value?

[imcheng, 2014-02-25 21:17:14]

On 2014/02/25 21:03:45, kalman wrote:
>
https://codereview.chromium.org/170063006/diff/260001/chrome/common/extension...
> File chrome/common/extensions/api/cast_streaming_rtp_stream.idl (right):
> 
>
https://codereview.chromium.org/170063006/diff/260001/chrome/common/extension...
> chrome/common/extensions/api/cast_streaming_rtp_stream.idl:64: // |rawEvents|:
> events encoded in protocol buffer format along with other
> On 2014/02/25 21:00:51, imcheng1 wrote:
> > On 2014/02/25 19:34:36, kalman wrote:
> > > encoded in protobuf? like the raw bytes? or are you unwrapping it into its
> > real
> > > key/values?
> > 
> > The events will be in serialized format as specified by protobuf library, so
> it
> > will be raw bytes. The exact format of the overall string is TBD because
there
> > will be additional metadata in addition to just the events. I changed the
> > comments a bit. The specification of the format can be found at the cc file
> > mentioned here.
> 
> It's a bit crazy to expect extensions in the wild to include a protobuf
parsing
> library in order to use this API. It's reasonable for the wire format to be
> protos but as a developer-facing API could you unwrap it into key-value?

The current use case we are thinking about is that the cast extension will
upload this compressed raw bytes version to the Crash for offline analysis. We
will be building a separate tool on the other end to parse this file. We do not
really expect other extensions in the wild to use this API because we are
optimizing for size here (i.e. upload systems have a size limit, so we try to
stuff as many logs in as possible by using encoding and compression).

I agree it's reasonable to have some kind of key-value returning API when we
have a use case for it. But I think we have to be careful when implementing it
because the same data in key-value format will be a lot larger.

[not at google - send to devlin, 2014-02-25 21:22:07]

On 2014/02/25 21:17:14, imcheng1 wrote:
> On 2014/02/25 21:03:45, kalman wrote:
> >
>
https://codereview.chromium.org/170063006/diff/260001/chrome/common/extension...
> > File chrome/common/extensions/api/cast_streaming_rtp_stream.idl (right):
> > 
> >
>
https://codereview.chromium.org/170063006/diff/260001/chrome/common/extension...
> > chrome/common/extensions/api/cast_streaming_rtp_stream.idl:64: //
|rawEvents|:
> > events encoded in protocol buffer format along with other
> > On 2014/02/25 21:00:51, imcheng1 wrote:
> > > On 2014/02/25 19:34:36, kalman wrote:
> > > > encoded in protobuf? like the raw bytes? or are you unwrapping it into
its
> > > real
> > > > key/values?
> > > 
> > > The events will be in serialized format as specified by protobuf library,
so
> > it
> > > will be raw bytes. The exact format of the overall string is TBD because
> there
> > > will be additional metadata in addition to just the events. I changed the
> > > comments a bit. The specification of the format can be found at the cc
file
> > > mentioned here.
> > 
> > It's a bit crazy to expect extensions in the wild to include a protobuf
> parsing
> > library in order to use this API. It's reasonable for the wire format to be
> > protos but as a developer-facing API could you unwrap it into key-value?
> 
> The current use case we are thinking about is that the cast extension will
> upload this compressed raw bytes version to the Crash for offline analysis. We
> will be building a separate tool on the other end to parse this file. We do
not
> really expect other extensions in the wild to use this API because we are
> optimizing for size here (i.e. upload systems have a size limit, so we try to
> stuff as many logs in as possible by using encoding and compression).
> 
> I agree it's reasonable to have some kind of key-value returning API when we
> have a use case for it. But I think we have to be careful when implementing it
> because the same data in key-value format will be a lot larger.

Ok... well then I don't really know what the place for this functionality is. I
could also imagine you just exposing an "upload debugging data" function and not
forcing the extension to deal with the data since there's no expectation it will
actually care.

[imcheng, 2014-02-25 21:37:02]

On 2014/02/25 21:22:07, kalman wrote:
> On 2014/02/25 21:17:14, imcheng1 wrote:
> > On 2014/02/25 21:03:45, kalman wrote:
> > >
> >
>
https://codereview.chromium.org/170063006/diff/260001/chrome/common/extension...
> > > File chrome/common/extensions/api/cast_streaming_rtp_stream.idl (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/170063006/diff/260001/chrome/common/extension...
> > > chrome/common/extensions/api/cast_streaming_rtp_stream.idl:64: //
> |rawEvents|:
> > > events encoded in protocol buffer format along with other
> > > On 2014/02/25 21:00:51, imcheng1 wrote:
> > > > On 2014/02/25 19:34:36, kalman wrote:
> > > > > encoded in protobuf? like the raw bytes? or are you unwrapping it into
> its
> > > > real
> > > > > key/values?
> > > > 
> > > > The events will be in serialized format as specified by protobuf
library,
> so
> > > it
> > > > will be raw bytes. The exact format of the overall string is TBD because
> > there
> > > > will be additional metadata in addition to just the events. I changed
the
> > > > comments a bit. The specification of the format can be found at the cc
> file
> > > > mentioned here.
> > > 
> > > It's a bit crazy to expect extensions in the wild to include a protobuf
> > parsing
> > > library in order to use this API. It's reasonable for the wire format to
be
> > > protos but as a developer-facing API could you unwrap it into key-value?
> > 
> > The current use case we are thinking about is that the cast extension will
> > upload this compressed raw bytes version to the Crash for offline analysis.
We
> > will be building a separate tool on the other end to parse this file. We do
> not
> > really expect other extensions in the wild to use this API because we are
> > optimizing for size here (i.e. upload systems have a size limit, so we try
to
> > stuff as many logs in as possible by using encoding and compression).
> > 
> > I agree it's reasonable to have some kind of key-value returning API when we
> > have a use case for it. But I think we have to be careful when implementing
it
> > because the same data in key-value format will be a lot larger.
> 
> Ok... well then I don't really know what the place for this functionality is.
I
> could also imagine you just exposing an "upload debugging data" function and
not
> forcing the extension to deal with the data since there's no expectation it
will
> actually care.

The reason why we are deciding to offer a API to get the data rather than just a
straight upload API is because it gives the extension more freedom with what to
do with the data. For example the cast extension might have several choices as
to where to upload the data, or it might just want to save the data locally.
This will be easier to implement with a "get data" API than a upload API.

Also while we don't expect other extensions to use this API, they could if they
*really* wanted to. We will build a tool that they can use to parse the data.

[not at google - send to devlin, 2014-02-25 21:51:11]

> 
> The reason why we are deciding to offer a API to get the data rather than just
a
> straight upload API is because it gives the extension more freedom with what
to
> do with the data. For example the cast extension might have several choices as
> to where to upload the data, or it might just want to save the data locally.
> This will be easier to implement with a "get data" API than a upload API.
> 
> Also while we don't expect other extensions to use this API, they could if
they
> *really* wanted to. We will build a tool that they can use to parse the data.

So one advantage of the higher level API for feedback was that the browser can
implement the retry logic (e.g. if the issue is network connectivity or being
offline then you can't upload that data obviously). The extension can probably
implement this but it would be inferior.

But, up to you. lgtm so you can at least get this rolling.

[Alpha Left Google, 2014-02-26 00:53:51]

https://codereview.chromium.org/170063006/diff/280001/chrome/renderer/media/c...
File chrome/renderer/media/cast_session.h (right):

https://codereview.chromium.org/170063006/diff/280001/chrome/renderer/media/c...
chrome/renderer/media/cast_session.h:64: void ToggleLogging(bool enable, bool
is_audio);
nit: The order of arguments should match GetEventLogsAndReset() to be
consistent. That means |is_audio| first and then |enable|.

https://codereview.chromium.org/170063006/diff/280001/chrome/renderer/media/c...
File chrome/renderer/media/cast_session_delegate.cc (right):

https://codereview.chromium.org/170063006/diff/280001/chrome/renderer/media/c...
chrome/renderer/media/cast_session_delegate.cc:135:
DCHECK(audio_event_subscriber_.get());
This branch is not right. The application can call disable multiple times.

https://codereview.chromium.org/170063006/diff/280001/chrome/renderer/media/c...
chrome/renderer/media/cast_session_delegate.cc:140:
DCHECK(video_event_subscriber_.get());
Same here. Application can break this DCHECK by calling ToggleLogging(false) a
couple times.

https://codereview.chromium.org/170063006/diff/280001/chrome/renderer/media/c...
chrome/renderer/media/cast_session_delegate.cc:154: DCHECK(subscriber);
What happens if logging has not started by this method is called?

[imcheng, 2014-02-26 01:21:07]

https://codereview.chromium.org/170063006/diff/280001/chrome/renderer/media/c...
File chrome/renderer/media/cast_session.h (right):

https://codereview.chromium.org/170063006/diff/280001/chrome/renderer/media/c...
chrome/renderer/media/cast_session.h:64: void ToggleLogging(bool enable, bool
is_audio);
On 2014/02/26 00:53:52, Alpha wrote:
> nit: The order of arguments should match GetEventLogsAndReset() to be
> consistent. That means |is_audio| first and then |enable|.

Done.

https://codereview.chromium.org/170063006/diff/280001/chrome/renderer/media/c...
File chrome/renderer/media/cast_session_delegate.cc (right):

https://codereview.chromium.org/170063006/diff/280001/chrome/renderer/media/c...
chrome/renderer/media/cast_session_delegate.cc:135:
DCHECK(audio_event_subscriber_.get());
On 2014/02/26 00:53:52, Alpha wrote:
> This branch is not right. The application can call disable multiple times.

You are right. These shouldn't be DCHECKs. I changed them to return early if
enable / disable is called multiple times in a row.

https://codereview.chromium.org/170063006/diff/280001/chrome/renderer/media/c...
chrome/renderer/media/cast_session_delegate.cc:140:
DCHECK(video_event_subscriber_.get());
On 2014/02/26 00:53:52, Alpha wrote:
> Same here. Application can break this DCHECK by calling ToggleLogging(false) a
> couple times.

Done.

https://codereview.chromium.org/170063006/diff/280001/chrome/renderer/media/c...
chrome/renderer/media/cast_session_delegate.cc:154: DCHECK(subscriber);
On 2014/02/26 00:53:52, Alpha wrote:
> What happens if logging has not started by this method is called?

I changed this to return early (callback not invoked). The behavior is the same
if serialization fails.

[imcheng, 2014-02-26 01:26:00]

https://codereview.chromium.org/170063006/diff/280001/chrome/renderer/media/c...
File chrome/renderer/media/cast_session_delegate.cc (right):

https://codereview.chromium.org/170063006/diff/280001/chrome/renderer/media/c...
chrome/renderer/media/cast_session_delegate.cc:154: DCHECK(subscriber);
On 2014/02/26 01:21:07, imcheng1 wrote:
> On 2014/02/26 00:53:52, Alpha wrote:
> > What happens if logging has not started by this method is called?
> 
> I changed this to return early (callback not invoked). The behavior is the
same
> if serialization fails.

I was thinking if it would be better if caller provided a error callback which
will be invoked if there is a failure? Or maybe fire an event or throw an
exception?

[Alpha Left Google, 2014-02-26 01:32:42]

If serialization fails we should just return a string.

https://codereview.chromium.org/170063006/diff/300001/chrome/renderer/media/c...
File chrome/renderer/media/cast_session_delegate.cc (right):

https://codereview.chromium.org/170063006/diff/300001/chrome/renderer/media/c...
chrome/renderer/media/cast_session_delegate.cc:158: if (!subscriber)
We should call the callback with an empty string in this case. Otherwise there
will be a permanent entry in the map in CastStreamingNativeHandlers.

[Alpha Left Google, 2014-02-26 01:33:42]

What about writing an additional API test for the case that application is doing
the wrong thing and receives an empty log.

[imcheng, 2014-02-26 03:00:16]

On 2014/02/26 01:33:42, Alpha wrote:
> What about writing an additional API test for the case that application is
doing
> the wrong thing and receives an empty log.

Added a new API test, with a new html and js files.

[imcheng, 2014-02-26 03:00:27]

https://codereview.chromium.org/170063006/diff/300001/chrome/renderer/media/c...
File chrome/renderer/media/cast_session_delegate.cc (right):

https://codereview.chromium.org/170063006/diff/300001/chrome/renderer/media/c...
chrome/renderer/media/cast_session_delegate.cc:158: if (!subscriber)
On 2014/02/26 01:32:43, Alpha wrote:
> We should call the callback with an empty string in this case. Otherwise there
> will be a permanent entry in the map in CastStreamingNativeHandlers.

Oh good point. Done.

[Alpha Left Google, 2014-02-26 03:06:26]

https://codereview.chromium.org/170063006/diff/360001/chrome/renderer/media/c...
File chrome/renderer/media/cast_session_delegate.cc (right):

https://codereview.chromium.org/170063006/diff/360001/chrome/renderer/media/c...
chrome/renderer/media/cast_session_delegate.cc:160: }
Call the callback and then return.

https://codereview.chromium.org/170063006/diff/360001/chrome/renderer/media/c...
chrome/renderer/media/cast_session_delegate.cc:175: }
Call the callback and then return.

[imcheng, 2014-02-26 04:41:56]

https://codereview.chromium.org/170063006/diff/360001/chrome/renderer/media/c...
File chrome/renderer/media/cast_session_delegate.cc (right):

https://codereview.chromium.org/170063006/diff/360001/chrome/renderer/media/c...
chrome/renderer/media/cast_session_delegate.cc:160: }
On 2014/02/26 03:06:26, Alpha wrote:
> Call the callback and then return.

Done.

https://codereview.chromium.org/170063006/diff/360001/chrome/renderer/media/c...
chrome/renderer/media/cast_session_delegate.cc:175: }
On 2014/02/26 03:06:26, Alpha wrote:
> Call the callback and then return.

Done.

[Alpha Left Google, 2014-02-26 07:19:26]

LGTM.

[imcheng, 2014-02-26 19:55:05]

The CQ bit was checked by imcheng@chromium.org

[commit-bot: I haz the power, 2014-02-26 19:56:31]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/imcheng@chromium.org/170063006/380001

[haibinlu, 2014-02-26 19:59:08]

lgtm

[imcheng, 2014-02-26 20:05:18]

The CQ bit was checked by imcheng@chromium.org

[commit-bot: I haz the power, 2014-02-26 20:20:19]

Failed to trigger a try job on chromium_presubmit
HTTP Error 400: Bad Request

[commit-bot: I haz the power, 2014-02-26 20:20:41]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/imcheng@chromium.org/170063006/400001

[commit-bot: I haz the power, 2014-02-26 21:06:25]

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

[commit-bot: I haz the power, 2014-02-26 21:06:26]

Retried try job too often on ios_dbg_simulator for step(s) base_unittests,
compile, components_unittests, content_unittests, crypto_unittests,
net_unittests, sql_unittests, ui_unittests, url_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=ios_dbg_si...

[imcheng, 2014-02-26 22:29:53]

The CQ bit was checked by imcheng@chromium.org

[commit-bot: I haz the power, 2014-02-26 22:30:20]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/imcheng@chromium.org/170063006/400001

[commit-bot: I haz the power, 2014-02-27 02:21:52]

Change committed as 253686