Plumb firstPartyForCookies() to MediaPlayerBridge

Plumb firstPartyForCookies() to MediaPlayerBridge

Currently, the MediaPlayerRenderer does not allow authenticated
playback.

This change adds the necessary plumbing to send the first party URL
from blink::WebLocalFrame to the MediaPlayerBridge, in order to access
(or to be denied access to) the first party cookies.

Also opened 659826 which might necessite further investigation.

BUG=636604
TEST=updated UTs for MediaUrlDemuxer

Committed: https://crrev.com/9881bc24551338fe9644f02265e7269e73a07765
Cr-Commit-Position: refs/heads/master@{#427924}

[tguilbert, 2016-10-20 23:00:55]

Patchset #1 (id:1) has been deleted

[tguilbert, 2016-10-20 23:18:57]

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

[tguilbert, 2016-10-20 23:18:58]

PTAL!

I think that DemuxerStreamProvider is confusing and a misnomer, and that we
should rename it, but I do not want to address it in this CL. I opened
crbug.com/658062 instead.

Thanks!

https://codereview.chromium.org/2434413002/diff/20001/media/base/media_url_pa...
File media/base/media_url_params.h (right):

https://codereview.chromium.org/2434413002/diff/20001/media/base/media_url_pa...
media/base/media_url_params.h:13: // playback (as opposed to stream based).
Open to suggestions as to how to reword this in a clear way.

[xhwang, 2016-10-20 23:23:28]

On 2016/10/20 23:18:58, tguilbert wrote:
> PTAL!
> 
> I think that DemuxerStreamProvider is confusing and a misnomer, and that we
> should rename it, but I do not want to address it in this CL. I opened
> crbug.com/658062 instead.
> 
> Thanks!

Agreed. Thanks!

>
https://codereview.chromium.org/2434413002/diff/20001/media/base/media_url_pa...
> File media/base/media_url_params.h (right):
> 
>
https://codereview.chromium.org/2434413002/diff/20001/media/base/media_url_pa...
> media/base/media_url_params.h:13: // playback (as opposed to stream based).
> Open to suggestions as to how to reword this in a clear way.

[xhwang, 2016-10-21 00:02:19]

looking pretty good. I just have a question.

https://codereview.chromium.org/2434413002/diff/40001/media/base/media_url_pa...
File media/base/media_url_params.h (right):

https://codereview.chromium.org/2434413002/diff/40001/media/base/media_url_pa...
media/base/media_url_params.h:20: GURL first_party_for_cookies;
Could you please add some more comments about this, it's not trivial to
understand for people familiar with media but not net.

Also, since this is a URL, does it make sense to name this something like
"first_party_url_" (as you have in your test).

[tguilbert, 2016-10-21 03:05:58]

https://codereview.chromium.org/2434413002/diff/40001/media/base/media_url_pa...
File media/base/media_url_params.h (right):

https://codereview.chromium.org/2434413002/diff/40001/media/base/media_url_pa...
media/base/media_url_params.h:20: GURL first_party_for_cookies;
On 2016/10/21 00:02:19, xhwang wrote:
> Could you please add some more comments about this, it's not trivial to
> understand for people familiar with media but not net.
> 
> Also, since this is a URL, does it make sense to name this something like
> "first_party_url_" (as you have in your test).

I was hesitant to add detailed comments, because I am not a net expert myself,
and didn't want to accidentally say anything misleading (especially if there
were security implications).

To the best of my knowledge, and to my current understanding, the comment I
added in this 2nd iteration is true.

I would rather leave it as first_party_for_cookies, because it seems to be the
name that is used across all the files that use it.

[xhwang, 2016-10-21 03:13:06]

Thanks! LGTM

[tguilbert, 2016-10-24 21:21:34]

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

[tguilbert, 2016-10-24 21:21:35]

+ochang Can you security review media/mojo/interfaces/renderer.mojom?

Thank you!

[Oliver Chang, 2016-10-24 21:27:08]

lgtm

mojom lgtm

[tguilbert, 2016-10-24 21:34:16]

The CQ bit was checked by tguilbert@chromium.org

[tguilbert, 2016-10-24 21:34:31]

The CQ bit was unchecked by tguilbert@chromium.org

[commit-bot: I haz the power, 2016-10-24 21:34:45]

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

[tguilbert, 2016-10-26 22:47:32]

Description was changed from

==========
Plumb firstPartyForCookies() to MediaPlayerBridge

