Add MediaPlayerRenderer/MediaPlayerRendererClient

Add MediaPlayerRenderer/MediaPlayerRendererClient

This change introduces MediaPlayerRender and MediaPlayerRendererClient.

The MediaPlayerRenderer uses an instance of an Android MediaPlayer to
render video to a given Android Surface. It adapts the MediaPlayer
interface into the media::Renderer interface. It lives in the Browser
process, and is owned by and instance of MojoRendererService.

The MediaPlayerRendereClient is a media::Renderer that integrates
into the WMPI pipeline. It forwards media::Renderer calls to its
associated MediaPlayerRenderer in the Browser process, via a
MojoRenderer (bound to the MojoRendererService). It also owns a
StreamTextureWrapper, which it uses to send VideoFrame to WMPI's
VideoFrameCompositor.

In order to be fully functional, the code is missing the repainting
of duplicate video frames (636002), the propagation of duration changes
(635991) and a mechanism for registering/retrieving the
StreamTextureWrapper's Surface (627658).

BUG=631199
Committed: https://crrev.com/cd478271f0ea7b3a25fd0b04799d97699598278a
Cr-Commit-Position: refs/heads/master@{#411527}

[tguilbert, 2016-08-09 19:30:07]

tguilbert@chromium.org changed reviewers:
+ dalecurtis@chromium.org, watk@chromium.org

[tguilbert, 2016-08-09 19:30:08]

PTAL! I already have the changes ready for 636002 and 635991, which will follow
as soon as this CL is done.

Should I be adding anyone else to the review? Specifically for the content/
changes.

Thank you!
Thomas

https://codereview.chromium.org/2230583002/diff/1/content/browser/media/andro...
File content/browser/media/android/media_player_renderer.cc (right):

https://codereview.chromium.org/2230583002/diff/1/content/browser/media/andro...
content/browser/media/android/media_player_renderer.cc:137: return nullptr;
Should I be concerned with this?

https://codereview.chromium.org/2230583002/diff/1/content/browser/media/andro...
content/browser/media/android/media_player_renderer.cc:223: // TODO(tguilbert):
Investigate whether the use of the MediaThrottler is
What is the MediaThrottler used for?

[watk, 2016-08-09 21:39:58]

Nice, I only had nits

https://codereview.chromium.org/2230583002/diff/20001/content/browser/media/a...
File content/browser/media/android/media_player_renderer.cc (right):

https://codereview.chromium.org/2230583002/diff/20001/content/browser/media/a...
content/browser/media/android/media_player_renderer.cc:36: DVLOG(1) <<
__FUNCTION__;
I was going to say we use __func__ now, but a bunch of media files still use the
old one.. not sure if there's a reason for that or no one's gotten around to it.

https://codereview.chromium.org/2230583002/diff/20001/content/browser/media/a...
content/browser/media/android/media_player_renderer.cc:66:
init_cb.Run(media::PIPELINE_ERROR_INITIALIZATION_FAILED);
We would usually put this in an early return above "if (GetType() != URL)"

https://codereview.chromium.org/2230583002/diff/20001/content/browser/media/a...
content/browser/media/android/media_player_renderer.cc:126: partition ?
partition->GetFileSystemContext() : NULL;
nullptr

https://codereview.chromium.org/2230583002/diff/20001/content/browser/media/a...
content/browser/media/android/media_player_renderer.cc:131:
web_contents->GetMainFrame()->GetRoutingID()));
Isn't render_frame_host_ the right one?

https://codereview.chromium.org/2230583002/diff/20001/content/browser/media/a...
content/browser/media/android/media_player_renderer.cc:185: if (error) {
Should we also check !has_error_ to avoid sending more than one error?

https://codereview.chromium.org/2230583002/diff/20001/content/browser/media/a...
File content/browser/media/android/media_player_renderer.h (right):

https://codereview.chromium.org/2230583002/diff/20001/content/browser/media/a...
content/browser/media/android/media_player_renderer.h:24: }
Both unneeded by the looks of it

https://codereview.chromium.org/2230583002/diff/20001/content/browser/media/a...
content/browser/media/android/media_player_renderer.h:27: class WebContents;
unused

https://codereview.chromium.org/2230583002/diff/20001/content/browser/media/a...
content/browser/media/android/media_player_renderer.h:93:
std::unique_ptr<media::MediaPlayerBridgej> media_player_;
MediaPlayerBridgej? I assume this was a last minute typo and it doesn't compile
:P

https://codereview.chromium.org/2230583002/diff/20001/content/browser/media/a...
content/browser/media/android/media_player_renderer.h:105: int width_;
 gfx::Size?

https://codereview.chromium.org/2230583002/diff/20001/content/renderer/media/...
File content/renderer/media/android/media_player_renderer_client.h (right):

