media: add basic MediaRenderer mojom and TypeConverters

media: add basic MediaRenderer mojom and TypeConverters

This patch is the basis for mojo transport between a media::Pipeline and a media::Renderer implementation, e.g:

[ Pipeline [ media::Renderer [ Mojo Transport ] media::RendererImpl ] Output ]

The plan is to continue to iterate on the 'Mojo Transport' chunk without blocking anyone wishing to prototype one level up on either side.  As such, some things in this CL like DecodeAndRender are not intended to be the proper long term solution, but allow immediate and mostly isolated prototyping to happen in parallel to development of a utility / class to allow framed data flowing over a mojo::DataPipe. As several TODOs in the CL mention, the current transport implementation incurs a high cost (many large allocations) and has inherently low throughput (can't write new buffer until each buffer is ACKed). 

BUG=410451
Committed: https://crrev.com/9f8fe767920001f734dddc678bad491be8cc363b
Cr-Commit-Position: refs/heads/master@{#293392}

[xhwang, 2014-08-22 23:30:02]

I know this is not for review. Just having some nits (mostly on naming) anyways
:)

https://codereview.chromium.org/491733004/diff/1/mojo/services/public/interfa...
File mojo/services/public/interfaces/media/media_renderer.mojom (right):

https://codereview.chromium.org/491733004/diff/1/mojo/services/public/interfa...
mojo/services/public/interfaces/media/media_renderer.mojom:28: GetMediaTime() =>
(int64 time_delta_usec);
Since we have OnTimeAdvanced, we probably don't need this at all. The
media::MojoRendererImpl can implement this based on the time updates.

https://codereview.chromium.org/491733004/diff/1/mojo/services/public/interfa...
mojo/services/public/interfaces/media/media_renderer.mojom:35:
OnNewStatistics(PipelineStatistics statistics);
nit: OnStatistics

https://codereview.chromium.org/491733004/diff/1/mojo/services/public/interfa...
mojo/services/public/interfaces/media/media_renderer.mojom:36:
OnTimeAdvanced(int64 time_delta_usec);
OnTimeUpdate() would be a more media-ish term :)

https://codereview.chromium.org/491733004/diff/1/mojo/services/public/interfa...
mojo/services/public/interfaces/media/media_renderer.mojom:38: OnEndOfStream();
OnEnded() to be more consistent with media Pipeline terms.

https://codereview.chromium.org/491733004/diff/1/mojo/services/public/interfa...
mojo/services/public/interfaces/media/media_renderer.mojom:39:
OnPipelineError(PipelineStatus error);
Renderer knows nothing about Pipeline. So just OnError()?

https://codereview.chromium.org/491733004/diff/1/mojo/services/public/interfa...
File mojo/services/public/interfaces/media/media_types.mojom (right):

https://codereview.chromium.org/491733004/diff/1/mojo/services/public/interfa...
mojo/services/public/interfaces/media/media_types.mojom:23: };
As discussed offline, you can just drop this. We only care about:
1, Whether initialization succeeded or failed
2, Whether any error happened.

[darin (slow to review), 2014-08-23 17:27:03]

I wonder if we shouldn't consider putting the mojom and impl-side under
src/media...
On Aug 22, 2014 4:30 PM, <xhwang@chromium.org> wrote:

