media: Add a web preference to enable encrypted media

media: Add a web preference to enable encrypted media

Add a WebPreference encrypted_media_enabled. If it's set to false, all requestMediaKeySystemAccess() calls will return a rejected promise with NotSupportedError.

Also connect |encrypted_media_enabled| with a new Chrome preference kWebKitEncryptedMediaEnabled and make sure when user changes that preference, the Web preference in the renderer process will get updated.

Note that the check is only done in requestMediaKeySystemAccess(). So if a user disabled the preference after a protected content playback already started, the playback session will not be affected. However, all newer protected content playback attempt will fail.

BUG=689778
TEST=Manually tested.

Review-Url: https://codereview.chromium.org/2676633006
Cr-Commit-Position: refs/heads/master@{#449048}
Committed: https://chromium.googlesource.com/chromium/src/+/d252d7b55af16f3edbb90d1132cf6ff360f7d11f

[xhwang, 2017-02-03 00:51:02]

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

[commit-bot: I haz the power, 2017-02-03 00:51:36]

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

[xhwang, 2017-02-03 00:57:06]

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

[commit-bot: I haz the power, 2017-02-03 00:57:49]

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, 2017-02-03 03:27:59]

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

[commit-bot: I haz the power, 2017-02-03 03:28:04]

Dry run: This issue passed the CQ dry run.

[xhwang, 2017-02-04 06:39:41]

Description was changed from

==========
media: Add an option to disable requestMediaKeySystemAccess()

BUG=
TEST=
==========

to

==========
media: Add a web preference to disable encrypted media

BUG=
TEST=
==========

[xhwang, 2017-02-04 06:40:23]

Patchset #1 (id:1) has been deleted

[xhwang, 2017-02-04 06:40:33]

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

[commit-bot: I haz the power, 2017-02-04 06:40:40]

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

[xhwang, 2017-02-04 06:45:02]

xhwang@chromium.org changed reviewers:
+ pastarmovj@chromium.org, tommycli@chromium.org

[xhwang, 2017-02-04 06:45:03]

Just FYI for now. As discussed offline.

[commit-bot: I haz the power, 2017-02-04 08:41:23]

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

[commit-bot: I haz the power, 2017-02-04 08:41:24]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build
URL)
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  chromium_presubmit on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build
URL)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT,
no build URL)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no
build URL)
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT,
no build URL)
  win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_TIMED_OUT,
no build URL)
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_TIMED_OUT, no build
URL)
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_TIMED_OUT, no
build URL)
  win_clang on master.tryserver.chromium.win (JOB_TIMED_OUT, no build URL)

[xhwang, 2017-02-06 20:31:37]

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

[commit-bot: I haz the power, 2017-02-06 20:32:41]

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, 2017-02-06 22:24:00]

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

[commit-bot: I haz the power, 2017-02-06 22:24:01]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[xhwang, 2017-02-06 23:13:37]

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

[commit-bot: I haz the power, 2017-02-06 23:14:43]

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

[xhwang, 2017-02-06 23:59:54]

xhwang@chromium.org changed reviewers:
+ jrummell@chromium.org
- pastarmovj@chromium.org

[xhwang, 2017-02-06 23:59:55]

tommycli, jrummell: PTAL
ddorwin, pastarmovj: FYI

[xhwang, 2017-02-07 00:07:08]

Description was changed from

==========
media: Add a web preference to disable encrypted media

BUG=
TEST=
==========

to

==========
media: Add a web preference to disable encrypted media

Add a WebPreference encrypted_media_enabled. If it's set to false, all
requestMediaKeySystemAccess() calls will return a rejected promise.

Also connect |encrypted_media_enabled| with the Chrome preference kEnableDRM and
make sure when user changes that preference, the renderer process will get
updated.

Note that the check is only done in requestMediaKeySystemAccess(). So if a user
disabled it after a protected content playback started, the playback session
will not be affected. However, all newer protected content playback attempt will
fail.

BUG=686430
TEST=Manually tested.
==========

[xhwang, 2017-02-07 00:07:46]

Description was changed from

==========
media: Add a web preference to disable encrypted media

Add a WebPreference encrypted_media_enabled. If it's set to false, all
requestMediaKeySystemAccess() calls will return a rejected promise.

Also connect |encrypted_media_enabled| with the Chrome preference kEnableDRM and
make sure when user changes that preference, the renderer process will get
updated.