https://codereview.chromium.org/2230583002/diff/20001/content/renderer/media/...
content/renderer/media/android/media_player_renderer_client.h:61: //
RendererClient implementation implementation.
buffalo buffalo buffalo..

media::RendererClient for consistency with media::Renderer above

https://codereview.chromium.org/2230583002/diff/20001/content/renderer/media/...
File content/renderer/media/android/media_player_renderer_client_factory.cc
(right):

https://codereview.chromium.org/2230583002/diff/20001/content/renderer/media/...
content/renderer/media/android/media_player_renderer_client_factory.cc:1: //
Copyright 2014 The Chromium Authors. All rights reserved.
2016

https://codereview.chromium.org/2230583002/diff/20001/content/renderer/media/...
content/renderer/media/android/media_player_renderer_client_factory.cc:9:
#include "media/renderers/video_overlay_factory.h"
delete?

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

https://codereview.chromium.org/2230583002/diff/20001/media/mojo/services/moj...
media/mojo/services/mojo_renderer_service.cc:42: const mojo::String& url,
Can you pass a gurl all the way?

[tguilbert, 2016-08-09 22:38:35]

https://codereview.chromium.org/2230583002/diff/20001/content/browser/media/a...
File content/browser/media/android/media_player_renderer.cc (right):

https://codereview.chromium.org/2230583002/diff/20001/content/browser/media/a...
content/browser/media/android/media_player_renderer.cc:36: DVLOG(1) <<
__FUNCTION__;
On 2016/08/09 21:39:58, watk wrote:
> I was going to say we use __func__ now, but a bunch of media files still use
the
> old one.. not sure if there's a reason for that or no one's gotten around to
it.

Oh, I didn't know about __func__. Done!

https://codereview.chromium.org/2230583002/diff/20001/content/browser/media/a...
content/browser/media/android/media_player_renderer.cc:66:
init_cb.Run(media::PIPELINE_ERROR_INITIALIZATION_FAILED);
On 2016/08/09 21:39:58, watk wrote:
> We would usually put this in an early return above "if (GetType() != URL)"

Done.

https://codereview.chromium.org/2230583002/diff/20001/content/browser/media/a...
content/browser/media/android/media_player_renderer.cc:126: partition ?
partition->GetFileSystemContext() : NULL;
On 2016/08/09 21:39:58, watk wrote:
> nullptr

Done.

https://codereview.chromium.org/2230583002/diff/20001/content/browser/media/a...
content/browser/media/android/media_player_renderer.cc:131:
web_contents->GetMainFrame()->GetRoutingID()));
On 2016/08/09 21:39:58, watk wrote:
> Isn't render_frame_host_ the right one?

I am TBH not sure what this code does. It copied it from WMPA and took look 2
classes deep, but didn'tany farther than that. How can this be cleaned up?

https://codereview.chromium.org/2230583002/diff/20001/content/browser/media/a...
content/browser/media/android/media_player_renderer.cc:185: if (error) {
On 2016/08/09 21:39:58, watk wrote:
> Should we also check !has_error_ to avoid sending more than one error?

I'm not sure. Won't the pipeline simply initiate the Stop() and ignore any
subsequent errors during "teardown"?

https://codereview.chromium.org/2230583002/diff/20001/content/browser/media/a...
File content/browser/media/android/media_player_renderer.h (right):

https://codereview.chromium.org/2230583002/diff/20001/content/browser/media/a...
content/browser/media/android/media_player_renderer.h:24: }
On 2016/08/09 21:39:58, watk wrote:
> Both unneeded by the looks of it

Done.

https://codereview.chromium.org/2230583002/diff/20001/content/browser/media/a...
content/browser/media/android/media_player_renderer.h:27: class WebContents;
On 2016/08/09 21:39:58, watk wrote:
> unused

Done.

https://codereview.chromium.org/2230583002/diff/20001/content/browser/media/a...
content/browser/media/android/media_player_renderer.h:93:
std::unique_ptr<media::MediaPlayerBridgej> media_player_;
On 2016/08/09 21:39:58, watk wrote:
> MediaPlayerBridgej? I assume this was a last minute typo and it doesn't
compile
> :P

Yup :P That was me navigating through the files in patch #1 of the CL

https://codereview.chromium.org/2230583002/diff/20001/content/browser/media/a...
content/browser/media/android/media_player_renderer.h:105: int width_;
On 2016/08/09 21:39:58, watk wrote:
>  gfx::Size?

Done.

https://codereview.chromium.org/2230583002/diff/20001/content/renderer/media/...
File content/renderer/media/android/media_player_renderer_client.h (right):

https://codereview.chromium.org/2230583002/diff/20001/content/renderer/media/...
content/renderer/media/android/media_player_renderer_client.h:61: //
RendererClient implementation implementation.
On 2016/08/09 21:39:58, watk wrote:
> buffalo buffalo buffalo..
> 
> media::RendererClient for consistency with media::Renderer above

Done.

