1222875 - [EME] Connect MediaKeySession.expiration to GMP

Status: RESOLVED FIXED

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

Description

[Chris Pearce [:cpearce (Not reading bugmail)]]

I never connected GMPDecryptorCallback::ExpirationChange to MediaKeySession. It's pretty easy to hookup.

Comment 1

[Chris Pearce [:cpearce (Not reading bugmail)]]

Attachment: 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.

Comment 2

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

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?

Comment 3

[Chris Pearce [:cpearce (Not reading bugmail)]]

(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!

Comment 4

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

(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

[Pulsebot]

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

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/1e8f29d8b466