> I know this is not for review. Just having some nits (mostly on naming)
> anyways
> :)
>
>
> https://codereview.chromium.org/491733004/diff/1/mojo/
> services/public/interfaces/media/media_renderer.mojom
> File mojo/services/public/interfaces/media/media_renderer.mojom (right):
>
> https://codereview.chromium.org/491733004/diff/1/mojo/
> services/public/interfaces/media/media_renderer.mojom#newcode28
> mojo/services/public/interfaces/media/media_renderer.mojom:28:
> GetMediaTime() => (int64 time_delta_usec);
> Since we have OnTimeAdvanced, we probably don't need this at all. The
> media::MojoRendererImpl can implement this based on the time updates.
>
> https://codereview.chromium.org/491733004/diff/1/mojo/
> services/public/interfaces/media/media_renderer.mojom#newcode35
> mojo/services/public/interfaces/media/media_renderer.mojom:35:
> OnNewStatistics(PipelineStatistics statistics);
> nit: OnStatistics
>
> https://codereview.chromium.org/491733004/diff/1/mojo/
> services/public/interfaces/media/media_renderer.mojom#newcode36
> mojo/services/public/interfaces/media/media_renderer.mojom:36:
> OnTimeAdvanced(int64 time_delta_usec);
> OnTimeUpdate() would be a more media-ish term :)
>
> https://codereview.chromium.org/491733004/diff/1/mojo/
> services/public/interfaces/media/media_renderer.mojom#newcode38
> mojo/services/public/interfaces/media/media_renderer.mojom:38:
> OnEndOfStream();
> OnEnded() to be more consistent with media Pipeline terms.
>
> https://codereview.chromium.org/491733004/diff/1/mojo/
> services/public/interfaces/media/media_renderer.mojom#newcode39
> mojo/services/public/interfaces/media/media_renderer.mojom:39:
> OnPipelineError(PipelineStatus error);
> Renderer knows nothing about Pipeline. So just OnError()?
>
> https://codereview.chromium.org/491733004/diff/1/mojo/
> services/public/interfaces/media/media_types.mojom
> File mojo/services/public/interfaces/media/media_types.mojom (right):
>
> https://codereview.chromium.org/491733004/diff/1/mojo/
> services/public/interfaces/media/media_types.mojom#newcode23
> mojo/services/public/interfaces/media/media_types.mojom:23: };
> As discussed offline, you can just drop this. We only care about:
> 1, Whether initialization succeeded or failed
> 2, Whether any error happened.
>
> https://codereview.chromium.org/491733004/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[tim (not reviewing), 2014-09-02 20:36:01]

Patchset #4 (id:80001) has been deleted

[tim (not reviewing), 2014-09-02 20:36:14]

Patchset #3 (id:60001) has been deleted

[tim (not reviewing), 2014-09-02 20:43:14]

tim@chromium.org changed reviewers:
+ darin@chromium.org

[tim (not reviewing), 2014-09-02 20:43:14]

Please read updated CL description / disclaimer, ping me if you're still
confused about the goal for this.

xhwang@: ready for another look!

darin@: moved to src/media. Can you look at mojo_variables.gyp (and
corresponding media.gyp change?), as well as media/mojo/DEPS?

alokp@, davemoore@, scherkus@: fyi

[darin (slow to review), 2014-09-02 20:51:47]

https://codereview.chromium.org/491733004/diff/100001/media/mojo/interfaces/m...
File media/mojo/interfaces/media_types.mojom (right):

https://codereview.chromium.org/491733004/diff/100001/media/mojo/interfaces/m...
media/mojo/interfaces/media_types.mojom:12: BUFFERING_HAVE_NOTHING,
doesn't this mean we end up with BUFFERING_STATE_BUFFERING_HAVE_NOTHING in C++?
it'd be nice to not repeat the word "BUFFERING" there, right?

[tim (not reviewing), 2014-09-02 21:49:13]

Patchset #4 (id:110001) has been deleted

[tim (not reviewing), 2014-09-02 21:55:50]

https://codereview.chromium.org/491733004/diff/100001/media/mojo/interfaces/m...
File media/mojo/interfaces/media_types.mojom (right):

https://codereview.chromium.org/491733004/diff/100001/media/mojo/interfaces/m...
media/mojo/interfaces/media_types.mojom:12: BUFFERING_HAVE_NOTHING,
On 2014/09/02 20:51:47, darin wrote:
> doesn't this mean we end up with BUFFERING_STATE_BUFFERING_HAVE_NOTHING in
C++?
> it'd be nice to not repeat the word "BUFFERING" there, right?