https://codereview.chromium.org/2230583002/diff/20001/content/renderer/media/...
File content/renderer/media/android/media_player_renderer_client_factory.cc
(right):

https://codereview.chromium.org/2230583002/diff/20001/content/renderer/media/...
content/renderer/media/android/media_player_renderer_client_factory.cc:1: //
Copyright 2014 The Chromium Authors. All rights reserved.
On 2016/08/09 21:39:58, watk wrote:
> 2016

Done.

https://codereview.chromium.org/2230583002/diff/20001/content/renderer/media/...
content/renderer/media/android/media_player_renderer_client_factory.cc:9:
#include "media/renderers/video_overlay_factory.h"
On 2016/08/09 21:39:58, watk wrote:
> delete?

Done.

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

https://codereview.chromium.org/2230583002/diff/20001/media/mojo/services/moj...
media/mojo/services/mojo_renderer_service.cc:42: const mojo::String& url,
On 2016/08/09 21:39:58, watk wrote:
> Can you pass a gurl all the way?

I had tried a while ago (3 months?) and it was not as easy as I thought. It
seems like they have added native GURL support since then. Done! :)

[tguilbert, 2016-08-09 22:39:45]

https://codereview.chromium.org/2230583002/diff/40001/content/browser/media/a...
File content/browser/media/android/media_player_renderer.h (right):

https://codereview.chromium.org/2230583002/diff/40001/content/browser/media/a...
content/browser/media/android/media_player_renderer.h:98: // Video size.
Will remove this comment.

[watk, 2016-08-09 23:04:15]

lgtm

https://codereview.chromium.org/2230583002/diff/20001/content/browser/media/a...
File content/browser/media/android/media_player_renderer.cc (right):

https://codereview.chromium.org/2230583002/diff/20001/content/browser/media/a...
content/browser/media/android/media_player_renderer.cc:131:
web_contents->GetMainFrame()->GetRoutingID()));
On 2016/08/09 22:38:34, ThomasGuilbert wrote:
> On 2016/08/09 21:39:58, watk wrote:
> > Isn't render_frame_host_ the right one?
> 
> I am TBH not sure what this code does. It copied it from WMPA and took look 2
> classes deep, but didn'tany farther than that. How can this be cleaned up?

Oh, I just meant pass render_frame_host_->GetRoutingID() instead of the main
frame id (which addresses the todo comment above it).

The MediaResourceGetter lets MPB get cookies to pass to MediaPlayer, and
metadata (size, duration) to return to WMPI before starting playback (when
preload=auto or preload=metadata).

https://codereview.chromium.org/2230583002/diff/20001/content/browser/media/a...
content/browser/media/android/media_player_renderer.cc:185: if (error) {
On 2016/08/09 22:38:34, ThomasGuilbert wrote:
> On 2016/08/09 21:39:58, watk wrote:
> > Should we also check !has_error_ to avoid sending more than one error?
> 
> I'm not sure. Won't the pipeline simply initiate the Stop() and ignore any
> subsequent errors during "teardown"?

True, should be fine.

[tguilbert, 2016-08-10 19:55:53]

The CQ bit was checked by tguilbert@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-10 19:56:32]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-10 20:00:30]

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

[commit-bot: I haz the power, 2016-08-10 20:00:31]

Dry run: Try jobs failed on following builders:
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[tguilbert, 2016-08-10 20:25:25]

The CQ bit was checked by tguilbert@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-10 20:26:16]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-10 20:29:52]

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

[commit-bot: I haz the power, 2016-08-10 20:29:53]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  chromeos_x86-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_x86-ge...)
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_clobber_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[tguilbert, 2016-08-10 20:34:25]

tguilbert@chromium.org changed reviewers:
+ ochang@chromium.org

[tguilbert, 2016-08-10 20:34:26]

+ochang for security review.
Can you please review the media/mojo/interfaces/renderer.mojom change?

Gentle ping @dalecurtis.

Thanks!

https://codereview.chromium.org/2230583002/diff/20001/content/browser/media/a...
File content/browser/media/android/media_player_renderer.cc (right):

https://codereview.chromium.org/2230583002/diff/20001/content/browser/media/a...
content/browser/media/android/media_player_renderer.cc:131:
web_contents->GetMainFrame()->GetRoutingID()));
On 2016/08/09 23:04:15, watk wrote:
> On 2016/08/09 22:38:34, ThomasGuilbert wrote:
> > On 2016/08/09 21:39:58, watk wrote:
> > > Isn't render_frame_host_ the right one?
> > 
> > I am TBH not sure what this code does. It copied it from WMPA and took look
2
> > classes deep, but didn'tany farther than that. How can this be cleaned up?
> 
> Oh, I just meant pass render_frame_host_->GetRoutingID() instead of the main
> frame id (which addresses the todo comment above it).
> 
> The MediaResourceGetter lets MPB get cookies to pass to MediaPlayer, and
> metadata (size, duration) to return to WMPI before starting playback (when
> preload=auto or preload=metadata).

