Use webrtc::MouseCursorMonitor for cursor shapes

Use webrtc::MouseCursorMonitor for cursor shapes instead of
webrtc::VideoFrameCapturer, in preparation for deprecating cursor shape
functionality in the latter.

Fix memory corruption in VideoSchedulerTests_StartAndStop, where a lingering
capture task could trigger a expectation action declared on the stack during
tear down. My changes to the test somehow made the race condition more likely.

BUG=324033

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=247689

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=248045

Review URL: https://codereview.chromium.org/92473002

Cr-Commit-Position: refs/heads/master@{#288226}
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@288226 0039d316-1c4b-4281-b951-d872f2087c98

remoting/host/basic_desktop_environment.cc

@@ -12,6 +12,8 @@
 #include "remoting/host/gnubby_auth_handler.h"
 #include "remoting/host/input_injector.h"
 #include "remoting/host/screen_controls.h"
+#include "third_party/webrtc/modules/desktop_capture/desktop_capture_options.h"
+#include "third_party/webrtc/modules/desktop_capture/mouse_cursor_monitor.h"
 #include "third_party/webrtc/modules/desktop_capture/screen_capturer.h"
 
 namespace remoting {
@@ -38,6 +40,14 @@ scoped_ptr<ScreenControls> BasicDesktopEnvironment::CreateScreenControls() {
   return scoped_ptr<ScreenControls>();
 }
 
+scoped_ptr<webrtc::MouseCursorMonitor>
+BasicDesktopEnvironment::CreateMouseCursorMonitor() {
+  return scoped_ptr<webrtc::MouseCursorMonitor>(
+      webrtc::MouseCursorMonitor::CreateForScreen(
+          *desktop_capture_options_,
+          webrtc::kFullDesktopScreenId));
+}
+
 std::string BasicDesktopEnvironment::GetCapabilities() const {
   return std::string();
 }
@@ -56,7 +66,8 @@ BasicDesktopEnvironment::CreateVideoCapturer() {
 
   // The basic desktop environment does not use X DAMAGE, since it is
   // broken on many systems - see http://crbug.com/73423.
-  return scoped_ptr<webrtc::ScreenCapturer>(webrtc::ScreenCapturer::Create());
+  return scoped_ptr<webrtc::ScreenCapturer>(
+      webrtc::ScreenCapturer::Create(*desktop_capture_options_));
 }
 
 BasicDesktopEnvironment::BasicDesktopEnvironment(
@@ -65,7 +76,10 @@ BasicDesktopEnvironment::BasicDesktopEnvironment(
     scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner)
     : caller_task_runner_(caller_task_runner),
       input_task_runner_(input_task_runner),
-      ui_task_runner_(ui_task_runner) {
+      ui_task_runner_(ui_task_runner),
+      desktop_capture_options_(
+          new webrtc::DesktopCaptureOptions(
+              webrtc::DesktopCaptureOptions::CreateDefault())) {
   DCHECK(caller_task_runner_->BelongsToCurrentThread());
 }
 

remoting/host/basic_desktop_environment.h

@@ -13,6 +13,12 @@
 #include "base/memory/scoped_ptr.h"
 #include "remoting/host/desktop_environment.h"
 
+namespace webrtc {
+
+class DesktopCaptureOptions;
+
+}  // namespace webrtc
+
 namespace remoting {
 
 class GnubbyAuthHandler;
@@ -28,6 +34,8 @@ class BasicDesktopEnvironment : public DesktopEnvironment {
   virtual scoped_ptr<InputInjector> CreateInputInjector() OVERRIDE;
   virtual scoped_ptr<ScreenControls> CreateScreenControls() OVERRIDE;
   virtual scoped_ptr<webrtc::ScreenCapturer> CreateVideoCapturer() OVERRIDE;
+  virtual scoped_ptr<webrtc::MouseCursorMonitor> CreateMouseCursorMonitor()
+      OVERRIDE;
   virtual std::string GetCapabilities() const OVERRIDE;
   virtual void SetCapabilities(const std::string& capabilities) OVERRIDE;
   virtual scoped_ptr<GnubbyAuthHandler> CreateGnubbyAuthHandler(
@@ -53,6 +61,10 @@ class BasicDesktopEnvironment : public DesktopEnvironment {
     return ui_task_runner_;
   }
 
+  webrtc::DesktopCaptureOptions* desktop_capture_options() {
+    return desktop_capture_options_.get();
+  }
+
  private:
   // Task runner on which methods of DesktopEnvironment interface should be
   // called.
@@ -64,6 +76,13 @@ class BasicDesktopEnvironment : public DesktopEnvironment {
   // Used to run UI code.
   scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner_;
 
+  // Options shared between |ScreenCapturer| and |MouseCursorMonitor|. It
+  // might contain expensive resources, thus justifying the sharing.
+  // Also: it's dynamically allocated to avoid having to bring in
+  // desktop_capture_options.h which brings in X11 headers which causes hard to
+  // find build errors.
+  scoped_ptr<webrtc::DesktopCaptureOptions> desktop_capture_options_;
+
   DISALLOW_COPY_AND_ASSIGN(BasicDesktopEnvironment);
 };
 

remoting/host/chromoting_host_unittest.cc

@@ -11,6 +11,7 @@
 #include "remoting/host/chromoting_host.h"
 #include "remoting/host/chromoting_host_context.h"
 #include "remoting/host/desktop_environment.h"
+#include "remoting/host/fake_mouse_cursor_monitor.h"
 #include "remoting/host/fake_screen_capturer.h"
 #include "remoting/host/host_mock_objects.h"
 #include "remoting/proto/video.pb.h"
@@ -249,6 +250,9 @@ class ChromotingHostTest : public testing::Test {
     EXPECT_CALL(*desktop_environment, CreateVideoCapturerPtr())
         .Times(AtMost(1))
         .WillOnce(Invoke(this, &ChromotingHostTest::CreateVideoCapturer));
+    EXPECT_CALL(*desktop_environment, CreateMouseCursorMonitorPtr())
+        .Times(AtMost(1))
+        .WillOnce(Invoke(this, &ChromotingHostTest::CreateMouseCursorMonitor));
     EXPECT_CALL(*desktop_environment, GetCapabilities())
         .Times(AtMost(1));
     EXPECT_CALL(*desktop_environment, SetCapabilities(_))
@@ -271,6 +275,12 @@ class ChromotingHostTest : public testing::Test {
     return new FakeScreenCapturer();
   }
 
+  // Creates a MockMouseCursorMonitor, to mock
+  // DesktopEnvironment::CreateMouseCursorMonitor().
+  webrtc::MouseCursorMonitor* CreateMouseCursorMonitor() {
+    return new FakeMouseCursorMonitor();
+  }
+
   void DisconnectAllClients() {
     host_->DisconnectAllClients();
   }

remoting/host/chromoting_messages.h

@@ -144,12 +144,6 @@ IPC_MESSAGE_CONTROL3(ChromotingDesktopNetworkMsg_CreateSharedBuffer,
 IPC_MESSAGE_CONTROL1(ChromotingDesktopNetworkMsg_ReleaseSharedBuffer,
                      int /* id */)
 
-IPC_STRUCT_TRAITS_BEGIN(webrtc::MouseCursorShape)
-  IPC_STRUCT_TRAITS_MEMBER(size)
-  IPC_STRUCT_TRAITS_MEMBER(hotspot)
-  IPC_STRUCT_TRAITS_MEMBER(data)
-IPC_STRUCT_TRAITS_END()
-
 // Serialized webrtc::DesktopFrame.
 IPC_STRUCT_BEGIN(SerializedDesktopFrame)
   // ID of the shared memory buffer containing the pixels.
@@ -179,8 +173,8 @@ IPC_MESSAGE_CONTROL1(ChromotingDesktopNetworkMsg_CaptureCompleted,
                      SerializedDesktopFrame /* frame */ )
 
 // Carries a cursor share update from the desktop session agent to the client.
-IPC_MESSAGE_CONTROL1(ChromotingDesktopNetworkMsg_CursorShapeChanged,
-                     webrtc::MouseCursorShape /* cursor_shape */ )
+IPC_MESSAGE_CONTROL1(ChromotingDesktopNetworkMsg_MouseCursor,
+                     webrtc::MouseCursor /* cursor */ )
 
 // Carries a clipboard event from the desktop session agent to the client.
 // |serialized_event| is a serialized protocol::ClipboardEvent.

remoting/host/chromoting_param_traits.cc

@@ -5,6 +5,7 @@
 #include "remoting/host/chromoting_param_traits.h"
 
 #include "base/strings/stringprintf.h"
+#include "third_party/webrtc/modules/desktop_capture/desktop_frame.h"
 
 namespace IPC {
 
@@ -87,6 +88,72 @@ void ParamTraits<webrtc::DesktopRect>::Log(const webrtc::DesktopRect& p,
                                p.left(), p.top(), p.right(), p.bottom()));
 }
 
+// static
+void ParamTraits<webrtc::MouseCursor>::Write(
+    Message* m,
+    const webrtc::MouseCursor& p) {
+  ParamTraits<webrtc::DesktopSize>::Write(m, p.image()->size());
+
+  // Data is serialized in such a way that size is exactly width * height *
+  // |kBytesPerPixel|.
+  std::string data;
+  uint8_t* current_row = p.image()->data();
+  for (int y = 0; y < p.image()->size().height(); ++y) {
+    data.append(current_row,
+                current_row + p.image()->size().width() *
+                    webrtc::DesktopFrame::kBytesPerPixel);
+    current_row += p.image()->stride();
+  }
+  m->WriteData(reinterpret_cast<const char*>(p.image()->data()), data.size());
+
+  ParamTraits<webrtc::DesktopVector>::Write(m, p.hotspot());
+}
+
+// static
+bool ParamTraits<webrtc::MouseCursor>::Read(
+    const Message* m,
+    PickleIterator* iter,
+    webrtc::MouseCursor* r) {
+  webrtc::DesktopSize size;
+  if (!ParamTraits<webrtc::DesktopSize>::Read(m, iter, &size) ||
+      size.width() <= 0 || size.width() > (SHRT_MAX / 2) ||
+      size.height() <= 0 || size.height() > (SHRT_MAX / 2)) {
+    return false;
+  }
+
+  const int expected_length =
+      size.width() * size.height() * webrtc::DesktopFrame::kBytesPerPixel;
+
+  const char* data;
+  int data_length;
+  if (!m->ReadData(iter, &data, &data_length) ||
+      data_length != expected_length) {
+    return false;
+  }
+
+  webrtc::DesktopVector hotspot;
+  if (!ParamTraits<webrtc::DesktopVector>::Read(m, iter, &hotspot))
+    return false;
+
+  webrtc::BasicDesktopFrame* image = new webrtc::BasicDesktopFrame(size);
+  memcpy(image->data(), data, data_length);
+
+  r->set_image(image);
+  r->set_hotspot(hotspot);
+  return true;
+}
+
+// static
+void ParamTraits<webrtc::MouseCursor>::Log(
+    const webrtc::MouseCursor& p,
+    std::string* l) {
+  l->append(base::StringPrintf(
+      "webrtc::DesktopRect{image(%d, %d), hotspot(%d, %d)}",
+      p.image()->size().width(), p.image()->size().height(),
+      p.hotspot().x(), p.hotspot().y()));
+}
+
+
 // static
 void ParamTraits<remoting::ScreenResolution>::Write(
     Message* m,

remoting/host/chromoting_param_traits.h

@@ -8,7 +8,9 @@
 #include "ipc/ipc_message.h"
 #include "ipc/ipc_param_traits.h"
 #include "remoting/host/screen_resolution.h"
+#include "third_party/webrtc/modules/desktop_capture/desktop_frame.h"
 #include "third_party/webrtc/modules/desktop_capture/desktop_geometry.h"
+#include "third_party/webrtc/modules/desktop_capture/mouse_cursor.h"
 
 namespace IPC {
 
@@ -36,6 +38,14 @@ struct ParamTraits<webrtc::DesktopRect> {
   static void Log(const param_type& p, std::string* l);
 };
 
+template <>
+struct ParamTraits<webrtc::MouseCursor> {
+  typedef webrtc::MouseCursor param_type;
+  static void Write(Message* m, const param_type& p);
+  static bool Read(const Message* m, PickleIterator* iter, param_type* r);
+  static void Log(const param_type& p, std::string* l);
+};
+
 template <>
 struct ParamTraits<remoting::ScreenResolution> {
   typedef remoting::ScreenResolution param_type;

remoting/host/client_session.cc

@@ -472,6 +472,7 @@ void ClientSession::ResetVideoPipeline() {
       video_encode_task_runner_,
       network_task_runner_,
       video_capturer.Pass(),
+      desktop_environment_->CreateMouseCursorMonitor(),
       video_encoder.Pass(),
       connection_->client_stub(),
       &mouse_clamping_filter_);

remoting/host/client_session_unittest.cc

@@ -16,6 +16,7 @@
 #include "remoting/host/client_session.h"
 #include "remoting/host/desktop_environment.h"
 #include "remoting/host/fake_host_extension.h"
+#include "remoting/host/fake_mouse_cursor_monitor.h"
 #include "remoting/host/fake_screen_capturer.h"
 #include "remoting/host/host_extension.h"
 #include "remoting/host/host_extension_session.h"
@@ -146,6 +147,10 @@ class ClientSessionTest : public testing::Test {
   // DesktopEnvironment::CreateVideoCapturer().
   webrtc::ScreenCapturer* CreateVideoCapturer();
 
+  // Creates a MockMouseCursorMonitor, to mock
+  // DesktopEnvironment::CreateMouseCursorMonitor
+  webrtc::MouseCursorMonitor* CreateMouseCursorMonitor();
+
   // Notifies the client session that the client connection has been
   // authenticated and channels have been connected. This effectively enables
   // the input pipe line and starts video capturing.
@@ -279,6 +284,9 @@ DesktopEnvironment* ClientSessionTest::CreateDesktopEnvironment() {
       .Times(AtMost(1));
   EXPECT_CALL(*desktop_environment, CreateVideoCapturerPtr())
       .WillRepeatedly(Invoke(this, &ClientSessionTest::CreateVideoCapturer));
+  EXPECT_CALL(*desktop_environment, CreateMouseCursorMonitorPtr())
+      .WillRepeatedly(
+          Invoke(this, &ClientSessionTest::CreateMouseCursorMonitor));
   EXPECT_CALL(*desktop_environment, GetCapabilities())
       .Times(AtMost(1))
        .WillOnce(Return(kDefaultTestCapability));
@@ -297,6 +305,10 @@ webrtc::ScreenCapturer* ClientSessionTest::CreateVideoCapturer() {
   return new FakeScreenCapturer();
 }
 
+webrtc::MouseCursorMonitor* ClientSessionTest::CreateMouseCursorMonitor() {
+  return new FakeMouseCursorMonitor();
+}
+
 void ClientSessionTest::ConnectClientSession() {
   client_session_->OnConnectionAuthenticated(client_session_->connection());
   client_session_->OnConnectionChannelsConnected(client_session_->connection());

remoting/host/desktop_environment.h

@@ -19,6 +19,7 @@ class SingleThreadTaskRunner;
 
 namespace webrtc {
 class ScreenCapturer;
+class MouseCursorMonitor;
 }  // namespace webrtc
 
 namespace remoting {
@@ -45,6 +46,7 @@ class DesktopEnvironment {
   virtual scoped_ptr<InputInjector> CreateInputInjector() = 0;
   virtual scoped_ptr<ScreenControls> CreateScreenControls() = 0;
   virtual scoped_ptr<webrtc::ScreenCapturer> CreateVideoCapturer() = 0;
+  virtual scoped_ptr<webrtc::MouseCursorMonitor> CreateMouseCursorMonitor() = 0;
 
   // Returns the set of all capabilities supported by |this|.
   virtual std::string GetCapabilities() const = 0;