Remoting: upgrade remoting/host shared memory to new API.

Change implementation of webrtc::SharedMemory to use
base::UnsafeSharedMemoryRegion. Should not change any
functionality outside of changing a read-only mapping of
shared memory to be a const mapping, as the former is no
longer supported by the chromium shared memory API.

Remoting: upgrade remoting/host shared memory to new API.

Bug: 795291, 961119
Change-Id: Iff24e1a6fd780d6f6b1618f20a588dbd596541f1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1602502
Commit-Queue: Matthew Cary <mattcary@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Yuwei Huang <yuweih@chromium.org>
Reviewed-by: Alex Ilin <alexilin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#663030}

base/memory/writable_shared_memory_region.h

@@ -10,6 +10,7 @@
 #include "base/memory/read_only_shared_memory_region.h"
 #include "base/memory/shared_memory_mapping.h"
 #include "base/memory/unsafe_shared_memory_region.h"
+#include "build/build_config.h"
 
 namespace base {
 
@@ -106,6 +107,15 @@ class BASE_EXPORT WritableSharedMemoryRegion {
     return handle_.GetGUID();
   }
 
+#if defined(OS_WIN)
+  // On Windows it is necessary in rare cases to take a writable handle from a
+  // region that will be converted to read-only. On this platform it is a safe
+  // operation, as the handle returned from this method will remain writable
+  // after the region is converted to read-only. However, it breaks chromium's
+  // WritableSharedMemoryRegion semantics and so should be use with care.
+  HANDLE UnsafeGetPlatformHandle() const { return handle_.GetPlatformHandle(); }
+#endif
+
  private:
   friend class SharedMemoryHooks;
 

remoting/host/chromoting_messages.h

@@ -8,7 +8,7 @@
 #include <stdint.h>
 
 #include "base/files/file_path.h"
-#include "base/memory/shared_memory_handle.h"
+#include "base/memory/unsafe_shared_memory_region.h"
 #include "base/time/time.h"
 #include "ipc/ipc_channel_handle.h"
 #include "ipc/ipc_platform_file.h"
@@ -148,7 +148,7 @@ IPC_MESSAGE_CONTROL(ChromotingDesktopDaemonMsg_InjectSas)
 // Notifies the network process that a shared buffer has been created.
 IPC_MESSAGE_CONTROL(ChromotingDesktopNetworkMsg_CreateSharedBuffer,
                     int /* id */,
-                    base::SharedMemoryHandle /* handle */,
+                    base::ReadOnlySharedMemoryRegion /* region */,
                     uint32_t /* size */)
 
 // Request the network process to stop using a shared buffer.

remoting/host/desktop_session_agent.cc

@@ -10,8 +10,9 @@
 #include "base/files/file_util.h"
 #include "base/logging.h"
 #include "base/macros.h"
+#include "base/memory/platform_shared_memory_region.h"
 #include "base/memory/ptr_util.h"
-#include "base/memory/shared_memory.h"
+#include "base/memory/read_only_shared_memory_region.h"
 #include "base/process/process_handle.h"
 #include "base/time/time.h"
 #include "build/build_config.h"
@@ -41,8 +42,92 @@
 #include "third_party/webrtc/modules/desktop_capture/mouse_cursor.h"
 #include "third_party/webrtc/modules/desktop_capture/shared_memory.h"
 
+#if defined(OS_WIN)
+#include "base/memory/writable_shared_memory_region.h"
+#endif
+
 namespace remoting {
 
+// webrtc::SharedMemory implementation that creates a
+// base::ReadOnlySharedMemoryRegion along with a writable mapping.
+//
+// This is declared outside the anonymous namespace so that it can be friended
+// with base::WritableSharedMemoryRegion. It is not exported in the header.
+class SharedMemoryImpl : public webrtc::SharedMemory {
+ public:
+  static std::unique_ptr<SharedMemoryImpl>
+  Create(size_t size, int id, const base::Closure& on_deleted_callback) {
+    webrtc::SharedMemory::Handle handle = webrtc::SharedMemory::kInvalidHandle;
+#if defined(OS_WIN)
+    // webrtc::ScreenCapturer uses webrtc::SharedMemory::handle() only on
+    // windows. This handle must be writable. A WritableSharedMemoryRegion is
+    // created, and then it is converted to read-only.  On the windows platform,
+    // it happens to be the case that converting a region to read-only does not
+    // change the status of existing handles. This is not true on all other
+    // platforms, so please don't emulate this behavior!
+    base::WritableSharedMemoryRegion region =
+        base::WritableSharedMemoryRegion::Create(size);
+    base::WritableSharedMemoryMapping mapping = region.Map();
+    // Converting |region| to read-only will close its associated handle, so we
+    // must duplicate it into the handle used for |webrtc::ScreenCapturer|.
+    HANDLE process = ::GetCurrentProcess();
+    BOOL success =
+        ::DuplicateHandle(process, region.UnsafeGetPlatformHandle(), process,
+                          &handle, 0, FALSE, DUPLICATE_SAME_ACCESS);
+    if (!success)
+      return nullptr;
+    base::ReadOnlySharedMemoryRegion read_only_region =
+        base::WritableSharedMemoryRegion::ConvertToReadOnly(std::move(region));
+#else
+    base::MappedReadOnlyRegion region_mapping =
+        base::ReadOnlySharedMemoryRegion::Create(size);
+    base::ReadOnlySharedMemoryRegion read_only_region =
+        std::move(region_mapping.region);
+    base::WritableSharedMemoryMapping mapping =
+        std::move(region_mapping.mapping);
+#endif
+    if (!mapping.IsValid())
+      return nullptr;
+    // The SharedMemoryImpl ctor is private, so std::make_unique can't be
+    // used.
+    return base::WrapUnique(new SharedMemoryImpl(std::move(read_only_region),
+                                                 std::move(mapping), handle, id,
+                                                 on_deleted_callback));
+  }
+
+  ~SharedMemoryImpl() override { on_deleted_callback_.Run(); }
+
+  const base::ReadOnlySharedMemoryRegion& region() const { return region_; }
+
+ private:
+  SharedMemoryImpl(base::ReadOnlySharedMemoryRegion region,
+                   base::WritableSharedMemoryMapping mapping,
+                   webrtc::SharedMemory::Handle handle,
+                   int id,
+                   const base::Closure& on_deleted_callback)
+      : SharedMemory(mapping.memory(), mapping.size(), handle, id),
+        on_deleted_callback_(on_deleted_callback)
+#if defined(OS_WIN)
+        ,
+        writable_handle_(handle)
+#endif
+  {
+    region_ = std::move(region);
+    mapping_ = std::move(mapping);
+  }
+
+  base::Closure on_deleted_callback_;
+  base::ReadOnlySharedMemoryRegion region_;
+  base::WritableSharedMemoryMapping mapping_;
+#if defined(OS_WIN)
+  // Owns the handle passed to the base class which is used by
+  // webrtc::ScreenCapturer.
+  base::win::ScopedHandle writable_handle_;
+#endif
+
+  DISALLOW_COPY_AND_ASSIGN(SharedMemoryImpl);
+};
+
 namespace {
 
 // Routes local clipboard events though the IPC channel to the network process.
@@ -72,46 +157,6 @@ void DesktopSessionClipboardStub::InjectClipboardEvent(
   desktop_session_agent_->InjectClipboardEvent(event);
 }
 
-// webrtc::SharedMemory implementation that creates base::SharedMemory.
-class SharedMemoryImpl : public webrtc::SharedMemory {
- public:
-  static std::unique_ptr<SharedMemoryImpl>
-  Create(size_t size, int id, const base::Closure& on_deleted_callback) {
-    std::unique_ptr<base::SharedMemory> memory(new base::SharedMemory());
-    if (!memory->CreateAndMapAnonymous(size))
-      return nullptr;
-    return base::WrapUnique(
-        new SharedMemoryImpl(std::move(memory), size, id, on_deleted_callback));
-  }
-
-  ~SharedMemoryImpl() override { on_deleted_callback_.Run(); }
-
-  base::SharedMemory* shared_memory() { return shared_memory_.get(); }
-
- private:
-  SharedMemoryImpl(std::unique_ptr<base::SharedMemory> memory,
-                   size_t size,
-                   int id,
-                   const base::Closure& on_deleted_callback)
-      : SharedMemory(memory->memory(),
-                     size,
-// webrtc::ScreenCapturer uses webrtc::SharedMemory::handle() only on Windows.
-#if defined(OS_WIN)
-                     memory->handle().GetHandle(),
-#else
-                     0,
-#endif
-                     id),
-        on_deleted_callback_(on_deleted_callback),
-        shared_memory_(std::move(memory)) {
-  }
-
-  base::Closure on_deleted_callback_;
-  std::unique_ptr<base::SharedMemory> shared_memory_;
-
-  DISALLOW_COPY_AND_ASSIGN(SharedMemoryImpl);
-};
-
 class SharedMemoryFactoryImpl : public webrtc::SharedMemoryFactory {
  public:
   typedef base::Callback<void(std::unique_ptr<IPC::Message> message)>
@@ -142,7 +187,7 @@ class SharedMemoryFactoryImpl : public webrtc::SharedMemoryFactory {
 
       send_message_callback_.Run(
           std::make_unique<ChromotingDesktopNetworkMsg_CreateSharedBuffer>(
-              buffer->id(), buffer->shared_memory()->handle(), buffer->size()));
+              buffer->id(), buffer->region().Duplicate(), buffer->size()));
     }
 
     return std::move(buffer);

remoting/host/desktop_session_proxy.cc

@@ -13,7 +13,6 @@
 #include "base/logging.h"
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
-#include "base/memory/shared_memory.h"
 #include "base/process/process_handle.h"
 #include "base/single_thread_task_runner.h"
 #include "build/build_config.h"
@@ -43,44 +42,44 @@
 #include "base/win/scoped_handle.h"
 #endif  // defined(OS_WIN)
 
-const bool kReadOnly = true;
-
 namespace remoting {
 
 class DesktopSessionProxy::IpcSharedBufferCore
     : public base::RefCountedThreadSafe<IpcSharedBufferCore> {
  public:
-  IpcSharedBufferCore(int id,
-                      base::SharedMemoryHandle handle,
-                      size_t size)
-      : id_(id),
-        shared_memory_(handle, kReadOnly),
-        size_(size) {
-    if (!shared_memory_.Map(size)) {
+  IpcSharedBufferCore(int id, base::ReadOnlySharedMemoryRegion region)
+      : id_(id) {
+    mapping_ = region.Map();
+    if (!mapping_.IsValid()) {
       LOG(ERROR) << "Failed to map a shared buffer: id=" << id
-                 << ", size=" << size;
+                 << ", size=" << region.GetSize();
     }
+    // After being mapped, |region| is no longer needed and can be discarded.
   }
 
-  int id() { return id_; }
-  size_t size() { return size_; }
-  void* memory() { return shared_memory_.memory(); }
+  int id() const { return id_; }
+  size_t size() const { return mapping_.size(); }
+  const void* memory() const { return mapping_.memory(); }
 
  private:
   virtual ~IpcSharedBufferCore() = default;
   friend class base::RefCountedThreadSafe<IpcSharedBufferCore>;
 
   int id_;
-  base::SharedMemory shared_memory_;
-  size_t size_;
+  base::ReadOnlySharedMemoryMapping mapping_;
 
   DISALLOW_COPY_AND_ASSIGN(IpcSharedBufferCore);
 };
 
 class DesktopSessionProxy::IpcSharedBuffer : public webrtc::SharedMemory {
  public:
+  // Note that the webrtc::SharedMemory class is used for both read-only and
+  // writable shared memory, necessitating the ugly const_cast here.
   IpcSharedBuffer(scoped_refptr<IpcSharedBufferCore> core)
-      : SharedMemory(core->memory(), core->size(), 0, core->id()),
+      : SharedMemory(const_cast<void*>(core->memory()),
+                     core->size(),
+                     0,
+                     core->id()),
         core_(core) {}
 
  private:
@@ -516,12 +515,12 @@ void DesktopSessionProxy::OnAudioPacket(const std::string& serialized_packet) {
 
 void DesktopSessionProxy::OnCreateSharedBuffer(
     int id,
-    base::SharedMemoryHandle handle,
+    base::ReadOnlySharedMemoryRegion region,
     uint32_t size) {
   DCHECK(caller_task_runner_->BelongsToCurrentThread());
 
   scoped_refptr<IpcSharedBufferCore> shared_buffer =
-      new IpcSharedBufferCore(id, handle, size);
+      new IpcSharedBufferCore(id, std::move(region));
 
   if (shared_buffer->memory() != nullptr &&
       !shared_buffers_.insert(std::make_pair(id, shared_buffer)).second) {

remoting/host/desktop_session_proxy.h

@@ -10,8 +10,8 @@
 #include <memory>
 
 #include "base/macros.h"
+#include "base/memory/read_only_shared_memory_region.h"
 #include "base/memory/ref_counted.h"
-#include "base/memory/shared_memory_handle.h"
 #include "base/memory/weak_ptr.h"
 #include "base/process/process.h"
 #include "base/sequenced_task_runner_helpers.h"
@@ -171,7 +171,7 @@ class DesktopSessionProxy
 
   // Registers a new shared buffer created by the desktop process.
   void OnCreateSharedBuffer(int id,
-                            base::SharedMemoryHandle handle,
+                            base::ReadOnlySharedMemoryRegion region,
                             uint32_t size);
 
   // Drops a cached reference to the shared buffer.