Done.

https://codereview.chromium.org/2230583002/diff/40001/content/browser/media/a...
File content/browser/media/android/media_player_renderer.h (right):

https://codereview.chromium.org/2230583002/diff/40001/content/browser/media/a...
content/browser/media/android/media_player_renderer.h:98: // Video size.
On 2016/08/09 22:39:45, ThomasGuilbert wrote:
> Will remove this comment.

Done.

[Oliver Chang, 2016-08-10 20:47:13]

lgtm

[DaleCurtis, 2016-08-10 21:14:06]

https://codereview.chromium.org/2230583002/diff/80001/content/browser/media/a...
File content/browser/media/android/media_player_renderer.cc (right):

https://codereview.chromium.org/2230583002/diff/80001/content/browser/media/a...
content/browser/media/android/media_player_renderer.cc:18: const int
kUnusedAndIrrelevantPlayerId = 0;
constexpr int..

https://codereview.chromium.org/2230583002/diff/80001/content/browser/media/a...
content/browser/media/android/media_player_renderer.cc:24:
duration_(base::TimeDelta::FromMicroseconds(0)),
Unnecessary, default is zero.

https://codereview.chromium.org/2230583002/diff/80001/content/browser/media/a...
content/browser/media/android/media_player_renderer.cc:37: LOG(ERROR) <<
"DemuxerStreamProvider is not of Type URL";
Prefer DLOG?

https://codereview.chromium.org/2230583002/diff/80001/content/browser/media/a...
content/browser/media/android/media_player_renderer.cc:53: GURL(),  //
first_party_for_cookies
I think this will be important to fill. How is WMPA filling this? +qinmin to
look over this. If you're not careful here, WMPA won't be able to load videos
which require some cookies / authentication.

https://codereview.chromium.org/2230583002/diff/80001/content/browser/media/a...
content/browser/media/android/media_player_renderer.cc:86:
media_player_->SeekTo(time);
Should SeekTo happen first?

https://codereview.chromium.org/2230583002/diff/80001/content/browser/media/a...
content/browser/media/android/media_player_renderer.cc:89: void
MediaPlayerRenderer::SetPlaybackRate(double playback_rate) {
Needs a comment about what's happening here. You're not using the rate for
anything other than "Stopped" or "started"

https://codereview.chromium.org/2230583002/diff/80001/content/browser/media/a...
content/browser/media/android/media_player_renderer.cc:119: WebContents*
web_contents =
This can only be done on the UI thread IIRC. Do you want to add some
DCHECK(BrowserThread::CurrentlyOn()); calls?

https://codereview.chromium.org/2230583002/diff/80001/content/browser/media/a...
content/browser/media/android/media_player_renderer.cc:134: return nullptr;
TODO plumb this through? Or is this overridden at a higher level?

https://codereview.chromium.org/2230583002/diff/80001/content/browser/media/a...
content/browser/media/android/media_player_renderer.cc:151: // TODO(tguilbert):
Add OnDurationChange to the RendererClient interface.
Probably this will be important. I think MP gets the duration wrong a lot.

https://codereview.chromium.org/2230583002/diff/80001/content/browser/media/a...
content/browser/media/android/media_player_renderer.cc:176: DVLOG(2) << __func__
<< " Error: " << error;
I'd make this a LOG(ERROR) for finding issues in the wild.

https://codereview.chromium.org/2230583002/diff/80001/content/browser/media/a...
content/browser/media/android/media_player_renderer.cc:176: DVLOG(2) << __func__
<< " Error: " << error;
I'd make this a LOG(ERROR) for finding issues in the wild.

https://codereview.chromium.org/2230583002/diff/80001/content/browser/media/a...
content/browser/media/android/media_player_renderer.cc:184:
renderer_client_->OnError(media::PIPELINE_ERROR_COULD_NOT_RENDER);
We may want a special error for external renderers here. I.e.
PIPELINE_ERROR_EXTERNAL_RENDERER_FAILED.

https://codereview.chromium.org/2230583002/diff/80001/content/browser/media/a...
content/browser/media/android/media_player_renderer.cc:212: return true;
May need to carry over some logic from WMPA/etc to make sure this doesn't allow
DoS of mediaserver.

https://codereview.chromium.org/2230583002/diff/80001/content/browser/media/a...
content/browser/media/android/media_player_renderer.cc:219: // TODO(tguilbert):
Investigate whether the use of the MediaThrottler is
I think you can instead use an exponential back off scheme instead.

https://codereview.chromium.org/2230583002/diff/80001/content/browser/media/a...
File content/browser/media/android/media_player_renderer.h (right):

https://codereview.chromium.org/2230583002/diff/80001/content/browser/media/a...
content/browser/media/android/media_player_renderer.h:40:
MediaPlayerRenderer(content::RenderFrameHost* render_frame_host);
explicit

https://codereview.chromium.org/2230583002/diff/80001/content/browser/media/a...
content/browser/media/android/media_player_renderer.h:89: // Used when creating
|media_player_|.
Functions above variables.

https://codereview.chromium.org/2230583002/diff/80001/content/renderer/media/...
File content/renderer/media/android/media_player_renderer_client.cc (right):

https://codereview.chromium.org/2230583002/diff/80001/content/renderer/media/...
content/renderer/media/android/media_player_renderer_client.cc:21:
media_task_runner_(media_task_runner),
std::move ?

https://codereview.chromium.org/2230583002/diff/80001/content/renderer/media/...
content/renderer/media/android/media_player_renderer_client.cc:22:
compositor_task_runner_(compositor_task_runner),
ditto

https://codereview.chromium.org/2230583002/diff/80001/content/renderer/media/...
content/renderer/media/android/media_player_renderer_client.cc:99: return true;
Some commentary is necessary for HasAudio/HasVideo always returning true.

https://codereview.chromium.org/2230583002/diff/80001/content/renderer/media/...
File content/renderer/media/android/media_player_renderer_client_factory.cc
(right):

https://codereview.chromium.org/2230583002/diff/80001/content/renderer/media/...
content/renderer/media/android/media_player_renderer_client_factory.cc:18:
compositor_task_runner_(compositor_task_runner),
std::move?

https://codereview.chromium.org/2230583002/diff/80001/media/mojo/clients/mojo...
File media/mojo/clients/mojo_renderer.h (right):

https://codereview.chromium.org/2230583002/diff/80001/media/mojo/clients/mojo...
media/mojo/clients/mojo_renderer.h:76: void
InitializeRendererFromStreams(media::RendererClient* client);
What is FromStreams?

https://codereview.chromium.org/2230583002/diff/80001/media/mojo/interfaces/r...
File media/mojo/interfaces/renderer.mojom (right):

https://codereview.chromium.org/2230583002/diff/80001/media/mojo/interfaces/r...
media/mojo/interfaces/renderer.mojom:18: url.mojom.Url url) => (bool success);
? for url parameter ?

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

