Converted VideoFrameCapturer callbacks into a VideoFrameCapturer::Delegate interface.

remoting/host/video_frame_capturer_linux.cc

Index: remoting/host/video_frame_capturer_linux.cc
diff --git a/remoting/host/video_frame_capturer_linux.cc b/remoting/host/video_frame_capturer_linux.cc
index c472f491a1b99ddaea94cd506c0cc970dccf2d9c..522eac710f4db6a113080072d8f22dae5809b633 100644
--- a/remoting/host/video_frame_capturer_linux.cc
+++ b/remoting/host/video_frame_capturer_linux.cc
@@ -83,12 +83,11 @@ class VideoFrameCapturerLinux : public VideoFrameCapturer {
   bool Init();  // TODO(ajwong): Do we really want this to be synchronous?
 
   // Capturer interface.
-  virtual void Start(const CursorShapeChangedCallback& callback) OVERRIDE;
+  virtual void Start(Delegate* delegate) OVERRIDE;
   virtual void Stop() OVERRIDE;
   virtual media::VideoFrame::Format pixel_format() const OVERRIDE;
   virtual void InvalidateRegion(const SkRegion& invalid_region) OVERRIDE;
-  virtual void CaptureInvalidRegion(
-      const CaptureCompletedCallback& callback) OVERRIDE;
+  virtual void CaptureInvalidRegion() OVERRIDE;
   virtual const SkISize& size_most_recent() const OVERRIDE;
 
  private:
@@ -110,8 +109,7 @@ class VideoFrameCapturerLinux : public VideoFrameCapturer {
   // previous capture.
   CaptureData* CaptureFrame();
 
-  // Capture the cursor image and call the CursorShapeChangedCallback if it
-  // has been set (using SetCursorShapeChangedCallback).
+  // Capture the cursor image and notify the delegate if it was captured.
   void CaptureCursor();
 
   // Called when the screen configuration is changed.
@@ -135,6 +133,8 @@ class VideoFrameCapturerLinux : public VideoFrameCapturer {
   void FastBlit(uint8* image, const SkIRect& rect, CaptureData* capture_data);
   void SlowBlit(uint8* image, const SkIRect& rect, CaptureData* capture_data);
 
+  Delegate* delegate_;
+
   // X11 graphics context.
   Display* display_;
   GC gc_;
@@ -159,9 +159,6 @@ class VideoFrameCapturerLinux : public VideoFrameCapturer {
   // recently captured screen.
   VideoFrameCapturerHelper helper_;
 
-  // Callback notified whenever the cursor shape is changed.
-  CursorShapeChangedCallback cursor_shape_changed_callback_;
-
   // Capture state.
   static const int kNumBuffers = 2;
   VideoFrameBuffer buffers_[kNumBuffers];
@@ -184,7 +181,8 @@ class VideoFrameCapturerLinux : public VideoFrameCapturer {
 };
 
 VideoFrameCapturerLinux::VideoFrameCapturerLinux()
-    : display_(NULL),
+    : delegate_(NULL),
+      display_(NULL),
       gc_(NULL),
       root_window_(BadValue),
       has_xfixes_(false),
@@ -293,9 +291,10 @@ void VideoFrameCapturerLinux::InitXDamage() {
   LOG(INFO) << "Using XDamage extension.";
 }
 
-void VideoFrameCapturerLinux::Start(
-    const CursorShapeChangedCallback& callback) {
-  cursor_shape_changed_callback_ = callback;
+void VideoFrameCapturerLinux::Start(Delegate* delegate) {
+  DCHECK(delegate_ == NULL);
+
+  delegate_ = delegate;
 }
 
 void VideoFrameCapturerLinux::Stop() {
@@ -309,8 +308,7 @@ void VideoFrameCapturerLinux::InvalidateRegion(const SkRegion& invalid_region) {
   helper_.InvalidateRegion(invalid_region);
 }
 
-void VideoFrameCapturerLinux::CaptureInvalidRegion(
-    const CaptureCompletedCallback& callback) {
+void VideoFrameCapturerLinux::CaptureInvalidRegion() {

[simonmorris, 2012/11/07 20:35:21]

DCHECK(delegate_ != NULL) ?

Or, looking at the removed line 371, return here if delegate_ is NULL?

[alexeypa (please no reviews), 2012/11/07 20:54:41]

No, why? It will crash nicely if |delegate_| is NULL. 

   // Process XEvents for XDamage and cursor shape tracking.
   ProcessPendingXEvents();
 
@@ -337,7 +335,7 @@ void VideoFrameCapturerLinux::CaptureInvalidRegion(
   last_buffer_ = current_buffer_;
   current_buffer_ = (current_buffer_ + 1) % kNumBuffers;
 
-  callback.Run(capture_data);
+  delegate_->OnCaptureCompleted(capture_data);
 }
 
 void VideoFrameCapturerLinux::ProcessPendingXEvents() {
@@ -368,8 +366,6 @@ void VideoFrameCapturerLinux::ProcessPendingXEvents() {
 
 void VideoFrameCapturerLinux::CaptureCursor() {
   DCHECK(has_xfixes_);

[simonmorris, 2012/11/07 20:35:21]

Is it clearly OK to remove the check? It's probably right to check, or DCHECK,
whether delegate_ is NULL.

[alexeypa (please no reviews), 2012/11/07 20:54:41]

Yes. It will crash if |delegate_| is NULL. No need for a separate CHECK or
DCHECK. This class is useless if |delegate_| is NULL, so crashing is a good way
to say what went wrong.

[simonmorris, 2012/11/07 21:08:57]

There is a check for delegate_ being NULL, so I agree there's no real need
to check it again.

-  if (cursor_shape_changed_callback_.is_null())
-    return;
 
   XFixesCursorImage* img = XFixesGetCursorImage(display_);
   if (!img) {
@@ -400,7 +396,7 @@ void VideoFrameCapturerLinux::CaptureCursor() {
   }
   XFree(img);
 
-  cursor_shape_changed_callback_.Run(cursor_proto.Pass());
+  delegate_->OnCursorShapeChanged(cursor_proto.Pass());
 }
 
 CaptureData* VideoFrameCapturerLinux::CaptureFrame() {