Break tight coupling of VideoCaptureDeviceClient to renderer_host

content/browser/renderer_host/media/video_capture_controller.h

Index: content/browser/renderer_host/media/video_capture_controller.h
diff --git a/content/browser/renderer_host/media/video_capture_controller.h b/content/browser/renderer_host/media/video_capture_controller.h
index a9b93206405f5d4a777ecdb0687bf18e9226a912..c71fc9427af4c9af1e9024144620e86bc1f352d6 100644
--- a/content/browser/renderer_host/media/video_capture_controller.h
+++ b/content/browser/renderer_host/media/video_capture_controller.h
@@ -57,7 +57,19 @@
 namespace content {
 class VideoCaptureBufferPool;
 
-class CONTENT_EXPORT VideoCaptureController {
+class CONTENT_EXPORT VideoFrameReceiver {
+ public:
+  virtual ~VideoFrameReceiver(){};
+
+  virtual void OnIncomingCapturedVideoFrame(
+      std::unique_ptr<media::VideoCaptureDevice::Client::Buffer> buffer,
+      const scoped_refptr<media::VideoFrame>& frame) = 0;

[miu, 2016/09/12 20:16:46]

Since this code is being modified, please use the new style for passing
ref-counted pointers when the method will add a reference:

  virtual void OnIncomingCapturedVideoFrame(
      std::unique_ptr<media::VideoCaptureDevice::Client::Buffer> buffer,
      scoped_refptr<media::VideoFrame> frame) = 0;

If the caller of this method no longer needs its reference to |frame|, make sure
to use std::move(). And, inside the method, use std::move() to pass the pointer
around until it reaches its final "owner" member.

See "calling conventions" section here:
https://www.chromium.org/developers/smart-pointer-guidelines (what applies to
unique_ptr also applies to scoped_refptr, per e-mail discussion on
chromium-dev@).

There are several of these in this change, so please be sure to fix them too. :)

[chfremer, 2016/09/12 22:23:33]

Thanks for making me aware of this.
I would like to keep the scope of this CL limited to breaking the dependencies.
Changing these signatures would require reviewing all call sites as well and
deciding on whether or not to use std::move(). I added a new planned CL
(1.6.1.2) to the design doc to do this work in a separate CL.

+  virtual void OnError() = 0;
+  virtual void OnLog(const std::string& message) = 0;
+  virtual void OnBufferDestroyed(int buffer_id_to_drop) = 0;
+};
+
+class CONTENT_EXPORT VideoCaptureController : public VideoFrameReceiver {
  public:
   // |max_buffers| is the maximum number of video frame buffers in-flight at any
   // one time.  This value should be based on the logical capacity of the
@@ -65,7 +77,7 @@ class CONTENT_EXPORT VideoCaptureController {
   // capture requires more buffers than webcam capture because the pipeline is
   // longer (it includes read-backs pending in the GPU pipeline).
   explicit VideoCaptureController(int max_buffers);
-  virtual ~VideoCaptureController();
+  ~VideoCaptureController() override;
 
   base::WeakPtr<VideoCaptureController> GetWeakPtrForIOThread();
 
@@ -125,13 +137,13 @@ class CONTENT_EXPORT VideoCaptureController {
 
   bool has_received_frames() const { return has_received_frames_; }
 
-  // Worker functions on IO thread. Called by the VideoCaptureDeviceClient.
-  virtual void DoIncomingCapturedVideoFrameOnIOThread(
+  // Implementation of VideoFrameReceiver interface:
+  void OnIncomingCapturedVideoFrame(
       std::unique_ptr<media::VideoCaptureDevice::Client::Buffer> buffer,
-      const scoped_refptr<media::VideoFrame>& frame);
-  virtual void DoErrorOnIOThread();
-  virtual void DoLogOnIOThread(const std::string& message);
-  virtual void DoBufferDestroyedOnIOThread(int buffer_id_to_drop);
+      const scoped_refptr<media::VideoFrame>& frame) override;
+  void OnError() override;
+  void OnLog(const std::string& message) override;
+  void OnBufferDestroyed(int buffer_id_to_drop) override;
 
  private:
   struct ControllerClient;