https://codereview.chromium.org/2230583002/diff/80001/media/mojo/services/moj...
media/mojo/services/mojo_renderer_service.cc:51: if (url.is_empty()) {
This is fine, but early return if empty instead of else block.

[tguilbert, 2016-08-11 00:29:00]

Addressed comments, and opened a few bugs to address in different CLs.

Thanks!

https://codereview.chromium.org/2230583002/diff/80001/content/browser/media/a...
File content/browser/media/android/media_player_renderer.cc (right):

https://codereview.chromium.org/2230583002/diff/80001/content/browser/media/a...
content/browser/media/android/media_player_renderer.cc:18: const int
kUnusedAndIrrelevantPlayerId = 0;
On 2016/08/10 21:14:05, DaleCurtis wrote:
> constexpr int..

Done.

https://codereview.chromium.org/2230583002/diff/80001/content/browser/media/a...
content/browser/media/android/media_player_renderer.cc:24:
duration_(base::TimeDelta::FromMicroseconds(0)),
On 2016/08/10 21:14:04, DaleCurtis wrote:
> Unnecessary, default is zero.

Done.

https://codereview.chromium.org/2230583002/diff/80001/content/browser/media/a...
content/browser/media/android/media_player_renderer.cc:37: LOG(ERROR) <<
"DemuxerStreamProvider is not of Type URL";
On 2016/08/10 21:14:04, DaleCurtis wrote:
> Prefer DLOG?

Done.

https://codereview.chromium.org/2230583002/diff/80001/content/browser/media/a...
content/browser/media/android/media_player_renderer.cc:53: GURL(),  //
first_party_for_cookies
On 2016/08/10 21:14:05, DaleCurtis wrote:
> I think this will be important to fill. How is WMPA filling this? +qinmin to
> look over this. If you're not careful here, WMPA won't be able to load videos
> which require some cookies / authentication.

WMPA gets it from the WebFrame.document().firstPartyForCookies(), with the
WebFrame being passed in by the contructor.

If I need to to pass that information from WMPI to the MediaPlayerRenderer, it
might be worth creating a new initialize method for mojom::Renderer, because, at
some point, I will need to pass
-- The URL that MediaPlayer will use to play.
-- The first party cookies URL.
-- Some token to retrieve the registered SurfaceTexture.

I opened 636604 to track this issue.

https://codereview.chromium.org/2230583002/diff/80001/content/browser/media/a...
content/browser/media/android/media_player_renderer.cc:86:
media_player_->SeekTo(time);
On 2016/08/10 21:14:05, DaleCurtis wrote:
> Should SeekTo happen first?

Either one works. Calling Start() first causes the MediaPlayer to start to
"prepare" first, and it will automatically seek to the last given |time| when it
is done preparing.

https://codereview.chromium.org/2230583002/diff/80001/content/browser/media/a...
content/browser/media/android/media_player_renderer.cc:89: void
MediaPlayerRenderer::SetPlaybackRate(double playback_rate) {
On 2016/08/10 21:14:04, DaleCurtis wrote:
> Needs a comment about what's happening here. You're not using the rate for
> anything other than "Stopped" or "started"

Good point. I added a comment in the header file.

MediaPlayerBridge does not expose MediaPlayer's SetPlaybackParams() method, but
we could add support for variable playback rate in the future.

https://codereview.chromium.org/2230583002/diff/80001/content/browser/media/a...
content/browser/media/android/media_player_renderer.cc:119: WebContents*
web_contents =
On 2016/08/10 21:14:04, DaleCurtis wrote:
> This can only be done on the UI thread IIRC. Do you want to add some
> DCHECK(BrowserThread::CurrentlyOn()); calls?

Added, but I don't know where else I should add this call.

https://codereview.chromium.org/2230583002/diff/80001/content/browser/media/a...
content/browser/media/android/media_player_renderer.cc:134: return nullptr;
On 2016/08/10 21:14:04, DaleCurtis wrote:
> TODO plumb this through? Or is this overridden at a higher level?

Opened 636588.

https://codereview.chromium.org/2230583002/diff/80001/content/browser/media/a...
content/browser/media/android/media_player_renderer.cc:151: // TODO(tguilbert):
Add OnDurationChange to the RendererClient interface.
On 2016/08/10 21:14:05, DaleCurtis wrote:
> Probably this will be important. I think MP gets the duration wrong a lot.

The CL is already done, I just took it out of this review to keep this review
simpler.

https://codereview.chromium.org/2230583002/diff/80001/content/browser/media/a...
content/browser/media/android/media_player_renderer.cc:176: DVLOG(2) << __func__
<< " Error: " << error;
On 2016/08/10 21:14:05, DaleCurtis wrote:
> I'd make this a LOG(ERROR) for finding issues in the wild.

Changed this to early return, to prevent the non-errors from being logged.

https://codereview.chromium.org/2230583002/diff/80001/content/browser/media/a...
content/browser/media/android/media_player_renderer.cc:184:
renderer_client_->OnError(media::PIPELINE_ERROR_COULD_NOT_RENDER);
On 2016/08/10 21:14:04, DaleCurtis wrote:
> We may want a special error for external renderers here. I.e.
> PIPELINE_ERROR_EXTERNAL_RENDERER_FAILED.

Done. Is there anything that needs updating UMA?

https://codereview.chromium.org/2230583002/diff/80001/content/browser/media/a...
content/browser/media/android/media_player_renderer.cc:212: return true;
On 2016/08/10 21:14:04, DaleCurtis wrote:
> May need to carry over some logic from WMPA/etc to make sure this doesn't
allow
> DoS of mediaserver.

I am not familiar with this area of the code at all. I will ask for a crash
course from you tomorrow, and have created 636615 in the meantime, to address
the issue in a different CL.

https://codereview.chromium.org/2230583002/diff/80001/content/browser/media/a...
content/browser/media/android/media_player_renderer.cc:219: // TODO(tguilbert):
Investigate whether the use of the MediaThrottler is
On 2016/08/10 21:14:05, DaleCurtis wrote:
> I think you can instead use an exponential back off scheme instead.

Created 636615 and added TODO.

https://codereview.chromium.org/2230583002/diff/80001/content/browser/media/a...
File content/browser/media/android/media_player_renderer.h (right):

https://codereview.chromium.org/2230583002/diff/80001/content/browser/media/a...
content/browser/media/android/media_player_renderer.h:40:
MediaPlayerRenderer(content::RenderFrameHost* render_frame_host);
On 2016/08/10 21:14:05, DaleCurtis wrote:
> explicit

Done.

https://codereview.chromium.org/2230583002/diff/80001/content/browser/media/a...
content/browser/media/android/media_player_renderer.h:89: // Used when creating
|media_player_|.
On 2016/08/10 21:14:05, DaleCurtis wrote:
> Functions above variables.

Done.

https://codereview.chromium.org/2230583002/diff/80001/content/renderer/media/...
File content/renderer/media/android/media_player_renderer_client.cc (right):

https://codereview.chromium.org/2230583002/diff/80001/content/renderer/media/...
content/renderer/media/android/media_player_renderer_client.cc:21:
media_task_runner_(media_task_runner),
On 2016/08/10 21:14:05, DaleCurtis wrote:
> std::move ?

Done.

https://codereview.chromium.org/2230583002/diff/80001/content/renderer/media/...
content/renderer/media/android/media_player_renderer_client.cc:22:
compositor_task_runner_(compositor_task_runner),
On 2016/08/10 21:14:05, DaleCurtis wrote:
> ditto

Done.

https://codereview.chromium.org/2230583002/diff/80001/content/renderer/media/...
content/renderer/media/android/media_player_renderer_client.cc:99: return true;
On 2016/08/10 21:14:05, DaleCurtis wrote:
> Some commentary is necessary for HasAudio/HasVideo always returning true.

Done.

https://codereview.chromium.org/2230583002/diff/80001/content/renderer/media/...
File content/renderer/media/android/media_player_renderer_client_factory.cc
(right):

https://codereview.chromium.org/2230583002/diff/80001/content/renderer/media/...
content/renderer/media/android/media_player_renderer_client_factory.cc:18:
compositor_task_runner_(compositor_task_runner),
On 2016/08/10 21:14:06, DaleCurtis wrote:
> std::move?

Done.

https://codereview.chromium.org/2230583002/diff/80001/media/mojo/clients/mojo...
File media/mojo/clients/mojo_renderer.h (right):

https://codereview.chromium.org/2230583002/diff/80001/media/mojo/clients/mojo...
media/mojo/clients/mojo_renderer.h:76: void
InitializeRendererFromStreams(media::RendererClient* client);
On 2016/08/10 21:14:06, DaleCurtis wrote:
> What is FromStreams?

Added comment.

https://codereview.chromium.org/2230583002/diff/80001/media/mojo/interfaces/r...
File media/mojo/interfaces/renderer.mojom (right):

https://codereview.chromium.org/2230583002/diff/80001/media/mojo/interfaces/r...
media/mojo/interfaces/renderer.mojom:18: url.mojom.Url url) => (bool success);
On 2016/08/10 21:14:06, DaleCurtis wrote:
> ? for url parameter ?

Added, and updated to use the proper base::Optional<GURL> where appropriate.

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

https://codereview.chromium.org/2230583002/diff/80001/media/mojo/services/moj...
media/mojo/services/mojo_renderer_service.cc:51: if (url.is_empty()) {
On 2016/08/10 21:14:06, DaleCurtis wrote:
> This is fine, but early return if empty instead of else block.

Done.

[tguilbert, 2016-08-11 00:40:01]

The CQ bit was checked by tguilbert@chromium.org to run a CQ dry run

[DaleCurtis, 2016-08-11 00:40:42]

lgtm, i'm pretty sure you have to solve the cookies issue. otherwise things like
signed in hulu playbacks, etc won't work.

[commit-bot: I haz the power, 2016-08-11 00:41:09]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-11 00:45:43]

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

[commit-bot: I haz the power, 2016-08-11 00:45:44]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  chromeos_x86-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_x86-ge...)
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_clobber_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[tguilbert, 2016-08-11 00:53:52]

