Unit test for WebMediaPlayerMS

content/renderer/media/webmediaplayer_ms_compositor.h

Index: content/renderer/media/webmediaplayer_ms_compositor.h
diff --git a/content/renderer/media/webmediaplayer_ms_compositor.h b/content/renderer/media/webmediaplayer_ms_compositor.h
index 23a4209fc119ba9287bc88bb276a5e7ca2afcd25..45d2941738beee6fd1d3f7876c1a563e6c62091a 100644
--- a/content/renderer/media/webmediaplayer_ms_compositor.h
+++ b/content/renderer/media/webmediaplayer_ms_compositor.h
@@ -43,10 +43,17 @@ namespace content {
 // frame, and submit it whenever asked by the compositor.
 class WebMediaPlayerMSCompositor : public cc::VideoFrameProvider {
  public:
-  // This |url| represents the media stream we are rendering.
+  // This |url| represents the media stream we are rendering. |url| is used to
+  // determine whether we enable algorithm or not by looking up
+  // MediaStreamDescriptor.

[mcasas, 2015/10/21 19:54:02]

Incorrect, |url| is used to find the MediaStream WMPMSC
is supposed to play. The usage of a sophisticated or more
conservative rendering algorithm is decided elsewhere.

[qiangchen, 2015/10/22 17:22:58]

It is decided here. And the main usage of |url| is to decide algorithm or not.

|url| --> remote? --> use algorithm? Added more comments.

   WebMediaPlayerMSCompositor(
       const scoped_refptr<base::SingleThreadTaskRunner>& compositor_task_runner,
       const blink::WebURL& url);
+
+  WebMediaPlayerMSCompositor(
+      const scoped_refptr<base::SingleThreadTaskRunner>& compositor_task_runner,
+      const bool algorithm_enabled);

[mcasas, 2015/10/21 19:54:02]

I think you could get rid of this ctor by:

a) registering your MS with a MockMediaStreamRegistry [1]
like is done in MediaRecorderHandlerTest [2] 
b) injecting the flag when appropriate using AppendSwitch() 
e.g. [3]

[1]
https://code.google.com/p/chromium/codesearch#chromium/src/content/renderer/m...
[2]
https://code.google.com/p/chromium/codesearch#chromium/src/content/renderer/m...
[3]
https://code.google.com/p/chromium/codesearch#chromium/src/content/test/webrt...

[qiangchen, 2015/10/22 17:22:58]

Plan a) sounds better, but here is some difficulty to implement it, so I'll
follow plan b):
1. In load(), we used static function lookupMediaStreamDescriptor to find web
stream, where static function can't be overriden.
2. In order to avoid that we have to pass web stream to load() as a parameter.
However changing signature of load() is a big project itself, because it is an
overriden function, and the origin function is in Blink.

+
   ~WebMediaPlayerMSCompositor() override;
 
   void EnqueueFrame(const scoped_refptr<media::VideoFrame>& frame);