Migrate MediaStream test code from content/shell/renderer/ to content/test/.

Migrate MediaStream test code from content/shell/renderer/ to content/test/.

ContentRendererClient::OverrideCreateMediaStreamClient() is replaced with UseMockMediaStreams() for overriding MediaStreamClient when running layout tests with content_shell. Doing so permits migration of MediaStream-related code from webkit/ to content/ without widening content's public API solely for test purposes.

BUG=251306
R=jam@chromium.org, jochen@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=210747

[scherkus (not reviewing), 2013-07-03 00:47:05]

jam: here you go -- now with no DEPS hacks!

I tried to keep it as simple of a move as possible.

[jam, 2013-07-03 17:44:32]

+jochen for his thoughts

basically my comments center around not exposing all this stuff in
content/public. i realize it used to be available outside of content before when
it was in src/webkit, but as we move stuff from src/webkit to src/content, we
have to be careful to only expose stuff that embedders need.

https://codereview.chromium.org/18590002/diff/8001/content/public/renderer/me...
File content/public/renderer/media_stream_audio_renderer.h (right):

https://codereview.chromium.org/18590002/diff/8001/content/public/renderer/me...
content/public/renderer/media_stream_audio_renderer.h:13: class
MediaStreamAudioRenderer
ditto, why is this public? i don't see this used anywhere outside of content.
the only thing that's done is that content shell returns NULL for this interface
in one place.
1) why does it do that?
2) it seems we can achieve the same thing and not expose the interface by asking
the embedder if an action is allowed and having a boolean return value

https://codereview.chromium.org/18590002/diff/8001/content/public/renderer/me...
File content/public/renderer/media_stream_client.h (right):

https://codereview.chromium.org/18590002/diff/8001/content/public/renderer/me...
content/public/renderer/media_stream_client.h:23: virtual bool
IsMediaStream(const GURL& url) = 0;
ditto for this whole interface. why do we need to expose this in content/public?
and if we only have to expose it, does it need to be a separate interface vs one
or two methods on ContentRendererClient?

https://codereview.chromium.org/18590002/diff/8001/content/public/renderer/vi...
File content/public/renderer/video_frame_provider.h (right):

https://codereview.chromium.org/18590002/diff/8001/content/public/renderer/vi...
content/public/renderer/video_frame_provider.h:20: class CONTENT_EXPORT
VideoFrameProvider
ditto for this interface

it seems that this and the other class are only there for layout tests. so
another alternative that avoids exposing all this stuff outsisde content is to
put the test implementations in content/test and have content itself instantiate
them when layout test mode is on (i.e. content::EnableRendererLayoutTestMode).

https://codereview.chromium.org/18590002/diff/8001/content/shell/renderer/she...
File content/shell/renderer/shell_media_stream_client.h (right):

https://codereview.chromium.org/18590002/diff/8001/content/shell/renderer/she...
content/shell/renderer/shell_media_stream_client.h:1: // Copyright 2013 The
Chromium Authors. All rights reserved.
I'm not familiar with this code. why does content shell not use the default one
but overrides it? the reason I ask is because now we're exposing these
interfaces in content/public just for it

[scherkus (not reviewing), 2013-07-03 20:22:57]

On 2013/07/03 17:44:32, jam wrote:
> +jochen for his thoughts
> 
> basically my comments center around not exposing all this stuff in
> content/public. i realize it used to be available outside of content before
when
> it was in src/webkit, but as we move stuff from src/webkit to src/content, we
> have to be careful to only expose stuff that embedders need.

I didn't write nor am I familiar with any of the MediaStream-related code you're
commenting on.

I agree that its current form is suboptimal as it both exposes an overly wide
API as well as mocks out a large amount of production code present in content/.
I don't know why it does that, but it's why I've been filing (and trying to fix)
bugs like http://crbug.com/239826 and emailing blink-dev@ about
content_browsertests vs. layout tests.

My primary goal here is to get things moved out of webkit/ and spruced up a bit
so others can start doing their own cleanup, not to do all the cleanup myself!

Anyway I'll give the content/test/ layout test mode approach a shot, possibly as
a separate CL.

[scherkus (not reviewing), 2013-07-03 23:04:56]

jam / jochen: PTAL

I moved the MediaStream crap to content/test/, poked a hole to override
RenderViewImpl's MediaStreamClient pointer via
content/public/test/layouttest_support.h, and removed the old override API from
content/public/renderer/

Things seem to still work. Tests still seem to pass. Hooray.

Let me know if you yay/nay this approach.

[scherkus (not reviewing), 2013-07-03 23:13:00]

On 2013/07/03 23:04:56, scherkus wrote:
> jam / jochen: PTAL
> 
> I moved the MediaStream crap to content/test/, poked a hole to override
> RenderViewImpl's MediaStreamClient pointer via
> content/public/test/layouttest_support.h, and removed the old override API
from
> content/public/renderer/
> 
> Things seem to still work. Tests still seem to pass. Hooray.
> 
> Let me know if you yay/nay this approach.