The CQ bit was checked by tguilbert@chromium.org

[tguilbert, 2016-08-11 00:53:53]

The patchset sent to the CQ was uploaded after l-g-t-m from watk@chromium.org,
ochang@chromium.org
Link to the patchset: https://codereview.chromium.org/2230583002/#ps100001
(title: "Addressing Dale's comments")

[commit-bot: I haz the power, 2016-08-11 00:54:39]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-11 00:58:31]

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

[commit-bot: I haz the power, 2016-08-11 00:58:34]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[tguilbert, 2016-08-11 01:45:46]

The CQ bit was checked by tguilbert@chromium.org

[tguilbert, 2016-08-11 01:45:47]

The patchset sent to the CQ was uploaded after l-g-t-m from watk@chromium.org,
ochang@chromium.org, dalecurtis@chromium.org
Link to the patchset: https://codereview.chromium.org/2230583002/#ps120001
(title: "Rebase")

[commit-bot: I haz the power, 2016-08-11 01:46:38]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-11 02:01:07]

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

[commit-bot: I haz the power, 2016-08-11 02:01:09]

Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[tguilbert, 2016-08-11 02:21:56]

The CQ bit was checked by tguilbert@chromium.org

[tguilbert, 2016-08-11 02:21:57]

The patchset sent to the CQ was uploaded after l-g-t-m from watk@chromium.org,
ochang@chromium.org, dalecurtis@chromium.org
Link to the patchset: https://codereview.chromium.org/2230583002/#ps140001
(title: "Fixed UTs")

