Closed Bug 1130932 Opened 5 years ago Closed 5 years ago

[EME] Be more lenient in handling mIsOpen in GMPDecryptorParent::RecvXXX

Categories

(Core :: Audio/Video, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla38
Tracking Status
firefox37 --- fixed
firefox38 --- fixed

People

(Reporter: jwwang, Assigned: jwwang)

References

Details

Attachments

(2 files, 1 obsolete file)

When running test_eme_canvas_blocked.html, chances are the test case finish and call GMPDecryptorParent::Close() before GMPDecryptorParent::RecvKeyStatusChanged() arrives. Since this is a valid case, we should return true in GMPDecryptorParent::RecvXXX on such conditions to avoid IPDL errors.
Don't return false in GMPDecryptorParent::RecvXXX functions.
Assignee: nobody → jwwang
Status: NEW → ASSIGNED
Attachment #8561213 - Flags: review?(edwin)
Blocks: 1129720
Comment on attachment 8561213 [details] [diff] [review]
1130932_allow_recv_calls_after_close-v1.patch

Review of attachment 8561213 [details] [diff] [review]:
-----------------------------------------------------------------

I don't think we should treat this as valid. The CDM should not emit any messages about a session after it has sent SessionClosed. (yes, this needs to be fixed in our ClearKey CDM; on my todo list).
Attachment #8561213 - Flags: review?(edwin) → review-
Comment on attachment 8561213 [details] [diff] [review]
1130932_allow_recv_calls_after_close-v1.patch

Review of attachment 8561213 [details] [diff] [review]:
-----------------------------------------------------------------

Actually, looking again at the parent-side code, looks like we close immediately after sending DecryptionComplete. Lame.

r+, but reduce to only the messages we're likely to receive at shutdown.
Attachment #8561213 - Flags: review- → review+
It is kinda hard to identify the messages we could receive during shutdown since you can close the page to trigger GMPDecryptorParent::Close() at any time.

For now I will leave GMPDecryptorParent::RecvKeyStatusChanged() only which should fix most problems we are facing in the test cases. In the long term, I think GMPDecryptorParent should wait for some response from the child after sending SendDecryptingComplete() to ensure shutdown sequence is done in a controlled way.
address comment 3.
Attachment #8561213 - Attachment is obsolete: true
Attachment #8561945 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/3f2ea7b6061f
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
Attached patch Beta patchSplinter Review
Patch for beta branch as part of EME platform uplift.
Comment on attachment 8572364 [details] [diff] [review]
Beta patch

Requesting retroactive approval for Beta landing as part of EME platform uplift.
Attachment #8572364 - Flags: approval-mozilla-beta?
Comment on attachment 8572364 [details] [diff] [review]
Beta patch

Approved for Beta as part of EME platform uplift.
Attachment #8572364 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.