media: Enable Unified Media Pipeline for MSE and EME on Android

content/renderer/render_frame_impl.cc

Index: content/renderer/render_frame_impl.cc
diff --git a/content/renderer/render_frame_impl.cc b/content/renderer/render_frame_impl.cc
index a41cdb260b8e4372543204bdd996cae5d36ee182..9f842d773d73099256248b9029127bdc094e8c37 100644
--- a/content/renderer/render_frame_impl.cc
+++ b/content/renderer/render_frame_impl.cc
@@ -214,8 +214,10 @@
 #endif
 
 #if defined(ENABLE_PEPPER_CDMS)
+#include "content/renderer/media/cdm/render_cdm_factory.h"
 #include "content/renderer/media/cdm/pepper_cdm_wrapper_impl.h"
 #elif defined(ENABLE_BROWSER_CDMS)
+#include "content/renderer/media/cdm/render_cdm_factory.h"
 #include "content/renderer/media/cdm/renderer_cdm_manager.h"
 #endif
 
@@ -225,8 +227,6 @@
 
 #if defined(ENABLE_MOJO_CDM)
 #include "media/mojo/services/mojo_cdm_factory.h"  // nogncheck
-#else
-#include "content/renderer/media/cdm/render_cdm_factory.h"
 #endif
 
 #if defined(ENABLE_MOJO_RENDERER)
@@ -760,11 +760,7 @@ bool IsContentWithCertificateErrorsRelevantToUI(
 //
 // Note that HLS and MP4 detection are pre-redirect and path-based. It is
 // possible to load such a URL and find different content.
-bool UseWebMediaPlayerImpl(blink::WebMediaPlayer::LoadType load_type,
-                           const GURL& url) {
-  if (load_type == blink::WebMediaPlayer::LoadTypeMediaSource)
-    return media::IsUnifiedMediaPipelineEnabledForMse();
-
+bool UseWebMediaPlayerImpl(const GURL& url) {

[DaleCurtis, 2016/03/23 17:51:54]

I wonder if we should have a if (!url.isHttpOrHttps()) return true here to avoid
any accidental blob/data urls ending with hls or mp4 extensions.

[xhwang, 2016/03/24 21:26:04]

For HLS, this is kinda covered in IsHLSPath().

For mp4, I have no strong opinions. We can probably add this check in in "if"
statement one line 770.

[wolenetz, 2016/03/24 22:16:26]

There should be no blob URL containing trailing .mp4 (or .m3u8). I think it'd be
incorrect currently for a properly formatted blob URL [1] [2] to have a 
trailing ".mp4" or (or ".m3u8"), since [.mpu] chars are not allowed within the
UUID portion of such a URL [3]. Otherwise, no strong opinion. We could
potentially protect against those specs changing by strengthening line 770. 

[1] code:
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...
[2] FILE API spec: https://www.w3.org/TR/FileAPI/#unicodeBlobURL
[3] RFC for the UUID portion of a blob URL: https://tools.ietf.org/html/rfc4122

   // WMPI does not support HLS.
   if (media::MediaCodecUtil::IsHLSPath(url))
     return false;
@@ -2382,7 +2378,6 @@ blink::WebPlugin* RenderFrameImpl::createPlugin(
 }
 
 blink::WebMediaPlayer* RenderFrameImpl::createMediaPlayer(
-    blink::WebMediaPlayer::LoadType load_type,
     const blink::WebURL& url,
     WebMediaPlayerClient* client,
     WebMediaPlayerEncryptedMediaClient* encrypted_client,
@@ -2429,7 +2424,7 @@ blink::WebMediaPlayer* RenderFrameImpl::createMediaPlayer(
       initial_cdm, media_surface_manager_, media_session);
 
 #if defined(OS_ANDROID)
-  if (!UseWebMediaPlayerImpl(load_type, url))
+  if (!UseWebMediaPlayerImpl(url))
     return CreateAndroidWebMediaPlayer(client, encrypted_client, params);
 #endif  // defined(OS_ANDROID)
 
@@ -5904,26 +5899,25 @@ bool RenderFrameImpl::AreSecureCodecsSupported() {
 }
 
 media::CdmFactory* RenderFrameImpl::GetCdmFactory() {
-#if defined(ENABLE_BROWSER_CDMS)
-  if (!cdm_manager_)
-    cdm_manager_ = new RendererCdmManager(this);
-#endif  // defined(ENABLE_BROWSER_CDMS)
-
-  if (!cdm_factory_) {
-    DCHECK(frame_);
+  if (cdm_factory_)
+    return cdm_factory_.get();
 
 #if defined(ENABLE_MOJO_CDM)
+  if (media::IsMojoCdmEnabled()) {

[Tima Vaisburd, 2016/03/23 19:27:52]

It looks like if ENABLE_MOJO_CDM is defined, and media::IsMojoCdmEnabled()
returns false, and neither CDMS is defined, this method returns nullptr. Is this
expected?

[xhwang, 2016/03/23 20:44:06]

Today we have ENABLE_BROWSER_CDMS enabled by default on Android. So in the case
you described, we'll fallback to linr 5919, which is the old WMPA EME path.

In the future, when to switch to mojo CDM exclusively and drop
ENABLE_BROWSER_CDMS, we'll end up with a null CDM factory, which is expected.

     cdm_factory_.reset(new media::MojoCdmFactory(GetMediaInterfaceProvider()));
-#else
-    cdm_factory_.reset(new RenderCdmFactory(
+    return cdm_factory_.get();
+  }
+#endif  //  defined(ENABLE_MOJO_CDM)
+
 #if defined(ENABLE_PEPPER_CDMS)
-        base::Bind(&PepperCdmWrapperImpl::Create, frame_)
+  DCHECK(frame_);
+  cdm_factory_.reset(
+      new RenderCdmFactory(base::Bind(&PepperCdmWrapperImpl::Create, frame_)));
 #elif defined(ENABLE_BROWSER_CDMS)
-        cdm_manager_
-#endif
-            ));
-#endif  //  defined(ENABLE_MOJO_CDM)
-  }
+  if (!cdm_manager_)
+    cdm_manager_ = new RendererCdmManager(this);
+  cdm_factory_.reset(new RenderCdmFactory(cdm_manager_));
+#endif  // defined(ENABLE_PEPPER_CDMS)
 
   return cdm_factory_.get();
 }