EME: Improve promise lifetime

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

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "modules/encryptedmedia/ContentDecryptionModuleResultPromise.h"
 
 #include "bindings/core/v8/ScriptPromise.h"
 #include "bindings/core/v8/ScriptState.h"
 #include "core/dom/DOMException.h"
 #include "core/dom/ExecutionContext.h"
@@ skipping 23 matching lines @@
       // the EME spec.
       return UnknownError;
   }
 
   NOTREACHED();
   return UnknownError;
 }
 
 ContentDecryptionModuleResultPromise::ContentDecryptionModuleResultPromise(
     ScriptState* scriptState)
-    : m_resolver(ScriptPromiseResolver::create(scriptState)) {}
+    : ContextLifecycleObserver(scriptState->getExecutionContext()),
+      m_resolver(ScriptPromiseResolver::create(scriptState)) {}
 
 ContentDecryptionModuleResultPromise::~ContentDecryptionModuleResultPromise() {}
 
 void ContentDecryptionModuleResultPromise::complete() {
   NOTREACHED();
   reject(InvalidStateError, "Unexpected completion.");
 }
 
 void ContentDecryptionModuleResultPromise::completeWithContentDecryptionModule(
     WebContentDecryptionModule* cdm) {
   NOTREACHED();
   reject(InvalidStateError, "Unexpected completion.");
 }
 
 void ContentDecryptionModuleResultPromise::completeWithSession(
     WebContentDecryptionModuleResult::SessionStatus status) {
   NOTREACHED();
   reject(InvalidStateError, "Unexpected completion.");
 }
 
 void ContentDecryptionModuleResultPromise::completeWithError(
     WebContentDecryptionModuleException exceptionCode,
     unsigned long systemCode,
     const WebString& errorMessage) {
+  if (!isValidToFulfillPromise())
+    return;
+
   // Non-zero |systemCode| is appended to the |errorMessage|. If the
   // |errorMessage| is empty, we'll report "Rejected with system code
   // (systemCode)".
   StringBuilder result;
   result.append(errorMessage);
   if (systemCode != 0) {
     if (result.isEmpty())
       result.append("Rejected with system code");
     result.append(" (");
     result.appendNumber(systemCode);
     result.append(')');
   }
   reject(WebCdmExceptionToExceptionCode(exceptionCode), result.toString());
 }
 
 ScriptPromise ContentDecryptionModuleResultPromise::promise() {
   return m_resolver->promise();
 }
 
 void ContentDecryptionModuleResultPromise::reject(ExceptionCode code,
                                                   const String& errorMessage) {
-  // Reject the promise asynchronously. This avoids problems when gc is
-  // destroying objects that result in unfulfilled promises being rejected.
-  // (Resolving promises is still done synchronously as there may be events
-  // already posted that need to happen only after the promise is resolved.)
-  // TODO(jrummell): Make resolving a promise asynchronous as well (including
-  // making sure events still happen after the promise is resolved).
-  if (getExecutionContext()) {
-    getExecutionContext()->postTask(
-        BLINK_FROM_HERE,
-        createSameThreadTask(
-            &ContentDecryptionModuleResultPromise::rejectInternal,
-            wrapPersistent(this), code, errorMessage));
-  }
-}
+  DCHECK(isValidToFulfillPromise());

[yhirano, 2016/10/14 01:05:25]

Is this change (from PS3) correct? You once said
and so I thought this function could be called when isValidToFulfillPromise
doesn't hold.

[jrummell, 2016/10/14 02:13:30]

All the complete...() methods have the check for isValidToFulfillPromise(), so
it should be checked before it gets to reject(). This just verifies it, in case
somebody in the future adds a call without checking it first. (e.g. see
completeWithError just above this). I just noticed that the other complete()
methods in this class have NOTREACHED() since they should never be called, but
will add the check to them as well, just in case one slips through.

 
-void ContentDecryptionModuleResultPromise::rejectInternal(
-    ExceptionCode code,
-    const String& errorMessage) {
   m_resolver->reject(DOMException::create(code, errorMessage));
   m_resolver.clear();
 }
 
-ExecutionContext* ContentDecryptionModuleResultPromise::getExecutionContext()
-    const {
-  return m_resolver->getExecutionContext();
+bool ContentDecryptionModuleResultPromise::isValidToFulfillPromise() {
+  // getExecutionContext() is no longer valid once the context is destroyed.
+  return getExecutionContext() &&
+         !getExecutionContext()->activeDOMObjectsAreStopped();
 }
 
 DEFINE_TRACE(ContentDecryptionModuleResultPromise) {
   visitor->trace(m_resolver);
   ContentDecryptionModuleResult::trace(visitor);
+  ContextLifecycleObserver::trace(visitor);
 }
 
 }  // namespace blink