Respect disable_webrtc command line.

content/renderer/render_view_impl.cc

Index: content/renderer/render_view_impl.cc
diff --git a/content/renderer/render_view_impl.cc b/content/renderer/render_view_impl.cc
index 022fcf3ea16073dd71bd0f1240da1e0ef11d7d54..677440cdf247c9ca5d772fab15eba8784c8a77f5 100644
--- a/content/renderer/render_view_impl.cc
+++ b/content/renderer/render_view_impl.cc
@@ -3074,82 +3074,16 @@ WebMediaPlayer* RenderViewImpl::createMediaPlayer(
   FOR_EACH_OBSERVER(
       RenderViewObserver, observers_, WillCreateMediaPlayer(frame, client));
 
-  const CommandLine* cmd_line = CommandLine::ForCurrentProcess();
-#if defined(ENABLE_WEBRTC)
-  if (!InitializeMediaStreamClient())
-    return NULL;

[michaelbai, 2013/11/01 18:41:50]

Here is the difference, the original code will return NULL, the new code is
returned if player is created successfully, looked through the
InitializeMediaStreamClient, it didn't make sense to return NULL here.

Here are the cases of returning false:

  if (!RenderThreadImpl::current())  // Will be NULL during unit tests.
    return false;

It only happened in unit test, from the trybot results, it seemed this is not an
issue.


The other 2 cases, we don't want to return NULL.

#if defined(OS_ANDROID)
  if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kDisableWebRTC))
    return false;
#endif

#else
  return false;
#endif

-
-#if !defined(GOOGLE_TV)
-  if (media_stream_client_->IsMediaStream(url)) {
-#if defined(OS_ANDROID) && defined(ARCH_CPU_ARMEL)
-    bool found_neon =
-        (android_getCpuFeatures() & ANDROID_CPU_ARM_FEATURE_NEON) != 0;
-    UMA_HISTOGRAM_BOOLEAN("Platform.WebRtcNEONFound", found_neon);
-#endif  // defined(OS_ANDROID) && defined(ARCH_CPU_ARMEL)
-    return new WebMediaPlayerMS(
-        frame, client, AsWeakPtr(), media_stream_client_, new RenderMediaLog());
-  }
-#endif  // !defined(GOOGLE_TV)
-#endif  // defined(ENABLE_WEBRTC)
+  WebMediaPlayer* player = CreateWebMediaPlayerForMediaStream(frame, url,
+                                                              client);
+  if (player != NULL)

[scherkus (not reviewing), 2013/11/04 20:38:50]

nit: remove != NULL check

[michaelbai, 2013/11/04 21:43:34]

Done.

+    return player;
 
 #if defined(OS_ANDROID)