Ah yes. Done.

[scherkus (not reviewing), 2014-09-02 23:50:19]

scherkus@chromium.org changed reviewers:
+ scherkus@chromium.org

[scherkus (not reviewing), 2014-09-02 23:50:20]

nits and a question about time

https://codereview.chromium.org/491733004/diff/130001/media/mojo/interfaces/m...
File media/mojo/interfaces/media_renderer.mojom (right):

https://codereview.chromium.org/491733004/diff/130001/media/mojo/interfaces/m...
media/mojo/interfaces/media_renderer.mojom:24: StartPlayingFrom(int64
time_delta_usec);
does mojo have any equivalent time types?

https://codereview.chromium.org/491733004/diff/130001/media/mojo/interfaces/m...
File media/mojo/interfaces/media_types.mojom (right):

https://codereview.chromium.org/491733004/diff/130001/media/mojo/interfaces/m...
media/mojo/interfaces/media_types.mojom:22: int64 timestamp;
_usec?

https://codereview.chromium.org/491733004/diff/130001/media/mojo/interfaces/m...
media/mojo/interfaces/media_types.mojom:37: // Indicates this buffer is part of
a splice around |splice_timestamp_|.
nit: drop trailing _

https://codereview.chromium.org/491733004/diff/130001/media/mojo/interfaces/m...
media/mojo/interfaces/media_types.mojom:38: int64 splice_timestamp;
_usec?

https://codereview.chromium.org/491733004/diff/130001/media/mojo/services/med...
File media/mojo/services/media_type_converters.h (right):

https://codereview.chromium.org/491733004/diff/130001/media/mojo/services/med...
media/mojo/services/media_type_converters.h:22: const
scoped_refptr<media::DecoderBuffer> input);
const-& ?

[xhwang, 2014-09-03 00:09:09]

looking pretty good. just a bunch of nits.

https://codereview.chromium.org/491733004/diff/130001/media/mojo/interfaces/m...
File media/mojo/interfaces/media_renderer.mojom (right):

https://codereview.chromium.org/491733004/diff/130001/media/mojo/interfaces/m...
media/mojo/interfaces/media_renderer.mojom:12: Initialize() => (bool succeeded);
FYI: The callback can be a closure after
https://codereview.chromium.org/526653002/ is landed.

https://codereview.chromium.org/491733004/diff/130001/media/mojo/interfaces/m...
media/mojo/interfaces/media_renderer.mojom:14: // Decode and render |buffer|,
calling back when more data is required.
nit: Decode_s_ and render_s_

https://codereview.chromium.org/491733004/diff/130001/media/mojo/interfaces/m...
media/mojo/interfaces/media_renderer.mojom:20: // Discards any buffered data,
executing callback when completed.
nit: Add something like "Fires OnError() before the callback if error happened."

https://codereview.chromium.org/491733004/diff/130001/media/mojo/interfaces/m...
media/mojo/interfaces/media_renderer.mojom:23: // Starts rendering from |time|.
nit: s/time/time_usec

https://codereview.chromium.org/491733004/diff/130001/media/mojo/interfaces/m...
media/mojo/interfaces/media_renderer.mojom:24: StartPlayingFrom(int64
time_delta_usec);
nit: s/time_delta_usec/time_usec?

https://codereview.chromium.org/491733004/diff/130001/media/mojo/interfaces/m...
media/mojo/interfaces/media_renderer.mojom:34: // Called whenever rendering has
advanced time by |time_delta_usec|.
nit: How about "Called to report media time advance of |time_delta_usec|"?

"whenever" is a bit misleading here since the caller decides when to call.

https://codereview.chromium.org/491733004/diff/130001/media/mojo/interfaces/m...
media/mojo/interfaces/media_renderer.mojom:43: // Executed if any error was
encountered during decode or rendering.
nit: If this error happens during an operation that has a completion callback,
call OnError() before firing the completion callback.