(and if you "yay" it I'll go ahead and update the CL description)

[jam, 2013-07-08 20:33:09]

lgtm

[commit-bot: I haz the power, 2013-07-08 21:38:15]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/scherkus@chromium.org/18590002/42001

[jochen (gone - plz use gerrit), 2013-07-09 07:10:42]

lgtm

[commit-bot: I haz the power, 2013-07-09 17:29:24]

Failed to apply patch for content/renderer/render_view_impl.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file content/renderer/render_view_impl.cc
  Hunk #1 succeeded at 805 (offset 12 lines).
  Hunk #2 FAILED at 2858.
  Hunk #3 FAILED at 2908.
  Hunk #4 succeeded at 4279 (offset 19 lines).
  Hunk #5 succeeded at 4292 (offset 19 lines).
  Hunk #6 succeeded at 6519 (offset 26 lines).
  Hunk #7 succeeded at 6789 (offset 26 lines).
  2 out of 7 hunks FAILED -- saving rejects to file
content/renderer/render_view_impl.cc.rej

Patch:       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
98c2b6928ac1d3531cf872de15dab108a3866f0b..5e86784564f5b5b7b0dd63b3d142e6b42742a2db
100644
--- a/content/renderer/render_view_impl.cc
+++ b/content/renderer/render_view_impl.cc
@@ -793,7 +793,8 @@ RenderViewImpl::RenderViewImpl(RenderViewImplParams* params)
       device_orientation_dispatcher_(NULL),
       media_stream_dispatcher_(NULL),
       browser_plugin_manager_(NULL),
-      media_stream_impl_(NULL),
+      media_stream_client_(NULL),
+      web_user_media_client_(NULL),
       devtools_agent_(NULL),
       accessibility_mode_(AccessibilityModeOff),
       renderer_accessibility_(NULL),
@@ -2857,22 +2858,15 @@ WebMediaPlayer* RenderViewImpl::createMediaPlayer(
 
   const CommandLine* cmd_line = CommandLine::ForCurrentProcess();
 #if defined(ENABLE_WEBRTC) && !defined(GOOGLE_TV)
-  webkit_media::MediaStreamClient* media_stream_client =
-      GetContentClient()->renderer()->OverrideCreateMediaStreamClient();
-  if (!media_stream_client) {
-    EnsureMediaStreamImpl();
-    media_stream_client = media_stream_impl_;
-  }
-
-  if (media_stream_client->IsMediaStream(url)) {
+  EnsureMediaStreamClient();
+  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)
-    EnsureMediaStreamImpl();
     return new webkit_media::WebMediaPlayerMS(
-        frame, client, AsWeakPtr(), media_stream_client, new RenderMediaLog());
+        frame, client, AsWeakPtr(), media_stream_client_, new
RenderMediaLog());
   }
 #endif
 
@@ -2907,14 +2901,14 @@ WebMediaPlayer* RenderViewImpl::createMediaPlayer(
               context_provider->Context3d(), gpu_channel_host, routing_id_),
           new RenderMediaLog()));
 #if defined(ENABLE_WEBRTC) && defined(GOOGLE_TV)
