EME: Make sure sessions are closed before they are destroyed

media/blink/webcontentdecryptionmodulesession_impl.cc

 // Copyright 2013 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 "webcontentdecryptionmodulesession_impl.h"
 
 #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"
 #include "media/base/cdm_key_information.h"
 #include "media/base/cdm_promise.h"
 #include "media/base/key_system_names.h"
 #include "media/base/key_systems.h"
 #include "media/base/limits.h"
 #include "media/base/media_keys.h"
 #include "media/blink/cdm_result_promise.h"
@@ skipping 197 matching lines @@
     std::string sanitized_data = GenerateJWKSet(keys, session_type);
     sanitized_response->assign(sanitized_data.begin(), sanitized_data.end());
     return true;
   }
 
   // TODO(jrummell): Verify responses for Widevine.
   sanitized_response->assign(response, response + response_length);
   return true;
 }
 
+// If we need to close the session while destroying this object, we need a
+// dummy promise that won't call back into this object (or try to pass
+// something back to blink).
+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
+    // "If a MediaKeySession object becomes inaccessible to the page and is
+    // not closed, the User Agent must use the CDM to close the session
+    // before User Agent state associated with the session is deleted."

[ddorwin, 2016/11/08 00:41:13]

You might note that this will happen after, but the visible result is the same.

I'm not quite sure why that is how it's spec'd.

[ddorwin, 2016/11/08 01:05:20]

Filed https://github.com/w3c/encrypted-media/issues/358 to remove this text.

[jrummell, 2016/11/10 00:04:57]

Updated comment copied from spec.

+    //
+    // 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) {
   client_ = client;
 }
 
 blink::WebString WebContentDecryptionModuleSessionImpl::sessionId() const {
   return blink::WebString::fromUTF8(session_id_);
 }
 
@@ skipping 140 matching lines @@
   adapter_->UpdateSession(
       session_id_, sanitized_response,
       std::unique_ptr<SimpleCdmPromise>(new CdmResultPromise<>(
           result, adapter_->GetKeySystemUMAPrefix() + kUpdateSessionUMAName)));
 }
 
 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<>(
           result, adapter_->GetKeySystemUMAPrefix() + kCloseSessionUMAName)));
 }
 
 void WebContentDecryptionModuleSessionImpl::remove(
     blink::WebContentDecryptionModuleResult result) {
   DCHECK(!session_id_.empty());
   DCHECK(thread_checker_.CalledOnValidThread());
@@ skipping 57 matching lines @@
 
   DCHECK(session_id_.empty()) << "Session ID may not be changed once set.";
   session_id_ = session_id;
   *status =
       adapter_->RegisterSession(session_id_, weak_ptr_factory_.GetWeakPtr())
           ? SessionInitStatus::NEW_SESSION
           : SessionInitStatus::SESSION_ALREADY_EXISTS;
 }
 
 }  // namespace media