Switch PlatformMessages to hold data in Mappings (flutter#25867)

ci/licenses_golden/licenses_flutter

@@ -173,6 +173,7 @@ FILE: ../../../flutter/fml/macros.h
 FILE: ../../../flutter/fml/make_copyable.h
 FILE: ../../../flutter/fml/mapping.cc
 FILE: ../../../flutter/fml/mapping.h
+FILE: ../../../flutter/fml/mapping_unittests.cc
 FILE: ../../../flutter/fml/memory/ref_counted.h
 FILE: ../../../flutter/fml/memory/ref_counted_internal.h
 FILE: ../../../flutter/fml/memory/ref_counted_unittest.cc

fml/BUILD.gn

@@ -262,6 +262,7 @@ if (enable_unittests) {
       "file_unittest.cc",
       "hash_combine_unittests.cc",
       "logging_unittests.cc",
+      "mapping_unittests.cc",
       "memory/ref_counted_unittest.cc",
       "memory/task_runner_checker_unittest.cc",
       "memory/weak_ptr_unittest.cc",

fml/mapping.cc

@@ -82,7 +82,6 @@ const uint8_t* DataMapping::GetMapping() const {
 }
 
 // NonOwnedMapping
-
 NonOwnedMapping::NonOwnedMapping(const uint8_t* data,
                                  size_t size,
                                  const ReleaseProc& release_proc)
@@ -102,6 +101,46 @@ const uint8_t* NonOwnedMapping::GetMapping() const {
   return data_;
 }
 
+// MallocMapping
+MallocMapping::MallocMapping() : data_(nullptr), size_(0) {}
+
+MallocMapping::MallocMapping(uint8_t* data, size_t size)
+    : data_(data), size_(size) {}
+
+MallocMapping::MallocMapping(fml::MallocMapping&& mapping)
+    : data_(mapping.data_), size_(mapping.size_) {
+  mapping.data_ = nullptr;
+  mapping.size_ = 0;
+}
+
+MallocMapping::~MallocMapping() {
+  free(data_);
+  data_ = nullptr;
+}
+
+MallocMapping MallocMapping::Copy(const void* begin, size_t length) {
+  auto result =
+      MallocMapping(reinterpret_cast<uint8_t*>(malloc(length)), length);
+  FML_CHECK(result.GetMapping() != nullptr);
+  memcpy(const_cast<uint8_t*>(result.GetMapping()), begin, length);
+  return result;
+}
+
+size_t MallocMapping::GetSize() const {
+  return size_;
+}
+
+const uint8_t* MallocMapping::GetMapping() const {
+  return data_;
+}
+
+uint8_t* MallocMapping::Release() {
+  uint8_t* result = data_;
+  data_ = nullptr;
+  size_ = 0;
+  return result;
+}
+
 // Symbol Mapping
 
 SymbolMapping::SymbolMapping(fml::RefPtr<fml::NativeLibrary> native_library,

fml/mapping.h

@@ -125,6 +125,54 @@ class NonOwnedMapping final : public Mapping {
   FML_DISALLOW_COPY_AND_ASSIGN(NonOwnedMapping);
 };
 
+/// A Mapping like NonOwnedMapping, but uses Free as its release proc.
+class MallocMapping final : public Mapping {
+ public:
+  MallocMapping();
+
+  /// Creates a MallocMapping for a region of memory (without copying it).
+  /// The function will `abort()` if the malloc fails.
+  /// @param data The starting address of the mapping.
+  /// @param size The size of the mapping in bytes.
+  MallocMapping(uint8_t* data, size_t size);
+
+  MallocMapping(fml::MallocMapping&& mapping);
+
+  ~MallocMapping() override;
+
+  /// Copies the data from `begin` to `end`.
+  /// It's templated since void* arithemetic isn't allowed and we want support
+  /// for `uint8_t` and `char`.
+  template <typename T>
+  static MallocMapping Copy(const T* begin, const T* end) {
+    FML_DCHECK(end > begin);
+    size_t length = end - begin;
+    return Copy(begin, length);
+  }
+
+  /// Copies a region of memory into a MallocMapping.
+  /// The function will `abort()` if the malloc fails.
+  /// @param begin The starting address of where we will copy.
+  /// @param length The length of the region to copy in bytes.
+  static MallocMapping Copy(const void* begin, size_t length);
+
+  // |Mapping|
+  size_t GetSize() const override;
+
+  // |Mapping|
+  const uint8_t* GetMapping() const override;
+
+  /// Removes ownership of the data buffer.
+  /// After this is called; the mapping will point to nullptr.
+  [[nodiscard]] uint8_t* Release();
+
+ private:
+  uint8_t* data_;
+  size_t size_;
+
+  FML_DISALLOW_COPY_AND_ASSIGN(MallocMapping);
+};
+
 class SymbolMapping final : public Mapping {
  public:
   SymbolMapping(fml::RefPtr<fml::NativeLibrary> native_library,

fml/mapping_unittests.cc

@@ -0,0 +1,55 @@
+// Copyright 2013 The Flutter 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 "flutter/fml/mapping.h"
+#include "flutter/testing/testing.h"
+
+namespace fml {
+
+TEST(MallocMapping, EmptyContructor) {
+  MallocMapping mapping;
+  ASSERT_EQ(nullptr, mapping.GetMapping());
+  ASSERT_EQ(0u, mapping.GetSize());
+}
+
+TEST(MallocMapping, NotEmptyContructor) {
+  size_t length = 10;
+  MallocMapping mapping(reinterpret_cast<uint8_t*>(malloc(length)), length);
+  ASSERT_NE(nullptr, mapping.GetMapping());
+  ASSERT_EQ(length, mapping.GetSize());
+}
+
+TEST(MallocMapping, MoveConstructor) {
+  size_t length = 10;
+  MallocMapping mapping(reinterpret_cast<uint8_t*>(malloc(length)), length);
+  MallocMapping moved = std::move(mapping);
+
+  ASSERT_EQ(nullptr, mapping.GetMapping());
+  ASSERT_EQ(0u, mapping.GetSize());
+  ASSERT_NE(nullptr, moved.GetMapping());
+  ASSERT_EQ(length, moved.GetSize());
+}
+
+TEST(MallocMapping, Copy) {
+  size_t length = 10;
+  MallocMapping mapping(reinterpret_cast<uint8_t*>(malloc(length)), length);
+  memset(const_cast<uint8_t*>(mapping.GetMapping()), 0xac, mapping.GetSize());
+  MallocMapping copied =
+      MallocMapping::Copy(mapping.GetMapping(), mapping.GetSize());
+
+  ASSERT_NE(mapping.GetMapping(), copied.GetMapping());
+  ASSERT_EQ(mapping.GetSize(), copied.GetSize());
+  ASSERT_EQ(
+      0, memcmp(mapping.GetMapping(), copied.GetMapping(), mapping.GetSize()));
+}
+
+TEST(MallocMapping, Release) {
+  size_t length = 10;
+  MallocMapping mapping(reinterpret_cast<uint8_t*>(malloc(length)), length);
+  free(const_cast<uint8_t*>(mapping.Release()));
+  ASSERT_EQ(nullptr, mapping.GetMapping());
+  ASSERT_EQ(0u, mapping.GetSize());
+}
+
+}  // namespace fml

lib/ui/window/platform_configuration.cc

@@ -130,7 +130,7 @@ Dart_Handle SendPlatformMessage(Dart_Handle window,
     const uint8_t* buffer = static_cast<const uint8_t*>(data.data());
     dart_state->platform_configuration()->client()->HandlePlatformMessage(
         std::make_unique<PlatformMessage>(
-            name, std::vector<uint8_t>(buffer, buffer + data.length_in_bytes()),
+            name, fml::MallocMapping::Copy(buffer, data.length_in_bytes()),
             response));
   }
 
@@ -190,8 +190,8 @@ void _RespondToKeyData(Dart_NativeArguments args) {
   tonic::DartCallStatic(&RespondToKeyData, args);
 }
 
-Dart_Handle ToByteData(const std::vector<uint8_t>& buffer) {
-  return tonic::DartByteData::Create(buffer.data(), buffer.size());
+Dart_Handle ToByteData(const fml::Mapping& buffer) {
+  return tonic::DartByteData::Create(buffer.GetMapping(), buffer.GetSize());
 }
 
 }  // namespace
@@ -338,15 +338,16 @@ void PlatformConfiguration::DispatchPlatformMessage(
 
 void PlatformConfiguration::DispatchSemanticsAction(int32_t id,
                                                     SemanticsAction action,
-                                                    std::vector<uint8_t> args) {
+                                                    fml::MallocMapping args) {
   std::shared_ptr<tonic::DartState> dart_state =
       dispatch_semantics_action_.dart_state().lock();
   if (!dart_state) {
     return;
   }
   tonic::DartState::Scope scope(dart_state);
 
-  Dart_Handle args_handle = (args.empty()) ? Dart_Null() : ToByteData(args);
+  Dart_Handle args_handle =
+      (args.GetSize() <= 0) ? Dart_Null() : ToByteData(args);
 
   if (Dart_IsError(args_handle)) {
     return;

lib/ui/window/platform_configuration.h

@@ -325,7 +325,7 @@ class PlatformConfiguration final {
   ///
   void DispatchSemanticsAction(int32_t id,
                                SemanticsAction action,
-                               std::vector<uint8_t> args);
+                               fml::MallocMapping args);
 
   //----------------------------------------------------------------------------
   /// @brief      Registers a callback to be invoked when the framework has

lib/ui/window/platform_message.cc

@@ -9,7 +9,7 @@
 namespace flutter {
 
 PlatformMessage::PlatformMessage(std::string channel,
-                                 std::vector<uint8_t> data,
+                                 fml::MallocMapping data,
                                  fml::RefPtr<PlatformMessageResponse> response)
     : channel_(std::move(channel)),
       data_(std::move(data)),

lib/ui/window/platform_message.h

@@ -17,23 +17,25 @@ namespace flutter {
 class PlatformMessage {
  public:
   PlatformMessage(std::string channel,
-                  std::vector<uint8_t> data,
+                  fml::MallocMapping data,
                   fml::RefPtr<PlatformMessageResponse> response);
   PlatformMessage(std::string channel,
                   fml::RefPtr<PlatformMessageResponse> response);
   ~PlatformMessage();
 
   const std::string& channel() const { return channel_; }
-  const std::vector<uint8_t>& data() const { return data_; }
+  const fml::MallocMapping& data() const { return data_; }
   bool hasData() { return hasData_; }
 
   const fml::RefPtr<PlatformMessageResponse>& response() const {
     return response_;
   }
 
+  fml::MallocMapping releaseData() { return std::move(data_); }
+
  private:
   std::string channel_;
-  std::vector<uint8_t> data_;
+  fml::MallocMapping data_;
   bool hasData_;
   fml::RefPtr<PlatformMessageResponse> response_;
 };

runtime/runtime_controller.cc

@@ -278,7 +278,7 @@ bool RuntimeController::DispatchKeyDataPacket(const KeyDataPacket& packet,
 
 bool RuntimeController::DispatchSemanticsAction(int32_t id,
                                                 SemanticsAction action,
-                                                std::vector<uint8_t> args) {
+                                                fml::MallocMapping args) {
   TRACE_EVENT1("flutter", "RuntimeController::DispatchSemanticsAction", "mode",
                "basic");
   if (auto* platform_configuration = GetPlatformConfigurationIfAvailable()) {

runtime/runtime_controller.h

@@ -453,7 +453,7 @@ class RuntimeController : public PlatformConfigurationClient {
   ///
   bool DispatchSemanticsAction(int32_t id,
                                SemanticsAction action,
-                               std::vector<uint8_t> args);
+                               fml::MallocMapping args);
 
   //----------------------------------------------------------------------------
   /// @brief      Gets the main port identifier of the root isolate.

shell/common/engine.cc

@@ -4,6 +4,7 @@
 
 #include "flutter/shell/common/engine.h"
 
+#include <cstring>
 #include <memory>
 #include <string>
 #include <utility>
@@ -35,6 +36,12 @@ static constexpr char kLocalizationChannel[] = "flutter/localization";
 static constexpr char kSettingsChannel[] = "flutter/settings";
 static constexpr char kIsolateChannel[] = "flutter/isolate";
 
+namespace {
+fml::MallocMapping MakeMapping(const std::string& str) {
+  return fml::MallocMapping::Copy(str.c_str(), str.length());
+}
+}  // namespace
+
 Engine::Engine(
     Delegate& delegate,
     const PointerDataDispatcherMaker& dispatcher_maker,
@@ -205,10 +212,7 @@ Engine::RunStatus Engine::Run(RunConfiguration configuration) {
   if (service_id.has_value()) {
     std::unique_ptr<PlatformMessage> service_id_message =
         std::make_unique<flutter::PlatformMessage>(
-            kIsolateChannel,
-            std::vector<uint8_t>(service_id.value().begin(),
-                                 service_id.value().end()),
-            nullptr);
+            kIsolateChannel, MakeMapping(service_id.value()), nullptr);
     HandlePlatformMessage(std::move(service_id_message));
   }
 
@@ -326,7 +330,8 @@ void Engine::DispatchPlatformMessage(std::unique_ptr<PlatformMessage> message) {
 
 bool Engine::HandleLifecyclePlatformMessage(PlatformMessage* message) {
   const auto& data = message->data();
-  std::string state(reinterpret_cast<const char*>(data.data()), data.size());
+  std::string state(reinterpret_cast<const char*>(data.GetMapping()),
+                    data.GetSize());
   if (state == "AppLifecycleState.paused" ||
       state == "AppLifecycleState.detached") {
     activity_running_ = false;
@@ -353,7 +358,8 @@ bool Engine::HandleNavigationPlatformMessage(
   const auto& data = message->data();
 
   rapidjson::Document document;
-  document.Parse(reinterpret_cast<const char*>(data.data()), data.size());
+  document.Parse(reinterpret_cast<const char*>(data.GetMapping()),
+                 data.GetSize());
   if (document.HasParseError() || !document.IsObject()) {
     return false;
   }
@@ -371,7 +377,8 @@ bool Engine::HandleLocalizationPlatformMessage(PlatformMessage* message) {
   const auto& data = message->data();
 
   rapidjson::Document document;
-  document.Parse(reinterpret_cast<const char*>(data.data()), data.size());
+  document.Parse(reinterpret_cast<const char*>(data.GetMapping()),
+                 data.GetSize());
   if (document.HasParseError() || !document.IsObject()) {
     return false;
   }
@@ -411,7 +418,8 @@ bool Engine::HandleLocalizationPlatformMessage(PlatformMessage* message) {
 
 void Engine::HandleSettingsPlatformMessage(PlatformMessage* message) {
   const auto& data = message->data();
-  std::string jsonData(reinterpret_cast<const char*>(data.data()), data.size());
+  std::string jsonData(reinterpret_cast<const char*>(data.GetMapping()),
+                       data.GetSize());
   if (runtime_controller_->SetUserSettingsData(std::move(jsonData)) &&
       have_surface_) {
     ScheduleFrame();
@@ -436,7 +444,7 @@ void Engine::DispatchKeyDataPacket(std::unique_ptr<KeyDataPacket> packet,
 
 void Engine::DispatchSemanticsAction(int id,
                                      SemanticsAction action,
-                                     std::vector<uint8_t> args) {
+                                     fml::MallocMapping args) {
   runtime_controller_->DispatchSemanticsAction(id, action, std::move(args));
 }
 
@@ -538,8 +546,8 @@ void Engine::HandleAssetPlatformMessage(
     return;
   }
   const auto& data = message->data();
-  std::string asset_name(reinterpret_cast<const char*>(data.data()),
-                         data.size());
+  std::string asset_name(reinterpret_cast<const char*>(data.GetMapping()),
+                         data.GetSize());
 
   if (asset_manager_) {
     std::unique_ptr<fml::Mapping> asset_mapping =

shell/common/engine.h

@@ -763,7 +763,7 @@ class Engine final : public RuntimeDelegate,
   ///
   void DispatchSemanticsAction(int id,
                                SemanticsAction action,
-                               std::vector<uint8_t> args);
+                               fml::MallocMapping args);
 
   //----------------------------------------------------------------------------
   /// @brief      Notifies the engine that the embedder has expressed an opinion

shell/common/engine_unittests.cc

@@ -96,7 +96,7 @@ std::unique_ptr<PlatformMessage> MakePlatformMessage(
   const uint8_t* data = reinterpret_cast<const uint8_t*>(buffer.GetString());
 
   std::unique_ptr<PlatformMessage> message = std::make_unique<PlatformMessage>(
-      channel, std::vector<uint8_t>(data, data + buffer.GetSize()), response);
+      channel, fml::MallocMapping::Copy(data, buffer.GetSize()), response);
   return message;
 }