https://codereview.chromium.org/491733004/diff/130001/media/mojo/interfaces/m...
File media/mojo/interfaces/media_types.mojom (right):

https://codereview.chromium.org/491733004/diff/130001/media/mojo/interfaces/m...
media/mojo/interfaces/media_types.mojom:23: int64 duration;
s/timestamp/timestamp_usec
s/duration/duration_usec

https://codereview.chromium.org/491733004/diff/130001/media/mojo/interfaces/m...
media/mojo/interfaces/media_types.mojom:38: int64 splice_timestamp;
splice_timestamp_usec?

https://codereview.chromium.org/491733004/diff/130001/media/mojo/interfaces/m...
media/mojo/interfaces/media_types.mojom:41: // TODO(tim): This currently results
in allocating a new, largeish datapipe
nit: s/datapipe/DataPipe

https://codereview.chromium.org/491733004/diff/130001/media/mojo/services/med...
File media/mojo/services/media_type_converters.cc (right):

https://codereview.chromium.org/491733004/diff/130001/media/mojo/services/med...
media/mojo/services/media_type_converters.cc:26: mojo_buffer->timestamp =
input->timestamp().ToInternalValue();
Here and below, s/ToInternalValue/InMicroseconds/ to avoid relying on TimeDelta
implementation detail?

https://codereview.chromium.org/491733004/diff/130001/media/mojo/services/med...
media/mojo/services/media_type_converters.cc:89:
buffer->set_timestamp(base::TimeDelta::FromInternalValue(input->timestamp));
ditto: FromMicroseconds()

https://codereview.chromium.org/491733004/diff/130001/media/mojo/services/med...
File media/mojo/services/media_type_converters.h (right):

https://codereview.chromium.org/491733004/diff/130001/media/mojo/services/med...
media/mojo/services/media_type_converters.h:22: const
scoped_refptr<media::DecoderBuffer> input);
nit: in media code, we pass scoped_refptr by const-ref. But feel free to use
whatever is default in the mojo world :)

https://codereview.chromium.org/491733004/diff/130001/media/mojo/services/med...
File media/mojo/services/media_type_converters_unittest.cc (right):

https://codereview.chromium.org/491733004/diff/130001/media/mojo/services/med...
media/mojo/services/media_type_converters_unittest.cc:24: reinterpret_cast<const
uint8*>(&kSideData), kSideDataSize));
Use non-default timestamp/duration to make sure they're set correctly?

[tim (not reviewing), 2014-09-03 00:43:51]

https://codereview.chromium.org/491733004/diff/130001/media/mojo/interfaces/m...
File media/mojo/interfaces/media_renderer.mojom (right):

https://codereview.chromium.org/491733004/diff/130001/media/mojo/interfaces/m...
media/mojo/interfaces/media_renderer.mojom:12: Initialize() => (bool succeeded);
On 2014/09/03 00:09:08, xhwang wrote:
> FYI: The callback can be a closure after
> https://codereview.chromium.org/526653002/ is landed.

Acknowledged.

https://codereview.chromium.org/491733004/diff/130001/media/mojo/interfaces/m...
media/mojo/interfaces/media_renderer.mojom:14: // Decode and render |buffer|,
calling back when more data is required.
On 2014/09/03 00:09:08, xhwang wrote:
> nit: Decode_s_ and render_s_

OK, Done.

https://codereview.chromium.org/491733004/diff/130001/media/mojo/interfaces/m...
media/mojo/interfaces/media_renderer.mojom:20: // Discards any buffered data,
executing callback when completed.
On 2014/09/03 00:09:08, xhwang wrote:
> nit: Add something like "Fires OnError() before the callback if error
happened."

Done.

https://codereview.chromium.org/491733004/diff/130001/media/mojo/interfaces/m...
media/mojo/interfaces/media_renderer.mojom:23: // Starts rendering from |time|.
On 2014/09/03 00:09:08, xhwang wrote:
> nit: s/time/time_usec