-  GpuChannelHost* gpu_channel_host =
-      RenderThreadImpl::current()->EstablishGpuChannelSync(
-          CAUSE_FOR_GPU_LAUNCH_VIDEODECODEACCELERATOR_INITIALIZE);
-  if (!gpu_channel_host) {
-    LOG(ERROR) << "Failed to establish GPU channel for media player";
-    return NULL;
-  }
-
-  scoped_ptr<StreamTextureFactory> stream_texture_factory;
-  if (UsingSynchronousRendererCompositor()) {
-    SynchronousCompositorFactory* factory =
-        SynchronousCompositorFactory::GetInstance();
-    stream_texture_factory = factory->CreateStreamTextureFactory(routing_id_);
-  } else {
-    scoped_refptr<cc::ContextProvider> context_provider =
-        RenderThreadImpl::current()->SharedMainThreadContextProvider();
-
-    if (!context_provider.get()) {
-      LOG(ERROR) << "Failed to get context3d for media player";
-      return NULL;
-    }
-
-    stream_texture_factory.reset(new StreamTextureFactoryImpl(
-        context_provider->Context3d(), gpu_channel_host, routing_id_));
-  }
-
-  scoped_ptr<WebMediaPlayerAndroid> web_media_player_android(
-      new WebMediaPlayerAndroid(
-          frame,
-          client,
-          AsWeakPtr(),
-          media_player_manager_,
-          stream_texture_factory.release(),
-          RenderThreadImpl::current()->GetMediaThreadMessageLoopProxy(),
-          new RenderMediaLog()));
-#if defined(ENABLE_WEBRTC) && defined(GOOGLE_TV)
-  if (media_stream_client_->IsMediaStream(url)) {
-    RTCVideoDecoderFactoryTv* factory = RenderThreadImpl::current()
-        ->GetMediaStreamDependencyFactory()->decoder_factory_tv();
-    // |media_stream_client| and |factory| outlives |web_media_player_android|.
-    if (!factory->AcquireDemuxer() ||
-        !web_media_player_android->InjectMediaStream(
-            media_stream_client_,
-            factory,
-            base::Bind(
-                base::IgnoreResult(&RTCVideoDecoderFactoryTv::ReleaseDemuxer),
-                base::Unretained(factory)))) {
-      LOG(ERROR) << "Failed to inject media stream.";
-      return NULL;
-    }
-  }
-#endif  // defined(ENABLE_WEBRTC) && defined(GOOGLE_TV)
-  return web_media_player_android.release();
-#endif  // defined(OS_ANDROID)
-
+  return CreateAndroidWebMediaPlayer(frame, url, client);
+#else
   scoped_refptr<media::AudioRendererSink> sink;
