Change back to destroy the CDM synchronously

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 811874b0ef452d084fb879c6deb708d4fcb3a93d..312da4ad712323d052ec2076f9d0609e1a12ff50 100644
--- a/third_party/WebKit/Source/modules/encryptedmedia/ContentDecryptionModuleResultPromise.cpp
+++ b/third_party/WebKit/Source/modules/encryptedmedia/ContentDecryptionModuleResultPromise.cpp
@@ -7,6 +7,8 @@
 #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"
 
@@ -85,6 +87,16 @@ ScriptPromise ContentDecryptionModuleResultPromise::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();
 }