Done.

https://codereview.chromium.org/491733004/diff/130001/media/mojo/interfaces/m...
media/mojo/interfaces/media_renderer.mojom:24: StartPlayingFrom(int64
time_delta_usec);
On 2014/09/03 00:09:08, xhwang wrote:
> nit: s/time_delta_usec/time_usec?

Done.

https://codereview.chromium.org/491733004/diff/130001/media/mojo/interfaces/m...
media/mojo/interfaces/media_renderer.mojom:24: StartPlayingFrom(int64
time_delta_usec);
On 2014/09/02 23:50:19, scherkus wrote:
> does mojo have any equivalent time types?

No. I asked the same question to either Darin or Trung when writing
TestTimeService (https://codereview.chromium.org/349303006) and it wasn't
something we wanted to add. I'm not sure I remember why aside from probably
"seems like extra bells and whistles we don't need (yet)"...

https://codereview.chromium.org/491733004/diff/130001/media/mojo/interfaces/m...
media/mojo/interfaces/media_renderer.mojom:34: // Called whenever rendering has
advanced time by |time_delta_usec|.
On 2014/09/03 00:09:08, xhwang wrote:
> nit: How about "Called to report media time advance of |time_delta_usec|"?
> 
> "whenever" is a bit misleading here since the caller decides when to call.

Agreed, done.

https://codereview.chromium.org/491733004/diff/130001/media/mojo/interfaces/m...
media/mojo/interfaces/media_renderer.mojom:43: // Executed if any error was
encountered during decode or rendering.
On 2014/09/03 00:09:08, xhwang wrote:
> nit: If this error happens during an operation that has a completion callback,
> call OnError() before firing the completion callback.

Done.

https://codereview.chromium.org/491733004/diff/130001/media/mojo/interfaces/m...
File media/mojo/interfaces/media_types.mojom (right):

https://codereview.chromium.org/491733004/diff/130001/media/mojo/interfaces/m...
media/mojo/interfaces/media_types.mojom:22: int64 timestamp;
On 2014/09/02 23:50:20, scherkus wrote:
> _usec?

Done.

https://codereview.chromium.org/491733004/diff/130001/media/mojo/interfaces/m...
media/mojo/interfaces/media_types.mojom:23: int64 duration;
On 2014/09/03 00:09:08, xhwang wrote:
> s/timestamp/timestamp_usec
> s/duration/duration_usec

Done.

https://codereview.chromium.org/491733004/diff/130001/media/mojo/interfaces/m...
media/mojo/interfaces/media_types.mojom:37: // Indicates this buffer is part of
a splice around |splice_timestamp_|.
On 2014/09/02 23:50:20, scherkus wrote:
> nit: drop trailing _

Done.

https://codereview.chromium.org/491733004/diff/130001/media/mojo/interfaces/m...
media/mojo/interfaces/media_types.mojom:38: int64 splice_timestamp;
On 2014/09/02 23:50:20, scherkus wrote:
> _usec?

Done.

https://codereview.chromium.org/491733004/diff/130001/media/mojo/interfaces/m...
media/mojo/interfaces/media_types.mojom:41: // TODO(tim): This currently results
in allocating a new, largeish datapipe
On 2014/09/03 00:09:08, xhwang wrote:
> nit: s/datapipe/DataPipe

Done.

https://codereview.chromium.org/491733004/diff/130001/media/mojo/services/med...
File media/mojo/services/media_type_converters.cc (right):

https://codereview.chromium.org/491733004/diff/130001/media/mojo/services/med...
media/mojo/services/media_type_converters.cc:26: mojo_buffer->timestamp =
input->timestamp().ToInternalValue();
On 2014/09/03 00:09:08, xhwang wrote:
> Here and below, s/ToInternalValue/InMicroseconds/ to avoid relying on
TimeDelta
> implementation detail?

Ah, good call.  Done.

https://codereview.chromium.org/491733004/diff/130001/media/mojo/services/med...
media/mojo/services/media_type_converters.cc:89:
buffer->set_timestamp(base::TimeDelta::FromInternalValue(input->timestamp));
On 2014/09/03 00:09:08, xhwang wrote:
> ditto: FromMicroseconds()

Done.

https://codereview.chromium.org/491733004/diff/130001/media/mojo/services/med...
File media/mojo/services/media_type_converters.h (right):

https://codereview.chromium.org/491733004/diff/130001/media/mojo/services/med...
media/mojo/services/media_type_converters.h:22: const
scoped_refptr<media::DecoderBuffer> input);
On 2014/09/03 00:09:08, xhwang wrote:
> nit: in media code, we pass scoped_refptr by const-ref. But feel free to use
> whatever is default in the mojo world :)