Note that the check is only done in requestMediaKeySystemAccess(). So if a user
disabled it after a protected content playback started, the playback session
will not be affected. However, all newer protected content playback attempt will
fail.

BUG=686430
TEST=Manually tested.
==========

to

==========
media: Add a web preference to disable encrypted media

Add a WebPreference encrypted_media_enabled. If it's set to false, all
requestMediaKeySystemAccess() calls will return a rejected promise.

Also connect |encrypted_media_enabled| with a Chrome preference and make sure
when user changes that preference, the renderer process will get updated.

Note that the check is only done in requestMediaKeySystemAccess(). So if a user
disabled it after a protected content playback started, the playback session
will not be affected. However, all newer protected content playback attempt will
fail.

BUG=686430
TEST=Manually tested.
==========

[tommycli, 2017-02-07 00:14:59]

overall looks good. I'd suggest writing either a Webkit-test or browser test
also.

https://codereview.chromium.org/2676633006/diff/80001/chrome/browser/ui/prefs...
File chrome/browser/ui/prefs/prefs_tab_helper.cc (right):

https://codereview.chromium.org/2676633006/diff/80001/chrome/browser/ui/prefs...
chrome/browser/ui/prefs/prefs_tab_helper.cc:535:
pref_defaults.encrypted_media_enabled);
nit: Maybe this should be moved under plugins_enabled to be consistent with the
ordering within WebPreferences.

I realize the pref name doesn't have the webkit prefix for historic reasons...
but oh well...

https://codereview.chromium.org/2676633006/diff/80001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/modules/encryptedmedia/NavigatorRequestMediaKeySystemAccess.cpp
(right):

https://codereview.chromium.org/2676633006/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/encryptedmedia/NavigatorRequestMediaKeySystemAccess.cpp:344:
if (!document->page()) {
Moving this clause changes the behavior when document->page() is nullptr and
there's an empty keySystem or supportedConfigurations argument.

Is the new behavior still consistent with the w3c spec?

[commit-bot: I haz the power, 2017-02-07 00:45:17]

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

[commit-bot: I haz the power, 2017-02-07 00:45:18]

Dry run: This issue passed the CQ dry run.

[xhwang, 2017-02-07 06:16:54]

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

[commit-bot: I haz the power, 2017-02-07 06:17:01]

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

[xhwang, 2017-02-07 06:18:10]

Comments addressed.

For the test, do you know any example where we toggle content settings and
expect different behavior?

https://codereview.chromium.org/2676633006/diff/80001/chrome/browser/ui/prefs...
File chrome/browser/ui/prefs/prefs_tab_helper.cc (right):

https://codereview.chromium.org/2676633006/diff/80001/chrome/browser/ui/prefs...
chrome/browser/ui/prefs/prefs_tab_helper.cc:535:
pref_defaults.encrypted_media_enabled);
On 2017/02/07 00:14:59, tommycli wrote:
> nit: Maybe this should be moved under plugins_enabled to be consistent with
the
> ordering within WebPreferences.

Done.

> I realize the pref name doesn't have the webkit prefix for historic reasons...
> but oh well...

Yeah, this was why I put it here :)

https://codereview.chromium.org/2676633006/diff/80001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/modules/encryptedmedia/NavigatorRequestMediaKeySystemAccess.cpp
(right):

https://codereview.chromium.org/2676633006/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/encryptedmedia/NavigatorRequestMediaKeySystemAccess.cpp:344:
if (!document->page()) {
On 2017/02/07 00:14:59, tommycli wrote:
> Moving this clause changes the behavior when document->page() is nullptr and
> there's an empty keySystem or supportedConfigurations argument.
> 
> Is the new behavior still consistent with the w3c spec?

I reverted this part of the change since the "document" is mentioned in that
part of the spec.

The spec does really specify what to do in this case so I don't think this
affect spec compliance. In either case, the promise will be rejected.

[commit-bot: I haz the power, 2017-02-07 06:19:32]

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

[commit-bot: I haz the power, 2017-02-07 06:19:33]

Dry run: Try jobs failed on following builders:
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

[jrummell, 2017-02-07 18:35:05]

lgtm

[xhwang, 2017-02-07 18:47:42]

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

[commit-bot: I haz the power, 2017-02-07 18:48:14]

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

[xhwang, 2017-02-07 20:29:19]

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

[commit-bot: I haz the power, 2017-02-07 20:29:58]

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

[xhwang, 2017-02-07 20:30:15]

Browser test added!

tommycli: PTAL again!
jrummell: FYI

[commit-bot: I haz the power, 2017-02-07 20:55:29]

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

[commit-bot: I haz the power, 2017-02-07 20:55:30]

Dry run: 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_...)