[commit-bot: I haz the power, 2016-08-11 02:22:55]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-11 02:43:07]

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

[commit-bot: I haz the power, 2016-08-11 02:43:09]

Try jobs failed on following builders:
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)

[tguilbert, 2016-08-11 04:47:52]

The CQ bit was checked by tguilbert@chromium.org

[tguilbert, 2016-08-11 04:47:53]

The patchset sent to the CQ was uploaded after l-g-t-m from watk@chromium.org,
ochang@chromium.org, dalecurtis@chromium.org
Link to the patchset: https://codereview.chromium.org/2230583002/#ps160001
(title: "Added override to virt. dtors.")

[commit-bot: I haz the power, 2016-08-11 04:48:09]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-11 04:49:43]

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

[commit-bot: I haz the power, 2016-08-11 04:49:45]

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

[tguilbert, 2016-08-11 06:22:55]

The CQ bit was checked by tguilbert@chromium.org

[commit-bot: I haz the power, 2016-08-11 06:23:05]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-11 06:42:32]

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

[commit-bot: I haz the power, 2016-08-11 06:42:35]

Try jobs failed on following builders:
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)

[tguilbert, 2016-08-11 07:26:02]

The CQ bit was checked by tguilbert@chromium.org

[tguilbert, 2016-08-11 07:26:02]

