Break tight coupling of VideoCaptureDeviceClient to renderer_host

content/browser/renderer_host/media/video_capture_controller.h

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 //
 // VideoCaptureController is the glue between a VideoCaptureDevice and all
 // VideoCaptureHosts that have connected to it. A controller exists on behalf of
 // one (and only one) VideoCaptureDevice; both are owned by the
 // VideoCaptureManager.
 //
 // The VideoCaptureController is responsible for:
@@ skipping 39 matching lines @@
 #include "base/process/process.h"
 #include "content/browser/renderer_host/media/video_capture_controller_event_handler.h"
 #include "content/common/content_export.h"
 #include "content/common/media/video_capture.h"
 #include "media/base/video_capture_types.h"
 #include "media/capture/video/video_capture_device.h"
 
 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
   // capture pipeline, and not on hardware performance.  For example, tab
   // 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();
 
   // Return a new VideoCaptureDeviceClient to forward capture events to this
   // instance.
   std::unique_ptr<media::VideoCaptureDevice::Client> NewDeviceClient();
 
   // Start video capturing and try to use the resolution specified in |params|.
   // Buffers will be shared to the client as necessary. The client will continue
   // to receive frames from the device until RemoveClient() is called.
@@ skipping 39 matching lines @@
   void ReturnBuffer(VideoCaptureControllerID id,
                     VideoCaptureControllerEventHandler* event_handler,
                     int buffer_id,
                     const gpu::SyncToken& sync_token,
                     double consumer_resource_utilization);
 
   const media::VideoCaptureFormat& GetVideoCaptureFormat() const;
 
   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;
   typedef std::list<ControllerClient*> ControllerClients;
 
   // Notify renderer that a new buffer has been created.
   void DoNewBufferOnIOThread(ControllerClient* client,
                              media::VideoCaptureDevice::Client::Buffer* buffer,
                              const scoped_refptr<media::VideoFrame>& frame);
 
@@ skipping 22 matching lines @@
   media::VideoCaptureFormat video_capture_format_;
 
   base::WeakPtrFactory<VideoCaptureController> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(VideoCaptureController);
 };
 
 }  // namespace content
 
 #endif  // CONTENT_BROWSER_RENDERER_HOST_MEDIA_VIDEO_CAPTURE_CONTROLLER_H_