Currently, the MediaPlayerRenderer does not allow authenticated
playback.

This change adds the necessary plumbing to send the first party URL
from blink::WebLocalFrame to the MediaPlayerBridge, in order to access
(or to be denied access to) the first party cookies.

BUG=636604
TEST=updated UTs for MediaUrlDemuxer, need test authenticated HLS
==========

to

==========
Plumb firstPartyForCookies() to MediaPlayerBridge

Currently, the MediaPlayerRenderer does not allow authenticated
playback.

This change adds the necessary plumbing to send the first party URL
from blink::WebLocalFrame to the MediaPlayerBridge, in order to access
(or to be denied access to) the first party cookies.

Also opened 659826 which will necessite further investigation.

BUG=636604
TEST=updated UTs for MediaUrlDemuxer
==========

[tguilbert, 2016-10-26 22:47:49]

Description was changed from

==========
Plumb firstPartyForCookies() to MediaPlayerBridge

Currently, the MediaPlayerRenderer does not allow authenticated
playback.

This change adds the necessary plumbing to send the first party URL
from blink::WebLocalFrame to the MediaPlayerBridge, in order to access
(or to be denied access to) the first party cookies.

Also opened 659826 which will necessite further investigation.

BUG=636604
TEST=updated UTs for MediaUrlDemuxer
==========

to

==========
Plumb firstPartyForCookies() to MediaPlayerBridge

Currently, the MediaPlayerRenderer does not allow authenticated
playback.

This change adds the necessary plumbing to send the first party URL
from blink::WebLocalFrame to the MediaPlayerBridge, in order to access
(or to be denied access to) the first party cookies.

Also opened 659826 which might necessite further investigation.

BUG=636604
TEST=updated UTs for MediaUrlDemuxer
==========

[tguilbert, 2016-10-27 01:36:49]

The CQ bit was checked by tguilbert@chromium.org

[commit-bot: I haz the power, 2016-10-27 01:37:21]

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-10-27 03:14:27]

Description was changed from

==========
Plumb firstPartyForCookies() to MediaPlayerBridge

Currently, the MediaPlayerRenderer does not allow authenticated
playback.

This change adds the necessary plumbing to send the first party URL
from blink::WebLocalFrame to the MediaPlayerBridge, in order to access
(or to be denied access to) the first party cookies.

Also opened 659826 which might necessite further investigation.

BUG=636604
TEST=updated UTs for MediaUrlDemuxer
==========

to

==========
Plumb firstPartyForCookies() to MediaPlayerBridge

Currently, the MediaPlayerRenderer does not allow authenticated
playback.

This change adds the necessary plumbing to send the first party URL
from blink::WebLocalFrame to the MediaPlayerBridge, in order to access
(or to be denied access to) the first party cookies.

Also opened 659826 which might necessite further investigation.

BUG=636604
TEST=updated UTs for MediaUrlDemuxer
==========

[commit-bot: I haz the power, 2016-10-27 03:14:28]

Committed patchset #3 (id:60001)

[commit-bot: I haz the power, 2016-10-27 03:18:30]

Description was changed from

==========
Plumb firstPartyForCookies() to MediaPlayerBridge

Currently, the MediaPlayerRenderer does not allow authenticated
playback.

This change adds the necessary plumbing to send the first party URL
from blink::WebLocalFrame to the MediaPlayerBridge, in order to access
(or to be denied access to) the first party cookies.

Also opened 659826 which might necessite further investigation.

BUG=636604
TEST=updated UTs for MediaUrlDemuxer
==========

to

==========
Plumb firstPartyForCookies() to MediaPlayerBridge

Currently, the MediaPlayerRenderer does not allow authenticated
playback.

This change adds the necessary plumbing to send the first party URL
from blink::WebLocalFrame to the MediaPlayerBridge, in order to access
(or to be denied access to) the first party cookies.

Also opened 659826 which might necessite further investigation.

BUG=636604
TEST=updated UTs for MediaUrlDemuxer

Committed: https://crrev.com/9881bc24551338fe9644f02265e7269e73a07765
Cr-Commit-Position: refs/heads/master@{#427924}
==========

[commit-bot: I haz the power, 2016-10-27 03:18:31]

Patchset 3 (id:??) landed as
https://crrev.com/9881bc24551338fe9644f02265e7269e73a07765
Cr-Commit-Position: refs/heads/master@{#427924}