[xhwang, 2017-02-07 21:20:51]

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

[commit-bot: I haz the power, 2017-02-07 21:22:18]

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

[tommycli, 2017-02-07 21:25:48]

nice test lgtm

[jrummell, 2017-02-07 21:30:48]

Thanks for adding a test. Still lgtm.

https://codereview.chromium.org/2676633006/diff/140001/chrome/browser/media/e...
File chrome/browser/media/encrypted_media_browsertest.cc (right):

https://codereview.chromium.org/2676633006/diff/140001/chrome/browser/media/e...
chrome/browser/media/encrypted_media_browsertest.cc:552: PlayTwice::NO,
kEmeNotSupportedError);
The code in NavigatorRequestMediaKeySystemAccess throws a TypeError. Although
the test page converts any error to return NotSupported, should RMKSA return the
DOMException NotSupported?

[xhwang, 2017-02-07 22:24:08]

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

[commit-bot: I haz the power, 2017-02-07 22:25:46]

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

[xhwang, 2017-02-07 22:27:05]

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

[commit-bot: I haz the power, 2017-02-07 22:28:09]

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

[xhwang, 2017-02-07 22:29:59]

https://codereview.chromium.org/2676633006/diff/140001/chrome/browser/media/e...
File chrome/browser/media/encrypted_media_browsertest.cc (right):

https://codereview.chromium.org/2676633006/diff/140001/chrome/browser/media/e...
chrome/browser/media/encrypted_media_browsertest.cc:552: PlayTwice::NO,
kEmeNotSupportedError);
On 2017/02/07 21:30:48, jrummell wrote:
> The code in NavigatorRequestMediaKeySystemAccess throws a TypeError. Although
> the test page converts any error to return NotSupported, should RMKSA return
the
> DOMException NotSupported?

Good point. Returns NotSupportedError in NavigatorRequestMediaKeySystemAccess
instead.

I thought about adding a helper page in the rejection message but this code is
in blink, so it's shared by all blink embedders and content embedders. I don't
feel it's easy to have a generic helper link there.

[xhwang, 2017-02-07 22:34:17]

xhwang@chromium.org changed reviewers:
+ dcheng@chromium.org, jam@chromium.org

[xhwang, 2017-02-07 22:34:18]

dcheng@chromium.org: Please OWNERS review changes in third_party/Webkit/

jam@chromium.org: Please OWNERS review changes in content/ and chrome/

[dcheng, 2017-02-07 23:28:01]

blink + ipc lgtm

[commit-bot: I haz the power, 2017-02-08 00:04:16]

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

[commit-bot: I haz the power, 2017-02-08 00:04:17]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[jam, 2017-02-08 01:43:58]

lgtm

[xhwang, 2017-02-08 02:20:31]

Description was changed from

==========
media: Add a web preference to disable encrypted media

Add a WebPreference encrypted_media_enabled. If it's set to false, all
requestMediaKeySystemAccess() calls will return a rejected promise.

Also connect |encrypted_media_enabled| with a Chrome preference and make sure
when user changes that preference, the renderer process will get updated.

Note that the check is only done in requestMediaKeySystemAccess(). So if a user
disabled it after a protected content playback started, the playback session
will not be affected. However, all newer protected content playback attempt will
fail.

BUG=686430
TEST=Manually tested.
==========

to

==========
media: Add a web preference to disable encrypted media

Add a WebPreference encrypted_media_enabled. If it's set to false, all
requestMediaKeySystemAccess() calls will return a rejected promise.

Also connect |encrypted_media_enabled| with a Chrome preference and make sure
when user changes that preference, the renderer process will get updated.

Note that the check is only done in requestMediaKeySystemAccess(). So if a user
disabled it after a protected content playback started, the playback session
will not be affected. However, all newer protected content playback attempt will
fail.

BUG=689778
TEST=Manually tested.
==========

[xhwang, 2017-02-08 02:33:07]

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

[commit-bot: I haz the power, 2017-02-08 02:33:51]

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, 2017-02-08 04:13:52]

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

[commit-bot: I haz the power, 2017-02-08 04:13:53]

Dry run: This issue passed the CQ dry run.

