Move SessionIdAdapter out of WebContentDecryptionModuleImpl

content/renderer/media/webcontentdecryptionmodule_impl.cc

Index: content/renderer/media/webcontentdecryptionmodule_impl.cc
diff --git a/content/renderer/media/webcontentdecryptionmodule_impl.cc b/content/renderer/media/webcontentdecryptionmodule_impl.cc
index f98bf14a4ca688c44f9c3471ddd59b6d1422f246..2f5294d4817c4ed1ff031068b4a9b91c08ea6764 100644
--- a/content/renderer/media/webcontentdecryptionmodule_impl.cc
+++ b/content/renderer/media/webcontentdecryptionmodule_impl.cc
@@ -8,161 +8,14 @@
 #include <vector>
 
 #include "base/basictypes.h"
-#include "base/bind.h"
-#include "base/callback_helpers.h"
 #include "base/logging.h"
-#include "base/memory/weak_ptr.h"
 #include "base/strings/string_util.h"
-#include "content/renderer/media/crypto/content_decryption_module_factory.h"
+#include "content/renderer/media/cdm_session_id_adapter.h"

[ddorwin, 2014/02/18 20:59:42]

We need a new name for this object. It's no longer just adapting - it owns the
CDM, and part of that is directing events at the correct Blink objects.

I can't come up with a good name, though, which is always a bad sign. Part of
the problem is that this is another layer around MediaKeys.h (should be CDM.h).
However, we don't want to force this type of logic into each implementation of
that interface.

Some ideas:
CdmProxy - ambiguous; is it really a proxy?
CdmHost - not really hosting
CdmWrapper - already used

[jrummell, 2014/02/19 00:20:29]

How about CdmSessionAdapter?

 #include "content/renderer/media/webcontentdecryptionmodulesession_impl.h"
 #include "media/base/media_keys.h"
