Improve relative mouse support on Windows.

The DesktopCapturer implementation used by ClientSession on Windows
uses read-only shared memory, and so the mouse cursor composition needs
to be done by the DesktopSessionAgent instead. This CL refactors that
logic so that it can be used in both places, cleans up some raw pointer
ugliness, and adds support for compositing the mouse cursor on Windows
when pointer lock is active.

Bug: 1047908
Change-Id: Ic21a750379a8f144227d662cce200cdc95ce3967
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2034104
Commit-Queue: Jamie Walch <jamiewalch@chromium.org>
Reviewed-by: Joe Downing <joedow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#738003}

remoting/host/BUILD.gn

@@ -129,6 +129,8 @@ static_library("common") {
     "daemon_process.cc",
     "daemon_process.h",
     "daemon_process_win.cc",
+    "desktop_and_cursor_conditional_composer.cc",
+    "desktop_and_cursor_conditional_composer.h",
     "desktop_capturer_checker.cc",
     "desktop_capturer_checker.h",
     "desktop_capturer_proxy.cc",

remoting/host/basic_desktop_environment.cc

@@ -94,6 +94,17 @@ uint32_t BasicDesktopEnvironment::GetDesktopSessionId() const {
   return UINT32_MAX;
 }
 
+std::unique_ptr<DesktopAndCursorConditionalComposer>
+BasicDesktopEnvironment::CreateComposingVideoCapturer() {
+#if defined(OS_MACOSX)
+  // MacOS always includes the mouse cursor in the captured image.
+  return nullptr;
+#else
+  return std::make_unique<DesktopAndCursorConditionalComposer>(
+      CreateVideoCapturer());
+#endif
+}
+
 std::unique_ptr<webrtc::DesktopCapturer>
 BasicDesktopEnvironment::CreateVideoCapturer() {
   DCHECK(caller_task_runner_->BelongsToCurrentThread());

remoting/host/basic_desktop_environment.h

@@ -47,6 +47,8 @@ class BasicDesktopEnvironment : public DesktopEnvironment {
   std::string GetCapabilities() const override;
   void SetCapabilities(const std::string& capabilities) override;
   uint32_t GetDesktopSessionId() const override;
+  std::unique_ptr<DesktopAndCursorConditionalComposer>
+  CreateComposingVideoCapturer() override;
 
  protected:
   friend class BasicDesktopEnvironmentFactory;

remoting/host/client_session.cc

@@ -334,20 +334,14 @@ void ClientSession::CreateMediaStreams() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
   // Create a VideoStream to pump frames from the capturer to the client.
-#if defined(OS_MACOSX) || defined(OS_WIN)
-  // On macOS the capturer includes the mouse cursor.
-  video_stream_ = connection_->StartVideoStream(
-      desktop_environment_->CreateVideoCapturer());
-#else
-  // On other platforms the mouse cursor is not present in the captured video,
-  // so the capturer must be wrapped in a DesktopAndCursorComposer so that the
-  // cursor can be included when mouse lock is enabled at the client.
-  auto composer =
-      webrtc::DesktopAndCursorComposer::CreateWithoutMouseCursorMonitor(
-          desktop_environment_->CreateVideoCapturer());
-  desktop_and_cursor_composer_raw_ = composer.get();
-  video_stream_ = connection_->StartVideoStream(std::move(composer));
-#endif
+  auto composer = desktop_environment_->CreateComposingVideoCapturer();
+  if (composer) {
+    desktop_and_cursor_composer_ = composer->GetWeakPtr();
+    video_stream_ = connection_->StartVideoStream(std::move(composer));
+  } else {
+    video_stream_ = connection_->StartVideoStream(
+        desktop_environment_->CreateVideoCapturer());
+  }
 
   // Create a AudioStream to pump audio from the capturer to the client.
   std::unique_ptr<protocol::AudioSource> audio_capturer =
@@ -426,7 +420,7 @@ void ClientSession::OnConnectionClosed(protocol::ErrorCode error) {
 
   // Stop components access the client, audio or video stubs, which are no
   // longer valid once ConnectionToClient calls OnConnectionClosed().
-  desktop_and_cursor_composer_raw_ = nullptr;
+  desktop_and_cursor_composer_.reset();
   audio_stream_.reset();
   mouse_shape_pump_.reset();
   video_stream_.reset();
@@ -504,30 +498,22 @@ ClientSessionControl* ClientSession::session_control() {
 }
 
 void ClientSession::OnPointerLockChanged(bool active) {
-  if (active) {
-    if (mouse_cursor_ && desktop_and_cursor_composer_raw_)
-      desktop_and_cursor_composer_raw_->OnMouseCursor(
-          webrtc::MouseCursor::CopyOf(*mouse_cursor_));
-  } else if (desktop_and_cursor_composer_raw_) {
-    webrtc::MouseCursor* empty = new webrtc::MouseCursor(
-        new webrtc::BasicDesktopFrame(webrtc::DesktopSize(0, 0)),
-        webrtc::DesktopVector(0, 0));
-    desktop_and_cursor_composer_raw_->OnMouseCursor(empty);
-  }
-  pointer_lock_active_ = active;
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  if (desktop_and_cursor_composer_)
+    desktop_and_cursor_composer_->SetComposeEnabled(active);
 }
 
 void ClientSession::OnMouseCursor(webrtc::MouseCursor* mouse_cursor) {
-  mouse_cursor_.reset(mouse_cursor);
-  if (pointer_lock_active_ && desktop_and_cursor_composer_raw_)
-    desktop_and_cursor_composer_raw_->OnMouseCursor(
-        webrtc::MouseCursor::CopyOf(*mouse_cursor_));
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  if (desktop_and_cursor_composer_)
+    desktop_and_cursor_composer_->SetMouseCursor(mouse_cursor);
 }
 
 void ClientSession::OnMouseCursorPosition(
     const webrtc::DesktopVector& position) {
-  if (pointer_lock_active_ && desktop_and_cursor_composer_raw_)
-    desktop_and_cursor_composer_raw_->OnMouseCursorPosition(position);
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  if (desktop_and_cursor_composer_)
+    desktop_and_cursor_composer_->SetMouseCursorPosition(position);
 }
 
 void ClientSession::RegisterCreateHandlerCallbackForTesting(

remoting/host/client_session.h

@@ -18,6 +18,7 @@
 #include "base/timer/timer.h"
 #include "remoting/host/client_session_control.h"
 #include "remoting/host/client_session_details.h"
+#include "remoting/host/desktop_and_cursor_conditional_composer.h"
 #include "remoting/host/desktop_display_info.h"
 #include "remoting/host/desktop_environment_options.h"
 #include "remoting/host/host_experiment_session_plugin.h"
@@ -38,7 +39,6 @@
 #include "remoting/protocol/mouse_input_filter.h"
 #include "remoting/protocol/pairing_registry.h"
 #include "remoting/protocol/video_stream.h"
-#include "third_party/webrtc/modules/desktop_capture/desktop_and_cursor_composer.h"
 #include "third_party/webrtc/modules/desktop_capture/desktop_capture_types.h"
 #include "third_party/webrtc/modules/desktop_capture/desktop_geometry.h"
 #include "third_party/webrtc/modules/desktop_capture/mouse_cursor.h"
@@ -316,13 +316,8 @@ class ClientSession : public protocol::HostStub,
   std::unique_ptr<MouseShapePump> mouse_shape_pump_;
   std::unique_ptr<KeyboardLayoutMonitor> keyboard_layout_monitor_;
 
-  // Raw pointer to the DesktopAndCursorComposer, owned by |video_stream_|.
-  // TODO(crbug.com/1043325): Replace this with something more robust if the
-  // relative pointer experiment is a success.
-  webrtc::DesktopAndCursorComposer* desktop_and_cursor_composer_raw_ = nullptr;
-
-  std::unique_ptr<webrtc::MouseCursor> mouse_cursor_;
-  bool pointer_lock_active_ = false;
+  base::WeakPtr<DesktopAndCursorConditionalComposer>
+      desktop_and_cursor_composer_;
 
   SEQUENCE_CHECKER(sequence_checker_);
 

remoting/host/desktop_and_cursor_conditional_composer.cc

+// Copyright 2020 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.
+
+#include "remoting/host/desktop_and_cursor_conditional_composer.h"
+
+namespace remoting {
+
+DesktopAndCursorConditionalComposer::DesktopAndCursorConditionalComposer(
+    std::unique_ptr<webrtc::DesktopCapturer> desktop_capturer)
+    : capturer_(
+          webrtc::DesktopAndCursorComposer::CreateWithoutMouseCursorMonitor(
+              std::move(desktop_capturer))) {}
+
+DesktopAndCursorConditionalComposer::~DesktopAndCursorConditionalComposer() =
+    default;
+
+base::WeakPtr<DesktopAndCursorConditionalComposer>
+DesktopAndCursorConditionalComposer::GetWeakPtr() {
+  return weak_factory_.GetWeakPtr();
+}
+
+void DesktopAndCursorConditionalComposer::SetComposeEnabled(bool enabled) {
+  if (enabled == compose_enabled_)
+    return;
+
+  if (enabled) {
+    if (mouse_cursor_)
+      capturer_->OnMouseCursor(webrtc::MouseCursor::CopyOf(*mouse_cursor_));
+  } else {
+    webrtc::MouseCursor* empty = new webrtc::MouseCursor(
+        new webrtc::BasicDesktopFrame(webrtc::DesktopSize(0, 0)),
+        webrtc::DesktopVector(0, 0));
+    capturer_->OnMouseCursor(empty);
+  }
+
+  compose_enabled_ = enabled;
+}
+
+void DesktopAndCursorConditionalComposer::SetMouseCursor(
+    webrtc::MouseCursor* mouse_cursor) {
+  mouse_cursor_.reset(mouse_cursor);
+  if (compose_enabled_)
+    capturer_->OnMouseCursor(webrtc::MouseCursor::CopyOf(*mouse_cursor_));
+}
+
+void DesktopAndCursorConditionalComposer::SetMouseCursorPosition(
+    const webrtc::DesktopVector& position) {
+  if (compose_enabled_)
+    capturer_->OnMouseCursorPosition(position);
+}
+
+void DesktopAndCursorConditionalComposer::Start(
+    webrtc::DesktopCapturer::Callback* callback) {
+  capturer_->Start(callback);
+}
+
+void DesktopAndCursorConditionalComposer::SetSharedMemoryFactory(
+    std::unique_ptr<webrtc::SharedMemoryFactory> shared_memory_factory) {
+  capturer_->SetSharedMemoryFactory(std::move(shared_memory_factory));
+}
+
+void DesktopAndCursorConditionalComposer::CaptureFrame() {
+  capturer_->CaptureFrame();
+}
+
+void DesktopAndCursorConditionalComposer::SetExcludedWindow(
+    webrtc::WindowId window) {
+  capturer_->SetExcludedWindow(window);
+}
+
+}  // namespace remoting

remoting/host/desktop_and_cursor_conditional_composer.h

+// Copyright 2020 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.
+
+#ifndef REMOTING_HOST_DESKTOP_AND_CURSOR_CONDITIONAL_COMPOSER_H_
+#define REMOTING_HOST_DESKTOP_AND_CURSOR_CONDITIONAL_COMPOSER_H_
+
+#include "base/memory/weak_ptr.h"
+#include "third_party/webrtc/modules/desktop_capture/desktop_and_cursor_composer.h"
+#include "third_party/webrtc/modules/desktop_capture/desktop_capturer.h"
+#include "third_party/webrtc/modules/desktop_capture/mouse_cursor.h"
+#include "third_party/webrtc/modules/desktop_capture/shared_memory.h"
+
+namespace remoting {
+
+// A wrapper for DesktopAndCursorComposer that allows compositing of the cursor
+// to be enabled and disabled, and which exposes a WeakPtr to simplify memory
+// management.
+class DesktopAndCursorConditionalComposer : public webrtc::DesktopCapturer {
+ public:
+  explicit DesktopAndCursorConditionalComposer(
+      std::unique_ptr<webrtc::DesktopCapturer> desktop_capturer);
+  ~DesktopAndCursorConditionalComposer() override;
+
+  base::WeakPtr<DesktopAndCursorConditionalComposer> GetWeakPtr();
+
+  void SetComposeEnabled(bool enabled);
+
+  void SetMouseCursor(webrtc::MouseCursor* mouse_cursor);
+  void SetMouseCursorPosition(const webrtc::DesktopVector& position);
+
+  // DesktopCapturer interface.
+  void Start(webrtc::DesktopCapturer::Callback* callback) override;
+  void SetSharedMemoryFactory(std::unique_ptr<webrtc::SharedMemoryFactory>
+                                  shared_memory_factory) override;
+  void CaptureFrame() override;
+  void SetExcludedWindow(webrtc::WindowId window) override;
+
+ private:
+  DesktopAndCursorConditionalComposer(
+      const DesktopAndCursorConditionalComposer&) = delete;
+  DesktopAndCursorConditionalComposer& operator=(
+      const DesktopAndCursorConditionalComposer&) = delete;
+
+  std::unique_ptr<webrtc::MouseCursor> mouse_cursor_;
+  bool compose_enabled_ = false;
+  std::unique_ptr<webrtc::DesktopAndCursorComposer> capturer_;
+  base::WeakPtrFactory<DesktopAndCursorConditionalComposer> weak_factory_{this};
+};
+
+}  // namespace remoting
+
+#endif  // REMOTING_HOST_DESKTOP_AND_CURSOR_CONDITIONAL_COMPOSER_H_

remoting/host/desktop_environment.h

@@ -12,6 +12,7 @@
 #include "base/callback_forward.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/weak_ptr.h"
+#include "remoting/host/desktop_and_cursor_conditional_composer.h"
 #include "remoting/host/desktop_environment_options.h"
 
 namespace webrtc {
@@ -53,6 +54,13 @@ class DesktopEnvironment {
           callback) = 0;
   virtual std::unique_ptr<FileOperations> CreateFileOperations() = 0;
 
+  // For platforms that require the mouse cursor to be composited into the video
+  // stream when it is not rendered by the client, returns a composing capturer.
+  // If the platform already does this, this method return null, and the caller
+  // should use CreateVideoCapturer() instead.
+  virtual std::unique_ptr<DesktopAndCursorConditionalComposer>
+  CreateComposingVideoCapturer() = 0;
+
   // Returns the set of all capabilities supported by |this|.
   virtual std::string GetCapabilities() const = 0;
 

remoting/host/desktop_session_agent.cc

@@ -407,13 +407,15 @@ void DesktopSessionAgent::OnStartSessionAgent(
   }
 
   // Start the video capturer and mouse cursor monitor.
-  video_capturer_ = desktop_environment_->CreateVideoCapturer();
+  video_capturer_ = std::make_unique<DesktopAndCursorConditionalComposer>(
+      desktop_environment_->CreateVideoCapturer());
   video_capturer_->Start(this);
   video_capturer_->SetSharedMemoryFactory(
       std::unique_ptr<webrtc::SharedMemoryFactory>(new SharedMemoryFactoryImpl(
           base::Bind(&DesktopSessionAgent::SendToNetwork, this))));
   mouse_cursor_monitor_ = desktop_environment_->CreateMouseCursorMonitor();
-  mouse_cursor_monitor_->Init(this, webrtc::MouseCursorMonitor::SHAPE_ONLY);
+  mouse_cursor_monitor_->Init(this,
+                              webrtc::MouseCursorMonitor::SHAPE_AND_POSITION);
   // Unretained is sound because callback will never be invoked once after
   // |keyboard_layout_monitor_| is destroyed.
   keyboard_layout_monitor_ = desktop_environment_->CreateKeyboardLayoutMonitor(
@@ -459,6 +461,17 @@ void DesktopSessionAgent::OnMouseCursor(webrtc::MouseCursor* cursor) {
 
   SendToNetwork(
       std::make_unique<ChromotingDesktopNetworkMsg_MouseCursor>(*owned_cursor));
+
+  if (video_capturer_)
+    video_capturer_->SetMouseCursor(owned_cursor.release());
+}
+
+void DesktopSessionAgent::OnMouseCursorPosition(
+    const webrtc::DesktopVector& position) {
+  DCHECK(caller_task_runner_->BelongsToCurrentThread());
+
+  if (video_capturer_)
+    video_capturer_->SetMouseCursorPosition(position);
 }
 
 void DesktopSessionAgent::InjectClipboardEvent(
@@ -655,6 +668,10 @@ void DesktopSessionAgent::OnInjectMouseEvent(
     return;
   }
 
+  if (video_capturer_)
+    video_capturer_->SetComposeEnabled(event.has_delta_x() ||
+                                       event.has_delta_y());
+
   // InputStub implementations must verify events themselves, so we don't need
   // verification here. This matches HostEventDispatcher.
   remote_input_filter_->InjectMouseEvent(event);

remoting/host/desktop_session_agent.h

@@ -21,6 +21,7 @@
 #include "mojo/public/cpp/system/message_pipe.h"
 #include "remoting/host/client_session_control.h"
 #include "remoting/host/current_process_stats_agent.h"
+#include "remoting/host/desktop_and_cursor_conditional_composer.h"
 #include "remoting/host/desktop_display_info.h"
 #include "remoting/host/desktop_environment_options.h"
 #include "remoting/host/file_transfer/session_file_operations_handler.h"
@@ -96,6 +97,7 @@ class DesktopSessionAgent
 
   // webrtc::MouseCursorMonitor::Callback implementation.
   void OnMouseCursor(webrtc::MouseCursor* cursor) override;
+  void OnMouseCursorPosition(const webrtc::DesktopVector& position) override;
 
   // Forwards a local clipboard event though the IPC channel to the network
   // process.
@@ -224,8 +226,8 @@ class DesktopSessionAgent
   // True if the desktop session agent has been started.
   bool started_ = false;
 
-  // Captures the screen.
-  std::unique_ptr<webrtc::DesktopCapturer> video_capturer_;
+  // Captures the screen and composites with the mouse cursor if necessary.
+  std::unique_ptr<DesktopAndCursorConditionalComposer> video_capturer_;
 
   // Captures mouse shapes.
   std::unique_ptr<webrtc::MouseCursorMonitor> mouse_cursor_monitor_;

remoting/host/fake_desktop_environment.cc

@@ -120,6 +120,11 @@ uint32_t FakeDesktopEnvironment::GetDesktopSessionId() const {
   return UINT32_MAX;
 }
 
+std::unique_ptr<DesktopAndCursorConditionalComposer>
+FakeDesktopEnvironment::CreateComposingVideoCapturer() {
+  return nullptr;
+}
+
 const DesktopEnvironmentOptions& FakeDesktopEnvironment::options() const {
   return options_;
 }

remoting/host/fake_desktop_environment.h

@@ -106,6 +106,8 @@ class FakeDesktopEnvironment : public DesktopEnvironment {
   std::string GetCapabilities() const override;
   void SetCapabilities(const std::string& capabilities) override;
   uint32_t GetDesktopSessionId() const override;
+  std::unique_ptr<DesktopAndCursorConditionalComposer>
+  CreateComposingVideoCapturer() override;
 
   base::WeakPtr<FakeInputInjector> last_input_injector() {
     return last_input_injector_;

remoting/host/host_mock_objects.cc

@@ -62,6 +62,11 @@ std::unique_ptr<FileOperations> MockDesktopEnvironment::CreateFileOperations() {
   return base::WrapUnique(CreateFileOperationsPtr());
 }
 
+std::unique_ptr<DesktopAndCursorConditionalComposer>
+MockDesktopEnvironment::CreateComposingVideoCapturer() {
+  return base::WrapUnique(CreateComposingVideoCapturerPtr());
+}
+
 MockDesktopEnvironmentFactory::MockDesktopEnvironmentFactory() = default;
 
 MockDesktopEnvironmentFactory::~MockDesktopEnvironmentFactory() = default;

remoting/host/host_mock_objects.h

@@ -54,6 +54,8 @@ class MockDesktopEnvironment : public DesktopEnvironment {
   MOCK_CONST_METHOD0(GetCapabilities, std::string());
   MOCK_METHOD1(SetCapabilities, void(const std::string&));
   MOCK_CONST_METHOD0(GetDesktopSessionId, uint32_t());
+  MOCK_METHOD0(CreateComposingVideoCapturerPtr,
+               DesktopAndCursorConditionalComposer*());
 
   // DesktopEnvironment implementation.
   std::unique_ptr<ActionExecutor> CreateActionExecutor() override;
@@ -67,6 +69,8 @@ class MockDesktopEnvironment : public DesktopEnvironment {
       base::RepeatingCallback<void(const protocol::KeyboardLayout&)> callback)
       override;
   std::unique_ptr<FileOperations> CreateFileOperations() override;
+  std::unique_ptr<DesktopAndCursorConditionalComposer>
+  CreateComposingVideoCapturer() override;
 };
 
 class MockClientSessionControl : public ClientSessionControl {

remoting/host/ipc_desktop_environment.cc

@@ -97,6 +97,12 @@ uint32_t IpcDesktopEnvironment::GetDesktopSessionId() const {
   return desktop_session_proxy_->desktop_session_id();
 }
 
+std::unique_ptr<DesktopAndCursorConditionalComposer>
+IpcDesktopEnvironment::CreateComposingVideoCapturer() {
+  // Cursor compositing is done by the desktop process if necessary.
+  return nullptr;
+}
+
 IpcDesktopEnvironmentFactory::IpcDesktopEnvironmentFactory(
     scoped_refptr<base::SingleThreadTaskRunner> audio_task_runner,
     scoped_refptr<base::SingleThreadTaskRunner> caller_task_runner,

remoting/host/ipc_desktop_environment.h

@@ -64,6 +64,8 @@ class IpcDesktopEnvironment : public DesktopEnvironment {
   std::string GetCapabilities() const override;
   void SetCapabilities(const std::string& capabilities) override;
   uint32_t GetDesktopSessionId() const override;
+  std::unique_ptr<DesktopAndCursorConditionalComposer>
+  CreateComposingVideoCapturer() override;
 
  private:
   scoped_refptr<DesktopSessionProxy> desktop_session_proxy_;

remoting/host/ipc_mouse_cursor_monitor.cc

@@ -18,7 +18,6 @@ IpcMouseCursorMonitor::~IpcMouseCursorMonitor() = default;
 void IpcMouseCursorMonitor::Init(Callback* callback, Mode mode) {
   DCHECK(!callback_);
   DCHECK(callback);
-  DCHECK_EQ(webrtc::MouseCursorMonitor::SHAPE_ONLY, mode);
   callback_ = callback;
   desktop_session_proxy_->SetMouseCursorMonitor(weak_factory_.GetWeakPtr());
 }