Oops. Syntactically valid typo :( Fixed.

https://codereview.chromium.org/491733004/diff/130001/media/mojo/services/med...
File media/mojo/services/media_type_converters_unittest.cc (right):

https://codereview.chromium.org/491733004/diff/130001/media/mojo/services/med...
media/mojo/services/media_type_converters_unittest.cc:24: reinterpret_cast<const
uint8*>(&kSideData), kSideDataSize));
On 2014/09/03 00:09:08, xhwang wrote:
> Use non-default timestamp/duration to make sure they're set correctly?

Forgot to come back and do that! Thanks, done.

[xhwang, 2014-09-03 16:09:38]

lgtm % nits

https://codereview.chromium.org/491733004/diff/150001/media/mojo/interfaces/m...
File media/mojo/interfaces/media_renderer.mojom (right):

https://codereview.chromium.org/491733004/diff/150001/media/mojo/interfaces/m...
media/mojo/interfaces/media_renderer.mojom:14: // Decodes and renders|buffer|,
calling back when more data is required.
add space after "renders"

https://codereview.chromium.org/491733004/diff/150001/media/mojo/interfaces/m...
media/mojo/interfaces/media_renderer.mojom:15: // NOTE: If an error occurs,
MediaRendererClient::OnError will be called
s/OnError/OnError()/

https://codereview.chromium.org/491733004/diff/150001/media/mojo/interfaces/m...
media/mojo/interfaces/media_renderer.mojom:23: // NOTE: If an error occurs,
MediaRendererClient::OnError can be called
ditto

https://codereview.chromium.org/491733004/diff/150001/media/mojo/interfaces/m...
media/mojo/interfaces/media_renderer.mojom:39: OnTimeUpdate(int64
time_delta_usec);
s/time_delta_usec/time_usec/

https://codereview.chromium.org/491733004/diff/150001/media/mojo/services/med...
File media/mojo/services/media_type_converters.cc (right):

https://codereview.chromium.org/491733004/diff/150001/media/mojo/services/med...
media/mojo/services/media_type_converters.cc:63:
scoped_refptr<media::DecoderBuffer>  TypeConverter<MediaDecoderBufferPtr,
add //static as well?

[tim (not reviewing), 2014-09-03 16:39:46]

Thanks for the review.

https://codereview.chromium.org/491733004/diff/150001/media/mojo/interfaces/m...
File media/mojo/interfaces/media_renderer.mojom (right):

https://codereview.chromium.org/491733004/diff/150001/media/mojo/interfaces/m...
media/mojo/interfaces/media_renderer.mojom:14: // Decodes and renders|buffer|,
calling back when more data is required.
On 2014/09/03 16:09:38, xhwang wrote:
> add space after "renders"

Done.

https://codereview.chromium.org/491733004/diff/150001/media/mojo/interfaces/m...
media/mojo/interfaces/media_renderer.mojom:15: // NOTE: If an error occurs,
MediaRendererClient::OnError will be called
On 2014/09/03 16:09:38, xhwang wrote:
> s/OnError/OnError()/

Done.