[xhwang, 2017-02-08 05:34:31]

Description was changed from

==========
media: Add a web preference to disable encrypted media

Add a WebPreference encrypted_media_enabled. If it's set to false, all
requestMediaKeySystemAccess() calls will return a rejected promise.

Also connect |encrypted_media_enabled| with a Chrome preference and make sure
when user changes that preference, the renderer process will get updated.

Note that the check is only done in requestMediaKeySystemAccess(). So if a user
disabled it after a protected content playback started, the playback session
will not be affected. However, all newer protected content playback attempt will
fail.

BUG=689778
TEST=Manually tested.
==========

to

==========
media: Add a web preference to enable encrypted media

Add a WebPreference encrypted_media_enabled. If it's set to false, all
requestMediaKeySystemAccess() calls will return a rejected promise with
NotSupportedError.

Also connect |encrypted_media_enabled| with a new Chrome preference
kWebKitEncryptedMediaEnabled and make sure when user changes that preference,
the Web preference in the renderer process will get updated.

Note that the check is only done in requestMediaKeySystemAccess(). So if a user
disabled the preference after a protected content playback already started, the
playback session will not be affected. However, all newer protected content
playback attempt will fail.

BUG=689778
TEST=Manually tested.
==========

[tommycli, 2017-02-08 18:21:58]

On 2017/02/08 01:43:58, jam wrote:
> lgtm

patchset 10->11 lgtm still

[xhwang, 2017-02-08 18:30:03]

jam: In PS11 I added a new Chrome preference instead of using an existing one to
avoid migration issues. This also reflects our latest design on the UI
(tommycli@ is working on that). Please let me know whether this still looks good
to you. I need to land this today, so your prompt feedback would be highly
appreciated!

[jam, 2017-02-08 18:36:56]

lgtm

[xhwang, 2017-02-08 18:37:40]

The CQ bit was checked by xhwang@chromium.org

[xhwang, 2017-02-08 18:37:40]

The patchset sent to the CQ was uploaded after l-g-t-m from
jrummell@chromium.org, dcheng@chromium.org
Link to the patchset: https://codereview.chromium.org/2676633006/#ps220001
(title: "use a new pref")

[commit-bot: I haz the power, 2017-02-08 18:38:27]

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, 2017-02-08 18:52:40]

CQ is committing da patch.

Bot data: {"patchset_id": 220001, "attempt_start_ts": 1486579060164390,
"parent_rev": "2512542164d696c21c6a62a0f103e1af9c37ca45", "commit_rev":
"d252d7b55af16f3edbb90d1132cf6ff360f7d11f"}

[commit-bot: I haz the power, 2017-02-08 18:53:22]

Description was changed from

==========
media: Add a web preference to enable encrypted media

Add a WebPreference encrypted_media_enabled. If it's set to false, all
requestMediaKeySystemAccess() calls will return a rejected promise with
NotSupportedError.

Also connect |encrypted_media_enabled| with a new Chrome preference
kWebKitEncryptedMediaEnabled and make sure when user changes that preference,
the Web preference in the renderer process will get updated.

Note that the check is only done in requestMediaKeySystemAccess(). So if a user
disabled the preference after a protected content playback already started, the
playback session will not be affected. However, all newer protected content
playback attempt will fail.

BUG=689778
TEST=Manually tested.
==========

to

==========
media: Add a web preference to enable encrypted media

Add a WebPreference encrypted_media_enabled. If it's set to false, all
requestMediaKeySystemAccess() calls will return a rejected promise with
NotSupportedError.

Also connect |encrypted_media_enabled| with a new Chrome preference
kWebKitEncryptedMediaEnabled and make sure when user changes that preference,
the Web preference in the renderer process will get updated.

Note that the check is only done in requestMediaKeySystemAccess(). So if a user
disabled the preference after a protected content playback already started, the
playback session will not be affected. However, all newer protected content
playback attempt will fail.

BUG=689778
TEST=Manually tested.

Review-Url: https://codereview.chromium.org/2676633006
Cr-Commit-Position: refs/heads/master@{#449048}
Committed:
https://chromium.googlesource.com/chromium/src/+/d252d7b55af16f3edbb90d1132cf...
==========

[commit-bot: I haz the power, 2017-02-08 18:53:23]

Committed patchset #11 (id:220001) as
https://chromium.googlesource.com/chromium/src/+/d252d7b55af16f3edbb90d1132cf...