From dc66ee31d32fd7358823a1fa7d46d08e7ac43ba6 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Fri, 30 Apr 2021 15:33:55 -0700 Subject: [PATCH 1/5] switched platformmessages to hold fml::NonOwnedMapping instead of std::vector --- fml/mapping.cc | 38 ++++++++++++++++++- fml/mapping.h | 34 +++++++++++++++-- lib/ui/window/platform_configuration.cc | 12 +++--- lib/ui/window/platform_configuration.h | 2 +- lib/ui/window/platform_message.cc | 2 +- lib/ui/window/platform_message.h | 8 ++-- runtime/runtime_controller.cc | 2 +- runtime/runtime_controller.h | 2 +- shell/common/engine.cc | 30 +++++++++------ shell/common/engine.h | 2 +- shell/common/engine_unittests.cc | 3 +- shell/common/platform_view.cc | 2 +- shell/common/platform_view.h | 4 +- shell/common/shell.cc | 14 ++++--- shell/common/shell.h | 2 +- shell/common/shell_unittests.cc | 5 ++- .../platform/android/platform_view_android.cc | 12 +++--- .../android/platform_view_android_jni_impl.cc | 6 +-- shell/platform/common/accessibility_bridge.cc | 4 +- shell/platform/common/accessibility_bridge.h | 10 ++--- .../common/flutter_platform_node_delegate.h | 4 +- .../common/test_accessibility_bridge.cc | 2 +- .../common/test_accessibility_bridge.h | 2 +- .../darwin/common/buffer_conversions.h | 8 ++-- .../darwin/common/buffer_conversions.mm | 17 +++++---- .../ios/framework/Source/FlutterEngine.mm | 2 +- .../ios/framework/Source/SemanticsObject.mm | 5 ++- .../framework/Source/accessibility_bridge.h | 2 +- .../framework/Source/accessibility_bridge.mm | 2 +- .../Source/accessibility_bridge_ios.h | 3 +- .../platform_message_response_darwin.mm | 2 +- .../Source/platform_message_router.mm | 4 +- .../Source/AccessibilityBridgeMacDelegate.h | 2 +- .../Source/AccessibilityBridgeMacDelegate.mm | 4 +- .../macos/framework/Source/FlutterEngine.mm | 4 +- .../framework/Source/FlutterEngine_Internal.h | 2 +- shell/platform/embedder/embedder.cc | 15 ++++---- shell/platform/embedder/embedder_engine.cc | 2 +- shell/platform/embedder/embedder_engine.h | 2 +- shell/testing/tester_main.cc | 8 ++-- 40 files changed, 186 insertions(+), 100 deletions(-) diff --git a/fml/mapping.cc b/fml/mapping.cc index 64d09e93464fc..66076a1e63ffc 100644 --- a/fml/mapping.cc +++ b/fml/mapping.cc @@ -82,11 +82,39 @@ const uint8_t* DataMapping::GetMapping() const { } // NonOwnedMapping +NonOwnedMapping::NonOwnedMapping() + : data_(nullptr), size_(0), release_proc_(), uses_free_(false) {} NonOwnedMapping::NonOwnedMapping(const uint8_t* data, size_t size, const ReleaseProc& release_proc) - : data_(data), size_(size), release_proc_(release_proc) {} + : data_(data), + size_(size), + release_proc_(release_proc), + uses_free_(false) {} + +NonOwnedMapping::NonOwnedMapping(fml::NonOwnedMapping&& mapping) + : data_(mapping.data_), + size_(mapping.size_), + release_proc_(mapping.release_proc_), + uses_free_(mapping.uses_free_) { + mapping.data_ = nullptr; + mapping.size_ = 0; + mapping.release_proc_ = nullptr; + mapping.uses_free_ = false; +} + +namespace { +void FreeProc(const uint8_t* data, size_t size) { + free(const_cast(data)); +} +} // namespace + +NonOwnedMapping NonOwnedMapping::WithFree(const uint8_t* data, size_t size) { + auto result = NonOwnedMapping(data, size, FreeProc); + result.uses_free_ = true; + return result; +} NonOwnedMapping::~NonOwnedMapping() { if (release_proc_) { @@ -102,6 +130,14 @@ const uint8_t* NonOwnedMapping::GetMapping() const { return data_; } +const uint8_t* NonOwnedMapping::Release() { + const uint8_t* result = data_; + data_ = nullptr; + size_ = 0; + release_proc_ = nullptr; + return result; +} + // Symbol Mapping SymbolMapping::SymbolMapping(fml::RefPtr native_library, diff --git a/fml/mapping.h b/fml/mapping.h index f6b54ed7bbb8a..7065e6841a6bb 100644 --- a/fml/mapping.h +++ b/fml/mapping.h @@ -105,22 +105,50 @@ class DataMapping final : public Mapping { class NonOwnedMapping final : public Mapping { public: using ReleaseProc = std::function; + NonOwnedMapping(); + NonOwnedMapping(const uint8_t* data, size_t size, const ReleaseProc& release_proc = nullptr); + NonOwnedMapping(fml::NonOwnedMapping&& mapping); + ~NonOwnedMapping() 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 + static NonOwnedMapping Copy(const T* begin, const T* end) { + size_t length = end - begin; + auto result = NonOwnedMapping::WithFree( + reinterpret_cast(malloc(length)), length); + memcpy(const_cast(result.GetMapping()), begin, length); + return result; + } + // |Mapping| size_t GetSize() const override; // |Mapping| const uint8_t* GetMapping() const override; + /// Removes ownership of the data buffer. + const uint8_t* Release(); + + /// Returns true if the Mapping uses `free` to delete the memory. + /// This is important for passing ownership to C/ObjC code like NSData. It + /// has to be a special case because std::function comparisions aren't + /// possible. + bool UsesFree() const { return uses_free_; } + private: - const uint8_t* const data_; - const size_t size_; - const ReleaseProc release_proc_; + static NonOwnedMapping WithFree(const uint8_t* data, size_t size); + + const uint8_t* data_; + size_t size_; + ReleaseProc release_proc_; + bool uses_free_; FML_DISALLOW_COPY_AND_ASSIGN(NonOwnedMapping); }; diff --git a/lib/ui/window/platform_configuration.cc b/lib/ui/window/platform_configuration.cc index 981cbbf8b74de..f36d97064d9c1 100644 --- a/lib/ui/window/platform_configuration.cc +++ b/lib/ui/window/platform_configuration.cc @@ -130,7 +130,8 @@ Dart_Handle SendPlatformMessage(Dart_Handle window, const uint8_t* buffer = static_cast(data.data()); dart_state->platform_configuration()->client()->HandlePlatformMessage( std::make_unique( - name, std::vector(buffer, buffer + data.length_in_bytes()), + name, + fml::NonOwnedMapping::Copy(buffer, buffer + data.length_in_bytes()), response)); } @@ -190,8 +191,8 @@ void _RespondToKeyData(Dart_NativeArguments args) { tonic::DartCallStatic(&RespondToKeyData, args); } -Dart_Handle ToByteData(const std::vector& 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,7 +339,7 @@ void PlatformConfiguration::DispatchPlatformMessage( void PlatformConfiguration::DispatchSemanticsAction(int32_t id, SemanticsAction action, - std::vector args) { + fml::NonOwnedMapping args) { std::shared_ptr dart_state = dispatch_semantics_action_.dart_state().lock(); if (!dart_state) { @@ -346,7 +347,8 @@ void PlatformConfiguration::DispatchSemanticsAction(int32_t id, } 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; diff --git a/lib/ui/window/platform_configuration.h b/lib/ui/window/platform_configuration.h index 11aef73a2fff1..0143eb3bb6100 100644 --- a/lib/ui/window/platform_configuration.h +++ b/lib/ui/window/platform_configuration.h @@ -325,7 +325,7 @@ class PlatformConfiguration final { /// void DispatchSemanticsAction(int32_t id, SemanticsAction action, - std::vector args); + fml::NonOwnedMapping args); //---------------------------------------------------------------------------- /// @brief Registers a callback to be invoked when the framework has diff --git a/lib/ui/window/platform_message.cc b/lib/ui/window/platform_message.cc index 8423da599ed29..9429dd8064f4d 100644 --- a/lib/ui/window/platform_message.cc +++ b/lib/ui/window/platform_message.cc @@ -9,7 +9,7 @@ namespace flutter { PlatformMessage::PlatformMessage(std::string channel, - std::vector data, + fml::NonOwnedMapping data, fml::RefPtr response) : channel_(std::move(channel)), data_(std::move(data)), diff --git a/lib/ui/window/platform_message.h b/lib/ui/window/platform_message.h index c0e807bf32aa9..6ccac3f03c5fc 100644 --- a/lib/ui/window/platform_message.h +++ b/lib/ui/window/platform_message.h @@ -17,23 +17,25 @@ namespace flutter { class PlatformMessage { public: PlatformMessage(std::string channel, - std::vector data, + fml::NonOwnedMapping data, fml::RefPtr response); PlatformMessage(std::string channel, fml::RefPtr response); ~PlatformMessage(); const std::string& channel() const { return channel_; } - const std::vector& data() const { return data_; } + const fml::NonOwnedMapping& data() const { return data_; } bool hasData() { return hasData_; } const fml::RefPtr& response() const { return response_; } + fml::NonOwnedMapping releaseData() { return std::move(data_); } + private: std::string channel_; - std::vector data_; + fml::NonOwnedMapping data_; bool hasData_; fml::RefPtr response_; }; diff --git a/runtime/runtime_controller.cc b/runtime/runtime_controller.cc index f532cc772e956..75815f7e1447c 100644 --- a/runtime/runtime_controller.cc +++ b/runtime/runtime_controller.cc @@ -278,7 +278,7 @@ bool RuntimeController::DispatchKeyDataPacket(const KeyDataPacket& packet, bool RuntimeController::DispatchSemanticsAction(int32_t id, SemanticsAction action, - std::vector args) { + fml::NonOwnedMapping args) { TRACE_EVENT1("flutter", "RuntimeController::DispatchSemanticsAction", "mode", "basic"); if (auto* platform_configuration = GetPlatformConfigurationIfAvailable()) { diff --git a/runtime/runtime_controller.h b/runtime/runtime_controller.h index 9f90b022ae530..dd31c3c1c69ea 100644 --- a/runtime/runtime_controller.h +++ b/runtime/runtime_controller.h @@ -453,7 +453,7 @@ class RuntimeController : public PlatformConfigurationClient { /// bool DispatchSemanticsAction(int32_t id, SemanticsAction action, - std::vector args); + fml::NonOwnedMapping args); //---------------------------------------------------------------------------- /// @brief Gets the main port identifier of the root isolate. diff --git a/shell/common/engine.cc b/shell/common/engine.cc index 0203f986a8a2c..35f11a409acfa 100644 --- a/shell/common/engine.cc +++ b/shell/common/engine.cc @@ -4,6 +4,7 @@ #include "flutter/shell/common/engine.h" +#include #include #include #include @@ -35,6 +36,12 @@ static constexpr char kLocalizationChannel[] = "flutter/localization"; static constexpr char kSettingsChannel[] = "flutter/settings"; static constexpr char kIsolateChannel[] = "flutter/isolate"; +namespace { +fml::NonOwnedMapping MakeMapping(const std::string& str) { + return fml::NonOwnedMapping::Copy(str.c_str(), 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 service_id_message = std::make_unique( - kIsolateChannel, - std::vector(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 message) { bool Engine::HandleLifecyclePlatformMessage(PlatformMessage* message) { const auto& data = message->data(); - std::string state(reinterpret_cast(data.data()), data.size()); + std::string state(reinterpret_cast(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(data.data()), data.size()); + document.Parse(reinterpret_cast(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(data.data()), data.size()); + document.Parse(reinterpret_cast(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(data.data()), data.size()); + std::string jsonData(reinterpret_cast(data.GetMapping()), + data.GetSize()); if (runtime_controller_->SetUserSettingsData(std::move(jsonData)) && have_surface_) { ScheduleFrame(); @@ -436,7 +444,7 @@ void Engine::DispatchKeyDataPacket(std::unique_ptr packet, void Engine::DispatchSemanticsAction(int id, SemanticsAction action, - std::vector args) { + fml::NonOwnedMapping 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(data.data()), - data.size()); + std::string asset_name(reinterpret_cast(data.GetMapping()), + data.GetSize()); if (asset_manager_) { std::unique_ptr asset_mapping = diff --git a/shell/common/engine.h b/shell/common/engine.h index a4d17893dbadb..12e139d15a89f 100644 --- a/shell/common/engine.h +++ b/shell/common/engine.h @@ -763,7 +763,7 @@ class Engine final : public RuntimeDelegate, /// void DispatchSemanticsAction(int id, SemanticsAction action, - std::vector args); + fml::NonOwnedMapping args); //---------------------------------------------------------------------------- /// @brief Notifies the engine that the embedder has expressed an opinion diff --git a/shell/common/engine_unittests.cc b/shell/common/engine_unittests.cc index 44a3cce1ea724..dd1a214558c5a 100644 --- a/shell/common/engine_unittests.cc +++ b/shell/common/engine_unittests.cc @@ -96,7 +96,8 @@ std::unique_ptr MakePlatformMessage( const uint8_t* data = reinterpret_cast(buffer.GetString()); std::unique_ptr message = std::make_unique( - channel, std::vector(data, data + buffer.GetSize()), response); + channel, fml::NonOwnedMapping::Copy(data, data + buffer.GetSize()), + response); return message; } diff --git a/shell/common/platform_view.cc b/shell/common/platform_view.cc index 6db8ba0c29be5..4eafd80d7e902 100644 --- a/shell/common/platform_view.cc +++ b/shell/common/platform_view.cc @@ -50,7 +50,7 @@ void PlatformView::DispatchKeyDataPacket(std::unique_ptr packet, void PlatformView::DispatchSemanticsAction(int32_t id, SemanticsAction action, - std::vector args) { + fml::NonOwnedMapping args) { delegate_.OnPlatformViewDispatchSemanticsAction(id, action, std::move(args)); } diff --git a/shell/common/platform_view.h b/shell/common/platform_view.h index 52d42cd19baef..d1362f7d22b1d 100644 --- a/shell/common/platform_view.h +++ b/shell/common/platform_view.h @@ -157,7 +157,7 @@ class PlatformView { virtual void OnPlatformViewDispatchSemanticsAction( int32_t id, SemanticsAction action, - std::vector args) = 0; + fml::NonOwnedMapping args) = 0; //-------------------------------------------------------------------------- /// @brief Notifies the delegate that the embedder has expressed an @@ -408,7 +408,7 @@ class PlatformView { /// void DispatchSemanticsAction(int32_t id, SemanticsAction action, - std::vector args); + fml::NonOwnedMapping args); //---------------------------------------------------------------------------- /// @brief Used by embedder to notify the running isolate hosted by the diff --git a/shell/common/shell.cc b/shell/common/shell.cc index 7387ec75d51db..148429fc3f24e 100644 --- a/shell/common/shell.cc +++ b/shell/common/shell.cc @@ -998,16 +998,17 @@ void Shell::OnPlatformViewDispatchKeyDataPacket( // |PlatformView::Delegate| void Shell::OnPlatformViewDispatchSemanticsAction(int32_t id, SemanticsAction action, - std::vector args) { + fml::NonOwnedMapping args) { FML_DCHECK(is_setup_); FML_DCHECK(task_runners_.GetPlatformTaskRunner()->RunsTasksOnCurrentThread()); task_runners_.GetUITaskRunner()->PostTask( - [engine = engine_->GetWeakPtr(), id, action, args = std::move(args)] { + fml::MakeCopyable([engine = engine_->GetWeakPtr(), id, action, + args = std::move(args)]() mutable { if (engine) { engine->DispatchSemanticsAction(id, action, std::move(args)); } - }); + })); } // |PlatformView::Delegate| @@ -1225,7 +1226,8 @@ void Shell::HandleEngineSkiaMessage(std::unique_ptr message) { const auto& data = message->data(); rapidjson::Document document; - document.Parse(reinterpret_cast(data.data()), data.size()); + document.Parse(reinterpret_cast(data.GetMapping()), + data.GetSize()); if (document.HasParseError() || !document.IsObject()) return; auto root = document.GetObject(); @@ -1806,7 +1808,9 @@ bool Shell::ReloadSystemFonts() { std::string message = buffer.GetString(); std::unique_ptr fontsChangeMessage = std::make_unique( - kSystemChannel, std::vector(message.begin(), message.end()), + kSystemChannel, + fml::NonOwnedMapping::Copy(message.c_str(), + message.c_str() + message.length()), nullptr); OnPlatformViewDispatchPlatformMessage(std::move(fontsChangeMessage)); diff --git a/shell/common/shell.h b/shell/common/shell.h index 4bd957e218255..92e0dbe4797cf 100644 --- a/shell/common/shell.h +++ b/shell/common/shell.h @@ -496,7 +496,7 @@ class Shell final : public PlatformView::Delegate, void OnPlatformViewDispatchSemanticsAction( int32_t id, SemanticsAction action, - std::vector args) override; + fml::NonOwnedMapping args) override; // |PlatformView::Delegate| void OnPlatformViewSetSemanticsEnabled(bool enabled) override; diff --git a/shell/common/shell_unittests.cc b/shell/common/shell_unittests.cc index 123a29a9f2c43..e3aa57e1532ef 100644 --- a/shell/common/shell_unittests.cc +++ b/shell/common/shell_unittests.cc @@ -69,7 +69,7 @@ class MockPlatformViewDelegate : public PlatformView::Delegate { MOCK_METHOD3(OnPlatformViewDispatchSemanticsAction, void(int32_t id, SemanticsAction action, - std::vector args)); + fml::NonOwnedMapping args)); MOCK_METHOD1(OnPlatformViewSetSemanticsEnabled, void(bool enabled)); @@ -1489,7 +1489,8 @@ TEST_F(ShellTest, SetResourceCacheSize) { "method": "Skia.setResourceCacheMaxBytes", "args": 10000 })json"; - std::vector data(request_json.begin(), request_json.end()); + auto data = fml::NonOwnedMapping::Copy( + request_json.c_str(), request_json.c_str() + request_json.length()); auto platform_message = std::make_unique( "flutter/skia", std::move(data), nullptr); SendEnginePlatformMessage(shell.get(), std::move(platform_message)); diff --git a/shell/platform/android/platform_view_android.cc b/shell/platform/android/platform_view_android.cc index 1420fbebe4b9b..586b12a85b7d5 100644 --- a/shell/platform/android/platform_view_android.cc +++ b/shell/platform/android/platform_view_android.cc @@ -180,8 +180,8 @@ void PlatformViewAndroid::DispatchPlatformMessage(JNIEnv* env, jint response_id) { uint8_t* message_data = static_cast(env->GetDirectBufferAddress(java_message_data)); - std::vector message = - std::vector(message_data, message_data + java_message_position); + fml::NonOwnedMapping message = fml::NonOwnedMapping::Copy( + message_data, message_data + java_message_position); fml::RefPtr response; if (response_id) { @@ -266,15 +266,15 @@ void PlatformViewAndroid::DispatchSemanticsAction(JNIEnv* env, jobject args, jint args_position) { if (env->IsSameObject(args, NULL)) { - std::vector args_vector; PlatformView::DispatchSemanticsAction( - id, static_cast(action), args_vector); + id, static_cast(action), + fml::NonOwnedMapping()); return; } uint8_t* args_data = static_cast(env->GetDirectBufferAddress(args)); - std::vector args_vector = - std::vector(args_data, args_data + args_position); + auto args_vector = + fml::NonOwnedMapping::Copy(args_data, args_data + args_position); PlatformView::DispatchSemanticsAction( id, static_cast(action), diff --git a/shell/platform/android/platform_view_android_jni_impl.cc b/shell/platform/android/platform_view_android_jni_impl.cc index 2e6e81c615376..16e58e46ce280 100644 --- a/shell/platform/android/platform_view_android_jni_impl.cc +++ b/shell/platform/android/platform_view_android_jni_impl.cc @@ -1108,10 +1108,10 @@ void PlatformViewAndroidJNIImpl::FlutterViewHandlePlatformMessage( if (message->hasData()) { fml::jni::ScopedJavaLocalRef message_array( - env, env->NewByteArray(message->data().size())); + env, env->NewByteArray(message->data().GetSize())); env->SetByteArrayRegion( - message_array.obj(), 0, message->data().size(), - reinterpret_cast(message->data().data())); + message_array.obj(), 0, message->data().GetSize(), + reinterpret_cast(message->data().GetMapping())); env->CallVoidMethod(java_object.obj(), g_handle_platform_message_method, java_channel.obj(), message_array.obj(), responseId); } else { diff --git a/shell/platform/common/accessibility_bridge.cc b/shell/platform/common/accessibility_bridge.cc index 3923815952d85..5e90e451ec0bc 100644 --- a/shell/platform/common/accessibility_bridge.cc +++ b/shell/platform/common/accessibility_bridge.cc @@ -539,8 +539,8 @@ gfx::RectF AccessibilityBridge::RelativeToGlobalBounds(const ui::AXNode* node, void AccessibilityBridge::DispatchAccessibilityAction( AccessibilityNodeId target, FlutterSemanticsAction action, - std::vector data) { - delegate_->DispatchAccessibilityAction(target, action, data); + fml::NonOwnedMapping data) { + delegate_->DispatchAccessibilityAction(target, action, std::move(data)); } } // namespace flutter diff --git a/shell/platform/common/accessibility_bridge.h b/shell/platform/common/accessibility_bridge.h index faebd452988c8..9efd425375657 100644 --- a/shell/platform/common/accessibility_bridge.h +++ b/shell/platform/common/accessibility_bridge.h @@ -7,6 +7,7 @@ #include +#include "flutter/fml/mapping.h" #include "flutter/shell/platform/embedder/embedder.h" #include "flutter/third_party/accessibility/ax/ax_event_generator.h" @@ -86,10 +87,9 @@ class AccessibilityBridge /// @param[in] action The generated flutter semantics action. /// @param[in] data Additional data associated with the /// action. - virtual void DispatchAccessibilityAction( - AccessibilityNodeId target, - FlutterSemanticsAction action, - const std::vector& data) = 0; + virtual void DispatchAccessibilityAction(AccessibilityNodeId target, + FlutterSemanticsAction action, + fml::NonOwnedMapping data) = 0; //--------------------------------------------------------------------------- /// @brief Creates a platform specific FlutterPlatformNodeDelegate. @@ -277,7 +277,7 @@ class AccessibilityBridge // |FlutterPlatformNodeDelegate::OwnerBridge| void DispatchAccessibilityAction(AccessibilityNodeId target, FlutterSemanticsAction action, - std::vector data) override; + fml::NonOwnedMapping data) override; // |FlutterPlatformNodeDelegate::OwnerBridge| gfx::RectF RelativeToGlobalBounds(const ui::AXNode* node, diff --git a/shell/platform/common/flutter_platform_node_delegate.h b/shell/platform/common/flutter_platform_node_delegate.h index ca65ff708f492..8959a14cd239d 100644 --- a/shell/platform/common/flutter_platform_node_delegate.h +++ b/shell/platform/common/flutter_platform_node_delegate.h @@ -5,8 +5,8 @@ #ifndef FLUTTER_SHELL_PLATFORM_COMMON_FLUTTER_PLATFORM_NODE_DELEGATE_H_ #define FLUTTER_SHELL_PLATFORM_COMMON_FLUTTER_PLATFORM_NODE_DELEGATE_H_ +#include "flutter/fml/mapping.h" #include "flutter/shell/platform/embedder/embedder.h" - #include "flutter/third_party/accessibility/ax/ax_event_generator.h" #include "flutter/third_party/accessibility/ax/platform/ax_platform_node_delegate_base.h" @@ -57,7 +57,7 @@ class FlutterPlatformNodeDelegate : public ui::AXPlatformNodeDelegateBase { /// action. virtual void DispatchAccessibilityAction(AccessibilityNodeId target, FlutterSemanticsAction action, - std::vector data) = 0; + fml::NonOwnedMapping data) = 0; //--------------------------------------------------------------------------- /// @brief Get the native accessibility node with the given id. diff --git a/shell/platform/common/test_accessibility_bridge.cc b/shell/platform/common/test_accessibility_bridge.cc index 79677a9660256..7495535f60a60 100644 --- a/shell/platform/common/test_accessibility_bridge.cc +++ b/shell/platform/common/test_accessibility_bridge.cc @@ -19,7 +19,7 @@ void TestAccessibilityBridgeDelegate::OnAccessibilityEvent( void TestAccessibilityBridgeDelegate::DispatchAccessibilityAction( AccessibilityNodeId target, FlutterSemanticsAction action, - const std::vector& data) { + fml::NonOwnedMapping data) { performed_actions.push_back(action); } diff --git a/shell/platform/common/test_accessibility_bridge.h b/shell/platform/common/test_accessibility_bridge.h index 892a00d1221d0..c51ac9478928b 100644 --- a/shell/platform/common/test_accessibility_bridge.h +++ b/shell/platform/common/test_accessibility_bridge.h @@ -18,7 +18,7 @@ class TestAccessibilityBridgeDelegate ui::AXEventGenerator::TargetedEvent targeted_event) override; void DispatchAccessibilityAction(AccessibilityNodeId target, FlutterSemanticsAction action, - const std::vector& data) override; + fml::NonOwnedMapping data) override; std::unique_ptr CreateFlutterPlatformNodeDelegate(); diff --git a/shell/platform/darwin/common/buffer_conversions.h b/shell/platform/darwin/common/buffer_conversions.h index 60559b2dfe230..135437f3bd2b0 100644 --- a/shell/platform/darwin/common/buffer_conversions.h +++ b/shell/platform/darwin/common/buffer_conversions.h @@ -13,13 +13,13 @@ namespace flutter { -std::vector GetVectorFromNSData(NSData* data); +fml::NonOwnedMapping CopyNSDataToMapping(NSData* data); -NSData* GetNSDataFromVector(const std::vector& buffer); +NSData* CopyMappingToNSData(fml::NonOwnedMapping buffer); -std::unique_ptr GetMappingFromNSData(NSData* data); +std::unique_ptr CopyNSDataToMappingPtr(NSData* data); -NSData* GetNSDataFromMapping(std::unique_ptr mapping); +NSData* CopyMappingPtrToNSData(std::unique_ptr mapping); } // namespace flutter diff --git a/shell/platform/darwin/common/buffer_conversions.mm b/shell/platform/darwin/common/buffer_conversions.mm index 69e81de46592b..ebf1a9ef03529 100644 --- a/shell/platform/darwin/common/buffer_conversions.mm +++ b/shell/platform/darwin/common/buffer_conversions.mm @@ -6,20 +6,21 @@ namespace flutter { -std::vector GetVectorFromNSData(NSData* data) { - const uint8_t* bytes = reinterpret_cast(data.bytes); - return std::vector(bytes, bytes + data.length); +fml::NonOwnedMapping CopyNSDataToMapping(NSData* data) { + const uint8_t* bytes = static_cast(data.bytes); + return fml::NonOwnedMapping::Copy(bytes, bytes + data.length); } -NSData* GetNSDataFromVector(const std::vector& buffer) { - return [NSData dataWithBytes:buffer.data() length:buffer.size()]; +NSData* CopyMappingToNSData(fml::NonOwnedMapping buffer) { + return [NSData dataWithBytes:const_cast(buffer.GetMapping()) length:buffer.GetSize()]; } -std::unique_ptr GetMappingFromNSData(NSData* data) { - return std::make_unique(GetVectorFromNSData(data)); +std::unique_ptr CopyNSDataToMappingPtr(NSData* data) { + auto mapping = CopyNSDataToMapping(data); + return std::make_unique(std::move(mapping)); } -NSData* GetNSDataFromMapping(std::unique_ptr mapping) { +NSData* CopyMappingPtrToNSData(std::unique_ptr mapping) { return [NSData dataWithBytes:mapping->GetMapping() length:mapping->GetSize()]; } diff --git a/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm b/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm index 780bac01ea0f4..8155fd2de1817 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm @@ -807,7 +807,7 @@ - (void)sendOnChannel:(NSString*)channel std::unique_ptr platformMessage = (message == nil) ? std::make_unique(channel.UTF8String, response) : std::make_unique( - channel.UTF8String, flutter::GetVectorFromNSData(message), response); + channel.UTF8String, flutter::CopyNSDataToMapping(message), response); _shell->GetPlatformView()->DispatchPlatformMessage(std::move(platformMessage)); } diff --git a/shell/platform/darwin/ios/framework/Source/SemanticsObject.mm b/shell/platform/darwin/ios/framework/Source/SemanticsObject.mm index e78fda0b3288e..a57896e1e344d 100644 --- a/shell/platform/darwin/ios/framework/Source/SemanticsObject.mm +++ b/shell/platform/darwin/ios/framework/Source/SemanticsObject.mm @@ -308,8 +308,9 @@ - (BOOL)onCustomAccessibilityAction:(FlutterCustomAccessibilityAction*)action { args.push_back(action_id >> 8); args.push_back(action_id >> 16); args.push_back(action_id >> 24); - [self bridge]->DispatchSemanticsAction([self uid], flutter::SemanticsAction::kCustomAction, - std::move(args)); + [self bridge]->DispatchSemanticsAction( + [self uid], flutter::SemanticsAction::kCustomAction, + fml::NonOwnedMapping::Copy(args.data(), args.data() + args.size() * sizeof(uint8_t))); return YES; } diff --git a/shell/platform/darwin/ios/framework/Source/accessibility_bridge.h b/shell/platform/darwin/ios/framework/Source/accessibility_bridge.h index bcafa90c478c7..ea378da819462 100644 --- a/shell/platform/darwin/ios/framework/Source/accessibility_bridge.h +++ b/shell/platform/darwin/ios/framework/Source/accessibility_bridge.h @@ -60,7 +60,7 @@ class AccessibilityBridge final : public AccessibilityBridgeIos { void DispatchSemanticsAction(int32_t id, flutter::SemanticsAction action) override; void DispatchSemanticsAction(int32_t id, flutter::SemanticsAction action, - std::vector args) override; + fml::NonOwnedMapping args) override; void AccessibilityObjectDidBecomeFocused(int32_t id) override; void AccessibilityObjectDidLoseFocus(int32_t id) override; diff --git a/shell/platform/darwin/ios/framework/Source/accessibility_bridge.mm b/shell/platform/darwin/ios/framework/Source/accessibility_bridge.mm index 1c9f6e4f2529d..5ab6be966233d 100644 --- a/shell/platform/darwin/ios/framework/Source/accessibility_bridge.mm +++ b/shell/platform/darwin/ios/framework/Source/accessibility_bridge.mm @@ -233,7 +233,7 @@ void PostAccessibilityNotification(UIAccessibilityNotifications notification, void AccessibilityBridge::DispatchSemanticsAction(int32_t uid, flutter::SemanticsAction action, - std::vector args) { + fml::NonOwnedMapping args) { platform_view_->DispatchSemanticsAction(uid, action, std::move(args)); } diff --git a/shell/platform/darwin/ios/framework/Source/accessibility_bridge_ios.h b/shell/platform/darwin/ios/framework/Source/accessibility_bridge_ios.h index 3be237473e9cc..a27ad31b7ddb7 100644 --- a/shell/platform/darwin/ios/framework/Source/accessibility_bridge_ios.h +++ b/shell/platform/darwin/ios/framework/Source/accessibility_bridge_ios.h @@ -8,6 +8,7 @@ #include #include +#import "flutter/fml/mapping.h" #include "flutter/lib/ui/semantics/semantics_node.h" @class UIView; @@ -24,7 +25,7 @@ class AccessibilityBridgeIos { virtual void DispatchSemanticsAction(int32_t id, flutter::SemanticsAction action) = 0; virtual void DispatchSemanticsAction(int32_t id, flutter::SemanticsAction action, - std::vector args) = 0; + fml::NonOwnedMapping args) = 0; /** * A callback that is called when a SemanticObject receives focus. * diff --git a/shell/platform/darwin/ios/framework/Source/platform_message_response_darwin.mm b/shell/platform/darwin/ios/framework/Source/platform_message_response_darwin.mm index e7e09855c4625..53b04e484d6fb 100644 --- a/shell/platform/darwin/ios/framework/Source/platform_message_response_darwin.mm +++ b/shell/platform/darwin/ios/framework/Source/platform_message_response_darwin.mm @@ -17,7 +17,7 @@ void PlatformMessageResponseDarwin::Complete(std::unique_ptr data) { fml::RefPtr self(this); platform_task_runner_->PostTask(fml::MakeCopyable([self, data = std::move(data)]() mutable { - self->callback_.get()(GetNSDataFromMapping(std::move(data))); + self->callback_.get()(CopyMappingPtrToNSData(std::move(data))); })); } diff --git a/shell/platform/darwin/ios/framework/Source/platform_message_router.mm b/shell/platform/darwin/ios/framework/Source/platform_message_router.mm index 70ae8d75cb586..db8b53afa4125 100644 --- a/shell/platform/darwin/ios/framework/Source/platform_message_router.mm +++ b/shell/platform/darwin/ios/framework/Source/platform_message_router.mm @@ -22,12 +22,12 @@ FlutterBinaryMessageHandler handler = it->second; NSData* data = nil; if (message->hasData()) { - data = GetNSDataFromVector(message->data()); + data = CopyMappingToNSData(message->releaseData()); } handler(data, ^(NSData* reply) { if (completer) { if (reply) { - completer->Complete(GetMappingFromNSData(reply)); + completer->Complete(CopyNSDataToMappingPtr(reply)); } else { completer->CompleteEmpty(); } diff --git a/shell/platform/darwin/macos/framework/Source/AccessibilityBridgeMacDelegate.h b/shell/platform/darwin/macos/framework/Source/AccessibilityBridgeMacDelegate.h index e192732c50807..ef0b464c4d3a7 100644 --- a/shell/platform/darwin/macos/framework/Source/AccessibilityBridgeMacDelegate.h +++ b/shell/platform/darwin/macos/framework/Source/AccessibilityBridgeMacDelegate.h @@ -30,7 +30,7 @@ class AccessibilityBridgeMacDelegate : public AccessibilityBridge::Accessibility // |AccessibilityBridge::AccessibilityBridgeDelegate| void DispatchAccessibilityAction(AccessibilityNodeId target, FlutterSemanticsAction action, - const std::vector& data) override; + fml::NonOwnedMapping data) override; // |AccessibilityBridge::AccessibilityBridgeDelegate| std::unique_ptr CreateFlutterPlatformNodeDelegate() override; diff --git a/shell/platform/darwin/macos/framework/Source/AccessibilityBridgeMacDelegate.mm b/shell/platform/darwin/macos/framework/Source/AccessibilityBridgeMacDelegate.mm index 5db727eb28ccb..494b935bd4b41 100644 --- a/shell/platform/darwin/macos/framework/Source/AccessibilityBridgeMacDelegate.mm +++ b/shell/platform/darwin/macos/framework/Source/AccessibilityBridgeMacDelegate.mm @@ -339,12 +339,12 @@ void AccessibilityBridgeMacDelegate::DispatchAccessibilityAction(ui::AXNode::AXID target, FlutterSemanticsAction action, - const std::vector& data) { + fml::NonOwnedMapping data) { NSCAssert(flutter_engine_, @"Flutter engine should not be deallocated"); NSCAssert(flutter_engine_.viewController.viewLoaded && flutter_engine_.viewController.view.window, @"The accessibility bridge should not receive accessibility actions if the flutter view" @"is not loaded or attached to a NSWindow."); - [flutter_engine_ dispatchSemanticsAction:action toTarget:target withData:data]; + [flutter_engine_ dispatchSemanticsAction:action toTarget:target withData:std::move(data)]; } std::unique_ptr diff --git a/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm b/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm index a95a0343fd154..f4464c713df63 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm @@ -465,8 +465,8 @@ - (void)setSemanticsEnabled:(BOOL)enabled { - (void)dispatchSemanticsAction:(FlutterSemanticsAction)action toTarget:(uint16_t)target - withData:(const std::vector&)data { - _embedderAPI.DispatchSemanticsAction(_engine, target, action, data.data(), data.size()); + withData:(fml::NonOwnedMapping)data { + _embedderAPI.DispatchSemanticsAction(_engine, target, action, data.GetMapping(), data.GetSize()); } #pragma mark - Private methods diff --git a/shell/platform/darwin/macos/framework/Source/FlutterEngine_Internal.h b/shell/platform/darwin/macos/framework/Source/FlutterEngine_Internal.h index 5d2176ea59098..ca909d8a490e8 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterEngine_Internal.h +++ b/shell/platform/darwin/macos/framework/Source/FlutterEngine_Internal.h @@ -77,6 +77,6 @@ */ - (void)dispatchSemanticsAction:(FlutterSemanticsAction)action toTarget:(uint16_t)target - withData:(const std::vector&)data; + withData:(fml::NonOwnedMapping)data; @end diff --git a/shell/platform/embedder/embedder.cc b/shell/platform/embedder/embedder.cc index b9d432e3f4a8d..6ff32bd1ec203 100644 --- a/shell/platform/embedder/embedder.cc +++ b/shell/platform/embedder/embedder.cc @@ -1104,8 +1104,8 @@ FlutterEngineResult FlutterEngineInitialize(size_t version, const FlutterPlatformMessage incoming_message = { sizeof(FlutterPlatformMessage), // struct_size message->channel().c_str(), // channel - message->data().data(), // message - message->data().size(), // message_size + message->data().GetMapping(), // message + message->data().GetSize(), // message_size handle, // response_handle }; handle->message = std::move(message); @@ -1637,7 +1637,7 @@ FlutterEngineResult FlutterEngineSendPlatformMessage( } else { message = std::make_unique( flutter_message->channel, - std::vector(message_data, message_data + message_size), + fml::NonOwnedMapping::Copy(message_data, message_data + message_size), response); } @@ -1829,7 +1829,7 @@ FlutterEngineResult FlutterEngineDispatchSemanticsAction( if (!reinterpret_cast(engine) ->DispatchSemanticsAction( id, engine_action, - std::vector({data, data + data_length}))) { + fml::NonOwnedMapping::Copy(data, data + data_length))) { return LOG_EMBEDDER_ERROR(kInternalInconsistency, "Could not dispatch semantics action."); } @@ -1953,9 +1953,10 @@ static bool DispatchJSONPlatformMessage(FLUTTER_API_SYMBOL(FlutterEngine) } auto platform_message = std::make_unique( - channel_name.c_str(), // channel - std::vector{message, message + buffer.GetSize()}, // message - nullptr // response + channel_name.c_str(), // channel + fml::NonOwnedMapping::Copy(message, + message + buffer.GetSize()), // message + nullptr // response ); return reinterpret_cast(engine) diff --git a/shell/platform/embedder/embedder_engine.cc b/shell/platform/embedder/embedder_engine.cc index f179130e27ad5..2f666407147db 100644 --- a/shell/platform/embedder/embedder_engine.cc +++ b/shell/platform/embedder/embedder_engine.cc @@ -210,7 +210,7 @@ bool EmbedderEngine::SetAccessibilityFeatures(int32_t flags) { bool EmbedderEngine::DispatchSemanticsAction(int id, flutter::SemanticsAction action, - std::vector args) { + fml::NonOwnedMapping args) { if (!IsValid()) { return false; } diff --git a/shell/platform/embedder/embedder_engine.h b/shell/platform/embedder/embedder_engine.h index a2b0e398fd599..84b4324bd0bcc 100644 --- a/shell/platform/embedder/embedder_engine.h +++ b/shell/platform/embedder/embedder_engine.h @@ -82,7 +82,7 @@ class EmbedderEngine { bool DispatchSemanticsAction(int id, flutter::SemanticsAction action, - std::vector args); + fml::NonOwnedMapping args); bool OnVsyncEvent(intptr_t baton, fml::TimePoint frame_start_time, diff --git a/shell/testing/tester_main.cc b/shell/testing/tester_main.cc index bd751ca8ed46c..4d306485b9646 100644 --- a/shell/testing/tester_main.cc +++ b/shell/testing/tester_main.cc @@ -254,12 +254,12 @@ int RunTester(const flutter::Settings& settings, const char* locale_json = "{\"method\":\"setLocale\",\"args\":[\"en\",\"US\",\"\",\"\",\"zh\"," "\"CN\",\"\",\"\"]}"; - std::vector locale_bytes(locale_json, - locale_json + std::strlen(locale_json)); + auto locale_bytes = fml::NonOwnedMapping::Copy( + locale_json, locale_json + std::strlen(locale_json)); fml::RefPtr response; shell->GetPlatformView()->DispatchPlatformMessage( - std::make_unique("flutter/localization", - locale_bytes, response)); + std::make_unique( + "flutter/localization", std::move(locale_bytes), response)); std::initializer_list protection = { fml::FileMapping::Protection::kRead}; From 46796d9ca1de362fb0c64094f034e8c180d3fcef Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Mon, 3 May 2021 10:56:15 -0700 Subject: [PATCH 2/5] switched to MallocMapping --- fml/mapping.cc | 61 +++++++++---------- fml/mapping.h | 47 ++++++++------ lib/ui/window/platform_configuration.cc | 4 +- lib/ui/window/platform_configuration.h | 2 +- lib/ui/window/platform_message.cc | 2 +- lib/ui/window/platform_message.h | 8 +-- runtime/runtime_controller.cc | 2 +- runtime/runtime_controller.h | 2 +- shell/common/engine.cc | 6 +- shell/common/engine.h | 2 +- shell/common/engine_unittests.cc | 2 +- shell/common/platform_view.cc | 2 +- shell/common/platform_view.h | 4 +- shell/common/shell.cc | 6 +- shell/common/shell.h | 7 +-- shell/common/shell_unittests.cc | 9 +-- .../platform/android/platform_view_android.cc | 6 +- shell/platform/common/BUILD.gn | 1 + shell/platform/common/accessibility_bridge.cc | 2 +- shell/platform/common/accessibility_bridge.h | 4 +- .../common/flutter_platform_node_delegate.h | 2 +- .../common/test_accessibility_bridge.cc | 2 +- .../common/test_accessibility_bridge.h | 2 +- .../darwin/common/buffer_conversions.h | 4 +- .../darwin/common/buffer_conversions.mm | 8 +-- .../Source/FlutterEnginePlatformViewTest.mm | 2 +- .../Source/FlutterPlatformViewsTest.mm | 2 +- .../ios/framework/Source/SemanticsObject.mm | 2 +- .../framework/Source/SemanticsObjectTest.mm | 2 +- .../framework/Source/accessibility_bridge.h | 2 +- .../framework/Source/accessibility_bridge.mm | 2 +- .../Source/accessibility_bridge_ios.h | 2 +- .../Source/accessibility_bridge_test.mm | 2 +- .../Source/AccessibilityBridgeMacDelegate.h | 2 +- .../Source/AccessibilityBridgeMacDelegate.mm | 2 +- .../macos/framework/Source/FlutterEngine.mm | 2 +- .../framework/Source/FlutterEngine_Internal.h | 2 +- shell/platform/embedder/embedder.cc | 10 +-- shell/platform/embedder/embedder_engine.cc | 2 +- shell/platform/embedder/embedder_engine.h | 2 +- .../platform/fuchsia/flutter/fuchsia_intl.cc | 4 +- shell/platform/fuchsia/flutter/fuchsia_intl.h | 3 +- .../fuchsia/flutter/fuchsia_intl_unittest.cc | 9 ++- .../platform/fuchsia/flutter/platform_view.cc | 39 +++++++----- .../fuchsia/flutter/platform_view_unittest.cc | 43 ++++++------- shell/testing/tester_main.cc | 2 +- 46 files changed, 176 insertions(+), 159 deletions(-) diff --git a/fml/mapping.cc b/fml/mapping.cc index 66076a1e63ffc..25821dc4b9abe 100644 --- a/fml/mapping.cc +++ b/fml/mapping.cc @@ -82,39 +82,10 @@ const uint8_t* DataMapping::GetMapping() const { } // NonOwnedMapping -NonOwnedMapping::NonOwnedMapping() - : data_(nullptr), size_(0), release_proc_(), uses_free_(false) {} - NonOwnedMapping::NonOwnedMapping(const uint8_t* data, size_t size, const ReleaseProc& release_proc) - : data_(data), - size_(size), - release_proc_(release_proc), - uses_free_(false) {} - -NonOwnedMapping::NonOwnedMapping(fml::NonOwnedMapping&& mapping) - : data_(mapping.data_), - size_(mapping.size_), - release_proc_(mapping.release_proc_), - uses_free_(mapping.uses_free_) { - mapping.data_ = nullptr; - mapping.size_ = 0; - mapping.release_proc_ = nullptr; - mapping.uses_free_ = false; -} - -namespace { -void FreeProc(const uint8_t* data, size_t size) { - free(const_cast(data)); -} -} // namespace - -NonOwnedMapping NonOwnedMapping::WithFree(const uint8_t* data, size_t size) { - auto result = NonOwnedMapping(data, size, FreeProc); - result.uses_free_ = true; - return result; -} + : data_(data), size_(size), release_proc_(release_proc) {} NonOwnedMapping::~NonOwnedMapping() { if (release_proc_) { @@ -130,11 +101,37 @@ const uint8_t* NonOwnedMapping::GetMapping() const { return data_; } -const uint8_t* NonOwnedMapping::Release() { +// MallocMapping +MallocMapping::MallocMapping() : data_(nullptr), size_(0) {} + +MallocMapping::MallocMapping(const 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() { + if (data_) { + free(const_cast(data_)); + data_ = nullptr; + } +} + +size_t MallocMapping::GetSize() const { + return size_; +} + +const uint8_t* MallocMapping::GetMapping() const { + return data_; +} + +const uint8_t* MallocMapping::Release() { const uint8_t* result = data_; data_ = nullptr; size_ = 0; - release_proc_ = nullptr; return result; } diff --git a/fml/mapping.h b/fml/mapping.h index 7065e6841a6bb..6eee51557823a 100644 --- a/fml/mapping.h +++ b/fml/mapping.h @@ -105,24 +105,45 @@ class DataMapping final : public Mapping { class NonOwnedMapping final : public Mapping { public: using ReleaseProc = std::function; - NonOwnedMapping(); - NonOwnedMapping(const uint8_t* data, size_t size, const ReleaseProc& release_proc = nullptr); - NonOwnedMapping(fml::NonOwnedMapping&& mapping); - ~NonOwnedMapping() override; + // |Mapping| + size_t GetSize() const override; + + // |Mapping| + const uint8_t* GetMapping() const override; + + private: + const uint8_t* const data_; + const size_t size_; + const ReleaseProc release_proc_; + + FML_DISALLOW_COPY_AND_ASSIGN(NonOwnedMapping); +}; + +/// A Mapping like NonOwnedMapping, but uses Free as its release proc. +class MallocMapping final : public Mapping { + public: + MallocMapping(); + + MallocMapping(const 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 - static NonOwnedMapping Copy(const T* begin, const T* end) { + static MallocMapping Copy(const T* begin, const T* end) { size_t length = end - begin; - auto result = NonOwnedMapping::WithFree( - reinterpret_cast(malloc(length)), length); + auto result = + MallocMapping(reinterpret_cast(malloc(length)), length); memcpy(const_cast(result.GetMapping()), begin, length); return result; } @@ -136,21 +157,11 @@ class NonOwnedMapping final : public Mapping { /// Removes ownership of the data buffer. const uint8_t* Release(); - /// Returns true if the Mapping uses `free` to delete the memory. - /// This is important for passing ownership to C/ObjC code like NSData. It - /// has to be a special case because std::function comparisions aren't - /// possible. - bool UsesFree() const { return uses_free_; } - private: - static NonOwnedMapping WithFree(const uint8_t* data, size_t size); - const uint8_t* data_; size_t size_; - ReleaseProc release_proc_; - bool uses_free_; - FML_DISALLOW_COPY_AND_ASSIGN(NonOwnedMapping); + FML_DISALLOW_COPY_AND_ASSIGN(MallocMapping); }; class SymbolMapping final : public Mapping { diff --git a/lib/ui/window/platform_configuration.cc b/lib/ui/window/platform_configuration.cc index f36d97064d9c1..ace575b0d39a7 100644 --- a/lib/ui/window/platform_configuration.cc +++ b/lib/ui/window/platform_configuration.cc @@ -131,7 +131,7 @@ Dart_Handle SendPlatformMessage(Dart_Handle window, dart_state->platform_configuration()->client()->HandlePlatformMessage( std::make_unique( name, - fml::NonOwnedMapping::Copy(buffer, buffer + data.length_in_bytes()), + fml::MallocMapping::Copy(buffer, buffer + data.length_in_bytes()), response)); } @@ -339,7 +339,7 @@ void PlatformConfiguration::DispatchPlatformMessage( void PlatformConfiguration::DispatchSemanticsAction(int32_t id, SemanticsAction action, - fml::NonOwnedMapping args) { + fml::MallocMapping args) { std::shared_ptr dart_state = dispatch_semantics_action_.dart_state().lock(); if (!dart_state) { diff --git a/lib/ui/window/platform_configuration.h b/lib/ui/window/platform_configuration.h index 0143eb3bb6100..a32b2def91f49 100644 --- a/lib/ui/window/platform_configuration.h +++ b/lib/ui/window/platform_configuration.h @@ -325,7 +325,7 @@ class PlatformConfiguration final { /// void DispatchSemanticsAction(int32_t id, SemanticsAction action, - fml::NonOwnedMapping args); + fml::MallocMapping args); //---------------------------------------------------------------------------- /// @brief Registers a callback to be invoked when the framework has diff --git a/lib/ui/window/platform_message.cc b/lib/ui/window/platform_message.cc index 9429dd8064f4d..14cd7bb21c9d1 100644 --- a/lib/ui/window/platform_message.cc +++ b/lib/ui/window/platform_message.cc @@ -9,7 +9,7 @@ namespace flutter { PlatformMessage::PlatformMessage(std::string channel, - fml::NonOwnedMapping data, + fml::MallocMapping data, fml::RefPtr response) : channel_(std::move(channel)), data_(std::move(data)), diff --git a/lib/ui/window/platform_message.h b/lib/ui/window/platform_message.h index 6ccac3f03c5fc..7fe09cdcac1c7 100644 --- a/lib/ui/window/platform_message.h +++ b/lib/ui/window/platform_message.h @@ -17,25 +17,25 @@ namespace flutter { class PlatformMessage { public: PlatformMessage(std::string channel, - fml::NonOwnedMapping data, + fml::MallocMapping data, fml::RefPtr response); PlatformMessage(std::string channel, fml::RefPtr response); ~PlatformMessage(); const std::string& channel() const { return channel_; } - const fml::NonOwnedMapping& data() const { return data_; } + const fml::MallocMapping& data() const { return data_; } bool hasData() { return hasData_; } const fml::RefPtr& response() const { return response_; } - fml::NonOwnedMapping releaseData() { return std::move(data_); } + fml::MallocMapping releaseData() { return std::move(data_); } private: std::string channel_; - fml::NonOwnedMapping data_; + fml::MallocMapping data_; bool hasData_; fml::RefPtr response_; }; diff --git a/runtime/runtime_controller.cc b/runtime/runtime_controller.cc index 75815f7e1447c..7da201b7a056f 100644 --- a/runtime/runtime_controller.cc +++ b/runtime/runtime_controller.cc @@ -278,7 +278,7 @@ bool RuntimeController::DispatchKeyDataPacket(const KeyDataPacket& packet, bool RuntimeController::DispatchSemanticsAction(int32_t id, SemanticsAction action, - fml::NonOwnedMapping args) { + fml::MallocMapping args) { TRACE_EVENT1("flutter", "RuntimeController::DispatchSemanticsAction", "mode", "basic"); if (auto* platform_configuration = GetPlatformConfigurationIfAvailable()) { diff --git a/runtime/runtime_controller.h b/runtime/runtime_controller.h index dd31c3c1c69ea..38976557496e9 100644 --- a/runtime/runtime_controller.h +++ b/runtime/runtime_controller.h @@ -453,7 +453,7 @@ class RuntimeController : public PlatformConfigurationClient { /// bool DispatchSemanticsAction(int32_t id, SemanticsAction action, - fml::NonOwnedMapping args); + fml::MallocMapping args); //---------------------------------------------------------------------------- /// @brief Gets the main port identifier of the root isolate. diff --git a/shell/common/engine.cc b/shell/common/engine.cc index 35f11a409acfa..9155019aea57d 100644 --- a/shell/common/engine.cc +++ b/shell/common/engine.cc @@ -37,8 +37,8 @@ static constexpr char kSettingsChannel[] = "flutter/settings"; static constexpr char kIsolateChannel[] = "flutter/isolate"; namespace { -fml::NonOwnedMapping MakeMapping(const std::string& str) { - return fml::NonOwnedMapping::Copy(str.c_str(), str.c_str() + str.length()); +fml::MallocMapping MakeMapping(const std::string& str) { + return fml::MallocMapping::Copy(str.c_str(), str.c_str() + str.length()); } } // namespace @@ -444,7 +444,7 @@ void Engine::DispatchKeyDataPacket(std::unique_ptr packet, void Engine::DispatchSemanticsAction(int id, SemanticsAction action, - fml::NonOwnedMapping args) { + fml::MallocMapping args) { runtime_controller_->DispatchSemanticsAction(id, action, std::move(args)); } diff --git a/shell/common/engine.h b/shell/common/engine.h index 12e139d15a89f..1b42fe244d180 100644 --- a/shell/common/engine.h +++ b/shell/common/engine.h @@ -763,7 +763,7 @@ class Engine final : public RuntimeDelegate, /// void DispatchSemanticsAction(int id, SemanticsAction action, - fml::NonOwnedMapping args); + fml::MallocMapping args); //---------------------------------------------------------------------------- /// @brief Notifies the engine that the embedder has expressed an opinion diff --git a/shell/common/engine_unittests.cc b/shell/common/engine_unittests.cc index dd1a214558c5a..8c2784c0f19ed 100644 --- a/shell/common/engine_unittests.cc +++ b/shell/common/engine_unittests.cc @@ -96,7 +96,7 @@ std::unique_ptr MakePlatformMessage( const uint8_t* data = reinterpret_cast(buffer.GetString()); std::unique_ptr message = std::make_unique( - channel, fml::NonOwnedMapping::Copy(data, data + buffer.GetSize()), + channel, fml::MallocMapping::Copy(data, data + buffer.GetSize()), response); return message; } diff --git a/shell/common/platform_view.cc b/shell/common/platform_view.cc index 4eafd80d7e902..7c7b4dd1d421e 100644 --- a/shell/common/platform_view.cc +++ b/shell/common/platform_view.cc @@ -50,7 +50,7 @@ void PlatformView::DispatchKeyDataPacket(std::unique_ptr packet, void PlatformView::DispatchSemanticsAction(int32_t id, SemanticsAction action, - fml::NonOwnedMapping args) { + fml::MallocMapping args) { delegate_.OnPlatformViewDispatchSemanticsAction(id, action, std::move(args)); } diff --git a/shell/common/platform_view.h b/shell/common/platform_view.h index d1362f7d22b1d..186237b02ad33 100644 --- a/shell/common/platform_view.h +++ b/shell/common/platform_view.h @@ -157,7 +157,7 @@ class PlatformView { virtual void OnPlatformViewDispatchSemanticsAction( int32_t id, SemanticsAction action, - fml::NonOwnedMapping args) = 0; + fml::MallocMapping args) = 0; //-------------------------------------------------------------------------- /// @brief Notifies the delegate that the embedder has expressed an @@ -408,7 +408,7 @@ class PlatformView { /// void DispatchSemanticsAction(int32_t id, SemanticsAction action, - fml::NonOwnedMapping args); + fml::MallocMapping args); //---------------------------------------------------------------------------- /// @brief Used by embedder to notify the running isolate hosted by the diff --git a/shell/common/shell.cc b/shell/common/shell.cc index 148429fc3f24e..27777640df5dd 100644 --- a/shell/common/shell.cc +++ b/shell/common/shell.cc @@ -998,7 +998,7 @@ void Shell::OnPlatformViewDispatchKeyDataPacket( // |PlatformView::Delegate| void Shell::OnPlatformViewDispatchSemanticsAction(int32_t id, SemanticsAction action, - fml::NonOwnedMapping args) { + fml::MallocMapping args) { FML_DCHECK(is_setup_); FML_DCHECK(task_runners_.GetPlatformTaskRunner()->RunsTasksOnCurrentThread()); @@ -1809,8 +1809,8 @@ bool Shell::ReloadSystemFonts() { std::unique_ptr fontsChangeMessage = std::make_unique( kSystemChannel, - fml::NonOwnedMapping::Copy(message.c_str(), - message.c_str() + message.length()), + fml::MallocMapping::Copy(message.c_str(), + message.c_str() + message.length()), nullptr); OnPlatformViewDispatchPlatformMessage(std::move(fontsChangeMessage)); diff --git a/shell/common/shell.h b/shell/common/shell.h index 92e0dbe4797cf..af0e85b7cef20 100644 --- a/shell/common/shell.h +++ b/shell/common/shell.h @@ -493,10 +493,9 @@ class Shell final : public PlatformView::Delegate, std::function callback) override; // |PlatformView::Delegate| - void OnPlatformViewDispatchSemanticsAction( - int32_t id, - SemanticsAction action, - fml::NonOwnedMapping args) override; + void OnPlatformViewDispatchSemanticsAction(int32_t id, + SemanticsAction action, + fml::MallocMapping args) override; // |PlatformView::Delegate| void OnPlatformViewSetSemanticsEnabled(bool enabled) override; diff --git a/shell/common/shell_unittests.cc b/shell/common/shell_unittests.cc index e3aa57e1532ef..313fed4db7b4a 100644 --- a/shell/common/shell_unittests.cc +++ b/shell/common/shell_unittests.cc @@ -69,7 +69,7 @@ class MockPlatformViewDelegate : public PlatformView::Delegate { MOCK_METHOD3(OnPlatformViewDispatchSemanticsAction, void(int32_t id, SemanticsAction action, - fml::NonOwnedMapping args)); + fml::MallocMapping args)); MOCK_METHOD1(OnPlatformViewSetSemanticsEnabled, void(bool enabled)); @@ -1489,7 +1489,7 @@ TEST_F(ShellTest, SetResourceCacheSize) { "method": "Skia.setResourceCacheMaxBytes", "args": 10000 })json"; - auto data = fml::NonOwnedMapping::Copy( + auto data = fml::MallocMapping::Copy( request_json.c_str(), request_json.c_str() + request_json.length()); auto platform_message = std::make_unique( "flutter/skia", std::move(data), nullptr); @@ -1812,10 +1812,7 @@ TEST_F(ShellTest, CanConvertToAndFromMappings) { buffer, buffer_size, MemsetPatternOp::kMemsetPatternOpSetBuffer)); std::unique_ptr mapping = - std::make_unique( - buffer, buffer_size, [](const uint8_t* buffer, size_t size) { - ::free(const_cast(buffer)); - }); + std::make_unique(buffer, buffer_size); ASSERT_EQ(mapping->GetSize(), buffer_size); diff --git a/shell/platform/android/platform_view_android.cc b/shell/platform/android/platform_view_android.cc index 586b12a85b7d5..380526a963573 100644 --- a/shell/platform/android/platform_view_android.cc +++ b/shell/platform/android/platform_view_android.cc @@ -180,7 +180,7 @@ void PlatformViewAndroid::DispatchPlatformMessage(JNIEnv* env, jint response_id) { uint8_t* message_data = static_cast(env->GetDirectBufferAddress(java_message_data)); - fml::NonOwnedMapping message = fml::NonOwnedMapping::Copy( + fml::MallocMapping message = fml::MallocMapping::Copy( message_data, message_data + java_message_position); fml::RefPtr response; @@ -268,13 +268,13 @@ void PlatformViewAndroid::DispatchSemanticsAction(JNIEnv* env, if (env->IsSameObject(args, NULL)) { PlatformView::DispatchSemanticsAction( id, static_cast(action), - fml::NonOwnedMapping()); + fml::MallocMapping()); return; } uint8_t* args_data = static_cast(env->GetDirectBufferAddress(args)); auto args_vector = - fml::NonOwnedMapping::Copy(args_data, args_data + args_position); + fml::MallocMapping::Copy(args_data, args_data + args_position); PlatformView::DispatchSemanticsAction( id, static_cast(action), diff --git a/shell/platform/common/BUILD.gn b/shell/platform/common/BUILD.gn index 323f8e5477853..8b3cf4c25a597 100644 --- a/shell/platform/common/BUILD.gn +++ b/shell/platform/common/BUILD.gn @@ -83,6 +83,7 @@ source_set("common_cpp_accessibility") { [ "//flutter/third_party/accessibility:accessibility_config" ] public_deps = [ + "//flutter/fml:fml", "//flutter/shell/platform/embedder:embedder_as_internal_library", "//flutter/third_party/accessibility", ] diff --git a/shell/platform/common/accessibility_bridge.cc b/shell/platform/common/accessibility_bridge.cc index 5e90e451ec0bc..e68a18ae371ff 100644 --- a/shell/platform/common/accessibility_bridge.cc +++ b/shell/platform/common/accessibility_bridge.cc @@ -539,7 +539,7 @@ gfx::RectF AccessibilityBridge::RelativeToGlobalBounds(const ui::AXNode* node, void AccessibilityBridge::DispatchAccessibilityAction( AccessibilityNodeId target, FlutterSemanticsAction action, - fml::NonOwnedMapping data) { + fml::MallocMapping data) { delegate_->DispatchAccessibilityAction(target, action, std::move(data)); } diff --git a/shell/platform/common/accessibility_bridge.h b/shell/platform/common/accessibility_bridge.h index 9efd425375657..ab8969ab47cb7 100644 --- a/shell/platform/common/accessibility_bridge.h +++ b/shell/platform/common/accessibility_bridge.h @@ -89,7 +89,7 @@ class AccessibilityBridge /// action. virtual void DispatchAccessibilityAction(AccessibilityNodeId target, FlutterSemanticsAction action, - fml::NonOwnedMapping data) = 0; + fml::MallocMapping data) = 0; //--------------------------------------------------------------------------- /// @brief Creates a platform specific FlutterPlatformNodeDelegate. @@ -277,7 +277,7 @@ class AccessibilityBridge // |FlutterPlatformNodeDelegate::OwnerBridge| void DispatchAccessibilityAction(AccessibilityNodeId target, FlutterSemanticsAction action, - fml::NonOwnedMapping data) override; + fml::MallocMapping data) override; // |FlutterPlatformNodeDelegate::OwnerBridge| gfx::RectF RelativeToGlobalBounds(const ui::AXNode* node, diff --git a/shell/platform/common/flutter_platform_node_delegate.h b/shell/platform/common/flutter_platform_node_delegate.h index 8959a14cd239d..3a562d1c345a0 100644 --- a/shell/platform/common/flutter_platform_node_delegate.h +++ b/shell/platform/common/flutter_platform_node_delegate.h @@ -57,7 +57,7 @@ class FlutterPlatformNodeDelegate : public ui::AXPlatformNodeDelegateBase { /// action. virtual void DispatchAccessibilityAction(AccessibilityNodeId target, FlutterSemanticsAction action, - fml::NonOwnedMapping data) = 0; + fml::MallocMapping data) = 0; //--------------------------------------------------------------------------- /// @brief Get the native accessibility node with the given id. diff --git a/shell/platform/common/test_accessibility_bridge.cc b/shell/platform/common/test_accessibility_bridge.cc index 7495535f60a60..7127989a8d4f7 100644 --- a/shell/platform/common/test_accessibility_bridge.cc +++ b/shell/platform/common/test_accessibility_bridge.cc @@ -19,7 +19,7 @@ void TestAccessibilityBridgeDelegate::OnAccessibilityEvent( void TestAccessibilityBridgeDelegate::DispatchAccessibilityAction( AccessibilityNodeId target, FlutterSemanticsAction action, - fml::NonOwnedMapping data) { + fml::MallocMapping data) { performed_actions.push_back(action); } diff --git a/shell/platform/common/test_accessibility_bridge.h b/shell/platform/common/test_accessibility_bridge.h index c51ac9478928b..427b88c276cbb 100644 --- a/shell/platform/common/test_accessibility_bridge.h +++ b/shell/platform/common/test_accessibility_bridge.h @@ -18,7 +18,7 @@ class TestAccessibilityBridgeDelegate ui::AXEventGenerator::TargetedEvent targeted_event) override; void DispatchAccessibilityAction(AccessibilityNodeId target, FlutterSemanticsAction action, - fml::NonOwnedMapping data) override; + fml::MallocMapping data) override; std::unique_ptr CreateFlutterPlatformNodeDelegate(); diff --git a/shell/platform/darwin/common/buffer_conversions.h b/shell/platform/darwin/common/buffer_conversions.h index 135437f3bd2b0..7dd90f558d6cb 100644 --- a/shell/platform/darwin/common/buffer_conversions.h +++ b/shell/platform/darwin/common/buffer_conversions.h @@ -13,9 +13,9 @@ namespace flutter { -fml::NonOwnedMapping CopyNSDataToMapping(NSData* data); +fml::MallocMapping CopyNSDataToMapping(NSData* data); -NSData* CopyMappingToNSData(fml::NonOwnedMapping buffer); +NSData* CopyMappingToNSData(fml::MallocMapping buffer); std::unique_ptr CopyNSDataToMappingPtr(NSData* data); diff --git a/shell/platform/darwin/common/buffer_conversions.mm b/shell/platform/darwin/common/buffer_conversions.mm index ebf1a9ef03529..5ddbe04a21d5d 100644 --- a/shell/platform/darwin/common/buffer_conversions.mm +++ b/shell/platform/darwin/common/buffer_conversions.mm @@ -6,18 +6,18 @@ namespace flutter { -fml::NonOwnedMapping CopyNSDataToMapping(NSData* data) { +fml::MallocMapping CopyNSDataToMapping(NSData* data) { const uint8_t* bytes = static_cast(data.bytes); - return fml::NonOwnedMapping::Copy(bytes, bytes + data.length); + return fml::MallocMapping::Copy(bytes, bytes + data.length); } -NSData* CopyMappingToNSData(fml::NonOwnedMapping buffer) { +NSData* CopyMappingToNSData(fml::MallocMapping buffer) { return [NSData dataWithBytes:const_cast(buffer.GetMapping()) length:buffer.GetSize()]; } std::unique_ptr CopyNSDataToMappingPtr(NSData* data) { auto mapping = CopyNSDataToMapping(data); - return std::make_unique(std::move(mapping)); + return std::make_unique(std::move(mapping)); } NSData* CopyMappingPtrToNSData(std::unique_ptr mapping) { diff --git a/shell/platform/darwin/ios/framework/Source/FlutterEnginePlatformViewTest.mm b/shell/platform/darwin/ios/framework/Source/FlutterEnginePlatformViewTest.mm index 5710f832332d1..0d339fbc722f6 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterEnginePlatformViewTest.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterEnginePlatformViewTest.mm @@ -29,7 +29,7 @@ void OnPlatformViewDispatchKeyDataPacket(std::unique_ptr packet, std::function callback) override {} void OnPlatformViewDispatchSemanticsAction(int32_t id, SemanticsAction action, - std::vector args) override {} + fml::MallocMapping args) override {} void OnPlatformViewSetSemanticsEnabled(bool enabled) override {} void OnPlatformViewSetAccessibilityFeatures(int32_t flags) override {} void OnPlatformViewRegisterTexture(std::shared_ptr texture) override {} diff --git a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViewsTest.mm b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViewsTest.mm index a22d182792e68..6cd7c349c8dfe 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViewsTest.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViewsTest.mm @@ -99,7 +99,7 @@ void OnPlatformViewDispatchKeyDataPacket(std::unique_ptr packet, std::function callback) override {} void OnPlatformViewDispatchSemanticsAction(int32_t id, SemanticsAction action, - std::vector args) override {} + fml::MallocMapping args) override {} void OnPlatformViewSetSemanticsEnabled(bool enabled) override {} void OnPlatformViewSetAccessibilityFeatures(int32_t flags) override {} void OnPlatformViewRegisterTexture(std::shared_ptr texture) override {} diff --git a/shell/platform/darwin/ios/framework/Source/SemanticsObject.mm b/shell/platform/darwin/ios/framework/Source/SemanticsObject.mm index a57896e1e344d..c17fac4a18071 100644 --- a/shell/platform/darwin/ios/framework/Source/SemanticsObject.mm +++ b/shell/platform/darwin/ios/framework/Source/SemanticsObject.mm @@ -310,7 +310,7 @@ - (BOOL)onCustomAccessibilityAction:(FlutterCustomAccessibilityAction*)action { args.push_back(action_id >> 24); [self bridge]->DispatchSemanticsAction( [self uid], flutter::SemanticsAction::kCustomAction, - fml::NonOwnedMapping::Copy(args.data(), args.data() + args.size() * sizeof(uint8_t))); + fml::MallocMapping::Copy(args.data(), args.data() + args.size() * sizeof(uint8_t))); return YES; } diff --git a/shell/platform/darwin/ios/framework/Source/SemanticsObjectTest.mm b/shell/platform/darwin/ios/framework/Source/SemanticsObjectTest.mm index b9179a27f6ad5..8380ce735aaeb 100644 --- a/shell/platform/darwin/ios/framework/Source/SemanticsObjectTest.mm +++ b/shell/platform/darwin/ios/framework/Source/SemanticsObjectTest.mm @@ -33,7 +33,7 @@ void DispatchSemanticsAction(int32_t id, SemanticsAction action) override { } void DispatchSemanticsAction(int32_t id, SemanticsAction action, - std::vector args) override { + fml::MallocMapping args) override { SemanticsActionObservation observation(id, action); observations.push_back(observation); } diff --git a/shell/platform/darwin/ios/framework/Source/accessibility_bridge.h b/shell/platform/darwin/ios/framework/Source/accessibility_bridge.h index ea378da819462..c1456ab446e1b 100644 --- a/shell/platform/darwin/ios/framework/Source/accessibility_bridge.h +++ b/shell/platform/darwin/ios/framework/Source/accessibility_bridge.h @@ -60,7 +60,7 @@ class AccessibilityBridge final : public AccessibilityBridgeIos { void DispatchSemanticsAction(int32_t id, flutter::SemanticsAction action) override; void DispatchSemanticsAction(int32_t id, flutter::SemanticsAction action, - fml::NonOwnedMapping args) override; + fml::MallocMapping args) override; void AccessibilityObjectDidBecomeFocused(int32_t id) override; void AccessibilityObjectDidLoseFocus(int32_t id) override; diff --git a/shell/platform/darwin/ios/framework/Source/accessibility_bridge.mm b/shell/platform/darwin/ios/framework/Source/accessibility_bridge.mm index 5ab6be966233d..77b1a95ff4bbe 100644 --- a/shell/platform/darwin/ios/framework/Source/accessibility_bridge.mm +++ b/shell/platform/darwin/ios/framework/Source/accessibility_bridge.mm @@ -233,7 +233,7 @@ void PostAccessibilityNotification(UIAccessibilityNotifications notification, void AccessibilityBridge::DispatchSemanticsAction(int32_t uid, flutter::SemanticsAction action, - fml::NonOwnedMapping args) { + fml::MallocMapping args) { platform_view_->DispatchSemanticsAction(uid, action, std::move(args)); } diff --git a/shell/platform/darwin/ios/framework/Source/accessibility_bridge_ios.h b/shell/platform/darwin/ios/framework/Source/accessibility_bridge_ios.h index a27ad31b7ddb7..a97f179ece53e 100644 --- a/shell/platform/darwin/ios/framework/Source/accessibility_bridge_ios.h +++ b/shell/platform/darwin/ios/framework/Source/accessibility_bridge_ios.h @@ -25,7 +25,7 @@ class AccessibilityBridgeIos { virtual void DispatchSemanticsAction(int32_t id, flutter::SemanticsAction action) = 0; virtual void DispatchSemanticsAction(int32_t id, flutter::SemanticsAction action, - fml::NonOwnedMapping args) = 0; + fml::MallocMapping args) = 0; /** * A callback that is called when a SemanticObject receives focus. * diff --git a/shell/platform/darwin/ios/framework/Source/accessibility_bridge_test.mm b/shell/platform/darwin/ios/framework/Source/accessibility_bridge_test.mm index f9bf1c2a2f86b..4ddda97355523 100644 --- a/shell/platform/darwin/ios/framework/Source/accessibility_bridge_test.mm +++ b/shell/platform/darwin/ios/framework/Source/accessibility_bridge_test.mm @@ -82,7 +82,7 @@ void OnPlatformViewDispatchKeyDataPacket(std::unique_ptr packet, std::function callback) override {} void OnPlatformViewDispatchSemanticsAction(int32_t id, SemanticsAction action, - std::vector args) override {} + fml::MallocMapping args) override {} void OnPlatformViewSetSemanticsEnabled(bool enabled) override {} void OnPlatformViewSetAccessibilityFeatures(int32_t flags) override {} void OnPlatformViewRegisterTexture(std::shared_ptr texture) override {} diff --git a/shell/platform/darwin/macos/framework/Source/AccessibilityBridgeMacDelegate.h b/shell/platform/darwin/macos/framework/Source/AccessibilityBridgeMacDelegate.h index ef0b464c4d3a7..ea51f93c2cf31 100644 --- a/shell/platform/darwin/macos/framework/Source/AccessibilityBridgeMacDelegate.h +++ b/shell/platform/darwin/macos/framework/Source/AccessibilityBridgeMacDelegate.h @@ -30,7 +30,7 @@ class AccessibilityBridgeMacDelegate : public AccessibilityBridge::Accessibility // |AccessibilityBridge::AccessibilityBridgeDelegate| void DispatchAccessibilityAction(AccessibilityNodeId target, FlutterSemanticsAction action, - fml::NonOwnedMapping data) override; + fml::MallocMapping data) override; // |AccessibilityBridge::AccessibilityBridgeDelegate| std::unique_ptr CreateFlutterPlatformNodeDelegate() override; diff --git a/shell/platform/darwin/macos/framework/Source/AccessibilityBridgeMacDelegate.mm b/shell/platform/darwin/macos/framework/Source/AccessibilityBridgeMacDelegate.mm index 494b935bd4b41..35746b04e64ed 100644 --- a/shell/platform/darwin/macos/framework/Source/AccessibilityBridgeMacDelegate.mm +++ b/shell/platform/darwin/macos/framework/Source/AccessibilityBridgeMacDelegate.mm @@ -339,7 +339,7 @@ void AccessibilityBridgeMacDelegate::DispatchAccessibilityAction(ui::AXNode::AXID target, FlutterSemanticsAction action, - fml::NonOwnedMapping data) { + fml::MallocMapping data) { NSCAssert(flutter_engine_, @"Flutter engine should not be deallocated"); NSCAssert(flutter_engine_.viewController.viewLoaded && flutter_engine_.viewController.view.window, @"The accessibility bridge should not receive accessibility actions if the flutter view" diff --git a/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm b/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm index f4464c713df63..58a212a1bb42c 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm @@ -465,7 +465,7 @@ - (void)setSemanticsEnabled:(BOOL)enabled { - (void)dispatchSemanticsAction:(FlutterSemanticsAction)action toTarget:(uint16_t)target - withData:(fml::NonOwnedMapping)data { + withData:(fml::MallocMapping)data { _embedderAPI.DispatchSemanticsAction(_engine, target, action, data.GetMapping(), data.GetSize()); } diff --git a/shell/platform/darwin/macos/framework/Source/FlutterEngine_Internal.h b/shell/platform/darwin/macos/framework/Source/FlutterEngine_Internal.h index ca909d8a490e8..a379e93d867a7 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterEngine_Internal.h +++ b/shell/platform/darwin/macos/framework/Source/FlutterEngine_Internal.h @@ -77,6 +77,6 @@ */ - (void)dispatchSemanticsAction:(FlutterSemanticsAction)action toTarget:(uint16_t)target - withData:(fml::NonOwnedMapping)data; + withData:(fml::MallocMapping)data; @end diff --git a/shell/platform/embedder/embedder.cc b/shell/platform/embedder/embedder.cc index 6ff32bd1ec203..e2b39e961e7c3 100644 --- a/shell/platform/embedder/embedder.cc +++ b/shell/platform/embedder/embedder.cc @@ -1637,7 +1637,7 @@ FlutterEngineResult FlutterEngineSendPlatformMessage( } else { message = std::make_unique( flutter_message->channel, - fml::NonOwnedMapping::Copy(message_data, message_data + message_size), + fml::MallocMapping::Copy(message_data, message_data + message_size), response); } @@ -1829,7 +1829,7 @@ FlutterEngineResult FlutterEngineDispatchSemanticsAction( if (!reinterpret_cast(engine) ->DispatchSemanticsAction( id, engine_action, - fml::NonOwnedMapping::Copy(data, data + data_length))) { + fml::MallocMapping::Copy(data, data + data_length))) { return LOG_EMBEDDER_ERROR(kInternalInconsistency, "Could not dispatch semantics action."); } @@ -1954,9 +1954,9 @@ static bool DispatchJSONPlatformMessage(FLUTTER_API_SYMBOL(FlutterEngine) auto platform_message = std::make_unique( channel_name.c_str(), // channel - fml::NonOwnedMapping::Copy(message, - message + buffer.GetSize()), // message - nullptr // response + fml::MallocMapping::Copy(message, + message + buffer.GetSize()), // message + nullptr // response ); return reinterpret_cast(engine) diff --git a/shell/platform/embedder/embedder_engine.cc b/shell/platform/embedder/embedder_engine.cc index 2f666407147db..7d1f74bdb3897 100644 --- a/shell/platform/embedder/embedder_engine.cc +++ b/shell/platform/embedder/embedder_engine.cc @@ -210,7 +210,7 @@ bool EmbedderEngine::SetAccessibilityFeatures(int32_t flags) { bool EmbedderEngine::DispatchSemanticsAction(int id, flutter::SemanticsAction action, - fml::NonOwnedMapping args) { + fml::MallocMapping args) { if (!IsValid()) { return false; } diff --git a/shell/platform/embedder/embedder_engine.h b/shell/platform/embedder/embedder_engine.h index 84b4324bd0bcc..30b0dd87a4155 100644 --- a/shell/platform/embedder/embedder_engine.h +++ b/shell/platform/embedder/embedder_engine.h @@ -82,7 +82,7 @@ class EmbedderEngine { bool DispatchSemanticsAction(int id, flutter::SemanticsAction action, - fml::NonOwnedMapping args); + fml::MallocMapping args); bool OnVsyncEvent(intptr_t baton, fml::TimePoint frame_start_time, diff --git a/shell/platform/fuchsia/flutter/fuchsia_intl.cc b/shell/platform/fuchsia/flutter/fuchsia_intl.cc index e1f4fa6238ce0..b75519301ab07 100644 --- a/shell/platform/fuchsia/flutter/fuchsia_intl.cc +++ b/shell/platform/fuchsia/flutter/fuchsia_intl.cc @@ -27,7 +27,7 @@ namespace flutter_runner { using fuchsia::intl::Profile; -std::vector MakeLocalizationPlatformMessageData( +fml::MallocMapping MakeLocalizationPlatformMessageData( const Profile& intl_profile) { rapidjson::Document document; auto& allocator = document.GetAllocator(); @@ -66,7 +66,7 @@ std::vector MakeLocalizationPlatformMessageData( rapidjson::Writer writer(buffer); document.Accept(writer); auto data = reinterpret_cast(buffer.GetString()); - return std::vector(data, data + buffer.GetSize()); + return fml::MallocMapping::Copy(data, data + buffer.GetSize()); } } // namespace flutter_runner diff --git a/shell/platform/fuchsia/flutter/fuchsia_intl.h b/shell/platform/fuchsia/flutter/fuchsia_intl.h index 349d6c516cd49..c769af9fd531d 100644 --- a/shell/platform/fuchsia/flutter/fuchsia_intl.h +++ b/shell/platform/fuchsia/flutter/fuchsia_intl.h @@ -6,6 +6,7 @@ #define FLUTTER_SHELL_PLATFORM_FUCHSIA_FUCHSIA_INTL_H_ #include +#include "flutter/fml/mapping.h" namespace flutter_runner { @@ -15,7 +16,7 @@ namespace flutter_runner { // This method does not return a `std::unique_ptr` for // testing convenience; that would require an unreasonably large set of // dependencies for the unit tests. -std::vector MakeLocalizationPlatformMessageData( +fml::MallocMapping MakeLocalizationPlatformMessageData( const fuchsia::intl::Profile& intl_profile); } // namespace flutter_runner diff --git a/shell/platform/fuchsia/flutter/fuchsia_intl_unittest.cc b/shell/platform/fuchsia/flutter/fuchsia_intl_unittest.cc index 08874f5390ca1..dddca90303dad 100644 --- a/shell/platform/fuchsia/flutter/fuchsia_intl_unittest.cc +++ b/shell/platform/fuchsia/flutter/fuchsia_intl_unittest.cc @@ -34,7 +34,8 @@ TEST_F(FuchsiaIntlTest, MakeLocalizationPlatformMessageData_SimpleLocale) { const std::string expected = R"({"method":"setLocale","args":["en","US","",""]})"; const auto actual = MakeLocalizationPlatformMessageData(profile); - ASSERT_EQ(expected, std::string(actual.begin(), actual.end())); + ASSERT_EQ(expected, std::string(actual.GetMapping(), + actual.GetMapping() + actual.GetSize())); } TEST_F(FuchsiaIntlTest, MakeLocalizationPlatformMessageData_OneLocale) { @@ -48,7 +49,8 @@ TEST_F(FuchsiaIntlTest, MakeLocalizationPlatformMessageData_OneLocale) { const std::string expected = R"({"method":"setLocale","args":["en","US","",""]})"; const auto actual = MakeLocalizationPlatformMessageData(profile); - ASSERT_EQ(expected, std::string(actual.begin(), actual.end())); + ASSERT_EQ(expected, std::string(actual.GetMapping(), + actual.GetMapping() + actual.GetSize())); } TEST_F(FuchsiaIntlTest, MakeLocalizationPlatformMessageData_MultipleLocales) { @@ -65,7 +67,8 @@ TEST_F(FuchsiaIntlTest, MakeLocalizationPlatformMessageData_MultipleLocales) { R"({"method":"setLocale","args":["en","US","","","sl","IT","Latn","nedis",)" R"("zh","","Hans","","sr","CS","Cyrl",""]})"; const auto actual = MakeLocalizationPlatformMessageData(profile); - ASSERT_EQ(expected, std::string(actual.begin(), actual.end())); + ASSERT_EQ(expected, std::string(actual.GetMapping(), + actual.GetMapping() + actual.GetSize())); } } // namespace diff --git a/shell/platform/fuchsia/flutter/platform_view.cc b/shell/platform/fuchsia/flutter/platform_view.cc index 0f1831f310c57..1c58cf16351f5 100644 --- a/shell/platform/fuchsia/flutter/platform_view.cc +++ b/shell/platform/fuchsia/flutter/platform_view.cc @@ -177,9 +177,9 @@ void PlatformView::DidUpdateState( const uint8_t* data = reinterpret_cast(buffer.GetString()); DispatchPlatformMessage(std::make_unique( - kTextInputChannel, // channel - std::vector(data, data + buffer.GetSize()), // message - nullptr) // response + kTextInputChannel, // channel + fml::MallocMapping::Copy(data, data + buffer.GetSize()), // message + nullptr) // response ); last_text_state_ = std::make_unique(state); @@ -207,9 +207,9 @@ void PlatformView::OnAction(fuchsia::ui::input::InputMethodAction action) { const uint8_t* data = reinterpret_cast(buffer.GetString()); DispatchPlatformMessage(std::make_unique( - kTextInputChannel, // channel - std::vector(data, data + buffer.GetSize()), // message - nullptr) // response + kTextInputChannel, // channel + fml::MallocMapping::Copy(data, data + buffer.GetSize()), // message + nullptr) // response ); } @@ -446,7 +446,8 @@ bool PlatformView::OnChildViewConnected(scenic::ResourceId view_holder_id) { std::unique_ptr message = std::make_unique( "flutter/platform_views", - std::vector(call.begin(), call.end()), nullptr); + fml::MallocMapping::Copy(call.c_str(), call.c_str() + call.size()), + nullptr); DispatchPlatformMessage(std::move(message)); return true; @@ -470,7 +471,8 @@ bool PlatformView::OnChildViewDisconnected(scenic::ResourceId view_holder_id) { std::unique_ptr message = std::make_unique( "flutter/platform_views", - std::vector(call.begin(), call.end()), nullptr); + fml::MallocMapping::Copy(call.c_str(), call.c_str() + call.size()), + nullptr); DispatchPlatformMessage(std::move(message)); return true; @@ -497,7 +499,8 @@ bool PlatformView::OnChildViewStateChanged(scenic::ResourceId view_holder_id, std::unique_ptr message = std::make_unique( "flutter/platform_views", - std::vector(call.begin(), call.end()), nullptr); + fml::MallocMapping::Copy(call.c_str(), call.c_str() + call.size()), + nullptr); DispatchPlatformMessage(std::move(message)); return true; @@ -646,9 +649,9 @@ void PlatformView::OnKeyEvent( const uint8_t* data = reinterpret_cast(buffer.GetString()); DispatchPlatformMessage(std::make_unique( - kKeyEventChannel, // channel - std::vector(data, data + buffer.GetSize()), // data - nullptr) // response + kKeyEventChannel, // channel + fml::MallocMapping::Copy(data, data + buffer.GetSize()), // data + nullptr) // response ); callback(fuchsia::ui::input3::KeyEventStatus::HANDLED); } @@ -766,7 +769,8 @@ void PlatformView::HandleAccessibilityChannelPlatformMessage( const flutter::StandardMessageCodec& standard_message_codec = flutter::StandardMessageCodec::GetInstance(nullptr); std::unique_ptr decoded = - standard_message_codec.DecodeMessage(message->data()); + standard_message_codec.DecodeMessage(message->data().GetMapping(), + message->data().GetSize()); flutter::EncodableMap map = std::get(*decoded); std::string type = @@ -789,7 +793,8 @@ void PlatformView::HandleFlutterPlatformChannelPlatformMessage( FML_DCHECK(message->channel() == kFlutterPlatformChannel); const auto& data = message->data(); rapidjson::Document document; - document.Parse(reinterpret_cast(data.data()), data.size()); + document.Parse(reinterpret_cast(data.GetMapping()), + data.GetSize()); if (document.HasParseError() || !document.IsObject()) { return; } @@ -810,7 +815,8 @@ void PlatformView::HandleFlutterTextInputChannelPlatformMessage( FML_DCHECK(message->channel() == kTextInputChannel); const auto& data = message->data(); rapidjson::Document document; - document.Parse(reinterpret_cast(data.data()), data.size()); + document.Parse(reinterpret_cast(data.GetMapping()), + data.GetSize()); if (document.HasParseError() || !document.IsObject()) { return; } @@ -900,7 +906,8 @@ void PlatformView::HandleFlutterPlatformViewsChannelPlatformMessage( FML_DCHECK(message->channel() == kFlutterPlatformViewsChannel); const auto& data = message->data(); rapidjson::Document document; - document.Parse(reinterpret_cast(data.data()), data.size()); + document.Parse(reinterpret_cast(data.GetMapping()), + data.GetSize()); if (document.HasParseError() || !document.IsObject()) { FML_LOG(ERROR) << "Could not parse document"; return; diff --git a/shell/platform/fuchsia/flutter/platform_view_unittest.cc b/shell/platform/fuchsia/flutter/platform_view_unittest.cc index fdb5ec0b46039..af72896c6f7a9 100644 --- a/shell/platform/fuchsia/flutter/platform_view_unittest.cc +++ b/shell/platform/fuchsia/flutter/platform_view_unittest.cc @@ -31,6 +31,11 @@ namespace flutter_runner::testing { namespace { +std::string ToString(const fml::Mapping& mapping) { + return std::string(mapping.GetMapping(), + mapping.GetMapping() + mapping.GetSize()); +} + class MockExternalViewEmbedder : public flutter::ExternalViewEmbedder { public: SkCanvas* GetRootCanvas() override { return nullptr; } @@ -96,7 +101,7 @@ class MockPlatformViewDelegate : public flutter::PlatformView::Delegate { // |flutter::PlatformView::Delegate| void OnPlatformViewDispatchSemanticsAction(int32_t id, flutter::SemanticsAction action, - std::vector args) {} + fml::MallocMapping args) {} // |flutter::PlatformView::Delegate| void OnPlatformViewSetSemanticsEnabled(bool enabled) { semantics_enabled_ = enabled; @@ -571,7 +576,7 @@ TEST_F(PlatformViewTests, EnableWireframeTest) { std::unique_ptr message = std::make_unique( "flutter/platform_views", - std::vector(txt, txt + sizeof(txt)), + fml::MallocMapping::Copy(txt, txt + sizeof(txt)), fml::RefPtr()); base_view->HandlePlatformMessage(std::move(message)); @@ -631,7 +636,7 @@ TEST_F(PlatformViewTests, CreateViewTest) { std::unique_ptr message = std::make_unique( "flutter/platform_views", - std::vector(txt, txt + sizeof(txt)), + fml::MallocMapping::Copy(txt, txt + sizeof(txt)), fml::RefPtr()); base_view->HandlePlatformMessage(std::move(message)); @@ -682,7 +687,7 @@ TEST_F(PlatformViewTests, UpdateViewTest) { std::unique_ptr message = std::make_unique( "flutter/platform_views", - std::vector(txt, txt + sizeof(txt)), + fml::MallocMapping::Copy(txt, txt + sizeof(txt)), fml::RefPtr()); base_view->HandlePlatformMessage(std::move(message)); @@ -739,7 +744,7 @@ TEST_F(PlatformViewTests, DestroyViewTest) { std::unique_ptr message = std::make_unique( "flutter/platform_views", - std::vector(txt, txt + sizeof(txt)), + fml::MallocMapping::Copy(txt, txt + sizeof(txt)), fml::RefPtr()); base_view->HandlePlatformMessage(std::move(message)); @@ -800,8 +805,9 @@ TEST_F(PlatformViewTests, ViewEventsTest) { static_cast(&platform_view) ->HandlePlatformMessage(std::make_unique( "flutter/platform_views", - std::vector(create_view_call.begin(), - create_view_call.end()), + fml::MallocMapping::Copy( + create_view_call.c_str(), + create_view_call.c_str() + create_view_call.size()), fml::RefPtr())); RunLoopUntilIdle(); @@ -827,8 +833,7 @@ TEST_F(PlatformViewTests, ViewEventsTest) { << " }" << "}"; EXPECT_EQ(view_connected_expected_out.str(), - std::string(view_connected_msg->data().begin(), - view_connected_msg->data().end())); + ToString(view_connected_msg->data())); // ViewDisconnected event. delegate.Reset(); @@ -852,8 +857,7 @@ TEST_F(PlatformViewTests, ViewEventsTest) { << " }" << "}"; EXPECT_EQ(view_disconnected_expected_out.str(), - std::string(view_disconnected_msg->data().begin(), - view_disconnected_msg->data().end())); + ToString(view_disconnected_msg->data())); // ViewStateChanged event. delegate.Reset(); @@ -883,8 +887,7 @@ TEST_F(PlatformViewTests, ViewEventsTest) { << " }" << "}"; EXPECT_EQ(view_state_changed_expected_out.str(), - std::string(view_state_changed_msg->data().begin(), - view_state_changed_msg->data().end())); + ToString(view_state_changed_msg->data())); } // This test makes sure that the PlatformView forwards messages on the @@ -939,14 +942,13 @@ TEST_F(PlatformViewTests, RequestFocusTest) { std::unique_ptr message = std::make_unique( "flutter/platform_views", - std::vector(buff, buff + sizeof(buff)), response); + fml::MallocMapping::Copy(buff, buff + sizeof(buff)), response); base_view->HandlePlatformMessage(std::move(message)); RunLoopUntilIdle(); EXPECT_TRUE(mock_focuser.request_focus_called()); - auto result = std::string((const char*)data_arg.data->GetMapping(), - data_arg.data->GetSize()); + auto result = ToString(*data_arg.data); EXPECT_EQ(std::string("[0]"), result); } @@ -1002,14 +1004,13 @@ TEST_F(PlatformViewTests, RequestFocusFailTest) { std::unique_ptr message = std::make_unique( "flutter/platform_views", - std::vector(buff, buff + sizeof(buff)), response); + fml::MallocMapping::Copy(buff, buff + sizeof(buff)), response); base_view->HandlePlatformMessage(std::move(message)); RunLoopUntilIdle(); EXPECT_TRUE(mock_focuser.request_focus_called()); - auto result = std::string((const char*)data_arg.data->GetMapping(), - data_arg.data->GetSize()); + auto result = ToString(*data_arg.data); std::ostringstream out; out << "[" << static_cast>( @@ -1094,8 +1095,8 @@ TEST_F(PlatformViewTests, OnKeyEvent) { key_event_status = status; }); RunLoopUntilIdle(); - const std::vector data = delegate.message()->data(); - const std::string message = std::string(data.begin(), data.end()); + const fml::MallocMapping data = delegate.message()->releaseData(); + const std::string message = ToString(data); EXPECT_EQ(event.expected_platform_message, message); EXPECT_EQ(key_event_status, event.expected_key_event_status); diff --git a/shell/testing/tester_main.cc b/shell/testing/tester_main.cc index 4d306485b9646..b96166c5faee6 100644 --- a/shell/testing/tester_main.cc +++ b/shell/testing/tester_main.cc @@ -254,7 +254,7 @@ int RunTester(const flutter::Settings& settings, const char* locale_json = "{\"method\":\"setLocale\",\"args\":[\"en\",\"US\",\"\",\"\",\"zh\"," "\"CN\",\"\",\"\"]}"; - auto locale_bytes = fml::NonOwnedMapping::Copy( + auto locale_bytes = fml::MallocMapping::Copy( locale_json, locale_json + std::strlen(locale_json)); fml::RefPtr response; shell->GetPlatformView()->DispatchPlatformMessage( From fe80b2a48b5dbd8f99970da616bae2510bbf13af Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 6 May 2021 09:54:10 -0700 Subject: [PATCH 3/5] added copy that takes in a length (everyone was calculating end position anyway) --- fml/mapping.cc | 7 +++++ fml/mapping.h | 7 +++-- lib/ui/window/platform_configuration.cc | 3 +-- shell/common/engine.cc | 2 +- shell/common/engine_unittests.cc | 3 +-- shell/common/shell.cc | 4 +-- shell/common/shell_unittests.cc | 4 +-- .../platform/android/platform_view_android.cc | 7 +++-- .../darwin/common/buffer_conversions.mm | 2 +- .../ios/framework/Source/SemanticsObject.mm | 2 +- shell/platform/embedder/embedder.cc | 9 +++---- .../platform/fuchsia/flutter/fuchsia_intl.cc | 2 +- .../platform/fuchsia/flutter/platform_view.cc | 27 +++++++++---------- .../fuchsia/flutter/platform_view_unittest.cc | 21 ++++++--------- 14 files changed, 46 insertions(+), 54 deletions(-) diff --git a/fml/mapping.cc b/fml/mapping.cc index 25821dc4b9abe..3778d982d37e1 100644 --- a/fml/mapping.cc +++ b/fml/mapping.cc @@ -120,6 +120,13 @@ MallocMapping::~MallocMapping() { } } +MallocMapping MallocMapping::Copy(const void* begin, size_t length) { + auto result = + MallocMapping(reinterpret_cast(malloc(length)), length); + memcpy(const_cast(result.GetMapping()), begin, length); + return result; +} + size_t MallocMapping::GetSize() const { return size_; } diff --git a/fml/mapping.h b/fml/mapping.h index 6eee51557823a..79b37e2e66635 100644 --- a/fml/mapping.h +++ b/fml/mapping.h @@ -142,12 +142,11 @@ class MallocMapping final : public Mapping { template static MallocMapping Copy(const T* begin, const T* end) { size_t length = end - begin; - auto result = - MallocMapping(reinterpret_cast(malloc(length)), length); - memcpy(const_cast(result.GetMapping()), begin, length); - return result; + return Copy(begin, length); } + static MallocMapping Copy(const void* begin, size_t length); + // |Mapping| size_t GetSize() const override; diff --git a/lib/ui/window/platform_configuration.cc b/lib/ui/window/platform_configuration.cc index ace575b0d39a7..18133a2e98827 100644 --- a/lib/ui/window/platform_configuration.cc +++ b/lib/ui/window/platform_configuration.cc @@ -130,8 +130,7 @@ Dart_Handle SendPlatformMessage(Dart_Handle window, const uint8_t* buffer = static_cast(data.data()); dart_state->platform_configuration()->client()->HandlePlatformMessage( std::make_unique( - name, - fml::MallocMapping::Copy(buffer, buffer + data.length_in_bytes()), + name, fml::MallocMapping::Copy(buffer, data.length_in_bytes()), response)); } diff --git a/shell/common/engine.cc b/shell/common/engine.cc index 9155019aea57d..db72230590b2a 100644 --- a/shell/common/engine.cc +++ b/shell/common/engine.cc @@ -38,7 +38,7 @@ static constexpr char kIsolateChannel[] = "flutter/isolate"; namespace { fml::MallocMapping MakeMapping(const std::string& str) { - return fml::MallocMapping::Copy(str.c_str(), str.c_str() + str.length()); + return fml::MallocMapping::Copy(str.c_str(), str.length()); } } // namespace diff --git a/shell/common/engine_unittests.cc b/shell/common/engine_unittests.cc index 8c2784c0f19ed..18163e7d1c404 100644 --- a/shell/common/engine_unittests.cc +++ b/shell/common/engine_unittests.cc @@ -96,8 +96,7 @@ std::unique_ptr MakePlatformMessage( const uint8_t* data = reinterpret_cast(buffer.GetString()); std::unique_ptr message = std::make_unique( - channel, fml::MallocMapping::Copy(data, data + buffer.GetSize()), - response); + channel, fml::MallocMapping::Copy(data, buffer.GetSize()), response); return message; } diff --git a/shell/common/shell.cc b/shell/common/shell.cc index 27777640df5dd..b9d1765e2a431 100644 --- a/shell/common/shell.cc +++ b/shell/common/shell.cc @@ -1809,9 +1809,7 @@ bool Shell::ReloadSystemFonts() { std::unique_ptr fontsChangeMessage = std::make_unique( kSystemChannel, - fml::MallocMapping::Copy(message.c_str(), - message.c_str() + message.length()), - nullptr); + fml::MallocMapping::Copy(message.c_str(), message.length()), nullptr); OnPlatformViewDispatchPlatformMessage(std::move(fontsChangeMessage)); return true; diff --git a/shell/common/shell_unittests.cc b/shell/common/shell_unittests.cc index 313fed4db7b4a..f2931ec109da2 100644 --- a/shell/common/shell_unittests.cc +++ b/shell/common/shell_unittests.cc @@ -1489,8 +1489,8 @@ TEST_F(ShellTest, SetResourceCacheSize) { "method": "Skia.setResourceCacheMaxBytes", "args": 10000 })json"; - auto data = fml::MallocMapping::Copy( - request_json.c_str(), request_json.c_str() + request_json.length()); + auto data = + fml::MallocMapping::Copy(request_json.c_str(), request_json.length()); auto platform_message = std::make_unique( "flutter/skia", std::move(data), nullptr); SendEnginePlatformMessage(shell.get(), std::move(platform_message)); diff --git a/shell/platform/android/platform_view_android.cc b/shell/platform/android/platform_view_android.cc index 380526a963573..ff514d93cb5d8 100644 --- a/shell/platform/android/platform_view_android.cc +++ b/shell/platform/android/platform_view_android.cc @@ -180,8 +180,8 @@ void PlatformViewAndroid::DispatchPlatformMessage(JNIEnv* env, jint response_id) { uint8_t* message_data = static_cast(env->GetDirectBufferAddress(java_message_data)); - fml::MallocMapping message = fml::MallocMapping::Copy( - message_data, message_data + java_message_position); + fml::MallocMapping message = + fml::MallocMapping::Copy(message_data, java_message_position); fml::RefPtr response; if (response_id) { @@ -273,8 +273,7 @@ void PlatformViewAndroid::DispatchSemanticsAction(JNIEnv* env, } uint8_t* args_data = static_cast(env->GetDirectBufferAddress(args)); - auto args_vector = - fml::MallocMapping::Copy(args_data, args_data + args_position); + auto args_vector = fml::MallocMapping::Copy(args_data, args_position); PlatformView::DispatchSemanticsAction( id, static_cast(action), diff --git a/shell/platform/darwin/common/buffer_conversions.mm b/shell/platform/darwin/common/buffer_conversions.mm index 5ddbe04a21d5d..1856ec0761151 100644 --- a/shell/platform/darwin/common/buffer_conversions.mm +++ b/shell/platform/darwin/common/buffer_conversions.mm @@ -8,7 +8,7 @@ fml::MallocMapping CopyNSDataToMapping(NSData* data) { const uint8_t* bytes = static_cast(data.bytes); - return fml::MallocMapping::Copy(bytes, bytes + data.length); + return fml::MallocMapping::Copy(bytes, data.length); } NSData* CopyMappingToNSData(fml::MallocMapping buffer) { diff --git a/shell/platform/darwin/ios/framework/Source/SemanticsObject.mm b/shell/platform/darwin/ios/framework/Source/SemanticsObject.mm index c17fac4a18071..1c80753d9272a 100644 --- a/shell/platform/darwin/ios/framework/Source/SemanticsObject.mm +++ b/shell/platform/darwin/ios/framework/Source/SemanticsObject.mm @@ -310,7 +310,7 @@ - (BOOL)onCustomAccessibilityAction:(FlutterCustomAccessibilityAction*)action { args.push_back(action_id >> 24); [self bridge]->DispatchSemanticsAction( [self uid], flutter::SemanticsAction::kCustomAction, - fml::MallocMapping::Copy(args.data(), args.data() + args.size() * sizeof(uint8_t))); + fml::MallocMapping::Copy(args.data(), args.size() * sizeof(uint8_t))); return YES; } diff --git a/shell/platform/embedder/embedder.cc b/shell/platform/embedder/embedder.cc index e2b39e961e7c3..23fe59db350b7 100644 --- a/shell/platform/embedder/embedder.cc +++ b/shell/platform/embedder/embedder.cc @@ -1637,8 +1637,7 @@ FlutterEngineResult FlutterEngineSendPlatformMessage( } else { message = std::make_unique( flutter_message->channel, - fml::MallocMapping::Copy(message_data, message_data + message_size), - response); + fml::MallocMapping::Copy(message_data, message_size), response); } return reinterpret_cast(engine) @@ -1829,7 +1828,7 @@ FlutterEngineResult FlutterEngineDispatchSemanticsAction( if (!reinterpret_cast(engine) ->DispatchSemanticsAction( id, engine_action, - fml::MallocMapping::Copy(data, data + data_length))) { + fml::MallocMapping::Copy(data, data_length))) { return LOG_EMBEDDER_ERROR(kInternalInconsistency, "Could not dispatch semantics action."); } @@ -1955,8 +1954,8 @@ static bool DispatchJSONPlatformMessage(FLUTTER_API_SYMBOL(FlutterEngine) auto platform_message = std::make_unique( channel_name.c_str(), // channel fml::MallocMapping::Copy(message, - message + buffer.GetSize()), // message - nullptr // response + buffer.GetSize()), // message + nullptr // response ); return reinterpret_cast(engine) diff --git a/shell/platform/fuchsia/flutter/fuchsia_intl.cc b/shell/platform/fuchsia/flutter/fuchsia_intl.cc index b75519301ab07..b3235385ce94d 100644 --- a/shell/platform/fuchsia/flutter/fuchsia_intl.cc +++ b/shell/platform/fuchsia/flutter/fuchsia_intl.cc @@ -66,7 +66,7 @@ fml::MallocMapping MakeLocalizationPlatformMessageData( rapidjson::Writer writer(buffer); document.Accept(writer); auto data = reinterpret_cast(buffer.GetString()); - return fml::MallocMapping::Copy(data, data + buffer.GetSize()); + return fml::MallocMapping::Copy(data, buffer.GetSize()); } } // namespace flutter_runner diff --git a/shell/platform/fuchsia/flutter/platform_view.cc b/shell/platform/fuchsia/flutter/platform_view.cc index 1c58cf16351f5..29bd90e018dc2 100644 --- a/shell/platform/fuchsia/flutter/platform_view.cc +++ b/shell/platform/fuchsia/flutter/platform_view.cc @@ -177,9 +177,9 @@ void PlatformView::DidUpdateState( const uint8_t* data = reinterpret_cast(buffer.GetString()); DispatchPlatformMessage(std::make_unique( - kTextInputChannel, // channel - fml::MallocMapping::Copy(data, data + buffer.GetSize()), // message - nullptr) // response + kTextInputChannel, // channel + fml::MallocMapping::Copy(data, buffer.GetSize()), // message + nullptr) // response ); last_text_state_ = std::make_unique(state); @@ -207,9 +207,9 @@ void PlatformView::OnAction(fuchsia::ui::input::InputMethodAction action) { const uint8_t* data = reinterpret_cast(buffer.GetString()); DispatchPlatformMessage(std::make_unique( - kTextInputChannel, // channel - fml::MallocMapping::Copy(data, data + buffer.GetSize()), // message - nullptr) // response + kTextInputChannel, // channel + fml::MallocMapping::Copy(data, buffer.GetSize()), // message + nullptr) // response ); } @@ -446,8 +446,7 @@ bool PlatformView::OnChildViewConnected(scenic::ResourceId view_holder_id) { std::unique_ptr message = std::make_unique( "flutter/platform_views", - fml::MallocMapping::Copy(call.c_str(), call.c_str() + call.size()), - nullptr); + fml::MallocMapping::Copy(call.c_str(), call.size()), nullptr); DispatchPlatformMessage(std::move(message)); return true; @@ -471,8 +470,7 @@ bool PlatformView::OnChildViewDisconnected(scenic::ResourceId view_holder_id) { std::unique_ptr message = std::make_unique( "flutter/platform_views", - fml::MallocMapping::Copy(call.c_str(), call.c_str() + call.size()), - nullptr); + fml::MallocMapping::Copy(call.c_str(), call.size()), nullptr); DispatchPlatformMessage(std::move(message)); return true; @@ -499,8 +497,7 @@ bool PlatformView::OnChildViewStateChanged(scenic::ResourceId view_holder_id, std::unique_ptr message = std::make_unique( "flutter/platform_views", - fml::MallocMapping::Copy(call.c_str(), call.c_str() + call.size()), - nullptr); + fml::MallocMapping::Copy(call.c_str(), call.size()), nullptr); DispatchPlatformMessage(std::move(message)); return true; @@ -649,9 +646,9 @@ void PlatformView::OnKeyEvent( const uint8_t* data = reinterpret_cast(buffer.GetString()); DispatchPlatformMessage(std::make_unique( - kKeyEventChannel, // channel - fml::MallocMapping::Copy(data, data + buffer.GetSize()), // data - nullptr) // response + kKeyEventChannel, // channel + fml::MallocMapping::Copy(data, buffer.GetSize()), // data + nullptr) // response ); callback(fuchsia::ui::input3::KeyEventStatus::HANDLED); } diff --git a/shell/platform/fuchsia/flutter/platform_view_unittest.cc b/shell/platform/fuchsia/flutter/platform_view_unittest.cc index af72896c6f7a9..ffc470ff6df52 100644 --- a/shell/platform/fuchsia/flutter/platform_view_unittest.cc +++ b/shell/platform/fuchsia/flutter/platform_view_unittest.cc @@ -575,8 +575,7 @@ TEST_F(PlatformViewTests, EnableWireframeTest) { std::unique_ptr message = std::make_unique( - "flutter/platform_views", - fml::MallocMapping::Copy(txt, txt + sizeof(txt)), + "flutter/platform_views", fml::MallocMapping::Copy(txt, sizeof(txt)), fml::RefPtr()); base_view->HandlePlatformMessage(std::move(message)); @@ -635,8 +634,7 @@ TEST_F(PlatformViewTests, CreateViewTest) { std::unique_ptr message = std::make_unique( - "flutter/platform_views", - fml::MallocMapping::Copy(txt, txt + sizeof(txt)), + "flutter/platform_views", fml::MallocMapping::Copy(txt, sizeof(txt)), fml::RefPtr()); base_view->HandlePlatformMessage(std::move(message)); @@ -686,8 +684,7 @@ TEST_F(PlatformViewTests, UpdateViewTest) { std::unique_ptr message = std::make_unique( - "flutter/platform_views", - fml::MallocMapping::Copy(txt, txt + sizeof(txt)), + "flutter/platform_views", fml::MallocMapping::Copy(txt, sizeof(txt)), fml::RefPtr()); base_view->HandlePlatformMessage(std::move(message)); @@ -743,8 +740,7 @@ TEST_F(PlatformViewTests, DestroyViewTest) { std::unique_ptr message = std::make_unique( - "flutter/platform_views", - fml::MallocMapping::Copy(txt, txt + sizeof(txt)), + "flutter/platform_views", fml::MallocMapping::Copy(txt, sizeof(txt)), fml::RefPtr()); base_view->HandlePlatformMessage(std::move(message)); @@ -805,9 +801,8 @@ TEST_F(PlatformViewTests, ViewEventsTest) { static_cast(&platform_view) ->HandlePlatformMessage(std::make_unique( "flutter/platform_views", - fml::MallocMapping::Copy( - create_view_call.c_str(), - create_view_call.c_str() + create_view_call.size()), + fml::MallocMapping::Copy(create_view_call.c_str(), + create_view_call.size()), fml::RefPtr())); RunLoopUntilIdle(); @@ -942,7 +937,7 @@ TEST_F(PlatformViewTests, RequestFocusTest) { std::unique_ptr message = std::make_unique( "flutter/platform_views", - fml::MallocMapping::Copy(buff, buff + sizeof(buff)), response); + fml::MallocMapping::Copy(buff, sizeof(buff)), response); base_view->HandlePlatformMessage(std::move(message)); RunLoopUntilIdle(); @@ -1004,7 +999,7 @@ TEST_F(PlatformViewTests, RequestFocusFailTest) { std::unique_ptr message = std::make_unique( "flutter/platform_views", - fml::MallocMapping::Copy(buff, buff + sizeof(buff)), response); + fml::MallocMapping::Copy(buff, sizeof(buff)), response); base_view->HandlePlatformMessage(std::move(message)); RunLoopUntilIdle(); From 9aaa52d6f32103416b29635f5427665b046d782f Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 6 May 2021 10:13:03 -0700 Subject: [PATCH 4/5] added unit tests for MallocMapping --- ci/licenses_golden/licenses_flutter | 1 + fml/BUILD.gn | 1 + fml/mapping_unittests.cc | 55 +++++++++++++++++++++++++++++ 3 files changed, 57 insertions(+) create mode 100644 fml/mapping_unittests.cc diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index 2d4afaff7f585..c17012f99d507 100755 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -170,6 +170,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 diff --git a/fml/BUILD.gn b/fml/BUILD.gn index 3a720fae2f9fb..55c88e2569d6f 100644 --- a/fml/BUILD.gn +++ b/fml/BUILD.gn @@ -252,6 +252,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", diff --git a/fml/mapping_unittests.cc b/fml/mapping_unittests.cc new file mode 100644 index 0000000000000..0a7309ccf3c4c --- /dev/null +++ b/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(malloc(length)), length); + ASSERT_NE(nullptr, mapping.GetMapping()); + ASSERT_EQ(length, mapping.GetSize()); +} + +TEST(MallocMapping, MoveConstructor) { + size_t length = 10; + MallocMapping mapping(reinterpret_cast(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(malloc(length)), length); + memset(const_cast(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(malloc(length)), length); + free(const_cast(mapping.Release())); + ASSERT_EQ(nullptr, mapping.GetMapping()); + ASSERT_EQ(0u, mapping.GetSize()); +} + +} // namespace fml From 63da9c4b9c7f3d27acc17e4f3fe2ef05d8d7e5bd Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Fri, 7 May 2021 13:51:16 -0700 Subject: [PATCH 5/5] chinmays feedback --- fml/mapping.cc | 13 ++++++------- fml/mapping.h | 16 +++++++++++++--- 2 files changed, 19 insertions(+), 10 deletions(-) diff --git a/fml/mapping.cc b/fml/mapping.cc index 3778d982d37e1..254ccafc441ea 100644 --- a/fml/mapping.cc +++ b/fml/mapping.cc @@ -104,7 +104,7 @@ const uint8_t* NonOwnedMapping::GetMapping() const { // MallocMapping MallocMapping::MallocMapping() : data_(nullptr), size_(0) {} -MallocMapping::MallocMapping(const uint8_t* data, size_t size) +MallocMapping::MallocMapping(uint8_t* data, size_t size) : data_(data), size_(size) {} MallocMapping::MallocMapping(fml::MallocMapping&& mapping) @@ -114,15 +114,14 @@ MallocMapping::MallocMapping(fml::MallocMapping&& mapping) } MallocMapping::~MallocMapping() { - if (data_) { - free(const_cast(data_)); - data_ = nullptr; - } + free(data_); + data_ = nullptr; } MallocMapping MallocMapping::Copy(const void* begin, size_t length) { auto result = MallocMapping(reinterpret_cast(malloc(length)), length); + FML_CHECK(result.GetMapping() != nullptr); memcpy(const_cast(result.GetMapping()), begin, length); return result; } @@ -135,8 +134,8 @@ const uint8_t* MallocMapping::GetMapping() const { return data_; } -const uint8_t* MallocMapping::Release() { - const uint8_t* result = data_; +uint8_t* MallocMapping::Release() { + uint8_t* result = data_; data_ = nullptr; size_ = 0; return result; diff --git a/fml/mapping.h b/fml/mapping.h index 79b37e2e66635..cd1be8e47d73d 100644 --- a/fml/mapping.h +++ b/fml/mapping.h @@ -130,7 +130,11 @@ class MallocMapping final : public Mapping { public: MallocMapping(); - MallocMapping(const uint8_t* data, size_t size); + /// 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); @@ -141,10 +145,15 @@ class MallocMapping final : public Mapping { /// for `uint8_t` and `char`. template 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| @@ -154,10 +163,11 @@ class MallocMapping final : public Mapping { const uint8_t* GetMapping() const override; /// Removes ownership of the data buffer. - const uint8_t* Release(); + /// After this is called; the mapping will point to nullptr. + [[nodiscard]] uint8_t* Release(); private: - const uint8_t* data_; + uint8_t* data_; size_t size_; FML_DISALLOW_COPY_AND_ASSIGN(MallocMapping);