media: Check kDisableInfobarForProtectedMediaIdentifier in ProtectedMediaIdentifierPermissionContex…

media: Check kDisableInfobarForProtectedMediaIdentifier in ProtectedMediaIdentifierPermissionContext.

Previously this flag was checked in BrowserCdmManager. When this flag is set,
we would NOT check any preference and content setting related to protected
media identifier. Permission was granted blindly by default.

This CL moves the check to ProtectedMediaIdentifierPermissionContext. Now the
impact of this flag is more restrictive: only when the permission status is
"ASK" will we replace it with "ALLOW". This applies to both permission request
and permission check. If the permission is "BLOCKED" for any reason (e.g.
invalid origin, disabled by master switch, blocked in content setting), the
permission request/check will be blocked.

BUG=456219
TEST=Manually tested with and without the switch.

Committed: https://crrev.com/5f80e74c8d9b52f246c4a276c25e6af6a35072ba
Cr-Commit-Position: refs/heads/master@{#320997}

[xhwang, 2015-03-17 00:19:46]

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

[xhwang, 2015-03-17 00:19:46]

PTAL

[ddorwin, 2015-03-17 00:43:12]

LGTM % comments. Thanks!

https://codereview.chromium.org/1007163003/diff/1/chrome/browser/media/protec...
File chrome/browser/media/protected_media_identifier_permission_context.cc
(right):

https://codereview.chromium.org/1007163003/diff/1/chrome/browser/media/protec...
chrome/browser/media/protected_media_identifier_permission_context.cc:29: #error
This file should only be used on ChromeOS or Android.
"This file currently only supports Chrome OS and Android."

Other platforms probably should use it, but we don't have support implemented
yet.

https://codereview.chromium.org/1007163003/diff/1/chrome/browser/media/protec...
chrome/browser/media/protected_media_identifier_permission_context.cc:83: 
Note: Not recording the Allow is important, especially since there is a flag to
enable this on Android, which also supports user permissions.

I worry about this code changing in such a way that we end up writing ALLOW when
the switch is present. One option would be to
DCHECK(!base::CommandLine::ForCurrentProcess()->HasSwitch(switches::kDisableInfobarForProtectedMediaIdentifier)).
Another would be to DCHECK wherever the settings get written, but that is more
generic code.

[ddorwin, 2015-03-17 00:44:15]

This is really BUG=456219

[xhwang, 2015-03-17 01:33:21]

comments addressed

[xhwang, 2015-03-17 01:33:28]

https://codereview.chromium.org/1007163003/diff/1/chrome/browser/media/protec...
File chrome/browser/media/protected_media_identifier_permission_context.cc
(right):

https://codereview.chromium.org/1007163003/diff/1/chrome/browser/media/protec...
chrome/browser/media/protected_media_identifier_permission_context.cc:29: #error
This file should only be used on ChromeOS or Android.
On 2015/03/17 00:43:11, ddorwin wrote:
> "This file currently only supports Chrome OS and Android."
> 
> Other platforms probably should use it, but we don't have support implemented
> yet.

Done.

https://codereview.chromium.org/1007163003/diff/1/chrome/browser/media/protec...
chrome/browser/media/protected_media_identifier_permission_context.cc:83: 
On 2015/03/17 00:43:11, ddorwin wrote:
> Note: Not recording the Allow is important, especially since there is a flag
to
> enable this on Android, which also supports user permissions.
> 
> I worry about this code changing in such a way that we end up writing ALLOW
when
> the switch is present. One option would be to
>
DCHECK(!base::CommandLine::ForCurrentProcess()->HasSwitch(switches::kDisableInfobarForProtectedMediaIdentifier)).
> Another would be to DCHECK wherever the settings get written, but that is more
> generic code.

Added DCHECK.

[ddorwin, 2015-03-17 02:43:09]

lgtm

Thanks.

https://codereview.chromium.org/1007163003/diff/20001/chrome/browser/media/pr...
File chrome/browser/media/protected_media_identifier_permission_context.cc
(right):

https://codereview.chromium.org/1007163003/diff/20001/chrome/browser/media/pr...
chrome/browser/media/protected_media_identifier_permission_context.cc:106:
DCHECK(!base::CommandLine::ForCurrentProcess()->HasSwitch(
Note: This only works because the file only supports Android and Chrome OS.

[xhwang, 2015-03-17 21:43:42]

https://codereview.chromium.org/1007163003/diff/20001/chrome/browser/media/pr...
File chrome/browser/media/protected_media_identifier_permission_context.cc
(right):

https://codereview.chromium.org/1007163003/diff/20001/chrome/browser/media/pr...
chrome/browser/media/protected_media_identifier_permission_context.cc:106:
DCHECK(!base::CommandLine::ForCurrentProcess()->HasSwitch(
On 2015/03/17 02:43:09, ddorwin wrote:
> Note: This only works because the file only supports Android and Chrome OS.

Acknowledged.

[xhwang, 2015-03-17 21:43:46]

The CQ bit was checked by xhwang@chromium.org

[xhwang, 2015-03-17 21:43:46]

The patchset sent to the CQ was uploaded after l-g-t-m from ddorwin@chromium.org
Link to the patchset: https://codereview.chromium.org/1007163003/#ps40001
(title: "rebase only")

[commit-bot: I haz the power, 2015-03-17 21:45:38]

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

[commit-bot: I haz the power, 2015-03-17 22:30:04]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-03-17 22:30:58]

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