Change back to destroy the CDM synchronously

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"
+#include "core/dom/ExecutionContextTask.h"
 #include "public/platform/WebString.h"
 #include "wtf/Assertions.h"
 
 namespace blink {
 
 ExceptionCode WebCdmExceptionToExceptionCode(WebContentDecryptionModuleException cdmException)
 {
     switch (cdmException) {
     case WebContentDecryptionModuleExceptionNotSupportedError:
         return NotSupportedError;
@@ skipping 58 matching lines @@
     reject(WebCdmExceptionToExceptionCode(exceptionCode), errorString);
 }
 
 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.

[haraken, 2016/06/07 23:45:52]

When a GC destroys the object, reject() won't be called. So this CL is just
making the promise unrejected forever (if it's not rejected before the object
gets destructed), isn't it?

[xhwang, 2016/06/08 06:52:56]

We do reject unfulfilled promises during destruction (e.g. when GC destroys the
object), which was the cause of the original issue. Previously this would cause
the promise being rejected synchronously in the same callstack where the GC
destroys the object, causing the crash. You can see an example of such callstack
at https://bugs.chromium.org/p/chromium/issues/detail?id=597355#c36

This CL basically force post a task for promise rejection such that we break
that callstack. The promise should still be rejected, but at a later time (after
GC).

+    // (Resolving promises is still done synchronously as there may be events
+    // already posted that need to happen only after the promise is resolved.)

[xhwang, 2016/06/07 23:33:46]

Can we add a TODO to improve the resolving part as well? It seems a good thing
to do to avoid reentering blink in both the resolving and rejecting cases.

[jrummell, 2016/06/08 22:40:37]

Done.

+    getExecutionContext()->postTask(BLINK_FROM_HERE,
+        createSameThreadTask(&ContentDecryptionModuleResultPromise::rejectLater, this, code, errorMessage));
+}
+
+void ContentDecryptionModuleResultPromise::rejectLater(ExceptionCode code, const String& errorMessage)

[xhwang, 2016/06/07 23:33:46]

rejectLater() actually rejects the promise immediately :)

Maybe just something like rejectInternal()?

[jrummell, 2016/06/08 22:40:37]

Done.

+{
     m_resolver->reject(DOMException::create(code, errorMessage));
     m_resolver.clear();
 }
 
 ExecutionContext* ContentDecryptionModuleResultPromise::getExecutionContext() const
 {
     return m_resolver->getExecutionContext();
 }
 
 DEFINE_TRACE(ContentDecryptionModuleResultPromise)
 {
     visitor->trace(m_resolver);
     ContentDecryptionModuleResult::trace(visitor);
 }
 
 } // namespace blink