https://codereview.chromium.org/491733004/diff/150001/media/mojo/interfaces/m...
media/mojo/interfaces/media_renderer.mojom:23: // NOTE: If an error occurs,
MediaRendererClient::OnError can be called
On 2014/09/03 16:09:38, xhwang wrote:
> ditto

Done.

https://codereview.chromium.org/491733004/diff/150001/media/mojo/interfaces/m...
media/mojo/interfaces/media_renderer.mojom:39: OnTimeUpdate(int64
time_delta_usec);
On 2014/09/03 16:09:38, xhwang wrote:
> s/time_delta_usec/time_usec/

Done.

https://codereview.chromium.org/491733004/diff/150001/media/mojo/services/med...
File media/mojo/services/media_type_converters.cc (right):

https://codereview.chromium.org/491733004/diff/150001/media/mojo/services/med...
media/mojo/services/media_type_converters.cc:63:
scoped_refptr<media::DecoderBuffer>  TypeConverter<MediaDecoderBufferPtr,
On 2014/09/03 16:09:38, xhwang wrote:
> add //static as well?

Done.

[scherkus (not reviewing), 2014-09-03 17:08:29]

lgtm w/ nits

OOC should we file a tracking BUG for media mojo stuff and update the CL
description?

https://codereview.chromium.org/491733004/diff/170001/media/mojo/DEPS
File media/mojo/DEPS (right):

https://codereview.chromium.org/491733004/diff/170001/media/mojo/DEPS#newcode2
media/mojo/DEPS:2: "+mojo/public",
nit: looks like an extra space sneaked in here

https://codereview.chromium.org/491733004/diff/170001/media/mojo/services/BUI...
File media/mojo/services/BUILD.gn (right):

https://codereview.chromium.org/491733004/diff/170001/media/mojo/services/BUI...
media/mojo/services/BUILD.gn:37: 
remove extra blank?

[tim (not reviewing), 2014-09-03 19:36:23]

All done, created bug 410451.

darin@ - will still require approval for mojo/mojo_variables (I'm not an mojo
OWNER yet).  Thanks!

https://codereview.chromium.org/491733004/diff/170001/media/mojo/DEPS
File media/mojo/DEPS (right):

https://codereview.chromium.org/491733004/diff/170001/media/mojo/DEPS#newcode2
media/mojo/DEPS:2: "+mojo/public",
On 2014/09/03 17:08:29, scherkus wrote:
> nit: looks like an extra space sneaked in here

Done.

https://codereview.chromium.org/491733004/diff/170001/media/mojo/services/BUI...
File media/mojo/services/BUILD.gn (right):

https://codereview.chromium.org/491733004/diff/170001/media/mojo/services/BUI...
media/mojo/services/BUILD.gn:37: 
On 2014/09/03 17:08:29, scherkus wrote:
> remove extra blank?

Done.

[tim (not reviewing), 2014-09-04 16:48:48]

tim@chromium.org changed reviewers:
+ davemoore@chromium.org

[tim (not reviewing), 2014-09-04 16:49:33]

+dave for mojo/ owners review

[darin (slow to review), 2014-09-04 21:40:36]

LGTM for mojo/mojo_variables.gypi

[tim (not reviewing), 2014-09-04 21:41:43]

The CQ bit was checked by tim@chromium.org

[commit-bot: I haz the power, 2014-09-04 21:47:19]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/tim@chromium.org/491733004/290001

[DaveMoore, 2014-09-04 21:51:11]

lgtm

[commit-bot: I haz the power, 2014-09-05 01:31:57]

Committed patchset #12 (id:290001) as a17e408b7e5ecd50312d8774c167869e12a10ac3

[commit-bot: I haz the power, 2014-09-10 03:34:54]

Patchset 12 (id:??) landed as
https://crrev.com/9f8fe767920001f734dddc678bad491be8cc363b
Cr-Commit-Position: refs/heads/master@{#293392}