Reland "Remove use of deprecated SharedMemory in chromeos/wilco_dtc_supportd"

Reason for reland: Changes were made to preserve compatibility with the
old version of mojo library.

Original change's description:
> Remove use of deprecated SharedMemory in chromeos/wilco_dtc_supportd
>
> Remove use of the deprecated class SharedMemory. Instead use
> the new classes: {Write,ReadOnly}SharedMemory{Region,Mapping}.
>
> The previous code was wrapping the shared memory inside a mojo
> handle. Instead, pass directly the ReadOnlySharedMemory to mojo,
> this is the preferred way.
>
> See Chromium Shared Memory Refactor design doc for more details:
> https://docs.google.com/document/d/1lk2-7W7Cy7FDqG-6lgS8LRkzXABYTOytf6VUsWNf8-0/edit#heading=h.7nki9mck5t64
>
> Bug: 795291
> Change-Id: I7ee031baf80533dac064fc779e31c40fc0eb202f
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1566307
> Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org>
> Reviewed-by: Polina Bondarenko <pbond@chromium.org>
> Reviewed-by: Alex Ilin <alexilin@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#667164}

Bug: 989503
Change-Id: I42079902caf9ad730e15f8c58d9d834f2dbf0174
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1729225Reviewed-by: Oleh Lamzin <lamzin@google.com>
Commit-Queue: Oleh Lamzin <lamzin@google.com>
Cr-Commit-Position: refs/heads/master@{#697242}

chrome/browser/chromeos/wilco_dtc_supportd/mojo_utils.cc

@@ -4,67 +4,65 @@
 
 #include "chrome/browser/chromeos/wilco_dtc_supportd/mojo_utils.h"
 
-#include <cstdint>
 #include <cstring>
-#include <utility>
 
-#include "base/file_descriptor_posix.h"
 #include "base/files/file.h"
-#include "base/memory/shared_memory_handle.h"
+#include "base/memory/platform_shared_memory_region.h"
+#include "base/memory/read_only_shared_memory_region.h"
 #include "base/unguessable_token.h"
 #include "mojo/public/c/system/types.h"
+#include "mojo/public/cpp/system/handle.h"
 #include "mojo/public/cpp/system/platform_handle.h"
 
 namespace chromeos {
 
 base::StringPiece GetStringPieceFromMojoHandle(
     mojo::ScopedHandle handle,
-    std::unique_ptr<base::SharedMemory>* shared_memory) {
+    base::ReadOnlySharedMemoryMapping* shared_memory) {
+  DCHECK(shared_memory);
+
   base::PlatformFile platform_file;
   auto result = mojo::UnwrapPlatformFile(std::move(handle), &platform_file);
-  shared_memory->reset();
-  if (result != MOJO_RESULT_OK) {
+  if (result != MOJO_RESULT_OK)
     return base::StringPiece();
-  }
-  base::UnguessableToken guid = base::UnguessableToken::Create();
-  *shared_memory = std::make_unique<base::SharedMemory>(
-      base::SharedMemoryHandle(
-          base::FileDescriptor(platform_file, true /* iauto_close */), 0u,
-          guid),
-      true /* read_only */);
 
-  base::SharedMemoryHandle dup_shared_memory_handle =
-      base::SharedMemory::DuplicateHandle((*shared_memory)->handle());
-  const int64_t file_size =
-      base::File(dup_shared_memory_handle.GetHandle()).GetLength();
-  if (file_size <= 0) {
-    shared_memory->reset();
+  base::File file(platform_file);
+  const size_t file_size = file.GetLength();
+  if (file_size <= 0)
     return base::StringPiece();
-  }
-  if (!(*shared_memory)->Map(file_size)) {
-    shared_memory->reset();
+
+  base::subtle::PlatformSharedMemoryRegion platform_region =
+      base::subtle::PlatformSharedMemoryRegion::Take(
+          base::ScopedFD(file.TakePlatformFile()),
+          base::subtle::PlatformSharedMemoryRegion::Mode::kReadOnly, file_size,
+          base::UnguessableToken::Create());
+
+  base::ReadOnlySharedMemoryRegion shm =
+      base::ReadOnlySharedMemoryRegion::Deserialize(std::move(platform_region));
+  *shared_memory = shm.Map();
+  if (!shared_memory->IsValid())
     return base::StringPiece();
-  }
-  return base::StringPiece(static_cast<const char*>((*shared_memory)->memory()),
-                           (*shared_memory)->mapped_size());
+
+  return base::StringPiece(static_cast<const char*>(shared_memory->memory()),
+                           shared_memory->size());
 }
 
 mojo::ScopedHandle CreateReadOnlySharedMemoryMojoHandle(
     const std::string& content) {
-  if (content.empty()) {
+  if (content.empty())
     return mojo::ScopedHandle();
-  }
 
-  base::SharedMemory shared_memory;
-  base::SharedMemoryCreateOptions options;
-  options.size = content.length();
-  options.share_read_only = true;
-  if (!shared_memory.Create(base::SharedMemoryCreateOptions(options)) ||
-      !shared_memory.Map(content.length())) {
+  base::MappedReadOnlyRegion shm =
+      base::ReadOnlySharedMemoryRegion::Create(content.size());
+  if (!shm.IsValid())
     return mojo::ScopedHandle();
-  }
-  memcpy(shared_memory.memory(), content.data(), content.length());
-  return mojo::WrapPlatformFile(shared_memory.TakeHandle().GetHandle());
+  memcpy(shm.mapping.memory(), content.data(), content.length());
+
+  base::subtle::PlatformSharedMemoryRegion platform_region =
+      base::ReadOnlySharedMemoryRegion::TakeHandleForSerialization(
+          std::move(shm.region));
+  return mojo::WrapPlatformFile(
+      platform_region.PassPlatformHandle().fd.release());
 }
 
 }  // namespace chromeos

chrome/browser/chromeos/wilco_dtc_supportd/mojo_utils.h

@@ -8,23 +8,27 @@
 #include <memory>
 #include <string>
 
-#include "base/memory/shared_memory.h"
+#include "base/memory/shared_memory_mapping.h"
 #include "base/strings/string_piece.h"
 #include "mojo/public/cpp/system/handle.h"
 
 namespace chromeos {
 
 // Allows to get access to the buffer in read only shared memory. It converts
-// mojo::Handle to base::SharedMemory and returns a string content.
+// mojo::Handle to base::ReadOnlySharedMemoryMapping and returns a string
+// content.
 //
 // |handle| must be a valid mojo handle of the non-empty buffer in the shared
 // memory.
-// |shared_memory| must be a valid allocated unique pointer.
 //
-// Returns an empty string and nullptr |shared_memory| if error.
+// Returns an empty string and an invalid |shared_memory| if error.
+//
+// TODO(crbug.com/989503): Use mojo::ScopedSharedBufferHandle or
+// base::ReadOnlySharedMemoryRegion instead of mojo::ScopedHandle
+// once ChromeOS updates to the required version of mojo library.
 base::StringPiece GetStringPieceFromMojoHandle(
     mojo::ScopedHandle handle,
-    std::unique_ptr<base::SharedMemory>* shared_memory);
+    base::ReadOnlySharedMemoryMapping* shared_memory);
 
 // Allocates buffer in shared memory, copies |content| to the buffer and
 // converts shared buffer handle into |mojo::ScopedHandle|.
@@ -32,6 +36,9 @@ base::StringPiece GetStringPieceFromMojoHandle(
 // Allocated shared memory is read only for another process.
 //
 // Returns invalid |mojo::ScopedHandle| if |content| is empty or error happened.
+//
+// TODO(crbug.com/989503): Remove mojo::ScopedHandle wrapping once
+// ChromeOS updates to the required version of mojo library.
 mojo::ScopedHandle CreateReadOnlySharedMemoryMojoHandle(
     const std::string& content);
 

chrome/browser/chromeos/wilco_dtc_supportd/wilco_dtc_supportd_bridge.cc

@@ -10,7 +10,7 @@
 #include "base/bind.h"
 #include "base/location.h"
 #include "base/logging.h"
-#include "base/memory/shared_memory.h"
+#include "base/memory/shared_memory_mapping.h"
 #include "base/process/process_handle.h"
 #include "base/strings/string_piece.h"
 #include "base/threading/thread_task_runner_handle.h"
@@ -261,9 +261,9 @@ void WilcoDtcSupportdBridge::PerformWebRequest(
   // Extract a GURL value from a ScopedHandle.
   GURL gurl;
   if (url.is_valid()) {
-    std::unique_ptr<base::SharedMemory> shared_memory;
+    base::ReadOnlySharedMemoryMapping shared_memory;
     gurl = GURL(GetStringPieceFromMojoHandle(std::move(url), &shared_memory));
-    if (!shared_memory) {
+    if (!shared_memory.IsValid()) {
       LOG(ERROR) << "Failed to read data from mojo handle";
       std::move(callback).Run(
           wilco_dtc_supportd::mojom::WilcoDtcSupportdWebRequestStatus::
@@ -275,16 +275,16 @@ void WilcoDtcSupportdBridge::PerformWebRequest(
 
   // Extract headers from ScopedHandle's.
   std::vector<base::StringPiece> header_contents;
-  std::vector<std::unique_ptr<base::SharedMemory>> shared_memories;
+  std::vector<base::ReadOnlySharedMemoryMapping> shared_memories;
   for (auto& header : headers) {
     if (!header.is_valid()) {
       header_contents.push_back("");
       continue;
     }
-    shared_memories.push_back(nullptr);
+    shared_memories.emplace_back();
     header_contents.push_back(GetStringPieceFromMojoHandle(
         std::move(header), &shared_memories.back()));
-    if (!shared_memories.back()) {
+    if (!shared_memories.back().IsValid()) {
       LOG(ERROR) << "Failed to read data from mojo handle";
       std::move(callback).Run(
           wilco_dtc_supportd::mojom::WilcoDtcSupportdWebRequestStatus::
@@ -297,10 +297,10 @@ void WilcoDtcSupportdBridge::PerformWebRequest(
   // Extract a string value from a ScopedHandle.
   std::string request_body_content;
   if (request_body.is_valid()) {
-    std::unique_ptr<base::SharedMemory> shared_memory;
+    base::ReadOnlySharedMemoryMapping shared_memory;
     request_body_content = std::string(
         GetStringPieceFromMojoHandle(std::move(request_body), &shared_memory));
-    if (!shared_memory) {
+    if (!shared_memory.IsValid()) {
       LOG(ERROR) << "Failed to read data from mojo handle";
       std::move(callback).Run(
           wilco_dtc_supportd::mojom::WilcoDtcSupportdWebRequestStatus::
@@ -350,7 +350,7 @@ void WilcoDtcSupportdBridge::SendWilcoDtcMessageToUi(
     SendWilcoDtcMessageToUiCallback callback) {
   // Extract the string value of the received message.
   DCHECK(json_message);
-  std::unique_ptr<base::SharedMemory> json_message_shared_memory;
+  base::ReadOnlySharedMemoryMapping json_message_shared_memory;
   base::StringPiece json_message_string = GetStringPieceFromMojoHandle(
       std::move(json_message), &json_message_shared_memory);
   if (json_message_string.empty()) {

chrome/browser/chromeos/wilco_dtc_supportd/wilco_dtc_supportd_messaging.cc

@@ -12,7 +12,7 @@
 #include "base/callback.h"
 #include "base/logging.h"
 #include "base/macros.h"
-#include "base/memory/shared_memory.h"
+#include "base/memory/shared_memory_mapping.h"
 #include "base/memory/weak_ptr.h"
 #include "base/stl_util.h"
 #include "base/strings/string_piece.h"
@@ -167,7 +167,7 @@ class WilcoDtcSupportdExtensionOwnedMessageHost final
       return;
     }
 
-    std::unique_ptr<base::SharedMemory> response_json_shared_memory;
+    base::ReadOnlySharedMemoryMapping response_json_shared_memory;
     base::StringPiece response_json_string = GetStringPieceFromMojoHandle(
         std::move(response_json_message), &response_json_shared_memory);
     if (response_json_string.empty()) {

chrome/browser/chromeos/wilco_dtc_supportd/wilco_dtc_supportd_messaging_unittest.cc

@@ -10,7 +10,7 @@
 #include "base/callback.h"
 #include "base/logging.h"
 #include "base/memory/scoped_refptr.h"
-#include "base/memory/shared_memory.h"
+#include "base/memory/shared_memory_mapping.h"
 #include "base/run_loop.h"
 #include "base/test/task_environment.h"
 #include "chrome/browser/chromeos/wilco_dtc_supportd/mojo_utils.h"
@@ -52,7 +52,7 @@ class MockNativeMessageHostClient
 std::string AssertGetStringFromMojoHandle(mojo::ScopedHandle handle) {
   if (!handle)
     return std::string();
-  std::unique_ptr<base::SharedMemory> shared_memory;
+  base::ReadOnlySharedMemoryMapping shared_memory;
   std::string contents =
       GetStringPieceFromMojoHandle(std::move(handle), &shared_memory)
           .as_string();

chrome/browser/chromeos/wilco_dtc_supportd/wilco_dtc_supportd_web_request_service_unittest.cc

@@ -10,7 +10,7 @@
 #include "base/bind.h"
 #include "base/logging.h"
 #include "base/memory/scoped_refptr.h"
-#include "base/memory/shared_memory.h"
+#include "base/memory/shared_memory_mapping.h"
 #include "base/run_loop.h"
 #include "base/strings/string_piece.h"
 #include "base/strings/stringprintf.h"
@@ -53,10 +53,10 @@ class WilcoDtcSupportdWebRequestServiceTest : public testing::Test {
         response_body = "";
         return;
       }
-      std::unique_ptr<base::SharedMemory> shared_memory;
+      base::ReadOnlySharedMemoryMapping shared_memory;
       response_body = std::string(GetStringPieceFromMojoHandle(
           std::move(response_body_handle), &shared_memory));
-      if (!shared_memory) {
+      if (!shared_memory.IsValid()) {
         response_body = "";
         return;
       }