-  if (!cmd_line->HasSwitch(switches::kDisableAudio)) {
+  if (!CommandLine::ForCurrentProcess()->HasSwitch(switches::kDisableAudio)) {
     sink = RenderThreadImpl::current()->GetAudioRendererMixerManager()->
         CreateInput(routing_id_);
     DVLOG(1) << "Using AudioRendererMixerManager-provided sink: " << sink.get();
@@ -3164,6 +3098,7 @@ WebMediaPlayer* RenderViewImpl::createMediaPlayer(
       RenderThreadImpl::current()->GetGpuFactories(),
       new RenderMediaLog());
   return new WebMediaPlayerImpl(frame, client, AsWeakPtr(), params);
+#endif  // defined(OS_ANDROID)
 }
 
 WebCookieJar* RenderViewImpl::cookieJar(WebFrame* frame) {
@@ -6307,6 +6242,30 @@ void RenderViewImpl::draggableRegionsChanged() {
       DraggableRegionsChanged(webview()->mainFrame()));
 }
 
+WebMediaPlayer* RenderViewImpl::CreateWebMediaPlayerForMediaStream(
+      WebFrame* frame,

[scherkus (not reviewing), 2013/11/04 20:38:50]

fix indent (should be 4-space indent)

[michaelbai, 2013/11/04 21:43:34]

Done.

+      const WebKit::WebURL& url,
+      WebMediaPlayerClient* client) {
+#if defined(ENABLE_WEBRTC)

[scherkus (not reviewing), 2013/11/01 00:21:36]

OOC aren't we shipping WebRTC? do you know if we can remove this or are there
bits of Chrome that require this define?

[michaelbai, 2013/11/01 00:31:50]

WebRTC is disabled in WebView. But, I don't know why both compile flag and
command line needed. +joth, he may comment on this.

On 2013/11/01 00:21:36, scherkus wrote:

+  if (!InitializeMediaStreamClient()) {
+    LOG(ERROR) << "Failed to initialize MediaStreamClient";
+    return NULL;
+  }
+#if !defined(GOOGLE_TV)
+  if (media_stream_client_->IsMediaStream(url)) {
+#if defined(OS_ANDROID) && defined(ARCH_CPU_ARMEL)
+    bool found_neon =
+        (android_getCpuFeatures() & ANDROID_CPU_ARM_FEATURE_NEON) != 0;
+    UMA_HISTOGRAM_BOOLEAN("Platform.WebRtcNEONFound", found_neon);
+#endif  // defined(OS_ANDROID) && defined(ARCH_CPU_ARMEL)
+    return new WebMediaPlayerMS(frame, client, AsWeakPtr(),
+                                media_stream_client_, new RenderMediaLog());
+  }
+#endif  // !defined(GOOGLE_TV)
+#endif  // defined(ENABLE_WEBRTC)
+  return NULL;
+}
+
 #if defined(OS_ANDROID)
 WebContentDetectionResult RenderViewImpl::detectContentAround(
     const WebHitTestResult& touch_hit) {
@@ -6362,6 +6321,65 @@ bool RenderViewImpl::openDateTimeChooser(
   return date_time_picker_client_->Open();
 }
 
+WebMediaPlayer* RenderViewImpl::CreateAndroidWebMediaPlayer(
+      WebFrame* frame,
+      const WebKit::WebURL& url,
+      WebMediaPlayerClient* client) {
+  GpuChannelHost* gpu_channel_host =
+      RenderThreadImpl::current()->EstablishGpuChannelSync(
+          CAUSE_FOR_GPU_LAUNCH_VIDEODECODEACCELERATOR_INITIALIZE);
+  if (!gpu_channel_host) {
+    LOG(ERROR) << "Failed to establish GPU channel for media player";
+    return NULL;
+  }
+
+  scoped_ptr<StreamTextureFactory> stream_texture_factory;
+  if (UsingSynchronousRendererCompositor()) {
+    SynchronousCompositorFactory* factory =
+        SynchronousCompositorFactory::GetInstance();
+    stream_texture_factory = factory->CreateStreamTextureFactory(routing_id_);
+  } else {
+    scoped_refptr<cc::ContextProvider> context_provider =
+        RenderThreadImpl::current()->SharedMainThreadContextProvider();
+
+    if (!context_provider.get()) {
+      LOG(ERROR) << "Failed to get context3d for media player";
+      return NULL;
+    }
+
+    stream_texture_factory.reset(new StreamTextureFactoryImpl(
+        context_provider->Context3d(), gpu_channel_host, routing_id_));
+  }
+
+  scoped_ptr<WebMediaPlayerAndroid> web_media_player_android(
+      new WebMediaPlayerAndroid(
+          frame,
+          client,
+          AsWeakPtr(),
+          media_player_manager_,
+          stream_texture_factory.release(),
+          RenderThreadImpl::current()->GetMediaThreadMessageLoopProxy(),
+          new RenderMediaLog()));
+#if defined(ENABLE_WEBRTC) && defined(GOOGLE_TV)
+  if (media_stream_client && media_stream_client_->IsMediaStream(url)) {

[scherkus (not reviewing), 2013/11/04 20:38:50]

this code looks broken (media_stream_client doesn't appear to be defined after
refactoring)

[michaelbai, 2013/11/04 21:43:34]

media_stream_client is defined unconditionally, 


On 2013/11/04 20:38:50, scherkus wrote:

[michaelbai, 2013/11/04 21:43:34]

The media_stream_client is defined unconditionally, and it could be NULL here,
so I added media_stream_client check. Did you see any problem here?


On 2013/11/04 20:38:50, scherkus wrote:

+    RTCVideoDecoderFactoryTv* factory = RenderThreadImpl::current()
+        ->GetMediaStreamDependencyFactory()->decoder_factory_tv();
+    // |media_stream_client| and |factory| outlives |web_media_player_android|.
+    if (!factory->AcquireDemuxer() ||
+        !web_media_player_android->InjectMediaStream(
+            media_stream_client_,
+            factory,
+            base::Bind(
+                base::IgnoreResult(&RTCVideoDecoderFactoryTv::ReleaseDemuxer),
+                base::Unretained(factory)))) {
+      LOG(ERROR) << "Failed to inject media stream.";
+      return NULL;
+    }
+  }
+#endif  // defined(ENABLE_WEBRTC) && defined(GOOGLE_TV)
+  return web_media_player_android.release();
+}
+
 #endif  // defined(OS_ANDROID)
 
 #if defined(OS_MACOSX)