EME: Improve promise lifetime

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

Index: third_party/WebKit/Source/modules/encryptedmedia/ContentDecryptionModuleResultPromise.cpp
diff --git a/third_party/WebKit/Source/modules/encryptedmedia/ContentDecryptionModuleResultPromise.cpp b/third_party/WebKit/Source/modules/encryptedmedia/ContentDecryptionModuleResultPromise.cpp
index 58b35116e66ac7d09d376be848413d8994f3eb3a..27e37c5077208e9ee812c71eee3d06d8fa3c5172 100644
--- a/third_party/WebKit/Source/modules/encryptedmedia/ContentDecryptionModuleResultPromise.cpp
+++ b/third_party/WebKit/Source/modules/encryptedmedia/ContentDecryptionModuleResultPromise.cpp
@@ -41,7 +41,9 @@ ExceptionCode WebCdmExceptionToExceptionCode(
 
 ContentDecryptionModuleResultPromise::ContentDecryptionModuleResultPromise(
     ScriptState* scriptState)
-    : m_resolver(ScriptPromiseResolver::create(scriptState)) {}
+    : ContextLifecycleObserver(scriptState->getExecutionContext()),
+      m_resolver(ScriptPromiseResolver::create(scriptState)),
+      m_contextDestroyed(false) {}
 
 ContentDecryptionModuleResultPromise::~ContentDecryptionModuleResultPromise() {}
 
@@ -81,42 +83,38 @@ void ContentDecryptionModuleResultPromise::completeWithError(
   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