Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(6)

Issue 2678433003: media: Require SecureContext for EME APIs (Closed)

Created:
1 year, 6 months ago by xhwang
Modified:
1 year, 6 months ago
Reviewers:
*jrummell, Torne, foolip
CC:
chromium-reviews, mlamouri+watch-blink_chromium.org, eric.carlson_apple.com, haraken, feature-media-reviews_chromium.org, blink-reviews, Srirama, ddorwin
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

media: Require SecureContext for EME APIs In this CL requestMediaKeySystemAccess() requires SecureContext. On non-SecureContext, this API will not be visible. Some notes for future reference: 1. file:// is always treated as SecureContext. 2. file:// is not a unique origin. The origin string is "file://" 3. LoadFromData is treated as non-SecureContext. See the updated [1] in this CL. 4. LoadFromData is also treated as unique origin. This is what [1] used to test without this CL. 5. allowFileAccessFromFileURLs has nothing to do with SecureContext. Rather it controls whether you can load other files from file://. - So if you run a simple html page on file://, and that file doesn't depend on any other file, you don't need allowFileAccessFromFileURLs. This is why for [2] we don't really need allowFileAccessFromFileURLs. - If the html page depends on other js files, without allowFileAccessFromFileURLs, those js files will not be loaded. This will cause a lot of test failures. That's why in content shell, we have allowFileAccessFromFileURLs enabled by default. [1] third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-unique-origin.html [2] android_webview/test/shell/assets/key-system-test.html BUG=672605 TEST=Manually tested and made sure EME APIs are not available on insecure origins. Review-Url: https://codereview.chromium.org/2678433003 Cr-Commit-Position: refs/heads/master@{#449344} Committed: https://chromium.googlesource.com/chromium/src/+/4de117544debac896ba8205f8835ba9ed484a4d3

Patch Set 1 #

Total comments: 4

Patch Set 2 : comments #

Patch Set 3 : rebase & fix upload error #

Total comments: 4

Patch Set 4 : 'requestMediaKeySystemAccess' in navigator #

Total comments: 2

Patch Set 5 : comments #

Patch Set 6 : rebase only #

