EME: Improve promise lifetime

third_party/WebKit/Source/modules/encryptedmedia/MediaKeySession.cpp

 /*
  * Copyright (C) 2013 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 20 matching lines @@
 #include "bindings/core/v8/ScriptState.h"
 #include "core/dom/DOMArrayBuffer.h"
 #include "core/dom/DOMException.h"
 #include "core/dom/ExceptionCode.h"
 #include "core/events/Event.h"
 #include "core/events/GenericEventQueue.h"
 #include "modules/encryptedmedia/ContentDecryptionModuleResultPromise.h"
 #include "modules/encryptedmedia/EncryptedMediaUtils.h"
 #include "modules/encryptedmedia/MediaKeyMessageEvent.h"
 #include "modules/encryptedmedia/MediaKeys.h"
-#include "modules/encryptedmedia/SimpleContentDecryptionModuleResultPromise.h"
 #include "platform/ContentDecryptionModuleResult.h"
 #include "platform/ContentType.h"
 #include "platform/Timer.h"
 #include "public/platform/WebContentDecryptionModule.h"
 #include "public/platform/WebContentDecryptionModuleException.h"
 #include "public/platform/WebContentDecryptionModuleSession.h"
 #include "public/platform/WebEncryptedMediaKeyInformation.h"
 #include "public/platform/WebString.h"
 #include "public/platform/WebURL.h"
 #include "wtf/ASCIICType.h"
@@ skipping 172 matching lines @@
 class NewSessionResultPromise : public ContentDecryptionModuleResultPromise {
  public:
   NewSessionResultPromise(ScriptState* scriptState, MediaKeySession* session)
       : ContentDecryptionModuleResultPromise(scriptState), m_session(session) {}
 
   ~NewSessionResultPromise() override {}
 
   // ContentDecryptionModuleResult implementation.
   void completeWithSession(
       WebContentDecryptionModuleResult::SessionStatus status) override {
+    if (!isValidToFulfillPromise())
+      return;
+
     if (status != WebContentDecryptionModuleResult::NewSession) {
       NOTREACHED();
       reject(InvalidStateError, "Unexpected completion.");
     }
 
     m_session->finishGenerateRequest();
     resolve();
   }
 
   DEFINE_INLINE_TRACE() {
@@ skipping 13 matching lines @@
 class LoadSessionResultPromise : public ContentDecryptionModuleResultPromise {
  public:
   LoadSessionResultPromise(ScriptState* scriptState, MediaKeySession* session)
       : ContentDecryptionModuleResultPromise(scriptState), m_session(session) {}
 
   ~LoadSessionResultPromise() override {}
 
   // ContentDecryptionModuleResult implementation.
   void completeWithSession(
       WebContentDecryptionModuleResult::SessionStatus status) override {
+    if (!isValidToFulfillPromise())
+      return;

[xhwang, 2016/10/13 19:29:58]

We are checking this in both the sub class and base class. Shall we just check
in one place (e.g. in the base class) to avoid confusion? Or if all subclasses
should check, can we DCHECK in the base class?

[jrummell, 2016/10/14 00:03:31]

We need to check before we attempt to create any object used to resolve/reject
the promise, so it's best to do it here (no need to do the work if it's not
going to be used).

+
     switch (status) {
       case WebContentDecryptionModuleResult::NewSession:
         m_session->finishLoad();
         resolve(true);
         return;
 
       case WebContentDecryptionModuleResult::SessionNotFound:
         resolve(false);
         return;
 
       case WebContentDecryptionModuleResult::SessionAlreadyExists:
         NOTREACHED();
         reject(InvalidStateError, "Unexpected completion.");
         return;
     }
 
     NOTREACHED();
   }
 
   DEFINE_INLINE_TRACE() {
     visitor->trace(m_session);
     ContentDecryptionModuleResultPromise::trace(visitor);
   }
 
  private:
   Member<MediaKeySession> m_session;
 };
 
+// This class wraps the promise resolver used by update/close/remove. The
+// implementation of complete() will resolve the promise with void. All other
+// complete() methods are not expected to be called (and will reject the
+// promise).
+class SimpleResultPromise : public ContentDecryptionModuleResultPromise {
+ public:
+  SimpleResultPromise(ScriptState* scriptState, MediaKeySession* session)
+      : ContentDecryptionModuleResultPromise(scriptState), m_session(session) {}
+
+  ~SimpleResultPromise() override {}
+
+  // ContentDecryptionModuleResultPromise implementation.
+  void complete() override { resolve(); }

[xhwang, 2016/10/13 19:29:58]

Just FWIW, we are not checking isValidToFulfillPromise() here, but as commented
above, we don't need to.

[jrummell, 2016/10/14 00:03:31]

Added.

+
+  DEFINE_INLINE_TRACE() {
+    visitor->trace(m_session);
+    ContentDecryptionModuleResultPromise::trace(visitor);
+  }
+
+ private:
+  Member<MediaKeySession> m_session;

[xhwang, 2016/10/13 19:29:58]

Add a comment about why we need this here.

[jrummell, 2016/10/14 00:03:31]

Done.

+};
+
 MediaKeySession* MediaKeySession::create(
     ScriptState* scriptState,
     MediaKeys* mediaKeys,
     WebEncryptedMediaSessionType sessionType) {
   MediaKeySession* session =
       new MediaKeySession(scriptState, mediaKeys, sessionType);
   session->suspendIfNeeded();
   return session;
 }
 
@@ skipping 234 matching lines @@
     return ScriptPromise::rejectWithDOMException(
         scriptState, DOMException::create(InvalidAccessError,
                                           "The response parameter is empty."));
   }
 
   // 3. Let response copy be a copy of the contents of the response parameter.
   DOMArrayBuffer* responseCopy =
       DOMArrayBuffer::create(response.data(), response.byteLength());
 
   // 4. Let promise be a new promise.
-  SimpleContentDecryptionModuleResultPromise* result =
-      new SimpleContentDecryptionModuleResultPromise(scriptState);
+  SimpleResultPromise* result = new SimpleResultPromise(scriptState, this);
   ScriptPromise promise = result->promise();
 
   // 5. Run the following steps asynchronously (documented in
   //    actionTimerFired())
   m_pendingActions.append(
       PendingAction::CreatePendingUpdate(result, responseCopy));
   if (!m_actionTimer.isActive())
     m_actionTimer.startOneShot(0, BLINK_FROM_HERE);
 
   // 6. Return promise.
@@ skipping 12 matching lines @@
   //    with a new DOMException whose name is "InvalidStateError".
   if (!m_isCallable)
     return CreateRejectedPromiseNotCallable(scriptState);
 
   // 2. If the Session Close algorithm has been run on this object,
   //    return a resolved promise.
   if (m_isClosed)
     return ScriptPromise::cast(scriptState, ScriptValue());
 
   // 3. Let promise be a new promise.
-  SimpleContentDecryptionModuleResultPromise* result =
-      new SimpleContentDecryptionModuleResultPromise(scriptState);
+  SimpleResultPromise* result = new SimpleResultPromise(scriptState, this);
   ScriptPromise promise = result->promise();
 
   // 4. Run the following steps asynchronously (documented in
   //    actionTimerFired()).
   m_pendingActions.append(PendingAction::CreatePendingClose(result));
   if (!m_actionTimer.isActive())
     m_actionTimer.startOneShot(0, BLINK_FROM_HERE);
 
   // 5. Return promise.
   return promise;
@@ skipping 25 matching lines @@
   // 3. If the Session Close algorithm has been run on this object, return a
   //    promise rejected with a new DOMException whose name is
   //    "InvalidStateError".
   if (m_isClosed) {
     return ScriptPromise::rejectWithDOMException(
         scriptState, DOMException::create(InvalidStateError,
                                           "The session is already closed."));
   }
 
   // 4. Let promise be a new promise.
-  SimpleContentDecryptionModuleResultPromise* result =
-      new SimpleContentDecryptionModuleResultPromise(scriptState);
+  SimpleResultPromise* result = new SimpleResultPromise(scriptState, this);
   ScriptPromise promise = result->promise();
 
   // 5. Run the following steps asynchronously (documented in
   //    actionTimerFired()).
   m_pendingActions.append(PendingAction::CreatePendingRemove(result));
   if (!m_actionTimer.isActive())
     m_actionTimer.startOneShot(0, BLINK_FROM_HERE);
 
   // 6. Return promise.
   return promise;
@@ skipping 315 matching lines @@
   visitor->trace(m_asyncEventQueue);
   visitor->trace(m_pendingActions);
   visitor->trace(m_mediaKeys);
   visitor->trace(m_keyStatusesMap);
   visitor->trace(m_closedPromise);
   EventTargetWithInlineData::trace(visitor);
   ActiveDOMObject::trace(visitor);
 }
 
 }  // namespace blink