Use webrtc::DesktopCapturer for screen capturer implementation.

remoting/host/desktop_session_agent.cc

Index: remoting/host/desktop_session_agent.cc
diff --git a/remoting/host/desktop_session_agent.cc b/remoting/host/desktop_session_agent.cc
index d3c8a3cfb975a263999f686969c2290e72388dbd..788101452ef018b5be5b18166e5023054b7a00bb 100644
--- a/remoting/host/desktop_session_agent.cc
+++ b/remoting/host/desktop_session_agent.cc
@@ -6,10 +6,10 @@
 
 #include "base/file_util.h"
 #include "base/logging.h"
+#include "base/memory/shared_memory.h"
 #include "ipc/ipc_channel_proxy.h"
 #include "ipc/ipc_message.h"
 #include "ipc/ipc_message_macros.h"
-#include "media/video/capture/screen/screen_capture_data.h"
 #include "remoting/base/auto_thread_task_runner.h"
 #include "remoting/base/constants.h"
 #include "remoting/host/audio_capturer.h"
@@ -24,7 +24,8 @@
 #include "remoting/proto/event.pb.h"
 #include "remoting/protocol/clipboard_stub.h"
 #include "remoting/protocol/input_event_tracker.h"
-#include "third_party/skia/include/core/SkRegion.h"
+#include "third_party/webrtc/modules/desktop_capture/desktop_frame.h"
+#include "third_party/webrtc/modules/desktop_capture/shared_memory.h"
 
 namespace remoting {
 
@@ -62,6 +63,47 @@ void DesktopSesssionClipboardStub::InjectClipboardEvent(
 
 }  // namespace
 
+class DesktopSessionAgent::SharedBuffer : public webrtc::SharedMemory {
+ public:
+  static scoped_ptr<SharedBuffer> Create(DesktopSessionAgent* agent,
+                                         size_t size,
+                                         int id) {
+    scoped_ptr<base::SharedMemory> memory(new base::SharedMemory());
+    if (!memory->CreateAndMapAnonymous(size))
+      return scoped_ptr<SharedBuffer>();
+    return scoped_ptr<SharedBuffer>(
+        new SharedBuffer(agent, memory.Pass(), size, id));
+  }
+
+  virtual ~SharedBuffer() {
+    if (agent_)
+      agent_->OnSharedBufferDeleted(id());

[alexeypa (please no reviews), 2013/04/26 21:33:58]

This class has the same confusing relationships with the owning class as the
EmittedBuffer. I'm having hard time following the logic. Is there a better way
to express and document how ownership of the frames passed around?

[Sergey Ulanov, 2013/05/07 22:25:50]

Actually it's much simpler here - we just need SharedMemory implementation that
can notify DesktopSessionAgent when it's deleted. Simplified it a bit -
DetachFromAgent() is not needed because the agent should always outlive
instances of this class. Also added a short comment to make it clear what this
class is for.

[alexeypa (please no reviews), 2013/05/08 22:24:59]

Sounds good.

+  }
+
+  void DetachFromAgent() { agent_ = NULL; }
+
+ private:
+  SharedBuffer(DesktopSessionAgent* agent,
+               scoped_ptr<base::SharedMemory> memory,
+               size_t size,
+               int id)
+      : SharedMemory(memory->memory(), size,
+#if defined(OS_WIN)
+                     memory->handle(),
+#else
+                     memory->handle().fd,
+#endif
+                     id),
+        agent_(agent),
+        shared_memory_(memory.Pass()) {
+  }
+
+  DesktopSessionAgent* agent_;
+  scoped_ptr<base::SharedMemory> shared_memory_;
+
+  DISALLOW_COPY_AND_ASSIGN(SharedBuffer);
+};
+
 DesktopSessionAgent::Delegate::~Delegate() {
 }
 
@@ -72,6 +114,12 @@ DesktopSessionAgent::~DesktopSessionAgent() {
   DCHECK(!screen_controls_);
   DCHECK(!video_capturer_);
 
+  for (SharedBuffers::iterator i = shared_buffers_.begin();
+       i != shared_buffers_.end(); ++i) {
+    i->second->DetachFromAgent();
+  }
+  shared_buffers_.clear();
+
   CloseDesktopPipeHandle();
 }
 
@@ -83,8 +131,6 @@ bool DesktopSessionAgent::OnMessageReceived(const IPC::Message& message) {
     IPC_BEGIN_MESSAGE_MAP(DesktopSessionAgent, message)
       IPC_MESSAGE_HANDLER(ChromotingNetworkDesktopMsg_CaptureFrame,
                           OnCaptureFrame)
-      IPC_MESSAGE_HANDLER(ChromotingNetworkDesktopMsg_SharedBufferCreated,
-                          OnSharedBufferCreated)

[alexeypa (please no reviews), 2013/04/26 21:33:58]

Removing this message will introduce a race between the network and desktop
processes. Consider the following sequence:

1. Desktop creates a frame A.
2. Desktop sends frame A to the network process.
3. Desktop deletes the frame A because the screen resolution changed.
4. Network receives frame A and fails to map memory.

This message is needed to implement shared between the two processes ownership
of the video frames.

[Sergey Ulanov, 2013/05/07 22:25:50]

I added |last_frame_| to make sure we hold the shared buffer before it's
received. Previously shared buffer was ref-counted, and it's not anymore, which
makes it harder to implement this message correctly. Let me know what you think
about |last_frame_| solution.

[alexeypa (please no reviews), 2013/05/08 22:24:59]

|last_frame_| will nit work either. First there may be two frames in flight.
Second, DesktopSessoinAgent has to receive confirmation from DesktopSessioProxy
that the latter saw the frame before DesktopSessoinAgent can delete the frame.
Otherwise it might delete it prematurely.

[Sergey Ulanov, 2013/05/09 18:49:02]

There assumption here is that DesktopCapturer::Capture() cannot be called until
OnCaptureCompleted() callback - this means that we will not receive
ChromotingNetworkDesktopMsg_CaptureFrame will not be received here until the
previous ChromotingDesktopNetworkMsg_CaptureCompleted is received by the network
process. 
Looks like our current VideoScheduler may call Capture() before previous capture
has completed - I think it's wrong and should be fixed (it still should be able
to process multiple frames at the same time).
Anyway, let's discuss this in person.

       IPC_MESSAGE_HANDLER(ChromotingNetworkDesktopMsg_InjectClipboardEvent,
                           OnInjectClipboardEvent)
       IPC_MESSAGE_HANDLER(ChromotingNetworkDesktopMsg_InjectKeyEvent,
@@ -127,14 +173,13 @@ void DesktopSessionAgent::OnChannelError() {
     delegate_->OnNetworkProcessDisconnected();
 }
 
-scoped_refptr<media::SharedBuffer> DesktopSessionAgent::CreateSharedBuffer(
-    uint32 size) {
+webrtc::SharedMemory* DesktopSessionAgent::CreateSharedMemory(size_t size) {
   DCHECK(video_capture_task_runner()->BelongsToCurrentThread());
 
-  scoped_refptr<media::SharedBuffer> buffer = new media::SharedBuffer(size);
-  if (buffer->ptr() != NULL) {
-    buffer->set_id(next_shared_buffer_id_);
-    shared_buffers_.push_back(buffer);
+  scoped_ptr<SharedBuffer> buffer =
+      SharedBuffer::Create(this, size, next_shared_buffer_id_);
+  if (buffer) {
+    shared_buffers_[buffer->id()] = buffer.get();
 
     // |next_shared_buffer_id_| starts from 1 and incrementing it by 2 makes
     // sure it is always odd and therefore zero is never used as a valid buffer
@@ -146,20 +191,17 @@ scoped_refptr<media::SharedBuffer> DesktopSessionAgent::CreateSharedBuffer(
     // speaking it never happens.
     next_shared_buffer_id_ += 2;
 
+    IPC::PlatformFileForTransit handle;
+#if defined(OS_WIN)
+    handle = buffer->handle();
+#else
+    handle = base::FileDescriptor(buffer->handle(), false);
+#endif
     SendToNetwork(new ChromotingDesktopNetworkMsg_CreateSharedBuffer(
-        buffer->id(), buffer->handle(), buffer->size()));
+        buffer->id(), handle, buffer->size()));
   }
 
-  return buffer;
-}
-
-void DesktopSessionAgent::ReleaseSharedBuffer(
-    scoped_refptr<media::SharedBuffer> buffer) {
-  DCHECK(video_capture_task_runner()->BelongsToCurrentThread());
-  DCHECK(buffer->id() != 0);
-
-  SendToNetwork(
-      new ChromotingDesktopNetworkMsg_ReleaseSharedBuffer(buffer->id()));
+  return buffer.release();
 }
 
 const std::string& DesktopSessionAgent::client_jid() const {
@@ -236,26 +278,27 @@ void DesktopSessionAgent::OnStartSessionAgent(
       FROM_HERE, base::Bind(&DesktopSessionAgent::StartVideoCapturer, this));
 }
 
-void DesktopSessionAgent::OnCaptureCompleted(
-    scoped_refptr<media::ScreenCaptureData> capture_data) {
+void DesktopSessionAgent::OnCaptureCompleted(webrtc::DesktopFrame* frame) {
   DCHECK(video_capture_task_runner()->BelongsToCurrentThread());
 
-  current_size_ = capture_data->size();
+  last_frame_.reset(frame);
 
-  // Serialize media::ScreenCaptureData
-  SerializedCapturedData serialized_data;
-  serialized_data.shared_buffer_id = capture_data->shared_buffer()->id();
-  serialized_data.bytes_per_row = capture_data->stride();
-  serialized_data.dimensions = capture_data->size();
-  serialized_data.capture_time_ms = capture_data->capture_time_ms();
-  serialized_data.client_sequence_number =
-      capture_data->client_sequence_number();
-  serialized_data.dpi = capture_data->dpi();
-  for (SkRegion::Iterator i(capture_data->dirty_region()); !i.done(); i.next())
-    serialized_data.dirty_region.push_back(i.rect());
+  current_size_ = frame->size();
+
+  // Serialize webrtc::DesktopFrame.
+  SerializedDesktopFrame serialized_frame;
+  serialized_frame.shared_buffer_id = frame->shared_memory()->id();
+  serialized_frame.bytes_per_row = frame->stride();
+  serialized_frame.dimensions = frame->size();
+  serialized_frame.capture_time_ms = frame->capture_time_ms();
+  serialized_frame.dpi = frame->dpi();
+  for (webrtc::DesktopRegion::Iterator i(frame->updated_region());
+       !i.IsAtEnd(); i.Advance()) {
+    serialized_frame.dirty_region.push_back(i.rect());
+  }
 
   SendToNetwork(
-      new ChromotingDesktopNetworkMsg_CaptureCompleted(serialized_data));
+      new ChromotingDesktopNetworkMsg_CaptureCompleted(serialized_frame));
 }
 
 void DesktopSessionAgent::OnCursorShapeChanged(
@@ -354,25 +397,7 @@ void DesktopSessionAgent::OnCaptureFrame() {
   // runner. If the client issues more requests, pixel data in captured frames
   // will likely be corrupted but stability of media::ScreenCapturer will not be
   // affected.
-  video_capturer_->CaptureFrame();
-}
-
-void DesktopSessionAgent::OnSharedBufferCreated(int id) {
-  if (!video_capture_task_runner()->BelongsToCurrentThread()) {
-    video_capture_task_runner()->PostTask(
-        FROM_HERE,
-        base::Bind(&DesktopSessionAgent::OnSharedBufferCreated, this, id));
-    return;
-  }
-
-  // Drop the cached reference to the buffer.
-  SharedBuffers::iterator i = shared_buffers_.begin();
-  for (; i != shared_buffers_.end(); ++i) {
-    if ((*i)->id() == id) {
-      shared_buffers_.erase(i);
-      break;
-    }
-  }
+  video_capturer_->Capture(webrtc::DesktopRegion());
 }
 
 void DesktopSessionAgent::OnInjectClipboardEvent(
@@ -429,7 +454,7 @@ void DesktopSessionAgent::SetScreenResolution(
     const ScreenResolution& resolution) {
   DCHECK(caller_task_runner()->BelongsToCurrentThread());
 
-  if (screen_controls_ && resolution.IsValid())
+  if (screen_controls_ && resolution.IsEmpty())
     screen_controls_->SetScreenResolution(resolution);
 }
 
@@ -466,17 +491,20 @@ void DesktopSessionAgent::StopAudioCapturer() {
 void DesktopSessionAgent::StartVideoCapturer() {
   DCHECK(video_capture_task_runner()->BelongsToCurrentThread());
 
-  if (video_capturer_)
+  if (video_capturer_) {
+    video_capturer_->SetMouseShapeObserver(this);
     video_capturer_->Start(this);
+  }
 }
 
 void DesktopSessionAgent::StopVideoCapturer() {
   DCHECK(video_capture_task_runner()->BelongsToCurrentThread());
 
   video_capturer_.reset();
+  last_frame_.reset();
 
-  // Free any shared buffers left.
-  shared_buffers_.clear();
+  // Video capturer must delete all buffers.
+  DCHECK(shared_buffers_.empty());
 }
 
 DesktopSessionAgent::DesktopSessionAgent(
@@ -492,12 +520,19 @@ DesktopSessionAgent::DesktopSessionAgent(
       video_capture_task_runner_(video_capture_task_runner),
       control_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)),
       desktop_pipe_(IPC::InvalidPlatformFileForTransit()),
-      current_size_(SkISize::Make(0, 0)),
       next_shared_buffer_id_(1),
       started_(false) {
   DCHECK(caller_task_runner_->BelongsToCurrentThread());
 }
 
+void DesktopSessionAgent::OnSharedBufferDeleted(int id) {
+  DCHECK(video_capture_task_runner()->BelongsToCurrentThread());
+  DCHECK(id != 0);
+
+  shared_buffers_.erase(id);
+  SendToNetwork(new ChromotingDesktopNetworkMsg_ReleaseSharedBuffer(id));
+}
+
 void DesktopSessionAgent::CloseDesktopPipeHandle() {
   if (!(desktop_pipe_ == IPC::InvalidPlatformFileForTransit())) {
 #if defined(OS_WIN)