[EME] test_eme_persistent_sessions does not handle multi-stream cases

RESOLVED FIXED in Firefox 40

Status

()

Core
Audio/Video
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: gerald, Assigned: gerald)

Tracking

(Blocks: 2 bugs)

unspecified
mozilla40
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox40 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

3 years ago
test_eme_persistent_sessions.html was only designed to handle one session at a time.
Some failures (e.g. bug 1148844) may be related to this.

Possible solutions:
1. Only use test cases with 1 session.
2. Modify code to handle multiple sessions.
Can you elaborate the problem? I think we should support multiple sessions in the real use case and our test case should also reflect the truth.
(Assignee)

Comment 2

3 years ago
The test was originally written at a time when there were only single-stream test cases, and it has some top-level variables meant to be used by 'the' session handler.
So when two sessions are created (as is the case in multi-stream videos since bug 1144409) the same variables are used by both session handlers, with some unexpected results that do not necessarily point to real code issues.

I agree that handling multiple sessions is expected, and will try to patch the test that way.
But in the worst case (too hard?), I'll fall back to option 1 for now and create a separate bug to later handle multi-stream tests.
(Assignee)

Updated

3 years ago
Blocks: 1152170
(Assignee)

Comment 3

3 years ago
eme.js:SetupEME() itself does not handle multiple sessions well, so this bug here will just disable multi-session test cases for now.
After bug 1142379 lands with real multi-stream tests, bug 1152170 will add full support for multi-stream tests in test_eme_persistent_sessions.
(Assignee)

Comment 4

3 years ago
Created attachment 8589526 [details] [diff] [review]
1152153-eme-persistent-test-single-sessions-only.patch

Only use test cases that generate 1 session.
Attachment #8589526 - Flags: review?(edwin)
(Assignee)

Comment 8

3 years ago
Trying with patch from bug 1154133:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=19326a3604ff
Depends on: 1154133
No longer depends on: 1152151
(Assignee)

Comment 9

3 years ago
Created attachment 8592554 [details] [diff] [review]
1152153-eme-persistent-test-single-sessions-only.patch

Rebase, no actual changes, carrying r+.
Attachment #8589526 - Attachment is obsolete: true
Attachment #8592554 - Flags: review+
(Assignee)

Updated

3 years ago
No longer depends on: 1154133
https://hg.mozilla.org/mozilla-central/rev/865713e00111
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox40: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
You need to log in before you can comment on or make changes to this bug.