Closed Bug 1310879 Opened 9 years ago Closed 8 years ago

[EME] Disable EME persistent-license sessions

Categories

(Core :: Audio/Video: Playback, defect)

Product:
Core
Component:
Audio/Video: Playback
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla52
Tracking Flags:
Tracking Status
firefox52 --- fixed

People

(Reporter: cpearce, Assigned: cpearce)

References

Details

Attachments

(3 files, 3 obsolete files)

Patch 1 - Pass MediaKeySystemConfiguration to MediaKeys
5.14 KB, patch
mozbugz
: review+
Details | Diff | Splinter Review
Patch 2 - Check that only supported session types are instantiated
2.87 KB, patch
mozbugz
: review+
Details | Diff | Splinter Review
Patch 3 - Remove EME persistent-license sessions
13.85 KB, patch
mozbugz
: review+
Details | Diff | Splinter Review
We have decided to disable persistent-license session types in our EME implementation. https://lists.w3.org/Archives/Public/public-html-media/2016Oct/0034.html So we should remove ClearKey's support for this session type, disable persistent-license mochitests, and disable the web platform tests that rely on it.
Comment on attachment 8801992 [details] Bug 1310879 - Disable Web Platform Tests for persistent session types. https://reviewboard.mozilla.org/r/86556/#review85580
Attachment #8801992 - Flags: review?(kikuo) → review+
Comment on attachment 8801990 [details] Bug 1310879 - Remove EME persistent-license sessions. https://reviewboard.mozilla.org/r/86552/#review85796
Attachment #8801990 - Flags: review?(kikuo) → review+
Comment on attachment 8801991 [details] Bug 1310879 - Remove persistent license support from gmp-clearkey. https://reviewboard.mozilla.org/r/86554/#review85804 Looks good to me.
Attachment #8801991 - Flags: review?(kikuo) → review+
Pushed by cpearce@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/2d160c261384 Remove EME persistent-license sessions. r=kikuo https://hg.mozilla.org/integration/autoland/rev/736a78915259 Remove persistent license support from gmp-clearkey. r=kikuo https://hg.mozilla.org/integration/autoland/rev/d344ae14671b Disable Web Platform Tests for persistent session types. r=kikuo
Backed out for failing browser-chrome test browser/components/contextualidentity/test/browser/browser_eme.js: https://hg.mozilla.org/integration/autoland/rev/27eef31f9434af59344a313481ada7366c151f59 https://hg.mozilla.org/integration/autoland/rev/9168ac4608163c233af299f6dc1b468c62199f85 https://hg.mozilla.org/integration/autoland/rev/d2d4f0ad9256faae13953dd3b422d9a7fe8e726f Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=d344ae14671b475b5cbc5374c7025a6ca3e6dce0 Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=5308255&repo=autoland 02:04:29 INFO - 64 INFO TEST-START | browser/components/contextualidentity/test/browser/browser_eme.js 02:04:29 INFO - TEST-INFO | started process screentopng 02:04:31 INFO - TEST-INFO | screentopng: exit 0 02:04:31 INFO - 65 INFO checking window state 02:04:31 INFO - 66 INFO Entering test bound setup 02:04:31 INFO - 67 INFO Leaving test bound setup 02:04:31 INFO - 68 INFO Entering test bound test 02:04:31 INFO - 69 INFO Console message: [JavaScript Error: "The character encoding of the HTML document was not declared. The document will render with garbled text in some browser configurations if the document contains characters from outside the US-ASCII range. The character encoding of the page must be declared in the document or in the transfer protocol." {file: "http://example.com/browser/browser/components/contextualidentity/test/browser/empty_file.html" line: 0}] 02:04:31 INFO - 70 INFO Console message: MediaKeySystemAccess::GetKeySystemStatus(org.w3.clearkey, minVer=-1) result=available version='1' msg='' 02:04:31 INFO - 71 INFO Console message: navigator.requestMediaKeySystemAccess promise rejected 0x80530009 'Key system configuration is not supported' 02:04:31 INFO - 72 INFO TEST-UNEXPECTED-FAIL | browser/components/contextualidentity/test/browser/browser_eme.js | Uncaught exception - NotSupportedError: Key system configuration is not supported 02:04:31 INFO - 73 INFO Leaving test bound test
Flags: needinfo?(cpearce)
OK. New plan. We pref of persistent-license in ClearKey instead of removing it, and pref it on again for those tests. Longer term, we'll need a better way to test the clearing of persistent CDM data and CDM isolation.
Assignee: nobody → cpearce
Status: NEW → ASSIGNED
MozReview-Commit-ID: 6XkToIXzZL8
Attachment #8805361 - Flags: review?(gsquelart)
This removes the ability for ClearKey to instantiate persistent-license sessions using the EME APIs. MozReview-Commit-ID: FXj5YORxpas
Attachment #8805362 - Flags: review?(gsquelart)
Comment on attachment 8805358 [details] [diff] [review] Patch 1 - Pass MediaKeySystemConfiguration to MediaKeys Sorry, I can't request review using Mozreview, as you can't start a new request once you've been backed out.
Attachment #8805358 - Attachment description: Pass MediaKeySystemConfiguration to MediaKeys → Patch 1 - Pass MediaKeySystemConfiguration to MediaKeys
Flags: needinfo?(cpearce)
Attachment #8805358 - Flags: review?(gsquelart)
Attachment #8801990 - Attachment is obsolete: true
Attachment #8801991 - Attachment is obsolete: true
Attachment #8801992 - Attachment is obsolete: true
Attachment #8805358 - Flags: review?(gsquelart) → review+
Attachment #8805361 - Flags: review?(gsquelart) → review+
Comment on attachment 8805362 [details] [diff] [review] Patch 3 - Remove EME persistent-license sessions Review of attachment 8805362 [details] [diff] [review]: ----------------------------------------------------------------- r+ with suggestion and question: ::: dom/media/eme/MediaKeySystemAccess.cpp @@ +431,5 @@ > clearkey.mInitDataTypes.AppendElement(NS_LITERAL_STRING("webm")); > clearkey.mPersistentState = KeySystemFeatureSupport::Requestable; > clearkey.mDistinctiveIdentifier = KeySystemFeatureSupport::Prohibited; > clearkey.mSessionTypes.AppendElement(MediaKeySessionType::Temporary); > + if (Preferences::GetBool("media.clearkey.persistent-license.enabled", false)) { Please consider using MediaPrefs for this pref, unless you think it's going to disappear soon. Also, I see you didn't add this pref to all.js, is that intended? (I don't know if there are guidelines about that)
Attachment #8805362 - Flags: review?(gsquelart) → review+
(In reply to Gerald Squelart [:gerald] from comment #17) > Also, I see you didn't add this pref to all.js, is that intended? (I don't > know if there are guidelines about that) The convention I was aware of is that pref's not in all.js are "hidden" prefs, in that they're less likely to be discovered by users who go poking around in about:config. So if you don't want people toggling a pref (for example like in this case where the pref is only present to allow testing of some other feature) a hidden pref is a good way to achieve that.
That makes sense, thank you for the explanation.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: