Adds hook for content embedders to get media::BrowserCdm*.

Adds hook for content embedders to get media::BrowserCdm*.

R=jam@chromium.org,erickung@chromium.org
BUG=444930
Committed: https://crrev.com/400ad00072c3f55fd449ed80373f261d06cf5348
Cr-Commit-Position: refs/heads/master@{#314937}

[gunsch, 2015-02-02 20:36:37]

@erickung: for chromecast/ changes
@jam: for content/ changes

[erickung1, 2015-02-02 21:51:35]

Don't see any issue at all, waiting for jam@ to finish the content side review

https://codereview.chromium.org/894173002/diff/20001/chromecast/browser/media...
File chromecast/browser/media/cma_message_filter_host.h (right):

https://codereview.chromium.org/894173002/diff/20001/chromecast/browser/media...
chromecast/browser/media/cma_message_filter_host.h:31: namespace content {
Nit: namespace in alphanumeric order. (move it to namespace gfx above)

https://codereview.chromium.org/894173002/diff/20001/content/public/browser/r...
File content/public/browser/render_process_host.h (right):

https://codereview.chromium.org/894173002/diff/20001/content/public/browser/r...
content/public/browser/render_process_host.h:277: // and |cdm_id| by invoking
|cdm_callback|, or nullptr if not found.
Nit: updates the comment since |cdm_callback| is not part of function
parameters.

[gunsch, 2015-02-02 23:43:13]

https://codereview.chromium.org/894173002/diff/20001/chromecast/browser/media...
File chromecast/browser/media/cma_message_filter_host.h (right):

https://codereview.chromium.org/894173002/diff/20001/chromecast/browser/media...
chromecast/browser/media/cma_message_filter_host.h:31: namespace content {
On 2015/02/02 21:51:34, erickung1 wrote:
> Nit: namespace in alphanumeric order. (move it to namespace gfx above) 

Done.

https://codereview.chromium.org/894173002/diff/20001/content/public/browser/r...
File content/public/browser/render_process_host.h (right):

https://codereview.chromium.org/894173002/diff/20001/content/public/browser/r...
content/public/browser/render_process_host.h:277: // and |cdm_id| by invoking
|cdm_callback|, or nullptr if not found.
On 2015/02/02 21:51:34, erickung1 wrote:
> Nit: updates the comment since |cdm_callback| is not part of function
> parameters.

Done.

[jam, 2015-02-03 05:51:45]

https://codereview.chromium.org/894173002/diff/60001/chromecast/browser/media...
File chromecast/browser/media/cma_message_filter_host.cc (right):

https://codereview.chromium.org/894173002/diff/60001/chromecast/browser/media...
chromecast/browser/media/cma_message_filter_host.cc:186:
render_process_host_->GetBrowserCdm(render_frame_id, cdm_id);
there's no guarantee that the RPH is alive at this point. you should keep the
RPH id as before, and when you want to access the RPH call RPH::FromID()

[gunsch, 2015-02-03 17:25:28]

https://codereview.chromium.org/894173002/diff/60001/chromecast/browser/media...
File chromecast/browser/media/cma_message_filter_host.cc (right):

https://codereview.chromium.org/894173002/diff/60001/chromecast/browser/media...
chromecast/browser/media/cma_message_filter_host.cc:186:
render_process_host_->GetBrowserCdm(render_frame_id, cdm_id);
On 2015/02/03 05:51:45, jam wrote:
> there's no guarantee that the RPH is alive at this point. you should keep the
> RPH id as before, and when you want to access the RPH call RPH::FromID()

Good to know, though shouldn't the CmaMessageFilterHost instance be dead in that
case, since it's attached as a filter to the RenderProcessHost?

[jam, 2015-02-05 23:02:15]

lgtm with comment

https://codereview.chromium.org/894173002/diff/60001/chromecast/browser/media...
File chromecast/browser/media/cma_message_filter_host.cc (right):

https://codereview.chromium.org/894173002/diff/60001/chromecast/browser/media...
chromecast/browser/media/cma_message_filter_host.cc:186:
render_process_host_->GetBrowserCdm(render_frame_id, cdm_id);
On 2015/02/03 17:25:28, gunsch wrote:
> On 2015/02/03 05:51:45, jam wrote:
> > there's no guarantee that the RPH is alive at this point. you should keep
the
> > RPH id as before, and when you want to access the RPH call RPH::FromID()
> 
> Good to know, though shouldn't the CmaMessageFilterHost instance be dead in
that
> case, since it's attached as a filter to the RenderProcessHost?

it's a race condition; i.e. the RPH goes away which causes the filters to
destruct. There could still be an outstanding PostTask from IO to UI thread, and
when it arrives on UI thread the RPH could be gone

https://codereview.chromium.org/894173002/diff/100001/chromecast/browser/medi...
File chromecast/browser/media/cma_message_filter_host.cc (right):

https://codereview.chromium.org/894173002/diff/100001/chromecast/browser/medi...
chromecast/browser/media/cma_message_filter_host.cc:171: base::Unretained(this),
it's confusing not keeping a reference to |this| while calling a non-static
member function. so either remove the method from this class and move it to the
anonymous namespace, or replace base::Unretained(this)  with |this| (and also in
that case, don't pass process_id_ and use it in the member method directly).

[gunsch, 2015-02-05 23:34:14]

New patchsets have been uploaded after l-g-t-m from jam@chromium.org

[gunsch, 2015-02-05 23:34:53]

https://codereview.chromium.org/894173002/diff/60001/chromecast/browser/media...
File chromecast/browser/media/cma_message_filter_host.cc (right):

https://codereview.chromium.org/894173002/diff/60001/chromecast/browser/media...
chromecast/browser/media/cma_message_filter_host.cc:186:
render_process_host_->GetBrowserCdm(render_frame_id, cdm_id);
On 2015/02/05 23:02:15, jam wrote:
> On 2015/02/03 17:25:28, gunsch wrote:
> > On 2015/02/03 05:51:45, jam wrote:
> > > there's no guarantee that the RPH is alive at this point. you should keep
> the
> > > RPH id as before, and when you want to access the RPH call RPH::FromID()
> > 
> > Good to know, though shouldn't the CmaMessageFilterHost instance be dead in
> that
> > case, since it's attached as a filter to the RenderProcessHost?
> 
> it's a race condition; i.e. the RPH goes away which causes the filters to
> destruct. There could still be an outstanding PostTask from IO to UI thread,
and
> when it arrives on UI thread the RPH could be gone

Acknowledged.

https://codereview.chromium.org/894173002/diff/100001/chromecast/browser/medi...
File chromecast/browser/media/cma_message_filter_host.cc (right):

https://codereview.chromium.org/894173002/diff/100001/chromecast/browser/medi...
chromecast/browser/media/cma_message_filter_host.cc:171: base::Unretained(this),
On 2015/02/05 23:02:15, jam wrote:
> it's confusing not keeping a reference to |this| while calling a non-static
> member function. so either remove the method from this class and move it to
the
> anonymous namespace, or replace base::Unretained(this)  with |this| (and also
in
> that case, don't pass process_id_ and use it in the member method directly).

Done: forgot that this class was ref-counted as a BrowserMessageFilter. That
makes things cleaner, thanks.

[erickung1, 2015-02-05 23:39:27]

On 2015/02/05 23:34:53, gunsch wrote:
>
https://codereview.chromium.org/894173002/diff/60001/chromecast/browser/media...
> File chromecast/browser/media/cma_message_filter_host.cc (right):
> 
>
https://codereview.chromium.org/894173002/diff/60001/chromecast/browser/media...
> chromecast/browser/media/cma_message_filter_host.cc:186:
> render_process_host_->GetBrowserCdm(render_frame_id, cdm_id);
> On 2015/02/05 23:02:15, jam wrote:
> > On 2015/02/03 17:25:28, gunsch wrote:
> > > On 2015/02/03 05:51:45, jam wrote:
> > > > there's no guarantee that the RPH is alive at this point. you should
keep
> > the
> > > > RPH id as before, and when you want to access the RPH call RPH::FromID()
> > > 
> > > Good to know, though shouldn't the CmaMessageFilterHost instance be dead
in
> > that
> > > case, since it's attached as a filter to the RenderProcessHost?
> > 
> > it's a race condition; i.e. the RPH goes away which causes the filters to
> > destruct. There could still be an outstanding PostTask from IO to UI thread,
> and
> > when it arrives on UI thread the RPH could be gone
> 
> Acknowledged.
> 
>
https://codereview.chromium.org/894173002/diff/100001/chromecast/browser/medi...
> File chromecast/browser/media/cma_message_filter_host.cc (right):
> 
>
https://codereview.chromium.org/894173002/diff/100001/chromecast/browser/medi...
> chromecast/browser/media/cma_message_filter_host.cc:171:
base::Unretained(this),
> On 2015/02/05 23:02:15, jam wrote:
> > it's confusing not keeping a reference to |this| while calling a non-static
> > member function. so either remove the method from this class and move it to
> the
> > anonymous namespace, or replace base::Unretained(this)  with |this| (and
also
> in
> > that case, don't pass process_id_ and use it in the member method directly).
> 
> Done: forgot that this class was ref-counted as a BrowserMessageFilter. That
> makes things cleaner, thanks.

lgtm

[gunsch, 2015-02-05 23:40:50]

The CQ bit was checked by gunsch@chromium.org

[commit-bot: I haz the power, 2015-02-05 23:42:19]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/894173002/120001

[commit-bot: I haz the power, 2015-02-06 01:20:47]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2015-02-06 01:21:33]

Patchset 7 (id:??) landed as
https://crrev.com/400ad00072c3f55fd449ed80373f261d06cf5348
Cr-Commit-Position: refs/heads/master@{#314937}