-  if (MediaStreamImpl::CheckMediaStream(url)) {
-    EnsureMediaStreamImpl();
+  EnsureMediaStreamClient();
+  if (media_stream_client_->IsMediaStream(url)) {
     RTCVideoDecoderFactoryTv* factory = RenderThreadImpl::current()
         ->GetMediaStreamDependencyFactory()->decoder_factory_tv();
     // |media_stream_impl_| and |factory| outlives |web_media_player_android|.
     if (!factory->AcquireDemuxer() ||
         !web_media_player_android->InjectMediaStream(
-            media_stream_impl_,
+            media_stream_client_,
             factory,
             base::Bind(
                 base::IgnoreResult(&RTCVideoDecoderFactoryTv::ReleaseDemuxer),
@@ -4259,7 +4253,7 @@ BrowserPluginManager*
RenderViewImpl::GetBrowserPluginManager() {
   return browser_plugin_manager_.get();
 }
 
-void RenderViewImpl::EnsureMediaStreamImpl() {
+void RenderViewImpl::EnsureMediaStreamClient() {
   if (!RenderThreadImpl::current())  // Will be NULL during unit tests.
     return;
 
@@ -4272,11 +4266,13 @@ void RenderViewImpl::EnsureMediaStreamImpl() {
   if (!media_stream_dispatcher_)
     media_stream_dispatcher_ = new MediaStreamDispatcher(this);
 
-  if (!media_stream_impl_) {
-    media_stream_impl_ = new MediaStreamImpl(
+  if (!media_stream_client_) {
+    MediaStreamImpl* media_stream_impl = new MediaStreamImpl(
         this,
         media_stream_dispatcher_,
         RenderThreadImpl::current()->GetMediaStreamDependencyFactory());
+    media_stream_client_ = media_stream_impl;
+    web_user_media_client_ = media_stream_impl;
   }
 #endif
 }
@@ -6490,8 +6486,8 @@ WebKit::WebPageVisibilityState
RenderViewImpl::visibilityState() const {
 }
 
 WebKit::WebUserMediaClient* RenderViewImpl::userMediaClient() {
-  EnsureMediaStreamImpl();
-  return media_stream_impl_;
+  EnsureMediaStreamClient();
+  return web_user_media_client_;
 }
 
 void RenderViewImpl::draggableRegionsChanged() {
@@ -6760,6 +6756,13 @@ void RenderViewImpl::DisableAutoResizeForTesting(const
gfx::Size& new_size) {
   OnDisableAutoResize(new_size);
 }
 
+void RenderViewImpl::SetMediaStreamClientForTesting(
+  webkit_media::MediaStreamClient* media_stream_client) {
+  DCHECK(!media_stream_client_);
+  DCHECK(!web_user_media_client_);
+  media_stream_client_ = media_stream_client;
+}
+
 void RenderViewImpl::OnReleaseDisambiguationPopupDIB(
     TransportDIB::Handle dib_handle) {
   TransportDIB* dib = TransportDIB::CreateWithHandle(dib_handle);

[commit-bot: I haz the power, 2013-07-09 18:37:18]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/scherkus@chromium.org/18590002/60001

[commit-bot: I haz the power, 2013-07-09 19:02:32]

Retried try job too often on win for step(s) compile
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win&number...

[commit-bot: I haz the power, 2013-07-09 20:14:25]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/scherkus@chromium.org/18590002/60001

[commit-bot: I haz the power, 2013-07-10 02:08:41]

Failed to apply patch for content/test/test_media_stream_client.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  A         content/test/test_media_stream_client.cc
  Copied content/shell/renderer/shell_media_stream_client.cc ->
content/test/test_media_stream_client.cc
  patching file content/test/test_media_stream_client.cc
  Hunk #1 FAILED at 2.
  1 out of 3 hunks FAILED -- saving rejects to file
content/test/test_media_stream_client.cc.rej

Patch:   NR 
content/shell/renderer/shell_media_stream_client.cc->content/test/test_media_stream_client.cc
Index: content/test/test_media_stream_client.cc
diff --git a/content/shell/renderer/shell_media_stream_client.cc
b/content/test/test_media_stream_client.cc
similarity index 77%
rename from content/shell/renderer/shell_media_stream_client.cc
rename to content/test/test_media_stream_client.cc
index
48580a6d6901583152568fb575165324408f3e36..01904f5b93a4e4340171c018065c864e61f08cb5
100644
--- a/content/shell/renderer/shell_media_stream_client.cc
+++ b/content/test/test_media_stream_client.cc
@@ -2,9 +2,9 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "content/shell/renderer/shell_media_stream_client.h"
+#include "content/test/test_media_stream_client.h"
 
-#include "content/shell/renderer/shell_video_frame_provider.h"
+#include "content/test/test_video_frame_provider.h"
 #include "googleurl/src/gurl.h"
 #include "third_party/WebKit/public/platform/WebMediaStream.h"
 #include "third_party/WebKit/public/platform/WebMediaStreamTrack.h"
@@ -38,23 +38,24 @@ bool IsMockMediaStreamWithVideo(const WebURL& url) {
 
 namespace content {
 
-ShellMediaStreamClient::ShellMediaStreamClient() {}
+TestMediaStreamClient::TestMediaStreamClient(RenderView* render_view)
+    : RenderViewObserver(render_view) {}
 
-ShellMediaStreamClient::~ShellMediaStreamClient() {}
+TestMediaStreamClient::~TestMediaStreamClient() {}
 
-bool ShellMediaStreamClient::IsMediaStream(const GURL& url) {
+bool TestMediaStreamClient::IsMediaStream(const GURL& url) {
   return IsMockMediaStreamWithVideo(url);
 }
 
 scoped_refptr<webkit_media::VideoFrameProvider>
-ShellMediaStreamClient::GetVideoFrameProvider(
+TestMediaStreamClient::GetVideoFrameProvider(
     const GURL& url,
     const base::Closure& error_cb,
     const webkit_media::VideoFrameProvider::RepaintCB& repaint_cb) {
   if (!IsMockMediaStreamWithVideo(url))
     return NULL;
 
-  return new ShellVideoFrameProvider(
+  return new TestVideoFrameProvider(
       gfx::Size(kVideoCaptureWidth, kVideoCaptureHeight),
       base::TimeDelta::FromMilliseconds(kVideoCaptureFrameDurationMs),
       error_cb,
@@ -62,7 +63,7 @@ ShellMediaStreamClient::GetVideoFrameProvider(
 }
 
 scoped_refptr<webkit_media::MediaStreamAudioRenderer>
-ShellMediaStreamClient::GetAudioRenderer(const GURL& url) {
+TestMediaStreamClient::GetAudioRenderer(const GURL& url) {
   return NULL;
 }

[scherkus (not reviewing), 2013-07-10 04:56:39]

Committed patchset #4 manually as r210747 (presubmit successful).