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)),
+      m_contextDestroyed(false) {}
 
 ContentDecryptionModuleResultPromise::~ContentDecryptionModuleResultPromise() {}
 
 void ContentDecryptionModuleResultPromise::complete() {
   NOTREACHED();
   reject(InvalidStateError, "Unexpected completion.");
 }
 
 void ContentDecryptionModuleResultPromise::completeWithContentDecryptionModule(
     WebContentDecryptionModule* cdm) {
@@ skipping 19 matching lines @@
   if (systemCode != 0) {
     if (result.isEmpty())
       result.append("Rejected with system code");
     result.append(" (");
     result.appendNumber(systemCode);
     result.append(')');
   }
   reject(WebCdmExceptionToExceptionCode(exceptionCode), result.toString());
 }
 
+void ContentDecryptionModuleResultPromise::contextDestroyed() {
+  m_contextDestroyed = true;
+}
+
 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));
-  }
-}
+  if (!isValidToFulfillPromise())
+    return;
 
-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 contextDestroyed() is called.
+  if (m_contextDestroyed ||

[haraken, 2016/10/11 03:51:24]

You can replace m_contextDestroyed with getExecutionContext().

[jrummell, 2016/10/11 21:38:27]

Done.

+      getExecutionContext()->activeDOMObjectsAreStopped()) {
+    m_resolver.clear();

[haraken, 2016/10/11 03:51:24]

Why do you need to clear the promise when isValidToFulfillPromise() gets called?
Alternately, shouldn't we clear the promise when contextDestroyed() gets called?

[jrummell, 2016/10/11 21:38:27]

Done. m_resolver was cleared to release the Member<> as soon as we were done
with it (and to ensure we don't call resolve/reject a second time). Much better
to do it in contextDestroyed().

+    return false;
+  }
+
+  return true;
 }
 
 DEFINE_TRACE(ContentDecryptionModuleResultPromise) {
   visitor->trace(m_resolver);
   ContentDecryptionModuleResult::trace(visitor);
+  ContextLifecycleObserver::trace(visitor);
 }
 
 }  // namespace blink