Move Renderer permanent callbacks into RendererClient interface.

Move Renderer permanent callbacks into RendererClient interface.

We need to move the callbacks for video size and opacity change from
VideoFrameCompositor to VideoRenderer. Doing so will exceed the gmock
limit of 10 arguments for VideoRenderer::Initialize. This patch moves
the callbacks passed in VideoRenderer::Initialize into a RendererClient
interface so that additional callbacks can be added.

BUG=571155
Committed: https://crrev.com/16bbeea8c7fed2b76b26bb9c7fcee326806514f7
Cr-Commit-Position: refs/heads/master@{#393396}

[alokp, 2016-05-06 17:53:34]

alokp@chromium.org changed reviewers:
+ sandersd@chromium.org, xhwang@chromium.org

[alokp, 2016-05-06 17:53:35]

I still need to update the unittests. Could you please provide early feedback on
the general direction of this patch? I have manually tested this patch on YT,
which seems to work fine.

[xhwang, 2016-05-09 18:13:23]

Overall looks good. Here are my first round of comments. Thank you again for
working on this nice refactoring/cleanup!

https://codereview.chromium.org/1955843002/diff/80001/media/base/audio_render...
File media/base/audio_renderer.h (right):

https://codereview.chromium.org/1955843002/diff/80001/media/base/audio_render...
media/base/audio_renderer.h:34: virtual void Initialize(RendererClient* client,
See comment about param order in renderer.h

https://codereview.chromium.org/1955843002/diff/80001/media/base/pipeline_impl.h
File media/base/pipeline_impl.h (left):

https://codereview.chromium.org/1955843002/diff/80001/media/base/pipeline_imp...
media/base/pipeline_impl.h:200: void OnRendererEnded();
remove?

https://codereview.chromium.org/1955843002/diff/80001/media/base/renderer.h
File media/base/renderer.h (right):

https://codereview.chromium.org/1955843002/diff/80001/media/base/renderer.h#n...
media/base/renderer.h:37: const PipelineStatusCB& init_cb) = 0;
Here and on AudioRenderer and VideoRenderer interfaces: We were follow the rule
of "putting input-only parameter before output parameters":

https://google.github.io/styleguide/cppguide.html#Function_Parameter_Ordering

The |client| is an output parameter. So let's put it after
|demuxer_stream_provider|?

https://codereview.chromium.org/1955843002/diff/80001/media/base/renderer_cli...
File media/base/renderer_client.h (right):

https://codereview.chromium.org/1955843002/diff/80001/media/base/renderer_cli...
media/base/renderer_client.h:10: class RendererClient {
Please add some documentation for this interface.

You can also mention that this interface is shared by Renderer, AudioRenderer
and VideoRenderer.

https://codereview.chromium.org/1955843002/diff/80001/media/base/video_render...
File media/base/video_renderer.h (right):

https://codereview.chromium.org/1955843002/diff/80001/media/base/video_render...
media/base/video_renderer.h:28: // completion.
I think the dropped line still has some value. People could go ahead calling
OnError on Client instead of firing init_cb.

https://codereview.chromium.org/1955843002/diff/80001/media/base/video_render...
media/base/video_renderer.h:35: virtual void Initialize(RendererClient* client,
ditto about parameter order

https://codereview.chromium.org/1955843002/diff/80001/media/mojo/services/moj...
File media/mojo/services/mojo_renderer_service.cc (right):

https://codereview.chromium.org/1955843002/diff/80001/media/mojo/services/moj...
media/mojo/services/mojo_renderer_service.cc:126: void
MojoRendererService::OnWaitingForDecryptionKey() {}
This CL makes it super clear that we should plumb OnStatisticsUpdate() and
OnWaitingForDecryptionKey() over the renderer mojom client interface :) Could
you please add a TODO somewhere to make sure we don't forget?

https://codereview.chromium.org/1955843002/diff/80001/media/renderers/audio_r...
File media/renderers/audio_renderer_impl.h (right):

https://codereview.chromium.org/1955843002/diff/80001/media/renderers/audio_r...
media/renderers/audio_renderer_impl.h:173: void
OnAudioBufferStreamInitialized(bool succes);
Add an empty line here since the comment above only applies to this one method.

https://codereview.chromium.org/1955843002/diff/80001/media/renderers/audio_r...
media/renderers/audio_renderer_impl.h:174: void OnPlaybackError(PipelineStatus
error);
Add comment for this group, e.g. callbacks for the |client| etc.

https://codereview.chromium.org/1955843002/diff/80001/media/renderers/rendere...
File media/renderers/renderer_impl.cc (right):

https://codereview.chromium.org/1955843002/diff/80001/media/renderers/rendere...
media/renderers/renderer_impl.cc:475: void
RendererImpl::OnStatisticsUpdate(AudioVideoRendererClient* client,
Why do you need |client| here?

https://codereview.chromium.org/1955843002/diff/80001/media/renderers/rendere...
media/renderers/renderer_impl.cc:483: const bool is_audio = client ==
audio_renderer_client_.get();
The old code here was a bit hacky (I blame legacy reasons and my laziness) and I
never liked the idea of comparing pointers to decide whether it's audio or
video.

Instead of changing to another way of comparing pointers. Can we take this
opportunity to fix it? 

I think we can at least add an extra parameter to
RendererImpl::OnBufferingStateChange to indicate whether it's audio or video,
and also pass in this parameter to AudioVideoRendererClient's constructor so
that it's super clear which client is for which.

https://codereview.chromium.org/1955843002/diff/80001/media/renderers/rendere...
media/renderers/renderer_impl.cc:614: }
ditto about not relying on pointer comparison

https://codereview.chromium.org/1955843002/diff/80001/media/renderers/rendere...
media/renderers/renderer_impl.cc:645: void
RendererImpl::OnError(AudioVideoRendererClient* client,
|client| not needed?

https://codereview.chromium.org/1955843002/diff/80001/media/renderers/rendere...
media/renderers/renderer_impl.cc:672: void
RendererImpl::OnWaitingForDecryptionKey(AudioVideoRendererClient* client) {
|client| not needed?

https://codereview.chromium.org/1955843002/diff/80001/media/renderers/rendere...
File media/renderers/renderer_impl.h (right):

https://codereview.chromium.org/1955843002/diff/80001/media/renderers/rendere...
media/renderers/renderer_impl.h:74: class AudioVideoRendererClient;
This can only be the client for audio OR video. So "AudioVideo" is a bit
confusing to me and doesn't really provide any additional info. How about just
RendererClientInternal?

https://codereview.chromium.org/1955843002/diff/80001/media/renderers/rendere...
File media/renderers/renderer_impl_unittest.cc (right):

https://codereview.chromium.org/1955843002/diff/80001/media/renderers/rendere...
media/renderers/renderer_impl_unittest.cc:55:
MOCK_METHOD0(OnWaitingForDecryptionKey, void());
Can you use the new MockRendererClient with StrictMock?

https://codereview.chromium.org/1955843002/diff/80001/media/renderers/video_r...
File media/renderers/video_renderer_impl.cc (right):

https://codereview.chromium.org/1955843002/diff/80001/media/renderers/video_r...
media/renderers/video_renderer_impl.cc:365:
client_->OnError(PIPELINE_ERROR_DECODE);
The old code posts the callback (probably because we are under the lock). Shall
we do the same?

https://codereview.chromium.org/1955843002/diff/80001/media/renderers/video_r...
media/renderers/video_renderer_impl.cc:459:
client_->OnBufferingStateChange(buffering_state_);
The old |buffering_state_cb_| involves a post (BindToCurrentLoop), should we
always post here?

https://codereview.chromium.org/1955843002/diff/80001/media/renderers/video_r...
media/renderers/video_renderer_impl.cc:470:
client_->OnBufferingStateChange(buffering_state_);
ditto about post

https://codereview.chromium.org/1955843002/diff/80001/media/renderers/video_r...
File media/renderers/video_renderer_impl.h (right):

https://codereview.chromium.org/1955843002/diff/80001/media/renderers/video_r...
media/renderers/video_renderer_impl.h:90: void
OnVideoFrameStreamInitialized(bool success);
empty line here

[alokp, 2016-05-09 21:31:44]

https://codereview.chromium.org/1955843002/diff/80001/media/base/audio_render...
File media/base/audio_renderer.h (right):

https://codereview.chromium.org/1955843002/diff/80001/media/base/audio_render...
media/base/audio_renderer.h:34: virtual void Initialize(RendererClient* client,
On 2016/05/09 18:13:22, xhwang wrote:
> See comment about param order in renderer.h

Done.

https://codereview.chromium.org/1955843002/diff/80001/media/base/pipeline_impl.h
File media/base/pipeline_impl.h (left):

https://codereview.chromium.org/1955843002/diff/80001/media/base/pipeline_imp...
media/base/pipeline_impl.h:200: void OnRendererEnded();
On 2016/05/09 18:13:22, xhwang wrote:
> remove?

Done.

https://codereview.chromium.org/1955843002/diff/80001/media/base/renderer.h
File media/base/renderer.h (right):

https://codereview.chromium.org/1955843002/diff/80001/media/base/renderer.h#n...
media/base/renderer.h:37: const PipelineStatusCB& init_cb) = 0;
On 2016/05/09 18:13:22, xhwang wrote:
> Here and on AudioRenderer and VideoRenderer interfaces: We were follow the
rule
> of "putting input-only parameter before output parameters":
> 
> https://google.github.io/styleguide/cppguide.html#Function_Parameter_Ordering
> 
> The |client| is an output parameter. So let's put it after
> |demuxer_stream_provider|?

Done.

https://codereview.chromium.org/1955843002/diff/80001/media/base/renderer_cli...
File media/base/renderer_client.h (right):

https://codereview.chromium.org/1955843002/diff/80001/media/base/renderer_cli...
media/base/renderer_client.h:10: class RendererClient {
On 2016/05/09 18:13:22, xhwang wrote:
> Please add some documentation for this interface.
> 
> You can also mention that this interface is shared by Renderer, AudioRenderer
> and VideoRenderer.

Done.

https://codereview.chromium.org/1955843002/diff/80001/media/base/video_render...
File media/base/video_renderer.h (right):

https://codereview.chromium.org/1955843002/diff/80001/media/base/video_render...
media/base/video_renderer.h:28: // completion.
On 2016/05/09 18:13:22, xhwang wrote:
> I think the dropped line still has some value. People could go ahead calling
> OnError on Client instead of firing init_cb.

Done.

https://codereview.chromium.org/1955843002/diff/80001/media/base/video_render...
media/base/video_renderer.h:35: virtual void Initialize(RendererClient* client,
On 2016/05/09 18:13:22, xhwang wrote:
> ditto about parameter order

Done.

https://codereview.chromium.org/1955843002/diff/80001/media/mojo/services/moj...
File media/mojo/services/mojo_renderer_service.cc (right):

https://codereview.chromium.org/1955843002/diff/80001/media/mojo/services/moj...
media/mojo/services/mojo_renderer_service.cc:126: void
MojoRendererService::OnWaitingForDecryptionKey() {}
On 2016/05/09 18:13:22, xhwang wrote:
> This CL makes it super clear that we should plumb OnStatisticsUpdate() and
> OnWaitingForDecryptionKey() over the renderer mojom client interface :) Could
> you please add a TODO somewhere to make sure we don't forget?

Done.

https://codereview.chromium.org/1955843002/diff/80001/media/renderers/audio_r...
File media/renderers/audio_renderer_impl.h (right):

https://codereview.chromium.org/1955843002/diff/80001/media/renderers/audio_r...
media/renderers/audio_renderer_impl.h:173: void
OnAudioBufferStreamInitialized(bool succes);
On 2016/05/09 18:13:23, xhwang wrote:
> Add an empty line here since the comment above only applies to this one
method.

Done.

https://codereview.chromium.org/1955843002/diff/80001/media/renderers/audio_r...
media/renderers/audio_renderer_impl.h:174: void OnPlaybackError(PipelineStatus
error);
On 2016/05/09 18:13:22, xhwang wrote:
> Add comment for this group, e.g. callbacks for the |client| etc.

Done.

https://codereview.chromium.org/1955843002/diff/80001/media/renderers/rendere...
File media/renderers/renderer_impl.cc (right):

https://codereview.chromium.org/1955843002/diff/80001/media/renderers/rendere...
media/renderers/renderer_impl.cc:483: const bool is_audio = client ==
audio_renderer_client_.get();
On 2016/05/09 18:13:23, xhwang wrote:
> The old code here was a bit hacky (I blame legacy reasons and my laziness) and
I
> never liked the idea of comparing pointers to decide whether it's audio or
> video.
> 
> Instead of changing to another way of comparing pointers. Can we take this
> opportunity to fix it? 
> 
> I think we can at least add an extra parameter to
> RendererImpl::OnBufferingStateChange to indicate whether it's audio or video,
> and also pass in this parameter to AudioVideoRendererClient's constructor so
> that it's super clear which client is for which.

SGTM. What do you think about using DemuxerStream::Type?

https://codereview.chromium.org/1955843002/diff/80001/media/renderers/rendere...
File media/renderers/renderer_impl.h (right):

https://codereview.chromium.org/1955843002/diff/80001/media/renderers/rendere...
media/renderers/renderer_impl.h:74: class AudioVideoRendererClient;
On 2016/05/09 18:13:23, xhwang wrote:
> This can only be the client for audio OR video. So "AudioVideo" is a bit
> confusing to me and doesn't really provide any additional info. How about just
> RendererClientInternal?

Done.

https://codereview.chromium.org/1955843002/diff/80001/media/renderers/rendere...
File media/renderers/renderer_impl_unittest.cc (right):

https://codereview.chromium.org/1955843002/diff/80001/media/renderers/rendere...
media/renderers/renderer_impl_unittest.cc:55:
MOCK_METHOD0(OnWaitingForDecryptionKey, void());
On 2016/05/09 18:13:23, xhwang wrote:
> Can you use the new MockRendererClient with StrictMock?

Do you mean derive from MockRendererClient instead of RendererClient so that we
do not have to mock these functions again?

https://codereview.chromium.org/1955843002/diff/80001/media/renderers/video_r...
File media/renderers/video_renderer_impl.cc (right):

https://codereview.chromium.org/1955843002/diff/80001/media/renderers/video_r...
media/renderers/video_renderer_impl.cc:365:
client_->OnError(PIPELINE_ERROR_DECODE);
On 2016/05/09 18:13:23, xhwang wrote:
> The old code posts the callback (probably because we are under the lock).
Shall
> we do the same?

Done.

https://codereview.chromium.org/1955843002/diff/80001/media/renderers/video_r...
media/renderers/video_renderer_impl.cc:459:
client_->OnBufferingStateChange(buffering_state_);
On 2016/05/09 18:13:23, xhwang wrote:
> The old |buffering_state_cb_| involves a post (BindToCurrentLoop), should we
> always post here?

Done.

https://codereview.chromium.org/1955843002/diff/80001/media/renderers/video_r...
media/renderers/video_renderer_impl.cc:470:
client_->OnBufferingStateChange(buffering_state_);
On 2016/05/09 18:13:23, xhwang wrote:
> ditto about post

Done.

https://codereview.chromium.org/1955843002/diff/80001/media/renderers/video_r...
File media/renderers/video_renderer_impl.h (right):

https://codereview.chromium.org/1955843002/diff/80001/media/renderers/video_r...
media/renderers/video_renderer_impl.h:90: void
OnVideoFrameStreamInitialized(bool success);
On 2016/05/09 18:13:23, xhwang wrote:
> empty line here

Done.

[sandersd (OOO until July 31), 2016-05-09 21:44:21]

I didn't review every single line, but the design and implementation lgtm.

[alokp, 2016-05-11 18:21:50]

I have update all unittests. This is ready land except for replacing the pointer
comparison pointed to by Xiaohan. I am debating whether to use
DemuxerStream::Type or just a is_audio boolean. I generally prefer enum for
clarity, but could be convinced to use a boolean in this case. Please let me
know.

https://codereview.chromium.org/1955843002/diff/80001/media/renderers/rendere...
File media/renderers/renderer_impl.cc (right):

https://codereview.chromium.org/1955843002/diff/80001/media/renderers/rendere...
media/renderers/renderer_impl.cc:483: const bool is_audio = client ==
audio_renderer_client_.get();
On 2016/05/09 21:31:44, alokp wrote:
> On 2016/05/09 18:13:23, xhwang wrote:
> > The old code here was a bit hacky (I blame legacy reasons and my laziness)
and
> I
> > never liked the idea of comparing pointers to decide whether it's audio or
> > video.
> > 
> > Instead of changing to another way of comparing pointers. Can we take this
> > opportunity to fix it? 
> > 
> > I think we can at least add an extra parameter to
> > RendererImpl::OnBufferingStateChange to indicate whether it's audio or
video,
> > and also pass in this parameter to AudioVideoRendererClient's constructor so
> > that it's super clear which client is for which.
> 
> SGTM. What do you think about using DemuxerStream::Type?

I have addressed all comments except this one. Xiaohan: Still waiting for you
thoughts about using DemuxerStream::Type instead of comparig pointers.

[xhwang, 2016-05-11 20:10:49]

On 2016/05/11 18:21:50, alokp wrote:
> I have update all unittests. This is ready land except for replacing the
pointer
> comparison pointed to by Xiaohan. I am debating whether to use
> DemuxerStream::Type or just a is_audio boolean. I generally prefer enum for
> clarity, but could be convinced to use a boolean in this case. Please let me
> know.
> 
>
https://codereview.chromium.org/1955843002/diff/80001/media/renderers/rendere...
> File media/renderers/renderer_impl.cc (right):
> 
>
https://codereview.chromium.org/1955843002/diff/80001/media/renderers/rendere...
> media/renderers/renderer_impl.cc:483: const bool is_audio = client ==
> audio_renderer_client_.get();
> On 2016/05/09 21:31:44, alokp wrote:
> > On 2016/05/09 18:13:23, xhwang wrote:
> > > The old code here was a bit hacky (I blame legacy reasons and my laziness)
> and
> > I
> > > never liked the idea of comparing pointers to decide whether it's audio or
> > > video.
> > > 
> > > Instead of changing to another way of comparing pointers. Can we take this
> > > opportunity to fix it? 
> > > 
> > > I think we can at least add an extra parameter to
> > > RendererImpl::OnBufferingStateChange to indicate whether it's audio or
> video,
> > > and also pass in this parameter to AudioVideoRendererClient's constructor
so
> > > that it's super clear which client is for which.
> > 
> > SGTM. What do you think about using DemuxerStream::Type?
> 
> I have addressed all comments except this one. Xiaohan: Still waiting for you
> thoughts about using DemuxerStream::Type instead of comparig pointers.

DemuxerStream::Type sgtm. Thanks!

[alokp, 2016-05-11 23:49:10]

> DemuxerStream::Type sgtm. Thanks!

DONE.

[xhwang, 2016-05-12 20:51:19]

looking good. One last comment (and some nits):

Can we use MockRendererClient where applicable?

https://chromiumcodereview.appspot.com/1955843002/diff/80001/media/renderers/...
File media/renderers/renderer_impl_unittest.cc (right):

https://chromiumcodereview.appspot.com/1955843002/diff/80001/media/renderers/...
media/renderers/renderer_impl_unittest.cc:55:
MOCK_METHOD0(OnWaitingForDecryptionKey, void());
On 2016/05/09 21:31:44, alokp wrote:
> On 2016/05/09 18:13:23, xhwang wrote:
> > Can you use the new MockRendererClient with StrictMock?
> 
> Do you mean derive from MockRendererClient instead of RendererClient so that
we
> do not have to mock these functions again?

The reason we use CallbackHelper is that we can use StrictMock on it to check
missing callbacks. Now we already have a MockRendererClient, we should be able
to use StrictMock<MockRendererClient> member directly, similar to these below:

std::unique_ptr<StrictMock<MockDemuxer>> demuxer_;
StrictMock<MockVideoRenderer>* video_renderer_;

Please use MockRendererClient in other tests as well where applicable.

https://chromiumcodereview.appspot.com/1955843002/diff/180001/media/mojo/serv...
File media/mojo/services/mojo_renderer_impl.cc (right):

https://chromiumcodereview.appspot.com/1955843002/diff/180001/media/mojo/serv...
media/mojo/services/mojo_renderer_impl.cc:34: // See http://crbug.com/585287
You can remove this now.

https://chromiumcodereview.appspot.com/1955843002/diff/180001/media/mojo/serv...
File media/mojo/services/mojo_renderer_service.cc (right):

https://chromiumcodereview.appspot.com/1955843002/diff/180001/media/mojo/serv...
media/mojo/services/mojo_renderer_service.cc:119: // TODO(alokp): Plumb the
event to interfaces::RendererClient.
Add bug 585287

https://chromiumcodereview.appspot.com/1955843002/diff/180001/media/mojo/serv...
media/mojo/services/mojo_renderer_service.cc:129: // TODO(alokp): Plumb the
event to interfaces::RendererClient.
ditto

https://chromiumcodereview.appspot.com/1955843002/diff/180001/media/renderers...
File media/renderers/audio_renderer_impl_unittest.cc (right):

https://chromiumcodereview.appspot.com/1955843002/diff/180001/media/renderers...
media/renderers/audio_renderer_impl_unittest.cc:68: class AudioRendererImplTest
: public ::testing::Test, public RendererClient {
Can you also use MockRendererClient here?

https://chromiumcodereview.appspot.com/1955843002/diff/180001/media/renderers...
File media/renderers/video_renderer_impl_unittest.cc (right):

https://chromiumcodereview.appspot.com/1955843002/diff/180001/media/renderers...
media/renderers/video_renderer_impl_unittest.cc:434:
MOCK_METHOD0(OnWaitingForDecryptionKey, void());
ditto about using MockRendererClient.

[alokp, 2016-05-12 21:46:51]

https://codereview.chromium.org/1955843002/diff/180001/media/mojo/services/mo...
File media/mojo/services/mojo_renderer_impl.cc (right):

https://codereview.chromium.org/1955843002/diff/180001/media/mojo/services/mo...
media/mojo/services/mojo_renderer_impl.cc:34: // See http://crbug.com/585287
On 2016/05/12 20:51:18, xhwang wrote:
> You can remove this now.

Done.

https://codereview.chromium.org/1955843002/diff/180001/media/mojo/services/mo...
File media/mojo/services/mojo_renderer_service.cc (right):

https://codereview.chromium.org/1955843002/diff/180001/media/mojo/services/mo...
media/mojo/services/mojo_renderer_service.cc:119: // TODO(alokp): Plumb the
event to interfaces::RendererClient.
On 2016/05/12 20:51:19, xhwang wrote:
> Add bug 585287

Done.

https://codereview.chromium.org/1955843002/diff/180001/media/mojo/services/mo...
media/mojo/services/mojo_renderer_service.cc:129: // TODO(alokp): Plumb the
event to interfaces::RendererClient.
On 2016/05/12 20:51:19, xhwang wrote:
> ditto

Done.

https://codereview.chromium.org/1955843002/diff/180001/media/renderers/audio_...
File media/renderers/audio_renderer_impl_unittest.cc (right):

https://codereview.chromium.org/1955843002/diff/180001/media/renderers/audio_...
media/renderers/audio_renderer_impl_unittest.cc:68: class AudioRendererImplTest
: public ::testing::Test, public RendererClient {
On 2016/05/12 20:51:19, xhwang wrote:
> Can you also use MockRendererClient here?

Using it here would require many changes in the test but can use it other
unittests.

https://codereview.chromium.org/1955843002/diff/180001/media/renderers/video_...
File media/renderers/video_renderer_impl_unittest.cc (right):

https://codereview.chromium.org/1955843002/diff/180001/media/renderers/video_...
media/renderers/video_renderer_impl_unittest.cc:434:
MOCK_METHOD0(OnWaitingForDecryptionKey, void());
On 2016/05/12 20:51:19, xhwang wrote:
> ditto about using MockRendererClient.

Done. And also in renderer_impl_unittest.cc

[xhwang, 2016-05-12 21:48:32]

lgtm

[alokp, 2016-05-12 21:59:24]

The CQ bit was checked by alokp@chromium.org

[alokp, 2016-05-12 21:59:25]

The patchset sent to the CQ was uploaded after l-g-t-m from
sandersd@chromium.org
Link to the patchset: https://codereview.chromium.org/1955843002/#ps200001
(title: "addressed comments")

[commit-bot: I haz the power, 2016-05-12 21:59:39]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1955843002/200001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1955843002/200001

[commit-bot: I haz the power, 2016-05-12 23:32:49]

Committed patchset #11 (id:200001)

[commit-bot: I haz the power, 2016-05-12 23:33:50]

Description was changed from

==========
Move Renderer permanent callbacks into RendererClient interface.

We need to move the callbacks for video size and opacity change from
VideoFrameCompositor to VideoRenderer. Doing so will exceed the gmock
limit of 10 arguments for VideoRenderer::Initialize. This patch moves
the callbacks passed in VideoRenderer::Initialize into a RendererClient
interface so that additional callbacks can be added.

BUG=571155
==========

to

==========
Move Renderer permanent callbacks into RendererClient interface.

We need to move the callbacks for video size and opacity change from
VideoFrameCompositor to VideoRenderer. Doing so will exceed the gmock
limit of 10 arguments for VideoRenderer::Initialize. This patch moves
the callbacks passed in VideoRenderer::Initialize into a RendererClient
interface so that additional callbacks can be added.

BUG=571155

Committed: https://crrev.com/16bbeea8c7fed2b76b26bb9c7fcee326806514f7
Cr-Commit-Position: refs/heads/master@{#393396}
==========

[commit-bot: I haz the power, 2016-05-12 23:33:51]

Patchset 11 (id:??) landed as
https://crrev.com/16bbeea8c7fed2b76b26bb9c7fcee326806514f7
Cr-Commit-Position: refs/heads/master@{#393396}