-#include "url/gurl.h"
 
 namespace content {
 
-// Forwards the session ID-based callbacks of the MediaKeys interface to the
-// appropriate session object.
-class SessionIdAdapter {
- public:
-  SessionIdAdapter();
-  ~SessionIdAdapter();
-
-  // On success, creates a MediaKeys, returns it in |media_keys|, returns true.
-  bool Initialize(const std::string& key_system,
-                  scoped_ptr<media::MediaKeys>* media_keys);
-
-  // Generates a unique internal session id.
-  uint32 GenerateSessionId();
-
-  // Adds a session to the internal map. Does not take ownership of the session.
-  void AddSession(uint32 session_id,
-                  WebContentDecryptionModuleSessionImpl* session);
-
-  // Removes a session from the internal map.
-  void RemoveSession(uint32 session_id);
-
- private:
-  typedef std::map<uint32, WebContentDecryptionModuleSessionImpl*> SessionMap;
-
-  // Callbacks for firing session events.
-  void OnSessionCreated(uint32 session_id, const std::string& web_session_id);
-  void OnSessionMessage(uint32 session_id,
-                        const std::vector<uint8>& message,
-                        const std::string& destination_url);
-  void OnSessionReady(uint32 session_id);
-  void OnSessionClosed(uint32 session_id);
-  void OnSessionError(uint32 session_id,
-                      media::MediaKeys::KeyError error_code,
-                      int system_code);
-
-  // Helper function of the callbacks.
-  WebContentDecryptionModuleSessionImpl* GetSession(uint32 session_id);
-
-  base::WeakPtrFactory<SessionIdAdapter> weak_ptr_factory_;
-
-  SessionMap sessions_;
-
-  // Session ID should be unique per renderer process for debugging purposes.
-  static uint32 next_session_id_;
-
-  DISALLOW_COPY_AND_ASSIGN(SessionIdAdapter);
-};
-
-const uint32 kStartingSessionId = 1;
-uint32 SessionIdAdapter::next_session_id_ = kStartingSessionId;
-COMPILE_ASSERT(kStartingSessionId > media::MediaKeys::kInvalidSessionId,
-               invalid_starting_value);
-
-SessionIdAdapter::SessionIdAdapter()
-    : weak_ptr_factory_(this) {
-}
-
-SessionIdAdapter::~SessionIdAdapter() {
-}
-
-bool SessionIdAdapter::Initialize(const std::string& key_system,
-                                  scoped_ptr<media::MediaKeys>* media_keys) {
-  DCHECK(media_keys);
-  DCHECK(!*media_keys);
-
-  base::WeakPtr<SessionIdAdapter> weak_this = weak_ptr_factory_.GetWeakPtr();
-  scoped_ptr<media::MediaKeys> created_media_keys =
-      ContentDecryptionModuleFactory::Create(
-          // TODO(ddorwin): Address lower in the stack: http://crbug.com/252065
-          "webkit-" + key_system,
-#if defined(ENABLE_PEPPER_CDMS)
-          // TODO(ddorwin): Support Pepper-based CDMs: http://crbug.com/250049
-          NULL,
-          NULL,
-          base::Closure(),
-#elif defined(OS_ANDROID)
-          // TODO(xhwang): Support Android.
-          NULL,
-          0,
-          // TODO(ddorwin): Get the URL for the frame containing the MediaKeys.
-          GURL(),
-#endif  // defined(ENABLE_PEPPER_CDMS)
-          base::Bind(&SessionIdAdapter::OnSessionCreated, weak_this),
-          base::Bind(&SessionIdAdapter::OnSessionMessage, weak_this),
-          base::Bind(&SessionIdAdapter::OnSessionReady, weak_this),
-          base::Bind(&SessionIdAdapter::OnSessionClosed, weak_this),
-          base::Bind(&SessionIdAdapter::OnSessionError, weak_this));
-  if (!created_media_keys)
-    return false;
-
-  *media_keys = created_media_keys.Pass();
-  return true;
-}
-
-uint32 SessionIdAdapter::GenerateSessionId() {
-  return next_session_id_++;
-}
-
-void SessionIdAdapter::AddSession(
-    uint32 session_id,
-    WebContentDecryptionModuleSessionImpl* session) {
-  DCHECK(sessions_.find(session_id) == sessions_.end());
-  sessions_[session_id] = session;
-}
-
-void SessionIdAdapter::RemoveSession(uint32 session_id) {
-  DCHECK(sessions_.find(session_id) != sessions_.end());
-  sessions_.erase(session_id);
-}
-
-void SessionIdAdapter::OnSessionCreated(uint32 session_id,
-                                        const std::string& web_session_id) {
-  GetSession(session_id)->OnSessionCreated(web_session_id);
-}
-
-void SessionIdAdapter::OnSessionMessage(uint32 session_id,
-                                        const std::vector<uint8>& message,
-                                        const std::string& destination_url) {
-  GetSession(session_id)->OnSessionMessage(message, destination_url);
-}
-
-void SessionIdAdapter::OnSessionReady(uint32 session_id) {
-  GetSession(session_id)->OnSessionReady();
-}
-
-void SessionIdAdapter::OnSessionClosed(uint32 session_id) {
-  GetSession(session_id)->OnSessionClosed();
-}
-
-void SessionIdAdapter::OnSessionError(uint32 session_id,
-                                      media::MediaKeys::KeyError error_code,
-                                      int system_code) {
-  GetSession(session_id)->OnSessionError(error_code, system_code);
-}
-
-WebContentDecryptionModuleSessionImpl* SessionIdAdapter::GetSession(
-    uint32 session_id) {
-  DCHECK(sessions_.find(session_id) != sessions_.end());
-  return sessions_[session_id];
-}
-
-//------------------------------------------------------------------------------
-
 WebContentDecryptionModuleImpl*
 WebContentDecryptionModuleImpl::Create(const base::string16& key_system) {
   // TODO(ddorwin): Guard against this in supported types check and remove this.
@@ -172,21 +25,16 @@ WebContentDecryptionModuleImpl::Create(const base::string16& key_system) {
     return NULL;
   }
 
-  // SessionIdAdapter creates the MediaKeys so it can provide its callbacks to
-  // during creation of the MediaKeys.
-  scoped_ptr<media::MediaKeys> media_keys;
-  scoped_ptr<SessionIdAdapter> adapter(new SessionIdAdapter());
-  if (!adapter->Initialize(UTF16ToASCII(key_system), &media_keys))
+  scoped_refptr<CdmSessionIdAdapter> adapter(new CdmSessionIdAdapter());
+  if (!adapter->Initialize(UTF16ToASCII(key_system)))
     return NULL;
 
-  return new WebContentDecryptionModuleImpl(media_keys.Pass(), adapter.Pass());
+  return new WebContentDecryptionModuleImpl(adapter);
 }
 
 WebContentDecryptionModuleImpl::WebContentDecryptionModuleImpl(
-    scoped_ptr<media::MediaKeys> media_keys,
-    scoped_ptr<SessionIdAdapter> adapter)
-    : media_keys_(media_keys.Pass()),
-      adapter_(adapter.Pass()) {
+    scoped_refptr<CdmSessionIdAdapter> adapter)
+    : adapter_(adapter) {
 }
 
 WebContentDecryptionModuleImpl::~WebContentDecryptionModuleImpl() {
@@ -196,26 +44,13 @@ WebContentDecryptionModuleImpl::~WebContentDecryptionModuleImpl() {
 blink::WebContentDecryptionModuleSession*
 WebContentDecryptionModuleImpl::createSession(
     blink::WebContentDecryptionModuleSession::Client* client) {
-  DCHECK(media_keys_);
   uint32 session_id = adapter_->GenerateSessionId();

[ddorwin, 2014/02/18 20:59:42]

The adapter could generate the ID itself when Add[New]Session() is called. Maybe
that function returns the session_id.

Another option to look at: Maybe the adapter should create the
WebCDMSessionImpl, providing it with a session_id in the constructor.

[jrummell, 2014/02/19 00:20:29]

Done.

-  WebContentDecryptionModuleSessionImpl* session =
-      new WebContentDecryptionModuleSessionImpl(
-          session_id,
-          media_keys_.get(),
-          client,
-          base::Bind(&WebContentDecryptionModuleImpl::OnSessionClosed,
-                     base::Unretained(this)));
-
-  adapter_->AddSession(session_id, session);
-  return session;
+  return new WebContentDecryptionModuleSessionImpl(
+      session_id, client, adapter_);
 }
 
 media::Decryptor* WebContentDecryptionModuleImpl::GetDecryptor() {
-  return media_keys_->GetDecryptor();
-}
-
-void WebContentDecryptionModuleImpl::OnSessionClosed(uint32 session_id) {
-  adapter_->RemoveSession(session_id);
+  return adapter_->GetDecryptor();
 }
 
 }  // namespace content