The patchset sent to the CQ was uploaded after l-g-t-m from watk@chromium.org,
ochang@chromium.org, dalecurtis@chromium.org
Link to the patchset: https://codereview.chromium.org/2230583002/#ps180001
(title: "Chromium style fix")

[commit-bot: I haz the power, 2016-08-11 07:26:17]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-11 07:59:06]

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

[commit-bot: I haz the power, 2016-08-11 07:59:08]

Try jobs failed on following builders:
  win_clang on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)

[tguilbert, 2016-08-11 18:42:28]

The CQ bit was checked by tguilbert@chromium.org

[tguilbert, 2016-08-11 18:42:28]

The patchset sent to the CQ was uploaded after l-g-t-m from watk@chromium.org,
ochang@chromium.org, dalecurtis@chromium.org
Link to the patchset: https://codereview.chromium.org/2230583002/#ps200001
(title: "Chromium style.")

[commit-bot: I haz the power, 2016-08-11 18:43:37]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-11 19:53:47]

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

[commit-bot: I haz the power, 2016-08-11 19:53:49]

Try jobs failed on following builders:
  win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)

[tguilbert, 2016-08-11 20:04:59]

The CQ bit was checked by tguilbert@chromium.org

[tguilbert, 2016-08-11 20:04:59]

The patchset sent to the CQ was uploaded after l-g-t-m from watk@chromium.org,
ochang@chromium.org, dalecurtis@chromium.org
Link to the patchset: https://codereview.chromium.org/2230583002/#ps220001
(title: "Added MEDIA_EXPORT.")

[commit-bot: I haz the power, 2016-08-11 20:05:30]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-11 23:34:09]

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

[commit-bot: I haz the power, 2016-08-11 23:34:11]

Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[tguilbert, 2016-08-11 23:46:13]

The CQ bit was checked by tguilbert@chromium.org

[commit-bot: I haz the power, 2016-08-11 23:47:12]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-12 02:40:32]

Committed patchset #12 (id:220001)

[commit-bot: I haz the power, 2016-08-12 02:42:15]

Description was changed from

==========
Add MediaPlayerRenderer/MediaPlayerRendererClient

This change introduces MediaPlayerRender and MediaPlayerRendererClient.

The MediaPlayerRenderer uses an instance of an Android MediaPlayer to
render video to a given Android Surface. It adapts the MediaPlayer
interface into the media::Renderer interface. It lives in the Browser
process, and is owned by and instance of MojoRendererService.

The MediaPlayerRendereClient is a media::Renderer that integrates
into the WMPI pipeline. It forwards media::Renderer calls to its
associated MediaPlayerRenderer in the Browser process, via a
MojoRenderer (bound to the MojoRendererService). It also owns a
StreamTextureWrapper, which it uses to send VideoFrame to WMPI's
VideoFrameCompositor.

In order to be fully functional, the code is missing the repainting
of duplicate video frames (636002), the propagation of duration changes
(635991) and a mechanism for registering/retrieving the
StreamTextureWrapper's Surface (627658).

BUG=631199
==========

to

==========
Add MediaPlayerRenderer/MediaPlayerRendererClient

This change introduces MediaPlayerRender and MediaPlayerRendererClient.

The MediaPlayerRenderer uses an instance of an Android MediaPlayer to
render video to a given Android Surface. It adapts the MediaPlayer
interface into the media::Renderer interface. It lives in the Browser
process, and is owned by and instance of MojoRendererService.

The MediaPlayerRendereClient is a media::Renderer that integrates
into the WMPI pipeline. It forwards media::Renderer calls to its
associated MediaPlayerRenderer in the Browser process, via a
MojoRenderer (bound to the MojoRendererService). It also owns a
StreamTextureWrapper, which it uses to send VideoFrame to WMPI's
VideoFrameCompositor.

In order to be fully functional, the code is missing the repainting
of duplicate video frames (636002), the propagation of duration changes
(635991) and a mechanism for registering/retrieving the
StreamTextureWrapper's Surface (627658).

BUG=631199

Committed: https://crrev.com/cd478271f0ea7b3a25fd0b04799d97699598278a
Cr-Commit-Position: refs/heads/master@{#411527}
==========

[commit-bot: I haz the power, 2016-08-12 02:42:17]

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