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 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) {
   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