[EME] Connect MediaKeySession.expiration to GMP

RESOLVED FIXED in Firefox 45

Status

()

Core
Audio/Video: Playback
P2
normal
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: cpearce, Assigned: cpearce)

Tracking

(Blocks: 1 bug)

unspecified
mozilla45
Points:
---

Firefox Tracking Flags

(firefox45 fixed)

Details

Attachments

(1 attachment)

I never connected GMPDecryptorCallback::ExpirationChange to MediaKeySession. It's pretty easy to hookup.
Created attachment 8684744 [details] [diff] [review]
Patch: Connect GMPDecryptorCallback::ExpirationChange to MediaKeySession.expiration

Calls in the GMP process to GMPDecryptorCallback::ExpirationChange() are already sent across to the parent process, we just need to finish the job and pass them onto affecting MediaKeySession.expiration.
Attachment #8684744 - Flags: review?(gsquelart)
Comment on attachment 8684744 [details] [diff] [review]
Patch: Connect GMPDecryptorCallback::ExpirationChange to MediaKeySession.expiration

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

r+, as it does the job of connecting GMPDecryptorCallback::ExpirationChange to MediaKeySession.expiration.

Nit and question:

::: dom/media/eme/CDMProxy.cpp
@@ +571,5 @@
>  {
>    MOZ_ASSERT(NS_IsMainThread());
> +  RefPtr<dom::MediaKeySession> session(mKeys->GetSession(aSessionId));
> +  if (session) {
> +    session->SetExpiry((double)aExpiryTime);

Please use C++ static_cast or converting constructor.

::: dom/media/eme/MediaKeySession.h
@@ +115,5 @@
>    const uint32_t mToken;
>    bool mIsClosed;
>    bool mUninitialized;
>    RefPtr<MediaKeyStatusMap> mKeyStatusMap;
> +  double mExpiry;

mExpiry is only set but not used (unless I'm missing something from the bigger picture?)
Will there be a future bug to actually handle it?
Attachment #8684744 - Flags: review?(gsquelart) → review+
(In reply to Gerald Squelart [:gerald] from comment #2)
> ::: dom/media/eme/MediaKeySession.h
> @@ +115,5 @@
> >    const uint32_t mToken;
> >    bool mIsClosed;
> >    bool mUninitialized;
> >    RefPtr<MediaKeyStatusMap> mKeyStatusMap;
> > +  double mExpiry;
> 
> mExpiry is only set but not used (unless I'm missing something from the
> bigger picture?)
> Will there be a future bug to actually handle it?

It should be returned in MediaKeySession::Expiration(). d'oh!
(In reply to Chris Pearce (:cpearce) from comment #3)
> (In reply to Gerald Squelart [:gerald] from comment #2)
> > mExpiry is only set but not used (unless I'm missing something from the
> > bigger picture?)
> > Will there be a future bug to actually handle it?
> 
> It should be returned in MediaKeySession::Expiration(). d'oh!

So I was wrong in my assessment that the job was done. "d'oh!" as well.

While you're at it, you probably should name the member variable 'mExpiration' to be consistent with 'Expiration()' and 'ExpirationChange'.

Comment 5

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/1e8f29d8b466

Comment 6

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/1e8f29d8b466
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox45: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
You need to log in before you can comment on or make changes to this bug.