[Mojo Video Capture] Replace RESOURCE_UTILIZATION with interface ReceiverLoadObserver

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 8aaf97cdc9dc142489b2b89d41f8ce36d8d03433..5c0f6406946525570c8153ae480104cf831c2d4c 100644
--- a/content/browser/renderer_host/media/video_capture_controller.h
+++ b/content/browser/renderer_host/media/video_capture_controller.h
@@ -51,6 +51,7 @@
 #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/capture/video/video_capture_device_client.h"
 #include "media/capture/video/video_frame_receiver.h"
 #include "media/capture/video_capture_types.h"
 
@@ -72,9 +73,17 @@ class CONTENT_EXPORT VideoCaptureController : public media::VideoFrameReceiver {
 
   base::WeakPtr<VideoCaptureController> GetWeakPtrForIOThread();
 
+  // Factory code creating instances of VideoCaptureController may optionally
+  // set a FrameReceiverObserver. Setting the observer is done in this method
+  // separate from the constructor to allow clients to create and use instances
+  // before they can provide the observer. (This is the case with
+  // VideoCaptureManager).
+  void SetFrameReceiverObserver(
+      media::FrameReceiverObserver* frame_receiver_observer);
+
   // Return a new VideoCaptureDeviceClient to forward capture events to this
   // instance.
-  std::unique_ptr<media::VideoCaptureDevice::Client> NewDeviceClient();
+  std::unique_ptr<media::VideoCaptureDeviceClient> 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
@@ -139,6 +148,31 @@ class CONTENT_EXPORT VideoCaptureController : public media::VideoFrameReceiver {
   struct ControllerClient;
   typedef std::list<std::unique_ptr<ControllerClient>> ControllerClients;
 
+  class BufferState {
+   public:
+    BufferState();

[miu, 2016/12/01 05:25:17]

IIRC, you don't need the zero-arg constructor if you work with iterators instead
of the map[] operators. Based on some of the impl, it looks like that might also
reduce the number of look-ups you're doing.

[chfremer, 2016/12/02 01:28:28]

Done.

+    explicit BufferState(
+        int buffer_id,
+        media::FrameReceiverObserver* frame_receiver_observer,
+        const scoped_refptr<media::VideoCaptureBufferPool>& buffer_pool,
+        const scoped_refptr<media::VideoFrame>& frame);
+    BufferState(BufferState&& other);
+    ~BufferState();
+    BufferState& operator=(BufferState&& other);
+    void RecordConsumerUtilization(double utilization);
+    void IncreaseConsumerCount();
+    void DecreaseConsumerCount();
+    bool HasZeroConsumerHoldCount();
+
+   private:
+    int buffer_id_;

[miu, 2016/12/01 05:25:17]

If you can eliminate the zero-arg ctor, several of these can become const.

[chfremer, 2016/12/02 01:28:28]

Done.

+    media::FrameReceiverObserver* frame_receiver_observer_;
+    scoped_refptr<media::VideoCaptureBufferPool> buffer_pool_;
+    scoped_refptr<media::VideoFrame> frame_;
+    double max_consumer_utilization_;
+    int consumer_hold_count_;
+  };
+
   // Notify renderer that a new buffer has been created.
   void DoNewBufferOnIOThread(ControllerClient* client,
                              media::VideoCaptureDevice::Client::Buffer* buffer,
@@ -156,6 +190,10 @@ class CONTENT_EXPORT VideoCaptureController : public media::VideoFrameReceiver {
   // The pool of shared-memory buffers used for capturing.
   const scoped_refptr<media::VideoCaptureBufferPool> buffer_pool_;
 
+  media::FrameReceiverObserver* frame_receiver_observer_;
+
+  std::map<int, BufferState> buffer_id_to_state_map_;

[miu, 2016/12/01 05:25:17]

naming nit: Up to you, but |buffer_states_| or |buffer_states_map_| seems more
readable to me.

[chfremer, 2016/12/02 01:28:28]

I could live with |buffer_states_|, but I prefer |buffer_id_to_state_map_|
because it makes it obvious to the reader that it is a map indexed by buffer
ids. With the shorter name, the reader would have to hunt down usages or a
comment in order to get that information.

@Miguel, please comment. If you prefer the shorter name as well, I will make the
change.

+
   // All clients served by this controller.
   ControllerClients controller_clients_;