EME: Make sure sessions are closed before they are destroyed

media/blink/webcontentdecryptionmodulesession_impl.cc

Index: media/blink/webcontentdecryptionmodulesession_impl.cc
diff --git a/media/blink/webcontentdecryptionmodulesession_impl.cc b/media/blink/webcontentdecryptionmodulesession_impl.cc
index ab138e7447d95a9786b94b88895939907fe4ab60..63bd46961538c5c1317e3786e48dee67a5b0f730 100644
--- a/media/blink/webcontentdecryptionmodulesession_impl.cc
+++ b/media/blink/webcontentdecryptionmodulesession_impl.cc
@@ -7,6 +7,7 @@
 #include "base/bind.h"
 #include "base/callback_helpers.h"
 #include "base/logging.h"
+#include "base/memory/ptr_util.h"
 #include "base/numerics/safe_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
@@ -224,16 +225,50 @@ static bool SanitizeResponse(const std::string& key_system,
   return true;
 }
 
+// If we need to call close() on destruction, we need a promise that won't

[ddorwin, 2016/11/07 19:45:30]

"call close()" (and similar wording in the description) are misleading IMO. This
is most likely to be interpreted as the MKS.close() JS API, but it is really
asking the implementation to close the session.

[jrummell, 2016/11/07 22:51:40]

Done.

+// do anything.
+class IgnoreResponsePromise : public SimpleCdmPromise {
+ public:
+  IgnoreResponsePromise() {}
+  ~IgnoreResponsePromise() override {}
+
+  // SimpleCdmPromise implementation.
+  void resolve() final { MarkPromiseSettled(); }
+  void reject(CdmPromise::Exception exception_code,
+              uint32_t system_code,
+              const std::string& error_message) final {
+    MarkPromiseSettled();
+  }
+};
+
 WebContentDecryptionModuleSessionImpl::WebContentDecryptionModuleSessionImpl(
     const scoped_refptr<CdmSessionAdapter>& adapter)
-    : adapter_(adapter), is_closed_(false), weak_ptr_factory_(this) {
-}
+    : adapter_(adapter),
+      is_closed_(false),
+      has_close_been_called_(false),
+      weak_ptr_factory_(this) {}
 
 WebContentDecryptionModuleSessionImpl::
     ~WebContentDecryptionModuleSessionImpl() {
   DCHECK(thread_checker_.CalledOnValidThread());
-  if (!session_id_.empty())
+
+  if (!session_id_.empty()) {
     adapter_->UnregisterSession(session_id_);
+
+    // From http://w3c.github.io/encrypted-media/#mediakeysession-interface

[ddorwin, 2016/11/07 19:45:30]

This text was just updated. (MediaKeySession destroyed algorithm has been
removed and collapsed into the prose.) You'll want to update the description
too.

[jrummell, 2016/11/07 22:51:40]

Done.

+    // "If a MediaKeySession object becomes inaccessible to the page and is not
+    // closed, the User Agent must run the MediaKeySession destroyed algorithm
+    // before User Agent state associated with the session is deleted."

[ddorwin, 2016/11/07 19:45:31]

We don't actually do this last part and can't because the API is asynchronous.
We'd need a "prepare to delete" method.

That's fine, though, since this is the observable behavior, and there's not
actually anything observable. There is a bit of an expectation that the CDM will
destroy the keys in memory, but that is not observable.

[jrummell, 2016/11/07 22:51:40]

Acknowledged.

+    //
+    // So if the session is not closed and CloseSession() has not yet been
+    // called, call CloseSession() now. Since this object is being destroyed,
+    // there is no need for the promise to do anything as this session will
+    // be gone.
+    if (!is_closed_ && !has_close_been_called_) {
+      adapter_->CloseSession(session_id_,
+                             base::MakeUnique<IgnoreResponsePromise>());
+    }
+  }
 }
 
 void WebContentDecryptionModuleSessionImpl::setClientInterface(Client* client) {
@@ -394,6 +429,8 @@ void WebContentDecryptionModuleSessionImpl::close(
     blink::WebContentDecryptionModuleResult result) {
   DCHECK(!session_id_.empty());
   DCHECK(thread_checker_.CalledOnValidThread());
+
+  has_close_been_called_ = true;
   adapter_->CloseSession(
       session_id_,
       std::unique_ptr<SimpleCdmPromise>(new CdmResultPromise<>(