Unified diffs Side-by-side diffs Delta from patch set Stats (+66 lines, -84 lines) Patch
M android_webview/javatests/src/org/chromium/android_webview/test/KeySystemTest.java View 1 2 3 4 1 chunk +2 lines, -30 lines 0 comments Download
M android_webview/test/BUILD.gn View 1 2 3 4 1 chunk +1 line, -0 lines 0 comments Download
A android_webview/test/shell/assets/key-system-test.html View 1 2 1 chunk +42 lines, -0 lines 0 comments Download
M third_party/WebKit/LayoutTests/http/tests/security/powerfulFeatureRestrictions/old-powerful-features-on-insecure-origin.html View 1 2 3 3 chunks +4 lines, -6 lines 0 comments Download
M third_party/WebKit/LayoutTests/http/tests/security/powerfulFeatureRestrictions/old-powerful-features-on-insecure-origin-expected.txt View 2 1 chunk +5 lines, -6 lines 0 comments Download
M third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-unique-origin.html View 1 2 3 3 chunks +9 lines, -16 lines 0 comments Download
M third_party/WebKit/Source/core/frame/Deprecation.cpp View 1 2 3 4 5 1 chunk +0 lines, -8 lines 0 comments Download
M third_party/WebKit/Source/core/frame/UseCounter.h View 1 2 3 4 5 1 chunk +0 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/modules/encryptedmedia/HTMLMediaElementEncryptedMedia.idl View 1 chunk +1 line, -2 lines 0 comments Download
M third_party/WebKit/Source/modules/encryptedmedia/MediaKeys.idl View 1 chunk +1 line, -0 lines 0 comments Download
M third_party/WebKit/Source/modules/encryptedmedia/NavigatorRequestMediaKeySystemAccess.cpp View 1 2 3 4 5 1 chunk +0 lines, -13 lines 0 comments Download
M third_party/WebKit/Source/modules/encryptedmedia/NavigatorRequestMediaKeySystemAccess.idl View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 61 (42 generated)
xhwang
jrummell: PTAL ddorwin: FYI
1 year, 6 months ago (2017-02-06 05:22:05 UTC) #18
jrummell
lgtm w/ suggestion to move properly working test to a separate file. https://codereview.chromium.org/2678433003/diff/60001/android_webview/test/shell/assets/key-system-test.html File android_webview/test/shell/assets/key-system-test.html ...
1 year, 6 months ago (2017-02-06 21:08:21 UTC) #19
xhwang
https://codereview.chromium.org/2678433003/diff/60001/android_webview/test/shell/assets/key-system-test.html File android_webview/test/shell/assets/key-system-test.html (right): https://codereview.chromium.org/2678433003/diff/60001/android_webview/test/shell/assets/key-system-test.html#newcode17 android_webview/test/shell/assets/key-system-test.html:17: navigator.requestMediaKeySystemAccess(keySystem, [{}]) On 2017/02/06 21:08:21, jrummell wrote: > You ...
1 year, 6 months ago (2017-02-06 21:51:01 UTC) #21
xhwang
torne@chromium.org: Please OWNERS review changes in android_webview/.
1 year, 6 months ago (2017-02-06 21:51:50 UTC) #24
xhwang
foolip: Please OWNERS review changes in third_party/WebKit/Source/core/*
1 year, 6 months ago (2017-02-06 23:57:47 UTC) #30
foolip
third_party/WebKit/ lgtm % nits https://codereview.chromium.org/2678433003/diff/100001/third_party/WebKit/LayoutTests/http/tests/security/powerfulFeatureRestrictions/old-powerful-features-on-insecure-origin.html File third_party/WebKit/LayoutTests/http/tests/security/powerfulFeatureRestrictions/old-powerful-features-on-insecure-origin.html (right): https://codereview.chromium.org/2678433003/diff/100001/third_party/WebKit/LayoutTests/http/tests/security/powerfulFeatureRestrictions/old-powerful-features-on-insecure-origin.html#newcode86 third_party/WebKit/LayoutTests/http/tests/security/powerfulFeatureRestrictions/old-powerful-features-on-insecure-origin.html:86: assert_false(navigator.hasOwnProperty('requestMediaKeySystemAccess'), navigator.hasOwnProperty('requestMediaKeySystemAccess') was false before ...
1 year, 6 months ago (2017-02-07 00:06:11 UTC) #31
xhwang
https://codereview.chromium.org/2678433003/diff/100001/third_party/WebKit/LayoutTests/http/tests/security/powerfulFeatureRestrictions/old-powerful-features-on-insecure-origin.html File third_party/WebKit/LayoutTests/http/tests/security/powerfulFeatureRestrictions/old-powerful-features-on-insecure-origin.html (right): https://codereview.chromium.org/2678433003/diff/100001/third_party/WebKit/LayoutTests/http/tests/security/powerfulFeatureRestrictions/old-powerful-features-on-insecure-origin.html#newcode86 third_party/WebKit/LayoutTests/http/tests/security/powerfulFeatureRestrictions/old-powerful-features-on-insecure-origin.html:86: assert_false(navigator.hasOwnProperty('requestMediaKeySystemAccess'), On 2017/02/07 00:06:11, foolip wrote: > navigator.hasOwnProperty('requestMediaKeySystemAccess') was ...
1 year, 6 months ago (2017-02-07 00:23:56 UTC) #32
foolip
https://codereview.chromium.org/2678433003/diff/100001/third_party/WebKit/LayoutTests/http/tests/security/powerfulFeatureRestrictions/old-powerful-features-on-insecure-origin.html File third_party/WebKit/LayoutTests/http/tests/security/powerfulFeatureRestrictions/old-powerful-features-on-insecure-origin.html (right): https://codereview.chromium.org/2678433003/diff/100001/third_party/WebKit/LayoutTests/http/tests/security/powerfulFeatureRestrictions/old-powerful-features-on-insecure-origin.html#newcode86 third_party/WebKit/LayoutTests/http/tests/security/powerfulFeatureRestrictions/old-powerful-features-on-insecure-origin.html:86: assert_false(navigator.hasOwnProperty('requestMediaKeySystemAccess'), On 2017/02/07 00:23:56, xhwang_slow wrote: > On 2017/02/07 ...
1 year, 6 months ago (2017-02-07 00:27:12 UTC) #33
xhwang
On 2017/02/07 00:27:12, foolip wrote: > https://codereview.chromium.org/2678433003/diff/100001/third_party/WebKit/LayoutTests/http/tests/security/powerfulFeatureRestrictions/old-powerful-features-on-insecure-origin.html > File > third_party/WebKit/LayoutTests/http/tests/security/powerfulFeatureRestrictions/old-powerful-features-on-insecure-origin.html > (right): > > ...
1 year, 6 months ago (2017-02-07 00:34:46 UTC) #34
xhwang
On 2017/02/07 00:34:46, xhwang_slow wrote: > On 2017/02/07 00:27:12, foolip wrote: > > > https://codereview.chromium.org/2678433003/diff/100001/third_party/WebKit/LayoutTests/http/tests/security/powerfulFeatureRestrictions/old-powerful-features-on-insecure-origin.html ...
1 year, 6 months ago (2017-02-07 00:53:23 UTC) #37
foolip
updated tests lgtm
1 year, 6 months ago (2017-02-07 02:27:07 UTC) #38
Torne
https://codereview.chromium.org/2678433003/diff/120001/android_webview/javatests/src/org/chromium/android_webview/test/KeySystemTest.java File android_webview/javatests/src/org/chromium/android_webview/test/KeySystemTest.java (right): https://codereview.chromium.org/2678433003/diff/120001/android_webview/javatests/src/org/chromium/android_webview/test/KeySystemTest.java#newcode54 android_webview/javatests/src/org/chromium/android_webview/test/KeySystemTest.java:54: allowFileAccessFromFileURLs(mAwContents); Several questions here: 1) I don't get why ...
1 year, 6 months ago (2017-02-07 11:24:54 UTC) #41
xhwang
Thanks for the comments. Please take a look again! https://codereview.chromium.org/2678433003/diff/120001/android_webview/javatests/src/org/chromium/android_webview/test/KeySystemTest.java File android_webview/javatests/src/org/chromium/android_webview/test/KeySystemTest.java (right): https://codereview.chromium.org/2678433003/diff/120001/android_webview/javatests/src/org/chromium/android_webview/test/KeySystemTest.java#newcode54 android_webview/javatests/src/org/chromium/android_webview/test/KeySystemTest.java:54: ...
1 year, 6 months ago (2017-02-07 18:43:56 UTC) #44
Torne
On 2017/02/07 18:43:56, xhwang_slow wrote: > Thanks for the comments. Please take a look again! ...
1 year, 6 months ago (2017-02-08 14:56:41 UTC) #47
xhwang
On 2017/02/08 14:56:41, Torne wrote: (removed older comments) > > > 2) Why is using ...
1 year, 6 months ago (2017-02-08 19:17:39 UTC) #48
xhwang
rebase only
1 year, 6 months ago (2017-02-09 06:50:23 UTC) #50
Torne
OK, thanks for checking stuff out here. LGTM as-is in that case :)
1 year, 6 months ago (2017-02-09 13:47:56 UTC) #55
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2678433003/160001
1 year, 6 months ago (2017-02-09 17:40:46 UTC) #58
commit-bot: I haz the power
1 year, 6 months ago (2017-02-09 17:47:55 UTC) #61
Message was sent while issue was closed.
Committed patchset #6 (id:160001) as
https://chromium.googlesource.com/chromium/src/+/4de117544debac896ba8205f8835...

Powered by Google App Engine
This is Rietveld 408576698