Introduce CdmFactory interface.

content/renderer/media/cdm_session_adapter.cc

Index: content/renderer/media/cdm_session_adapter.cc
diff --git a/content/renderer/media/cdm_session_adapter.cc b/content/renderer/media/cdm_session_adapter.cc
index 312a1a6fbceb0759b159f4435c9f67ff1ced326a..50b3597eff8a46dba070a1cb2d02eb7b7e75e210 100644
--- a/content/renderer/media/cdm_session_adapter.cc
+++ b/content/renderer/media/cdm_session_adapter.cc
@@ -8,13 +8,17 @@
 #include "base/logging.h"
 #include "base/memory/weak_ptr.h"
 #include "base/stl_util.h"
-#include "content/renderer/media/crypto/content_decryption_module_factory.h"
 #include "content/renderer/media/crypto/key_systems.h"
 #include "content/renderer/media/webcontentdecryptionmodulesession_impl.h"
+#include "media/base/cdm_factory.h"
 #include "media/base/cdm_promise.h"
 #include "media/base/media_keys.h"
 #include "url/gurl.h"
 
+#if defined(ENABLE_BROWSER_CDMS)
+#include "content/renderer/media/crypto/renderer_cdm_manager.h"
+#endif  // defined(ENABLE_BROWSER_CDMS)
+
 namespace content {
 
 const char kMediaEME[] = "Media.EME.";
@@ -22,29 +26,26 @@ const char kDot[] = ".";
 
 CdmSessionAdapter::CdmSessionAdapter() :
 #if defined(ENABLE_BROWSER_CDMS)
-    cdm_id_(0),
+    // TODO(xhwang): Move kInvalidCdmId to src/media.
+    cdm_id_(RendererCdmManager::kInvalidCdmId),
 #endif
     weak_ptr_factory_(this) {}
 
 CdmSessionAdapter::~CdmSessionAdapter() {}
 
-bool CdmSessionAdapter::Initialize(
-#if defined(ENABLE_PEPPER_CDMS)
-    const CreatePepperCdmCB& create_pepper_cdm_cb,
-#elif defined(ENABLE_BROWSER_CDMS)
-    RendererCdmManager* manager,
-#endif  // defined(ENABLE_PEPPER_CDMS)
-    const std::string& key_system,
-    const GURL& security_origin) {
+bool CdmSessionAdapter::Initialize(scoped_ptr<media::CdmFactory> cdm_factory,

[ddorwin, 2014/10/16 17:32:51]

It seems odd to pass ownership of a factory. It forces the caller to create a
new factory for each call, regardless of what makes sense. Since this function
is synchronous, we could pass a pointer and have the caller decide whether to
destroy it or not.

[xhwang, 2014/10/16 20:08:39]

Good point. Done.

Currently we are still creating a new factory every time we create a new CDM,
this is because the |frame| is passed from Blink. We could have a
set_frame(frame) function on RenderCdmFactory, but since this is a simple class,
I am not sure whether it's worth it.

+                                   const std::string& key_system,
+                                   const GURL& security_origin) {
+  // TODO(xhwang): This is why we need to include "key_systems.h". Move
+  // KeySystemNameForUMA out of src/content so we can move CdmSessionAdapter to

[ddorwin, 2014/10/16 17:32:51]

We probably need to inject the key systems into media/.
UMA name is currently a special case, but I think we can replace the hard-coding
and use the same mechanism we use for adding other key system info.

[xhwang, 2014/10/16 20:08:39]

Acknowledged.

+  // src/media.
   key_system_uma_prefix_ = kMediaEME + KeySystemNameForUMA(key_system) + kDot;
+
   base::WeakPtr<CdmSessionAdapter> weak_this = weak_ptr_factory_.GetWeakPtr();
-  media_keys_ = ContentDecryptionModuleFactory::Create(
+  media_keys_ = cdm_factory->Create(
       key_system,
       security_origin,
-#if defined(ENABLE_PEPPER_CDMS)
-      create_pepper_cdm_cb,
-#elif defined(ENABLE_BROWSER_CDMS)
-      manager,
+#if defined(ENABLE_BROWSER_CDMS)

[ddorwin, 2014/10/16 17:32:51]

We've eliminated other ifdef's. Can we eliminate this  eventually?

[xhwang, 2014/10/16 20:08:39]

Totally agreed. I am thinking about it...

This is the history:
https://codereview.chromium.org/193523002/diff/1/media/base/media_keys.h#newc...

How about reintroduce GetCdmId() to MediaKeys, but put it in the #if
defined(ENABLE_BROWSER_CDMS)?

[ddorwin, 2014/10/16 20:33:20]

Sounds good. Please make a separate CL after this lands so we can see how the
footprint changes.

[xhwang, 2014/10/17 00:11:48]

Acknowledged.

       &cdm_id_,
 #endif  // defined(ENABLE_PEPPER_CDMS)
       base::Bind(&CdmSessionAdapter::OnSessionMessage, weak_this),