From 8dac13cb192f609835f259ad23d5ff6c8fdbda0e Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 4 Aug 2022 14:16:29 -0700 Subject: [PATCH 01/19] Started handling messages from background isolates. --- ci/licenses_golden/licenses_flutter | 3 + fml/memory/threadsafe_unique_ptr.h | 111 ++++++++++++++++++ lib/ui/BUILD.gn | 2 + lib/ui/dart_ui.cc | 3 + lib/ui/platform_dispatcher.dart | 28 ++++- lib/ui/ui_dart_state.cc | 24 +++- lib/ui/ui_dart_state.h | 20 +++- lib/ui/window/platform_configuration.cc | 82 +++++++++++-- lib/ui/window/platform_configuration.h | 25 ++++ .../platform_message_response_dart_port.cc | 72 ++++++++++++ .../platform_message_response_dart_port.h | 34 ++++++ lib/web_ui/lib/platform_dispatcher.dart | 10 ++ .../lib/src/engine/platform_dispatcher.dart | 20 ++++ runtime/dart_isolate.cc | 8 +- runtime/dart_isolate_group_data.cc | 17 +++ runtime/dart_isolate_group_data.h | 18 ++- shell/common/engine.cc | 1 + shell/common/shell.cc | 7 +- .../ios/platform_message_handler_ios.mm | 5 +- 19 files changed, 471 insertions(+), 19 deletions(-) create mode 100644 fml/memory/threadsafe_unique_ptr.h create mode 100644 lib/ui/window/platform_message_response_dart_port.cc create mode 100644 lib/ui/window/platform_message_response_dart_port.h diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index 2dfc3857d0e25..bc13cb0f0b191 100644 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -281,6 +281,7 @@ FILE: ../../../flutter/fml/memory/task_runner_checker.cc FILE: ../../../flutter/fml/memory/task_runner_checker.h FILE: ../../../flutter/fml/memory/task_runner_checker_unittest.cc FILE: ../../../flutter/fml/memory/thread_checker.h +FILE: ../../../flutter/fml/memory/threadsafe_unique_ptr.h FILE: ../../../flutter/fml/memory/weak_ptr.h FILE: ../../../flutter/fml/memory/weak_ptr_internal.cc FILE: ../../../flutter/fml/memory/weak_ptr_internal.h @@ -1124,6 +1125,8 @@ FILE: ../../../flutter/lib/ui/window/platform_message_response.h FILE: ../../../flutter/lib/ui/window/platform_message_response_dart.cc FILE: ../../../flutter/lib/ui/window/platform_message_response_dart.h FILE: ../../../flutter/lib/ui/window/platform_message_response_dart_unittests.cc +FILE: ../../../flutter/lib/ui/window/platform_message_response_dart_port.cc +FILE: ../../../flutter/lib/ui/window/platform_message_response_dart_port.h FILE: ../../../flutter/lib/ui/window/pointer_data.cc FILE: ../../../flutter/lib/ui/window/pointer_data.h FILE: ../../../flutter/lib/ui/window/pointer_data_packet.cc diff --git a/fml/memory/threadsafe_unique_ptr.h b/fml/memory/threadsafe_unique_ptr.h new file mode 100644 index 0000000000000..2b8925976d789 --- /dev/null +++ b/fml/memory/threadsafe_unique_ptr.h @@ -0,0 +1,111 @@ +// 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. + +#ifndef FLUTTER_FML_THREADSAFE_UNIQUE_PTR_H_ +#define FLUTTER_FML_THREADSAFE_UNIQUE_PTR_H_ + +#include +#include + +#include "flutter/fml/macros.h" + +namespace fml { + +/// A single-owner smart pointer that allows weak references that can be +/// accessed on different threads. +/// This smart-pointer makes the following guarantees: +/// * There is only ever one threadsafe_unique_ptr per object. +/// * The object is deleted when ~threadsafe_unique_ptr() is called. +/// * The object will not be deleted whilst another thread has a lock_ptr. +/// * The thread that owns the object can access it without a lock. +/// WARNING: weak_ptr's should only be used to invoke thread-safe methods. +template +class threadsafe_unique_ptr { + private: + struct Data { + Data(std::unique_ptr&& arg_ptr) : ptr(std::move(arg_ptr)) {} + std::unique_ptr ptr; + std::mutex mutex; + }; + + public: + threadsafe_unique_ptr() + : data_(new Data(std::unique_ptr())), fast_ptr_(nullptr) {} + + threadsafe_unique_ptr(std::unique_ptr&& ptr) + : data_(new Data(std::move(ptr))) { + fast_ptr_ = data_->ptr.get(); + } + + threadsafe_unique_ptr(threadsafe_unique_ptr&& ptr) + : data_(std::move(ptr.data_)), fast_ptr_(std::move(ptr.fast_ptr_)) {} + + threadsafe_unique_ptr& operator=(threadsafe_unique_ptr&& rvalue) { + data_ = std::move(rvalue.data_); + fast_ptr_ = std::move(rvalue.fast_ptr_); + return *this; + } + + ~threadsafe_unique_ptr() { + if (data_) { + std::scoped_lock lock(data_->mutex); + data_->ptr.reset(); + } + } + + T* operator->() const { return fast_ptr_; } + + T* get() const { return fast_ptr_; } + + operator bool() const { return fast_ptr_ != nullptr; } + + class lock_ptr; + + /// A non-owning smart pointer for a `threadsafe_unique_ptr`. + class weak_ptr { + public: + weak_ptr() {} + + weak_ptr(std::weak_ptr weak_ptr) : weak_ptr_(weak_ptr) {} + + private: + friend class lock_ptr; + std::weak_ptr weak_ptr_; + }; + + /// A temporary owning smart pointer for a `threadsafe_unique_ptr`. This + /// guarantees that the object will not be deleted while in scope. + class lock_ptr { + public: + lock_ptr(const weak_ptr& weak) : strong_ptr_(weak.weak_ptr_.lock()) { + if (strong_ptr_) { + lock_ = std::unique_lock(strong_ptr_->mutex); + } + } + + T* operator->() { return strong_ptr_ ? strong_ptr_->ptr.get() : nullptr; } + + operator bool() const { + return strong_ptr_ ? static_cast(strong_ptr_->ptr) : false; + } + + private: + FML_DISALLOW_COPY_AND_ASSIGN(lock_ptr); + std::shared_ptr strong_ptr_; + std::unique_lock lock_; + }; + + weak_ptr GetWeakPtr() const { return weak_ptr(data_); } + + private: + FML_DISALLOW_COPY_AND_ASSIGN(threadsafe_unique_ptr); + std::shared_ptr data_; + // Clients that own the threadsafe_unique_ptr can access the pointer directly + // since there is no risk that it will be deleted whilst being accessed. + T* fast_ptr_; +}; + +} // namespace fml + +#endif // FLUTTER_FML_THREADSAFE_UNIQUE_PTR_H_ diff --git a/lib/ui/BUILD.gn b/lib/ui/BUILD.gn index 6e5c8296d3793..fedaaf584cc8b 100644 --- a/lib/ui/BUILD.gn +++ b/lib/ui/BUILD.gn @@ -124,6 +124,8 @@ source_set("ui") { "window/platform_message_response.h", "window/platform_message_response_dart.cc", "window/platform_message_response_dart.h", + "window/platform_message_response_dart_port.cc", + "window/platform_message_response_dart_port.h", "window/pointer_data.cc", "window/pointer_data.h", "window/pointer_data_packet.cc", diff --git a/lib/ui/dart_ui.cc b/lib/ui/dart_ui.cc index fead4f06814a6..4fd108454046d 100644 --- a/lib/ui/dart_ui.cc +++ b/lib/ui/dart_ui.cc @@ -100,6 +100,9 @@ typedef CanvasPath Path; V(PlatformConfigurationNativeApi::ComputePlatformResolvedLocale, 1) \ V(PlatformConfigurationNativeApi::SendPlatformMessage, 3) \ V(PlatformConfigurationNativeApi::RespondToPlatformMessage, 2) \ + V(PlatformConfigurationNativeApi::RegisterRootIsolate, 0) \ + V(PlatformConfigurationNativeApi::RegisterBackgroundIsolate, 1) \ + V(PlatformConfigurationNativeApi::SendPortPlatformMessage, 4) \ V(DartRuntimeHooks::Logger_PrintDebugString, 1) \ V(DartRuntimeHooks::Logger_PrintString, 1) \ V(DartRuntimeHooks::ScheduleMicrotask, 1) \ diff --git a/lib/ui/platform_dispatcher.dart b/lib/ui/platform_dispatcher.dart index 7c0af5dec2f54..2dccd6a90a23c 100644 --- a/lib/ui/platform_dispatcher.dart +++ b/lib/ui/platform_dispatcher.dart @@ -532,12 +532,38 @@ class PlatformDispatcher { } } - String? _sendPlatformMessage(String name,PlatformMessageResponseCallback? callback, ByteData? data) => + String? _sendPlatformMessage(String name, PlatformMessageResponseCallback? callback, ByteData? data) => __sendPlatformMessage(name, callback, data); @FfiNative('PlatformConfigurationNativeApi::SendPlatformMessage') external static String? __sendPlatformMessage(String name, PlatformMessageResponseCallback? callback, ByteData? data); + void sendPortPlatformMessage( + String name, + ByteData? data, + int identifier, + SendPort port) { + final String? error = + _sendPortPlatformMessage(name, identifier, port.nativePort, data); + if (error != null) { + throw Exception(error); + } + } + + String? _sendPortPlatformMessage(String name, int identifier, int port, ByteData? data) => + __sendPortPlatformMessage(name, identifier, port, data); + + @FfiNative('PlatformConfigurationNativeApi::SendPortPlatformMessage') + external static String? __sendPortPlatformMessage(String name, int identifier, int port, ByteData? data); + + int registerRootIsolate() => __registerRootIsolate(); + @FfiNative('PlatformConfigurationNativeApi::RegisterRootIsolate') + external static int __registerRootIsolate(); + + void registerBackgroundIsolate(int rootIsolateId) => __registerBackgroundIsolate(rootIsolateId); + @FfiNative('PlatformConfigurationNativeApi::RegisterBackgroundIsolate') + external static void __registerBackgroundIsolate(int rootIsolateId); + /// Called whenever this platform dispatcher receives a message from a /// platform-specific plugin. /// diff --git a/lib/ui/ui_dart_state.cc b/lib/ui/ui_dart_state.cc index ed60f8cbf2c5b..e0155c76b3b00 100644 --- a/lib/ui/ui_dart_state.cc +++ b/lib/ui/ui_dart_state.cc @@ -106,7 +106,8 @@ UIDartState* UIDartState::Current() { } void UIDartState::SetPlatformConfiguration( - std::unique_ptr platform_configuration) { + fml::threadsafe_unique_ptr platform_configuration) { + FML_DCHECK(IsRootIsolate()); platform_configuration_ = std::move(platform_configuration); if (platform_configuration_) { platform_configuration_->client()->UpdateIsolateDescription(debug_name_, @@ -114,6 +115,13 @@ void UIDartState::SetPlatformConfiguration( } } +void UIDartState::SetWeakPlatformConfiguration( + fml::threadsafe_unique_ptr::weak_ptr + platform_configuration) { + FML_DCHECK(!IsRootIsolate()); + weak_platform_configuration_ = platform_configuration; +} + const TaskRunners& UIDartState::GetTaskRunners() const { return context_.task_runners; } @@ -214,4 +222,18 @@ bool UIDartState::enable_skparagraph() const { return enable_skparagraph_; } +void UIDartState::HandlePlatformMessage( + std::unique_ptr message) { + if (platform_configuration_) { + platform_configuration_->client()->HandlePlatformMessage( + std::move(message)); + } else { + fml::threadsafe_unique_ptr::lock_ptr lock( + weak_platform_configuration_); + if (lock) { + lock->client()->HandlePlatformMessage(std::move(message)); + } + } +} + } // namespace flutter diff --git a/lib/ui/ui_dart_state.h b/lib/ui/ui_dart_state.h index bdcb49beadf4a..ca145e0046efb 100644 --- a/lib/ui/ui_dart_state.h +++ b/lib/ui/ui_dart_state.h @@ -13,6 +13,7 @@ #include "flutter/common/task_runners.h" #include "flutter/flow/skia_gpu_object.h" #include "flutter/fml/build_config.h" +#include "flutter/fml/memory/threadsafe_unique_ptr.h" #include "flutter/fml/memory/weak_ptr.h" #include "flutter/fml/synchronization/waitable_event.h" #include "flutter/lib/ui/io_manager.h" @@ -30,6 +31,7 @@ namespace flutter { class FontSelector; class ImageGeneratorRegistry; class PlatformConfiguration; +class PlatformMessage; class UIDartState : public tonic::DartState { public: @@ -106,6 +108,18 @@ class UIDartState : public tonic::DartState { return platform_configuration_.get(); } + fml::threadsafe_unique_ptr::weak_ptr + GetWeakPlatformConfiguration() const { + return platform_configuration_ ? platform_configuration_.GetWeakPtr() + : weak_platform_configuration_; + } + + void SetWeakPlatformConfiguration( + fml::threadsafe_unique_ptr::weak_ptr + platform_configuration); + + void HandlePlatformMessage(std::unique_ptr message); + const TaskRunners& GetTaskRunners() const; void ScheduleMicrotask(Dart_Handle handle); @@ -167,7 +181,7 @@ class UIDartState : public tonic::DartState { ~UIDartState() override; void SetPlatformConfiguration( - std::unique_ptr platform_configuration); + fml::threadsafe_unique_ptr platform_configuration); const std::string& GetAdvisoryScriptURI() const; @@ -180,7 +194,9 @@ class UIDartState : public tonic::DartState { Dart_Port main_port_ = ILLEGAL_PORT; const bool is_root_isolate_; std::string debug_name_; - std::unique_ptr platform_configuration_; + fml::threadsafe_unique_ptr platform_configuration_; + fml::threadsafe_unique_ptr::weak_ptr + weak_platform_configuration_; tonic::DartMicrotaskQueue microtask_queue_; UnhandledExceptionCallback unhandled_exception_callback_; LogMessageCallback log_message_callback_; diff --git a/lib/ui/window/platform_configuration.cc b/lib/ui/window/platform_configuration.cc index 1a98b204a1456..50670d4c63850 100644 --- a/lib/ui/window/platform_configuration.cc +++ b/lib/ui/window/platform_configuration.cc @@ -9,6 +9,7 @@ #include "flutter/lib/ui/compositing/scene.h" #include "flutter/lib/ui/ui_dart_state.h" #include "flutter/lib/ui/window/platform_message_response_dart.h" +#include "flutter/lib/ui/window/platform_message_response_dart_port.h" #include "flutter/lib/ui/window/viewport_metrics.h" #include "flutter/lib/ui/window/window.h" #include "third_party/tonic/converter/dart_converter.h" @@ -284,6 +285,25 @@ void PlatformConfigurationNativeApi::SetNeedsReportTimings(bool value) { ->SetNeedsReportTimings(value); } +namespace { +void HandlePlatformMessage( + UIDartState* dart_state, + const std::string& name, + Dart_Handle data_handle, + const fml::RefPtr& response) { + if (Dart_IsNull(data_handle)) { + dart_state->HandlePlatformMessage( + std::make_unique(name, response)); + } else { + tonic::DartByteData data(data_handle); + const uint8_t* buffer = static_cast(data.data()); + dart_state->HandlePlatformMessage(std::make_unique( + name, fml::MallocMapping::Copy(buffer, data.length_in_bytes()), + response)); + } +} +} // namespace + Dart_Handle PlatformConfigurationNativeApi::SendPlatformMessage( const std::string& name, Dart_Handle callback, @@ -292,7 +312,8 @@ Dart_Handle PlatformConfigurationNativeApi::SendPlatformMessage( if (!dart_state->platform_configuration()) { return tonic::ToDart( - "Platform messages can only be sent from the main isolate"); + "Sending messages off the root isolate should happen via " + "SendPortPlatformMessage."); } fml::RefPtr response; @@ -301,18 +322,31 @@ Dart_Handle PlatformConfigurationNativeApi::SendPlatformMessage( tonic::DartPersistentValue(dart_state, callback), dart_state->GetTaskRunners().GetUITaskRunner(), name); } - if (Dart_IsNull(data_handle)) { - dart_state->platform_configuration()->client()->HandlePlatformMessage( - std::make_unique(name, response)); - } else { - tonic::DartByteData data(data_handle); - const uint8_t* buffer = static_cast(data.data()); - dart_state->platform_configuration()->client()->HandlePlatformMessage( - std::make_unique( - name, fml::MallocMapping::Copy(buffer, data.length_in_bytes()), - response)); + HandlePlatformMessage(dart_state, name, data_handle, response); + + return Dart_Null(); +} + +Dart_Handle PlatformConfigurationNativeApi::SendPortPlatformMessage( + const std::string& name, + Dart_Handle identifier, + Dart_Handle send_port, + Dart_Handle data_handle) { + // This can be executed on any isolate. + UIDartState* dart_state = UIDartState::Current(); + + int64_t c_send_port = tonic::DartConverter::FromDart(send_port); + if (c_send_port == ILLEGAL_PORT) { + return tonic::ToDart("Invalid port specified"); } + fml::RefPtr response = + fml::MakeRefCounted( + c_send_port, tonic::DartConverter::FromDart(identifier), + name); + + HandlePlatformMessage(dart_state, name, data_handle, response); + return Dart_Null(); } @@ -396,4 +430,30 @@ std::string PlatformConfigurationNativeApi::DefaultRouteName() { ->DefaultRouteName(); } +int64_t PlatformConfigurationNativeApi::RegisterRootIsolate() { + UIDartState* dart_state = UIDartState::Current(); + FML_DCHECK(dart_state && dart_state->IsRootIsolate()); + if (dart_state->IsRootIsolate()) { + int64_t identifier = reinterpret_cast(dart_state); + (*static_cast*>( + Dart_CurrentIsolateGroupData())) + ->SetPlatformConfiguration(identifier, + dart_state->GetWeakPlatformConfiguration()); + return identifier; + } else { + return 0; + } +} + +void PlatformConfigurationNativeApi::RegisterBackgroundIsolate( + int64_t isolate_id) { + UIDartState* dart_state = UIDartState::Current(); + FML_DCHECK(dart_state && !dart_state->IsRootIsolate()); + auto weak_platform_configuration = + (*static_cast*>( + Dart_CurrentIsolateGroupData())) + ->GetPlatformConfiguration(isolate_id); + dart_state->SetWeakPlatformConfiguration(weak_platform_configuration); +} + } // namespace flutter diff --git a/lib/ui/window/platform_configuration.h b/lib/ui/window/platform_configuration.h index 632bac5c9d1fa..272470431403b 100644 --- a/lib/ui/window/platform_configuration.h +++ b/lib/ui/window/platform_configuration.h @@ -12,6 +12,7 @@ #include #include "flutter/assets/asset_manager.h" +#include "flutter/fml/memory/threadsafe_unique_ptr.h" #include "flutter/fml/time/time_point.h" #include "flutter/lib/ui/semantics/semantics_update.h" #include "flutter/lib/ui/window/pointer_data_packet.h" @@ -442,6 +443,21 @@ class PlatformConfiguration final { pending_responses_; }; +//---------------------------------------------------------------------------- +/// An inteface that the result of `Dart_CurrentIsolateGroupData` should +/// implement for registering background isolates to work. +class PlatformConfigurationStorage { + public: + virtual ~PlatformConfigurationStorage() = default; + virtual void SetPlatformConfiguration( + int64_t root_isolate_id, + fml::threadsafe_unique_ptr::weak_ptr + platform_configuration) = 0; + + virtual fml::threadsafe_unique_ptr::weak_ptr + GetPlatformConfiguration(int64_t root_isolate_id) const = 0; +}; + //---------------------------------------------------------------------------- // API exposed as FFI calls in Dart. // @@ -475,6 +491,11 @@ class PlatformConfigurationNativeApi { Dart_Handle callback, Dart_Handle data_handle); + static Dart_Handle SendPortPlatformMessage(const std::string& name, + Dart_Handle identifier, + Dart_Handle send_port, + Dart_Handle data_handle); + static void RespondToPlatformMessage(int response_id, const tonic::DartByteData& data); @@ -494,6 +515,10 @@ class PlatformConfigurationNativeApi { /// mode does. /// static int RequestDartPerformanceMode(int mode); + + static int64_t RegisterRootIsolate(); + + static void RegisterBackgroundIsolate(int64_t isolate_id); }; } // namespace flutter diff --git a/lib/ui/window/platform_message_response_dart_port.cc b/lib/ui/window/platform_message_response_dart_port.cc new file mode 100644 index 0000000000000..17801733ca9b4 --- /dev/null +++ b/lib/ui/window/platform_message_response_dart_port.cc @@ -0,0 +1,72 @@ +// 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/lib/ui/window/platform_message_response_dart_port.h" + +#include + +#include "flutter/common/task_runners.h" +#include "flutter/fml/make_copyable.h" +#include "flutter/fml/trace_event.h" +#include "third_party/dart/runtime/include/dart_native_api.h" +#include "third_party/tonic/converter/dart_converter.h" +#include "third_party/tonic/dart_state.h" +#include "third_party/tonic/logging/dart_invoke.h" +#include "third_party/tonic/typed_data/dart_byte_data.h" + +namespace flutter { +namespace { +void FreeFinalizer(void* isolate_callback_data, void* peer) { + free(peer); +} +} // namespace + +PlatformMessageResponseDartPort::PlatformMessageResponseDartPort( + Dart_Port send_port, + int64_t identifier, + const std::string& channel) + : send_port_(send_port), identifier_(identifier), channel_(channel) { + FML_DCHECK(send_port != ILLEGAL_PORT); +} + +void PlatformMessageResponseDartPort::Complete( + std::unique_ptr data) { + is_complete_ = true; + Dart_CObject response_identifier = { + .type = Dart_CObject_kInt64, + }; + response_identifier.value.as_int64 = identifier_; + Dart_CObject response_data = { + .type = Dart_CObject_kExternalTypedData, + }; + uint8_t* copy = static_cast(malloc(data->GetSize())); + memcpy(copy, data->GetMapping(), data->GetSize()); + response_data.value.as_external_typed_data.type = Dart_TypedData_kUint8; + response_data.value.as_external_typed_data.length = data->GetSize(); + response_data.value.as_external_typed_data.data = copy; + response_data.value.as_external_typed_data.peer = copy; + response_data.value.as_external_typed_data.callback = FreeFinalizer; + + Dart_CObject* response_values[2] = {&response_identifier, &response_data}; + + Dart_CObject response = { + .type = Dart_CObject_kArray, + }; + response.value.as_array.length = 2; + response.value.as_array.values = response_values; + + bool did_send = Dart_PostCObject(send_port_, &response); + FML_CHECK(did_send); +} + +void PlatformMessageResponseDartPort::CompleteEmpty() { + is_complete_ = true; + Dart_CObject response = { + .type = Dart_CObject_kNull, + }; + bool did_send = Dart_PostCObject(send_port_, &response); + FML_CHECK(did_send); +} + +} // namespace flutter diff --git a/lib/ui/window/platform_message_response_dart_port.h b/lib/ui/window/platform_message_response_dart_port.h new file mode 100644 index 0000000000000..5ca375437e8fa --- /dev/null +++ b/lib/ui/window/platform_message_response_dart_port.h @@ -0,0 +1,34 @@ +// 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. + +#ifndef FLUTTER_LIB_UI_PLATFORM_PLATFORM_MESSAGE_RESPONSE_DART_PORT_H_ +#define FLUTTER_LIB_UI_PLATFORM_PLATFORM_MESSAGE_RESPONSE_DART_PORT_H_ + +#include "flutter/fml/message_loop.h" +#include "flutter/lib/ui/window/platform_message_response.h" +#include "third_party/tonic/dart_persistent_value.h" + +namespace flutter { + +class PlatformMessageResponseDartPort : public PlatformMessageResponse { + FML_FRIEND_MAKE_REF_COUNTED(PlatformMessageResponseDartPort); + + public: + // Callable on any thread. + void Complete(std::unique_ptr data) override; + void CompleteEmpty() override; + + protected: + explicit PlatformMessageResponseDartPort(Dart_Port send_port, + int64_t identifier, + const std::string& channel); + + Dart_Port send_port_; + int64_t identifier_; + const std::string channel_; +}; + +} // namespace flutter + +#endif // FLUTTER_LIB_UI_PLATFORM_PLATFORM_MESSAGE_RESPONSE_DART_PORT_H_ diff --git a/lib/web_ui/lib/platform_dispatcher.dart b/lib/web_ui/lib/platform_dispatcher.dart index 043d4999a350e..cf81b34c01965 100644 --- a/lib/web_ui/lib/platform_dispatcher.dart +++ b/lib/web_ui/lib/platform_dispatcher.dart @@ -49,6 +49,16 @@ abstract class PlatformDispatcher { PlatformMessageResponseCallback? callback, ); + void sendPortPlatformMessage( + String name, + ByteData? data, + int identifier, + Object port); + + int registerRootIsolate(); + + void registerBackgroundIsolate(int rootIsolateId); + PlatformMessageCallback? get onPlatformMessage; set onPlatformMessage(PlatformMessageCallback? callback); diff --git a/lib/web_ui/lib/src/engine/platform_dispatcher.dart b/lib/web_ui/lib/src/engine/platform_dispatcher.dart index 311be4fa5e391..288e541acd4f8 100644 --- a/lib/web_ui/lib/src/engine/platform_dispatcher.dart +++ b/lib/web_ui/lib/src/engine/platform_dispatcher.dart @@ -343,6 +343,26 @@ class EnginePlatformDispatcher extends ui.PlatformDispatcher { name, data, _zonedPlatformMessageResponseCallback(callback)); } + @override + void sendPortPlatformMessage( + String name, + ByteData? data, + int identifier, + Object port, + ) { + throw Exception("Isolates aren't supported in web."); + } + + @override + int registerRootIsolate() { + throw Exception("Isolates aren't supported in web."); + } + + @override + void registerBackgroundIsolate(int rootIsolateId) { + throw Exception("Isolates aren't supported in web."); + } + // TODO(ianh): Deprecate onPlatformMessage once the framework is moved over // to using channel buffers exclusively. @override diff --git a/runtime/dart_isolate.cc b/runtime/dart_isolate.cc index e12059d229e95..fbd0776fb63da 100644 --- a/runtime/dart_isolate.cc +++ b/runtime/dart_isolate.cc @@ -269,7 +269,9 @@ std::weak_ptr DartIsolate::CreateRootIsolate( static_cast*>(Dart_IsolateData(vm_isolate)); (*root_isolate_data) - ->SetPlatformConfiguration(std::move(platform_configuration)); + ->SetPlatformConfiguration( + fml::threadsafe_unique_ptr( + std::move(platform_configuration))); return (*root_isolate_data)->GetWeakIsolatePtr(); } @@ -960,6 +962,10 @@ bool DartIsolate::DartIsolateInitializeCallback(void** child_callback_data, false, // is_root_isolate context))); // context + (*embedder_isolate.get()) + ->SetWeakPlatformConfiguration( + (*isolate_group_data)->GetPlatformConfiguration(1)); + // root isolate should have been created via CreateRootIsolate if (!InitializeIsolate(*embedder_isolate, isolate, error)) { return false; diff --git a/runtime/dart_isolate_group_data.cc b/runtime/dart_isolate_group_data.cc index 5850d04ea68b3..1a40712943cd8 100644 --- a/runtime/dart_isolate_group_data.cc +++ b/runtime/dart_isolate_group_data.cc @@ -64,4 +64,21 @@ void DartIsolateGroupData::SetChildIsolatePreparer( child_isolate_preparer_ = value; } +void DartIsolateGroupData::SetPlatformConfiguration( + int64_t root_isolate_id, + fml::threadsafe_unique_ptr::weak_ptr + platform_configuration) { + std::scoped_lock lock(platform_configurations_mutex_); + platform_configurations_[root_isolate_id] = platform_configuration; +} + +fml::threadsafe_unique_ptr::weak_ptr +DartIsolateGroupData::GetPlatformConfiguration(int64_t root_isolate_id) const { + std::scoped_lock lock(platform_configurations_mutex_); + auto it = platform_configurations_.find(root_isolate_id); + return it == platform_configurations_.end() + ? fml::threadsafe_unique_ptr::weak_ptr() + : it->second; +} + } // namespace flutter diff --git a/runtime/dart_isolate_group_data.h b/runtime/dart_isolate_group_data.h index 4306bca1d6b00..7c9e5ae195fea 100644 --- a/runtime/dart_isolate_group_data.h +++ b/runtime/dart_isolate_group_data.h @@ -5,12 +5,15 @@ #ifndef FLUTTER_RUNTIME_DART_ISOLATE_GROUP_DATA_H_ #define FLUTTER_RUNTIME_DART_ISOLATE_GROUP_DATA_H_ +#include #include #include #include "flutter/common/settings.h" #include "flutter/fml/closure.h" #include "flutter/fml/memory/ref_ptr.h" +#include "flutter/fml/memory/threadsafe_unique_ptr.h" +#include "flutter/lib/ui/window/platform_configuration.h" namespace flutter { @@ -25,7 +28,7 @@ using ChildIsolatePreparer = std::function; // // This object must be thread safe because the Dart VM can invoke the isolate // group cleanup callback on any thread. -class DartIsolateGroupData { +class DartIsolateGroupData : public PlatformConfigurationStorage { public: DartIsolateGroupData(const Settings& settings, fml::RefPtr isolate_snapshot, @@ -53,6 +56,16 @@ class DartIsolateGroupData { void SetChildIsolatePreparer(const ChildIsolatePreparer& value); + /// |PlatformConfigurationStorage| + void SetPlatformConfiguration( + int64_t root_isolate_id, + fml::threadsafe_unique_ptr::weak_ptr + platform_configuration) override; + + /// |PlatformConfigurationStorage| + fml::threadsafe_unique_ptr::weak_ptr + GetPlatformConfiguration(int64_t root_isolate_id) const override; + private: const Settings settings_; const fml::RefPtr isolate_snapshot_; @@ -62,6 +75,9 @@ class DartIsolateGroupData { ChildIsolatePreparer child_isolate_preparer_; const fml::closure isolate_create_callback_; const fml::closure isolate_shutdown_callback_; + std::map::weak_ptr> + platform_configurations_; + mutable std::mutex platform_configurations_mutex_; FML_DISALLOW_COPY_AND_ASSIGN(DartIsolateGroupData); }; diff --git a/shell/common/engine.cc b/shell/common/engine.cc index c2cbc9f512b98..2efe028c4f7e7 100644 --- a/shell/common/engine.cc +++ b/shell/common/engine.cc @@ -460,6 +460,7 @@ void Engine::UpdateSemantics(SemanticsNodeUpdates update, void Engine::HandlePlatformMessage(std::unique_ptr message) { if (message->channel() == kAssetChannel) { + FML_DCHECK(task_runners_.GetUITaskRunner()->RunsTasksOnCurrentThread()); HandleAssetPlatformMessage(std::move(message)); } else { delegate_.OnEngineHandlePlatformMessage(std::move(message)); diff --git a/shell/common/shell.cc b/shell/common/shell.cc index 50f81d8d57b92..a9882dfe2dce9 100644 --- a/shell/common/shell.cc +++ b/shell/common/shell.cc @@ -1212,10 +1212,15 @@ void Shell::OnEngineUpdateSemantics(SemanticsNodeUpdates update, // |Engine::Delegate| void Shell::OnEngineHandlePlatformMessage( std::unique_ptr message) { + /// Called from any isolate's thread. This is safe because the only instance + /// variables accessed here are set once at startup, except + /// `route_messages_through_platform_thread_` which if misread is not a + /// logical error. `UIDartState` has a lock that makes sure that calling this + /// doesn't happen while the `Shell` is being destructed. FML_DCHECK(is_setup_); - FML_DCHECK(task_runners_.GetUITaskRunner()->RunsTasksOnCurrentThread()); if (message->channel() == kSkiaChannel) { + FML_DCHECK(task_runners_.GetUITaskRunner()->RunsTasksOnCurrentThread()); HandleEngineSkiaMessage(std::move(message)); return; } diff --git a/shell/platform/darwin/ios/platform_message_handler_ios.mm b/shell/platform/darwin/ios/platform_message_handler_ios.mm index 7f6efcd63cfc0..a3d9c75456bb9 100644 --- a/shell/platform/darwin/ios/platform_message_handler_ios.mm +++ b/shell/platform/darwin/ios/platform_message_handler_ios.mm @@ -47,10 +47,13 @@ - (void)dispatch:(dispatch_block_t)block { : task_runners_(task_runners) {} void PlatformMessageHandlerIos::HandlePlatformMessage(std::unique_ptr message) { - FML_CHECK(task_runners_.GetUITaskRunner()->RunsTasksOnCurrentThread()); + // This can be called from any isolate's thread. fml::RefPtr completer = message->response(); HandlerInfo handler_info; { + // TODO(gaaclarke): This mutex is a bottleneck for multiple isolates sending + // messages at the same time. This could be potentially changed to a + // read-write lock. std::lock_guard lock(message_handlers_mutex_); auto it = message_handlers_.find(message->channel()); if (it != message_handlers_.end()) { From 359010b3a9fe592af72e3218c6f2945d74cc3422 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 18 Aug 2022 14:42:55 -0700 Subject: [PATCH 02/19] - switched to `get rootIsolateId` - Introduced RootIsolateToken --- lib/ui/dart_ui.cc | 2 +- lib/ui/platform_dispatcher.dart | 19 ++++++++++++++----- lib/ui/window/platform_configuration.cc | 3 +-- lib/ui/window/platform_configuration.h | 2 +- 4 files changed, 17 insertions(+), 9 deletions(-) diff --git a/lib/ui/dart_ui.cc b/lib/ui/dart_ui.cc index 4fd108454046d..8f33cbd70ec7d 100644 --- a/lib/ui/dart_ui.cc +++ b/lib/ui/dart_ui.cc @@ -100,7 +100,7 @@ typedef CanvasPath Path; V(PlatformConfigurationNativeApi::ComputePlatformResolvedLocale, 1) \ V(PlatformConfigurationNativeApi::SendPlatformMessage, 3) \ V(PlatformConfigurationNativeApi::RespondToPlatformMessage, 2) \ - V(PlatformConfigurationNativeApi::RegisterRootIsolate, 0) \ + V(PlatformConfigurationNativeApi::GetRootIsolateId, 0) \ V(PlatformConfigurationNativeApi::RegisterBackgroundIsolate, 1) \ V(PlatformConfigurationNativeApi::SendPortPlatformMessage, 4) \ V(DartRuntimeHooks::Logger_PrintDebugString, 1) \ diff --git a/lib/ui/platform_dispatcher.dart b/lib/ui/platform_dispatcher.dart index 2dccd6a90a23c..7203c3867c4e2 100644 --- a/lib/ui/platform_dispatcher.dart +++ b/lib/ui/platform_dispatcher.dart @@ -73,6 +73,19 @@ const String _kFlutterKeyDataChannel = 'flutter/keydata'; ByteData? _wrapUnmodifiableByteData(ByteData? byteData) => byteData == null ? null : UnmodifiableByteDataView(byteData); +class RootIsolateToken { + final int _rootIsolateId; + const RootIsolateToken._(this._rootIsolateId); + + static RootIsolateToken? get instance { + final int rootIsolateId = __getRootIsolateId(); + return rootIsolateId == 0 ? null : RootIsolateToken._(rootIsolateId); + } + + @FfiNative('PlatformConfigurationNativeApi::GetRootIsolateId') + external static int __getRootIsolateId(); +} + /// Platform event dispatcher singleton. /// /// The most basic interface to the host operating system's interface. @@ -556,11 +569,7 @@ class PlatformDispatcher { @FfiNative('PlatformConfigurationNativeApi::SendPortPlatformMessage') external static String? __sendPortPlatformMessage(String name, int identifier, int port, ByteData? data); - int registerRootIsolate() => __registerRootIsolate(); - @FfiNative('PlatformConfigurationNativeApi::RegisterRootIsolate') - external static int __registerRootIsolate(); - - void registerBackgroundIsolate(int rootIsolateId) => __registerBackgroundIsolate(rootIsolateId); + void registerBackgroundIsolate(RootIsolateToken token) => __registerBackgroundIsolate(token._rootIsolateId); @FfiNative('PlatformConfigurationNativeApi::RegisterBackgroundIsolate') external static void __registerBackgroundIsolate(int rootIsolateId); diff --git a/lib/ui/window/platform_configuration.cc b/lib/ui/window/platform_configuration.cc index 50670d4c63850..6d421e7a341eb 100644 --- a/lib/ui/window/platform_configuration.cc +++ b/lib/ui/window/platform_configuration.cc @@ -430,9 +430,8 @@ std::string PlatformConfigurationNativeApi::DefaultRouteName() { ->DefaultRouteName(); } -int64_t PlatformConfigurationNativeApi::RegisterRootIsolate() { +int64_t PlatformConfigurationNativeApi::GetRootIsolateId() { UIDartState* dart_state = UIDartState::Current(); - FML_DCHECK(dart_state && dart_state->IsRootIsolate()); if (dart_state->IsRootIsolate()) { int64_t identifier = reinterpret_cast(dart_state); (*static_cast*>( diff --git a/lib/ui/window/platform_configuration.h b/lib/ui/window/platform_configuration.h index 272470431403b..55c083d9a6df6 100644 --- a/lib/ui/window/platform_configuration.h +++ b/lib/ui/window/platform_configuration.h @@ -516,7 +516,7 @@ class PlatformConfigurationNativeApi { /// static int RequestDartPerformanceMode(int mode); - static int64_t RegisterRootIsolate(); + static int64_t GetRootIsolateId(); static void RegisterBackgroundIsolate(int64_t isolate_id); }; From 41b710bb47ec3d27eed4ae6b1463d80287bb8386 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Tue, 23 Aug 2022 11:10:22 -0700 Subject: [PATCH 03/19] switched to storing a platform message handler on the isolate group data --- ci/licenses_golden/licenses_flutter | 1 - fml/memory/threadsafe_unique_ptr.h | 111 ------------------------ lib/ui/platform_dispatcher.dart | 9 +- lib/ui/ui_dart_state.cc | 21 +++-- lib/ui/ui_dart_state.h | 21 ++--- lib/ui/window/platform_configuration.cc | 21 ++--- lib/ui/window/platform_configuration.h | 15 ++-- runtime/dart_isolate.cc | 8 +- runtime/dart_isolate.h | 8 +- runtime/dart_isolate_group_data.cc | 21 +++-- runtime/dart_isolate_group_data.h | 23 +++-- runtime/runtime_controller.h | 2 + shell/common/engine.cc | 1 - shell/common/engine.h | 7 ++ shell/common/shell.cc | 18 ++-- 15 files changed, 84 insertions(+), 203 deletions(-) delete mode 100644 fml/memory/threadsafe_unique_ptr.h diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index bc13cb0f0b191..285319b6df3de 100644 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -281,7 +281,6 @@ FILE: ../../../flutter/fml/memory/task_runner_checker.cc FILE: ../../../flutter/fml/memory/task_runner_checker.h FILE: ../../../flutter/fml/memory/task_runner_checker_unittest.cc FILE: ../../../flutter/fml/memory/thread_checker.h -FILE: ../../../flutter/fml/memory/threadsafe_unique_ptr.h FILE: ../../../flutter/fml/memory/weak_ptr.h FILE: ../../../flutter/fml/memory/weak_ptr_internal.cc FILE: ../../../flutter/fml/memory/weak_ptr_internal.h diff --git a/fml/memory/threadsafe_unique_ptr.h b/fml/memory/threadsafe_unique_ptr.h deleted file mode 100644 index 2b8925976d789..0000000000000 --- a/fml/memory/threadsafe_unique_ptr.h +++ /dev/null @@ -1,111 +0,0 @@ -// 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. - -#ifndef FLUTTER_FML_THREADSAFE_UNIQUE_PTR_H_ -#define FLUTTER_FML_THREADSAFE_UNIQUE_PTR_H_ - -#include -#include - -#include "flutter/fml/macros.h" - -namespace fml { - -/// A single-owner smart pointer that allows weak references that can be -/// accessed on different threads. -/// This smart-pointer makes the following guarantees: -/// * There is only ever one threadsafe_unique_ptr per object. -/// * The object is deleted when ~threadsafe_unique_ptr() is called. -/// * The object will not be deleted whilst another thread has a lock_ptr. -/// * The thread that owns the object can access it without a lock. -/// WARNING: weak_ptr's should only be used to invoke thread-safe methods. -template -class threadsafe_unique_ptr { - private: - struct Data { - Data(std::unique_ptr&& arg_ptr) : ptr(std::move(arg_ptr)) {} - std::unique_ptr ptr; - std::mutex mutex; - }; - - public: - threadsafe_unique_ptr() - : data_(new Data(std::unique_ptr())), fast_ptr_(nullptr) {} - - threadsafe_unique_ptr(std::unique_ptr&& ptr) - : data_(new Data(std::move(ptr))) { - fast_ptr_ = data_->ptr.get(); - } - - threadsafe_unique_ptr(threadsafe_unique_ptr&& ptr) - : data_(std::move(ptr.data_)), fast_ptr_(std::move(ptr.fast_ptr_)) {} - - threadsafe_unique_ptr& operator=(threadsafe_unique_ptr&& rvalue) { - data_ = std::move(rvalue.data_); - fast_ptr_ = std::move(rvalue.fast_ptr_); - return *this; - } - - ~threadsafe_unique_ptr() { - if (data_) { - std::scoped_lock lock(data_->mutex); - data_->ptr.reset(); - } - } - - T* operator->() const { return fast_ptr_; } - - T* get() const { return fast_ptr_; } - - operator bool() const { return fast_ptr_ != nullptr; } - - class lock_ptr; - - /// A non-owning smart pointer for a `threadsafe_unique_ptr`. - class weak_ptr { - public: - weak_ptr() {} - - weak_ptr(std::weak_ptr weak_ptr) : weak_ptr_(weak_ptr) {} - - private: - friend class lock_ptr; - std::weak_ptr weak_ptr_; - }; - - /// A temporary owning smart pointer for a `threadsafe_unique_ptr`. This - /// guarantees that the object will not be deleted while in scope. - class lock_ptr { - public: - lock_ptr(const weak_ptr& weak) : strong_ptr_(weak.weak_ptr_.lock()) { - if (strong_ptr_) { - lock_ = std::unique_lock(strong_ptr_->mutex); - } - } - - T* operator->() { return strong_ptr_ ? strong_ptr_->ptr.get() : nullptr; } - - operator bool() const { - return strong_ptr_ ? static_cast(strong_ptr_->ptr) : false; - } - - private: - FML_DISALLOW_COPY_AND_ASSIGN(lock_ptr); - std::shared_ptr strong_ptr_; - std::unique_lock lock_; - }; - - weak_ptr GetWeakPtr() const { return weak_ptr(data_); } - - private: - FML_DISALLOW_COPY_AND_ASSIGN(threadsafe_unique_ptr); - std::shared_ptr data_; - // Clients that own the threadsafe_unique_ptr can access the pointer directly - // since there is no risk that it will be deleted whilst being accessed. - T* fast_ptr_; -}; - -} // namespace fml - -#endif // FLUTTER_FML_THREADSAFE_UNIQUE_PTR_H_ diff --git a/lib/ui/platform_dispatcher.dart b/lib/ui/platform_dispatcher.dart index 7203c3867c4e2..0b1778f3c1551 100644 --- a/lib/ui/platform_dispatcher.dart +++ b/lib/ui/platform_dispatcher.dart @@ -74,13 +74,14 @@ ByteData? _wrapUnmodifiableByteData(ByteData? byteData) => byteData == null ? null : UnmodifiableByteDataView(byteData); class RootIsolateToken { + RootIsolateToken._(this._rootIsolateId); + final int _rootIsolateId; - const RootIsolateToken._(this._rootIsolateId); - static RootIsolateToken? get instance { - final int rootIsolateId = __getRootIsolateId(); + late final RootIsolateToken? instance = () { + int rootIsolateId = __getRootIsolateId(); return rootIsolateId == 0 ? null : RootIsolateToken._(rootIsolateId); - } + }(); @FfiNative('PlatformConfigurationNativeApi::GetRootIsolateId') external static int __getRootIsolateId(); diff --git a/lib/ui/ui_dart_state.cc b/lib/ui/ui_dart_state.cc index e0155c76b3b00..76e46708ac205 100644 --- a/lib/ui/ui_dart_state.cc +++ b/lib/ui/ui_dart_state.cc @@ -106,7 +106,7 @@ UIDartState* UIDartState::Current() { } void UIDartState::SetPlatformConfiguration( - fml::threadsafe_unique_ptr platform_configuration) { + std::unique_ptr platform_configuration) { FML_DCHECK(IsRootIsolate()); platform_configuration_ = std::move(platform_configuration); if (platform_configuration_) { @@ -115,11 +115,10 @@ void UIDartState::SetPlatformConfiguration( } } -void UIDartState::SetWeakPlatformConfiguration( - fml::threadsafe_unique_ptr::weak_ptr - platform_configuration) { +void UIDartState::SetPlatformMessageHandler( + std::weak_ptr handler) { FML_DCHECK(!IsRootIsolate()); - weak_platform_configuration_ = platform_configuration; + platform_message_handler_ = handler; } const TaskRunners& UIDartState::GetTaskRunners() const { @@ -228,12 +227,16 @@ void UIDartState::HandlePlatformMessage( platform_configuration_->client()->HandlePlatformMessage( std::move(message)); } else { - fml::threadsafe_unique_ptr::lock_ptr lock( - weak_platform_configuration_); - if (lock) { - lock->client()->HandlePlatformMessage(std::move(message)); + std::shared_ptr handler = + platform_message_handler_.lock(); + if (handler) { + handler->HandlePlatformMessage(std::move(message)); } } } +int64_t UIDartState::GetRootIsolateId() const { + return IsRootIsolate() ? reinterpret_cast(this) : 0; +} + } // namespace flutter diff --git a/lib/ui/ui_dart_state.h b/lib/ui/ui_dart_state.h index ca145e0046efb..618645330b1b1 100644 --- a/lib/ui/ui_dart_state.h +++ b/lib/ui/ui_dart_state.h @@ -13,7 +13,6 @@ #include "flutter/common/task_runners.h" #include "flutter/flow/skia_gpu_object.h" #include "flutter/fml/build_config.h" -#include "flutter/fml/memory/threadsafe_unique_ptr.h" #include "flutter/fml/memory/weak_ptr.h" #include "flutter/fml/synchronization/waitable_event.h" #include "flutter/lib/ui/io_manager.h" @@ -21,6 +20,7 @@ #include "flutter/lib/ui/painting/image_decoder.h" #include "flutter/lib/ui/snapshot_delegate.h" #include "flutter/lib/ui/volatile_path_tracker.h" +#include "flutter/shell/common/platform_message_handler.h" #include "third_party/dart/runtime/include/dart_api.h" #include "third_party/skia/include/gpu/GrDirectContext.h" #include "third_party/tonic/dart_microtask_queue.h" @@ -108,15 +108,7 @@ class UIDartState : public tonic::DartState { return platform_configuration_.get(); } - fml::threadsafe_unique_ptr::weak_ptr - GetWeakPlatformConfiguration() const { - return platform_configuration_ ? platform_configuration_.GetWeakPtr() - : weak_platform_configuration_; - } - - void SetWeakPlatformConfiguration( - fml::threadsafe_unique_ptr::weak_ptr - platform_configuration); + void SetPlatformMessageHandler(std::weak_ptr handler); void HandlePlatformMessage(std::unique_ptr message); @@ -167,6 +159,8 @@ class UIDartState : public tonic::DartState { return unhandled_exception_callback_; } + int64_t GetRootIsolateId() const; + protected: UIDartState(TaskObserverAdd add_callback, TaskObserverRemove remove_callback, @@ -181,7 +175,7 @@ class UIDartState : public tonic::DartState { ~UIDartState() override; void SetPlatformConfiguration( - fml::threadsafe_unique_ptr platform_configuration); + std::unique_ptr platform_configuration); const std::string& GetAdvisoryScriptURI() const; @@ -194,9 +188,8 @@ class UIDartState : public tonic::DartState { Dart_Port main_port_ = ILLEGAL_PORT; const bool is_root_isolate_; std::string debug_name_; - fml::threadsafe_unique_ptr platform_configuration_; - fml::threadsafe_unique_ptr::weak_ptr - weak_platform_configuration_; + std::unique_ptr platform_configuration_; + std::weak_ptr platform_message_handler_; tonic::DartMicrotaskQueue microtask_queue_; UnhandledExceptionCallback unhandled_exception_callback_; LogMessageCallback log_message_callback_; diff --git a/lib/ui/window/platform_configuration.cc b/lib/ui/window/platform_configuration.cc index 6d421e7a341eb..c31f035868103 100644 --- a/lib/ui/window/platform_configuration.cc +++ b/lib/ui/window/platform_configuration.cc @@ -432,27 +432,20 @@ std::string PlatformConfigurationNativeApi::DefaultRouteName() { int64_t PlatformConfigurationNativeApi::GetRootIsolateId() { UIDartState* dart_state = UIDartState::Current(); - if (dart_state->IsRootIsolate()) { - int64_t identifier = reinterpret_cast(dart_state); - (*static_cast*>( - Dart_CurrentIsolateGroupData())) - ->SetPlatformConfiguration(identifier, - dart_state->GetWeakPlatformConfiguration()); - return identifier; - } else { - return 0; - } + FML_DCHECK(dart_state); + return dart_state->GetRootIsolateId(); } void PlatformConfigurationNativeApi::RegisterBackgroundIsolate( int64_t isolate_id) { UIDartState* dart_state = UIDartState::Current(); FML_DCHECK(dart_state && !dart_state->IsRootIsolate()); - auto weak_platform_configuration = - (*static_cast*>( + auto weak_platform_message_handler = + (*static_cast*>( Dart_CurrentIsolateGroupData())) - ->GetPlatformConfiguration(isolate_id); - dart_state->SetWeakPlatformConfiguration(weak_platform_configuration); + ->GetPlatformMessageHandler(isolate_id); + FML_DCHECK(weak_platform_message_handler.lock()); + dart_state->SetPlatformMessageHandler(weak_platform_message_handler); } } // namespace flutter diff --git a/lib/ui/window/platform_configuration.h b/lib/ui/window/platform_configuration.h index 55c083d9a6df6..9ce0f7e46d845 100644 --- a/lib/ui/window/platform_configuration.h +++ b/lib/ui/window/platform_configuration.h @@ -12,7 +12,6 @@ #include #include "flutter/assets/asset_manager.h" -#include "flutter/fml/memory/threadsafe_unique_ptr.h" #include "flutter/fml/time/time_point.h" #include "flutter/lib/ui/semantics/semantics_update.h" #include "flutter/lib/ui/window/pointer_data_packet.h" @@ -24,6 +23,7 @@ namespace flutter { class FontCollection; class PlatformMessage; +class PlatformMessageHandler; class Scene; //-------------------------------------------------------------------------- @@ -446,16 +446,15 @@ class PlatformConfiguration final { //---------------------------------------------------------------------------- /// An inteface that the result of `Dart_CurrentIsolateGroupData` should /// implement for registering background isolates to work. -class PlatformConfigurationStorage { +class PlatformMessageHandlerStorage { public: - virtual ~PlatformConfigurationStorage() = default; - virtual void SetPlatformConfiguration( + virtual ~PlatformMessageHandlerStorage() = default; + virtual void SetPlatformMessageHandler( int64_t root_isolate_id, - fml::threadsafe_unique_ptr::weak_ptr - platform_configuration) = 0; + std::weak_ptr handler) = 0; - virtual fml::threadsafe_unique_ptr::weak_ptr - GetPlatformConfiguration(int64_t root_isolate_id) const = 0; + virtual std::weak_ptr GetPlatformMessageHandler( + int64_t root_isolate_id) const = 0; }; //---------------------------------------------------------------------------- diff --git a/runtime/dart_isolate.cc b/runtime/dart_isolate.cc index fbd0776fb63da..e12059d229e95 100644 --- a/runtime/dart_isolate.cc +++ b/runtime/dart_isolate.cc @@ -269,9 +269,7 @@ std::weak_ptr DartIsolate::CreateRootIsolate( static_cast*>(Dart_IsolateData(vm_isolate)); (*root_isolate_data) - ->SetPlatformConfiguration( - fml::threadsafe_unique_ptr( - std::move(platform_configuration))); + ->SetPlatformConfiguration(std::move(platform_configuration)); return (*root_isolate_data)->GetWeakIsolatePtr(); } @@ -962,10 +960,6 @@ bool DartIsolate::DartIsolateInitializeCallback(void** child_callback_data, false, // is_root_isolate context))); // context - (*embedder_isolate.get()) - ->SetWeakPlatformConfiguration( - (*isolate_group_data)->GetPlatformConfiguration(1)); - // root isolate should have been created via CreateRootIsolate if (!InitializeIsolate(*embedder_isolate, isolate, error)) { return false; diff --git a/runtime/dart_isolate.h b/runtime/dart_isolate.h index 3fe9cf2f81dc4..c51eb7081ab37 100644 --- a/runtime/dart_isolate.h +++ b/runtime/dart_isolate.h @@ -385,6 +385,10 @@ class DartIsolate : public UIDartState { const std::string error_message, bool transient); + DartIsolateGroupData& GetIsolateGroupData(); + + const DartIsolateGroupData& GetIsolateGroupData() const; + private: friend class IsolateConfiguration; class AutoFireClosure { @@ -444,10 +448,6 @@ class DartIsolate : public UIDartState { void OnShutdownCallback(); - DartIsolateGroupData& GetIsolateGroupData(); - - const DartIsolateGroupData& GetIsolateGroupData() const; - // |Dart_IsolateGroupCreateCallback| static Dart_Isolate DartIsolateGroupCreateCallback( const char* advisory_script_uri, diff --git a/runtime/dart_isolate_group_data.cc b/runtime/dart_isolate_group_data.cc index 1a40712943cd8..2c77ff1859c04 100644 --- a/runtime/dart_isolate_group_data.cc +++ b/runtime/dart_isolate_group_data.cc @@ -64,20 +64,19 @@ void DartIsolateGroupData::SetChildIsolatePreparer( child_isolate_preparer_ = value; } -void DartIsolateGroupData::SetPlatformConfiguration( +void DartIsolateGroupData::SetPlatformMessageHandler( int64_t root_isolate_id, - fml::threadsafe_unique_ptr::weak_ptr - platform_configuration) { - std::scoped_lock lock(platform_configurations_mutex_); - platform_configurations_[root_isolate_id] = platform_configuration; + std::weak_ptr handler) { + std::scoped_lock lock(platform_message_handlers_mutex_); + platform_message_handlers_[root_isolate_id] = handler; } -fml::threadsafe_unique_ptr::weak_ptr -DartIsolateGroupData::GetPlatformConfiguration(int64_t root_isolate_id) const { - std::scoped_lock lock(platform_configurations_mutex_); - auto it = platform_configurations_.find(root_isolate_id); - return it == platform_configurations_.end() - ? fml::threadsafe_unique_ptr::weak_ptr() +std::weak_ptr +DartIsolateGroupData::GetPlatformMessageHandler(int64_t root_isolate_id) const { + std::scoped_lock lock(platform_message_handlers_mutex_); + auto it = platform_message_handlers_.find(root_isolate_id); + return it == platform_message_handlers_.end() + ? std::weak_ptr() : it->second; } diff --git a/runtime/dart_isolate_group_data.h b/runtime/dart_isolate_group_data.h index 7c9e5ae195fea..2aacf6cf5ed19 100644 --- a/runtime/dart_isolate_group_data.h +++ b/runtime/dart_isolate_group_data.h @@ -12,13 +12,13 @@ #include "flutter/common/settings.h" #include "flutter/fml/closure.h" #include "flutter/fml/memory/ref_ptr.h" -#include "flutter/fml/memory/threadsafe_unique_ptr.h" #include "flutter/lib/ui/window/platform_configuration.h" namespace flutter { class DartIsolate; class DartSnapshot; +class PlatformMessageHandler; using ChildIsolatePreparer = std::function; @@ -28,7 +28,7 @@ using ChildIsolatePreparer = std::function; // // This object must be thread safe because the Dart VM can invoke the isolate // group cleanup callback on any thread. -class DartIsolateGroupData : public PlatformConfigurationStorage { +class DartIsolateGroupData : public PlatformMessageHandlerStorage { public: DartIsolateGroupData(const Settings& settings, fml::RefPtr isolate_snapshot, @@ -56,15 +56,14 @@ class DartIsolateGroupData : public PlatformConfigurationStorage { void SetChildIsolatePreparer(const ChildIsolatePreparer& value); - /// |PlatformConfigurationStorage| - void SetPlatformConfiguration( + // |PlatformMessageHandlerStorage| + void SetPlatformMessageHandler( int64_t root_isolate_id, - fml::threadsafe_unique_ptr::weak_ptr - platform_configuration) override; + std::weak_ptr handler) override; - /// |PlatformConfigurationStorage| - fml::threadsafe_unique_ptr::weak_ptr - GetPlatformConfiguration(int64_t root_isolate_id) const override; + // |PlatformMessageHandlerStorage| + std::weak_ptr GetPlatformMessageHandler( + int64_t root_isolate_id) const override; private: const Settings settings_; @@ -75,9 +74,9 @@ class DartIsolateGroupData : public PlatformConfigurationStorage { ChildIsolatePreparer child_isolate_preparer_; const fml::closure isolate_create_callback_; const fml::closure isolate_shutdown_callback_; - std::map::weak_ptr> - platform_configurations_; - mutable std::mutex platform_configurations_mutex_; + std::map> + platform_message_handlers_; + mutable std::mutex platform_message_handlers_mutex_; FML_DISALLOW_COPY_AND_ASSIGN(DartIsolateGroupData); }; diff --git a/runtime/runtime_controller.h b/runtime/runtime_controller.h index d62c8dc252013..44343231e907c 100644 --- a/runtime/runtime_controller.h +++ b/runtime/runtime_controller.h @@ -558,6 +558,8 @@ class RuntimeController : public PlatformConfigurationClient { return context_.snapshot_delegate; } + std::weak_ptr GetRootIsolate() { return root_isolate_; } + std::weak_ptr GetRootIsolate() const { return root_isolate_; } diff --git a/shell/common/engine.cc b/shell/common/engine.cc index 2efe028c4f7e7..c2cbc9f512b98 100644 --- a/shell/common/engine.cc +++ b/shell/common/engine.cc @@ -460,7 +460,6 @@ void Engine::UpdateSemantics(SemanticsNodeUpdates update, void Engine::HandlePlatformMessage(std::unique_ptr message) { if (message->channel() == kAssetChannel) { - FML_DCHECK(task_runners_.GetUITaskRunner()->RunsTasksOnCurrentThread()); HandleAssetPlatformMessage(std::move(message)); } else { delegate_.OnEngineHandlePlatformMessage(std::move(message)); diff --git a/shell/common/engine.h b/shell/common/engine.h index 04b968f106c41..d1eec04d99755 100644 --- a/shell/common/engine.h +++ b/shell/common/engine.h @@ -872,6 +872,13 @@ class Engine final : public RuntimeDelegate, PointerDataDispatcher::Delegate { return runtime_controller_.get(); } + //-------------------------------------------------------------------------- + /// @brief Accessor for the RuntimeController. + /// + RuntimeController* GetRuntimeController() { + return runtime_controller_.get(); + } + const std::weak_ptr GetVsyncWaiter() const; private: diff --git a/shell/common/shell.cc b/shell/common/shell.cc index a9882dfe2dce9..7253dc8544033 100644 --- a/shell/common/shell.cc +++ b/shell/common/shell.cc @@ -20,6 +20,7 @@ #include "flutter/fml/message_loop.h" #include "flutter/fml/paths.h" #include "flutter/fml/trace_event.h" +#include "flutter/runtime/dart_isolate_group_data.h" #include "flutter/runtime/dart_vm.h" #include "flutter/shell/common/engine.h" #include "flutter/shell/common/skia_event_tracer_impl.h" @@ -593,7 +594,8 @@ void Shell::RunEngine( task_runners_.GetUITaskRunner(), fml::MakeCopyable( [run_configuration = std::move(run_configuration), - weak_engine = weak_engine_, result]() mutable { + weak_engine = weak_engine_, result, + platform_message_handler = platform_message_handler_]() mutable { if (!weak_engine) { FML_LOG(ERROR) << "Could not launch engine with configuration - no engine."; @@ -604,6 +606,13 @@ void Shell::RunEngine( if (run_result == flutter::Engine::RunStatus::Failure) { FML_LOG(ERROR) << "Could not launch engine with configuration."; } + + std::shared_ptr root_isolate = + weak_engine->GetRuntimeController()->GetRootIsolate().lock(); + FML_DCHECK(root_isolate); + root_isolate->GetIsolateGroupData().SetPlatformMessageHandler( + root_isolate->GetRootIsolateId(), platform_message_handler); + result(run_result); })); } @@ -1212,15 +1221,10 @@ void Shell::OnEngineUpdateSemantics(SemanticsNodeUpdates update, // |Engine::Delegate| void Shell::OnEngineHandlePlatformMessage( std::unique_ptr message) { - /// Called from any isolate's thread. This is safe because the only instance - /// variables accessed here are set once at startup, except - /// `route_messages_through_platform_thread_` which if misread is not a - /// logical error. `UIDartState` has a lock that makes sure that calling this - /// doesn't happen while the `Shell` is being destructed. FML_DCHECK(is_setup_); + FML_DCHECK(task_runners_.GetUITaskRunner()->RunsTasksOnCurrentThread()); if (message->channel() == kSkiaChannel) { - FML_DCHECK(task_runners_.GetUITaskRunner()->RunsTasksOnCurrentThread()); HandleEngineSkiaMessage(std::move(message)); return; } From 9bc7750665534b5fffa196e8e34885d4df8ce300 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Tue, 23 Aug 2022 15:09:32 -0700 Subject: [PATCH 04/19] cleanup --- lib/ui/platform_dispatcher.dart | 13 +++++++++++++ lib/ui/window/platform_configuration.cc | 4 +--- lib/ui/window/platform_message_response_dart_port.h | 1 + shell/common/shell.cc | 1 + 4 files changed, 16 insertions(+), 3 deletions(-) diff --git a/lib/ui/platform_dispatcher.dart b/lib/ui/platform_dispatcher.dart index 0b1778f3c1551..80a9ea7f3fe4d 100644 --- a/lib/ui/platform_dispatcher.dart +++ b/lib/ui/platform_dispatcher.dart @@ -73,11 +73,14 @@ const String _kFlutterKeyDataChannel = 'flutter/keydata'; ByteData? _wrapUnmodifiableByteData(ByteData? byteData) => byteData == null ? null : UnmodifiableByteDataView(byteData); +/// A token that represents a root isolate. class RootIsolateToken { RootIsolateToken._(this._rootIsolateId); final int _rootIsolateId; + /// The token for the root isolate that is executing this Dart code. If this + /// Dart code is not executing on a root isolate [instance] will be null. late final RootIsolateToken? instance = () { int rootIsolateId = __getRootIsolateId(); return rootIsolateId == 0 ? null : RootIsolateToken._(rootIsolateId); @@ -552,6 +555,13 @@ class PlatformDispatcher { @FfiNative('PlatformConfigurationNativeApi::SendPlatformMessage') external static String? __sendPlatformMessage(String name, PlatformMessageResponseCallback? callback, ByteData? data); + /// Sends a message to a platform-specific plugin via a [SendPort]. + /// + /// This operates similarly to [sendPlatformMessage] but is used when sending + /// messages from background isolates. [port] allows Flutter to know which + /// isolate to send the result to. [name] is the name of the channel + /// communication will happen on. [data] is the payload of the message. + /// [identifier] is a unique integer assigned to the message. void sendPortPlatformMessage( String name, ByteData? data, @@ -570,6 +580,9 @@ class PlatformDispatcher { @FfiNative('PlatformConfigurationNativeApi::SendPortPlatformMessage') external static String? __sendPortPlatformMessage(String name, int identifier, int port, ByteData? data); + /// Registers the current isolate with the isolate identified with by the + /// [token]. This is required if platform channels are to be used on a + /// background isolate. void registerBackgroundIsolate(RootIsolateToken token) => __registerBackgroundIsolate(token._rootIsolateId); @FfiNative('PlatformConfigurationNativeApi::RegisterBackgroundIsolate') external static void __registerBackgroundIsolate(int rootIsolateId); diff --git a/lib/ui/window/platform_configuration.cc b/lib/ui/window/platform_configuration.cc index c31f035868103..e2c24e6e448c9 100644 --- a/lib/ui/window/platform_configuration.cc +++ b/lib/ui/window/platform_configuration.cc @@ -311,9 +311,7 @@ Dart_Handle PlatformConfigurationNativeApi::SendPlatformMessage( UIDartState* dart_state = UIDartState::Current(); if (!dart_state->platform_configuration()) { - return tonic::ToDart( - "Sending messages off the root isolate should happen via " - "SendPortPlatformMessage."); + return tonic::ToDart("SendPlatformMessage only works on the root isolate."); } fml::RefPtr response; diff --git a/lib/ui/window/platform_message_response_dart_port.h b/lib/ui/window/platform_message_response_dart_port.h index 5ca375437e8fa..f32806980ee48 100644 --- a/lib/ui/window/platform_message_response_dart_port.h +++ b/lib/ui/window/platform_message_response_dart_port.h @@ -11,6 +11,7 @@ namespace flutter { +/// A \ref PlatformMessageResponse that will respond over a Dart port. class PlatformMessageResponseDartPort : public PlatformMessageResponse { FML_FRIEND_MAKE_REF_COUNTED(PlatformMessageResponseDartPort); diff --git a/shell/common/shell.cc b/shell/common/shell.cc index 7253dc8544033..9bb498456bf2c 100644 --- a/shell/common/shell.cc +++ b/shell/common/shell.cc @@ -607,6 +607,7 @@ void Shell::RunEngine( FML_LOG(ERROR) << "Could not launch engine with configuration."; } + // Enable platform channels for background isolates. std::shared_ptr root_isolate = weak_engine->GetRuntimeController()->GetRootIsolate().lock(); FML_DCHECK(root_isolate); From e14b1d1bb28334322944860ee31cc970c32b2c83 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Wed, 24 Aug 2022 12:53:51 -0700 Subject: [PATCH 05/19] fixed build files --- lib/ui/BUILD.gn | 1 + shell/common/BUILD.gn | 8 +++++++- shell/common/platform_message_handler.h | 4 +++- shell/platform/darwin/ios/platform_message_handler_ios.mm | 1 + 4 files changed, 12 insertions(+), 2 deletions(-) diff --git a/lib/ui/BUILD.gn b/lib/ui/BUILD.gn index fedaaf584cc8b..36d4b1b47a2cb 100644 --- a/lib/ui/BUILD.gn +++ b/lib/ui/BUILD.gn @@ -154,6 +154,7 @@ source_set("ui") { "//flutter/impeller/runtime_stage", "//flutter/runtime:dart_plugin_registrant", "//flutter/runtime:test_font", + "//flutter/shell/common:platform_message_handler", "//flutter/third_party/tonic", "//third_party/dart/runtime/bin:dart_io_api", "//third_party/rapidjson", diff --git a/shell/common/BUILD.gn b/shell/common/BUILD.gn index 137b906dfba34..3d6b73ec587e2 100644 --- a/shell/common/BUILD.gn +++ b/shell/common/BUILD.gn @@ -59,6 +59,12 @@ template("dart_embedder_resources") { } } +source_set("platform_message_handler") { + sources = [ "platform_message_handler.h" ] + public_configs = [ "//flutter:config" ] + deps = [ "//flutter/fml:fml" ] +} + source_set("common") { sources = [ "animator.cc", @@ -75,7 +81,6 @@ source_set("common") { "engine.h", "pipeline.cc", "pipeline.h", - "platform_message_handler.h", "platform_view.cc", "platform_view.h", "pointer_data_dispatcher.cc", @@ -115,6 +120,7 @@ source_set("common") { public_configs = [ "//flutter:config" ] public_deps = [ + ":platform_message_handler", "//flutter/shell/version", "//flutter/third_party/tonic", "//flutter/third_party/txt", diff --git a/shell/common/platform_message_handler.h b/shell/common/platform_message_handler.h index 8e3f77e448236..725a341e72463 100644 --- a/shell/common/platform_message_handler.h +++ b/shell/common/platform_message_handler.h @@ -7,10 +7,12 @@ #include -#include "flutter/lib/ui/window/platform_message.h" +#include "flutter/fml/mapping.h" namespace flutter { +class PlatformMessage; + /// An interface over the ability to handle PlatformMessages that are being sent /// from Flutter to the host platform. class PlatformMessageHandler { diff --git a/shell/platform/darwin/ios/platform_message_handler_ios.mm b/shell/platform/darwin/ios/platform_message_handler_ios.mm index a3d9c75456bb9..f6c4f392d97d4 100644 --- a/shell/platform/darwin/ios/platform_message_handler_ios.mm +++ b/shell/platform/darwin/ios/platform_message_handler_ios.mm @@ -5,6 +5,7 @@ #import "flutter/shell/platform/darwin/ios/platform_message_handler_ios.h" #import "flutter/fml/trace_event.h" +#import "flutter/lib/ui/window/platform_message.h" #import "flutter/shell/platform/darwin/common/buffer_conversions.h" #import "flutter/shell/platform/darwin/common/framework/Headers/FlutterBinaryMessenger.h" From a4c9807eb0fe6c227ccfc8975276d77d4ac1cf96 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Wed, 24 Aug 2022 16:07:21 -0700 Subject: [PATCH 06/19] jasons feedback on communicating the platform message handler --- lib/ui/BUILD.gn | 2 +- runtime/runtime_controller.cc | 6 ++++++ runtime/runtime_controller.h | 2 -- runtime/runtime_delegate.h | 4 ++++ shell/common/engine.cc | 5 +++++ shell/common/engine.h | 17 ++++++++++------- shell/common/engine_unittests.cc | 4 ++++ shell/common/shell.cc | 8 -------- shell/common/shell.h | 6 ++---- 9 files changed, 32 insertions(+), 22 deletions(-) diff --git a/lib/ui/BUILD.gn b/lib/ui/BUILD.gn index 36d4b1b47a2cb..25bb688666b75 100644 --- a/lib/ui/BUILD.gn +++ b/lib/ui/BUILD.gn @@ -142,6 +142,7 @@ source_set("ui") { public_deps = [ "//flutter/flow", + "//flutter/shell/common:platform_message_handler", "//flutter/third_party/txt", ] @@ -154,7 +155,6 @@ source_set("ui") { "//flutter/impeller/runtime_stage", "//flutter/runtime:dart_plugin_registrant", "//flutter/runtime:test_font", - "//flutter/shell/common:platform_message_handler", "//flutter/third_party/tonic", "//third_party/dart/runtime/bin:dart_io_api", "//third_party/rapidjson", diff --git a/runtime/runtime_controller.cc b/runtime/runtime_controller.cc index d195f42e8e2ed..6d16fae39fb82 100644 --- a/runtime/runtime_controller.cc +++ b/runtime/runtime_controller.cc @@ -11,6 +11,7 @@ #include "flutter/lib/ui/window/platform_configuration.h" #include "flutter/lib/ui/window/viewport_metrics.h" #include "flutter/lib/ui/window/window.h" +#include "flutter/runtime/dart_isolate_group_data.h" #include "flutter/runtime/isolate_configuration.h" #include "flutter/runtime/runtime_delegate.h" #include "third_party/tonic/dart_message_handler.h" @@ -387,6 +388,11 @@ bool RuntimeController::LaunchRootIsolate( return false; } + // Enable platform channels for background isolates. + strong_root_isolate->GetIsolateGroupData().SetPlatformMessageHandler( + strong_root_isolate->GetRootIsolateId(), + client_.GetPlatformMessageHandler()); + // The root isolate ivar is weak. root_isolate_ = strong_root_isolate; diff --git a/runtime/runtime_controller.h b/runtime/runtime_controller.h index 44343231e907c..d62c8dc252013 100644 --- a/runtime/runtime_controller.h +++ b/runtime/runtime_controller.h @@ -558,8 +558,6 @@ class RuntimeController : public PlatformConfigurationClient { return context_.snapshot_delegate; } - std::weak_ptr GetRootIsolate() { return root_isolate_; } - std::weak_ptr GetRootIsolate() const { return root_isolate_; } diff --git a/runtime/runtime_delegate.h b/runtime/runtime_delegate.h index ad680677338e0..679922e29d732 100644 --- a/runtime/runtime_delegate.h +++ b/runtime/runtime_delegate.h @@ -14,6 +14,7 @@ #include "flutter/lib/ui/semantics/semantics_node.h" #include "flutter/lib/ui/text/font_collection.h" #include "flutter/lib/ui/window/platform_message.h" +#include "flutter/shell/common/platform_message_handler.h" #include "third_party/dart/runtime/include/dart_api.h" namespace flutter { @@ -49,6 +50,9 @@ class RuntimeDelegate { virtual void RequestDartDeferredLibrary(intptr_t loading_unit_id) = 0; + virtual std::weak_ptr GetPlatformMessageHandler() + const = 0; + protected: virtual ~RuntimeDelegate(); }; diff --git a/shell/common/engine.cc b/shell/common/engine.cc index c2cbc9f512b98..3eeea5ccd0a10 100644 --- a/shell/common/engine.cc +++ b/shell/common/engine.cc @@ -540,6 +540,11 @@ void Engine::RequestDartDeferredLibrary(intptr_t loading_unit_id) { return delegate_.RequestDartDeferredLibrary(loading_unit_id); } +std::weak_ptr Engine::GetPlatformMessageHandler() + const { + return delegate_.GetPlatformMessageHandler(); +} + void Engine::LoadDartDeferredLibrary( intptr_t loading_unit_id, std::unique_ptr snapshot_data, diff --git a/shell/common/engine.h b/shell/common/engine.h index d1eec04d99755..cc178c31735e8 100644 --- a/shell/common/engine.h +++ b/shell/common/engine.h @@ -288,6 +288,12 @@ class Engine final : public RuntimeDelegate, PointerDataDispatcher::Delegate { /// This method is primarily provided to allow tests to control /// Any methods that rely on advancing the clock. virtual fml::TimePoint GetCurrentTimePoint() = 0; + + //---------------------------------------------------------------------------- + /// @brief Returns the delegate object that handles PlatformMessage's from + /// Flutter to the host platform (and its responses). + virtual const std::shared_ptr& + GetPlatformMessageHandler() const = 0; }; //---------------------------------------------------------------------------- @@ -872,13 +878,6 @@ class Engine final : public RuntimeDelegate, PointerDataDispatcher::Delegate { return runtime_controller_.get(); } - //-------------------------------------------------------------------------- - /// @brief Accessor for the RuntimeController. - /// - RuntimeController* GetRuntimeController() { - return runtime_controller_.get(); - } - const std::weak_ptr GetVsyncWaiter() const; private: @@ -909,6 +908,10 @@ class Engine final : public RuntimeDelegate, PointerDataDispatcher::Delegate { // |RuntimeDelegate| void RequestDartDeferredLibrary(intptr_t loading_unit_id) override; + // |RuntimeDelegate| + std::weak_ptr GetPlatformMessageHandler() + const override; + void SetNeedsReportTimings(bool value) override; bool HandleLifecyclePlatformMessage(PlatformMessage* message); diff --git a/shell/common/engine_unittests.cc b/shell/common/engine_unittests.cc index 6dfc93c555c36..365012b84f507 100644 --- a/shell/common/engine_unittests.cc +++ b/shell/common/engine_unittests.cc @@ -36,6 +36,8 @@ class MockDelegate : public Engine::Delegate { const std::vector&)); MOCK_METHOD1(RequestDartDeferredLibrary, void(intptr_t)); MOCK_METHOD0(GetCurrentTimePoint, fml::TimePoint()); + MOCK_CONST_METHOD0(GetPlatformMessageHandler, + const std::shared_ptr&()); }; class MockResponse : public PlatformMessageResponse { @@ -61,6 +63,8 @@ class MockRuntimeDelegate : public RuntimeDelegate { std::unique_ptr>( const std::vector&)); MOCK_METHOD1(RequestDartDeferredLibrary, void(intptr_t)); + MOCK_CONST_METHOD0(GetPlatformMessageHandler, + std::weak_ptr()); }; class MockRuntimeController : public RuntimeController { diff --git a/shell/common/shell.cc b/shell/common/shell.cc index 9bb498456bf2c..f71d914978a14 100644 --- a/shell/common/shell.cc +++ b/shell/common/shell.cc @@ -20,7 +20,6 @@ #include "flutter/fml/message_loop.h" #include "flutter/fml/paths.h" #include "flutter/fml/trace_event.h" -#include "flutter/runtime/dart_isolate_group_data.h" #include "flutter/runtime/dart_vm.h" #include "flutter/shell/common/engine.h" #include "flutter/shell/common/skia_event_tracer_impl.h" @@ -607,13 +606,6 @@ void Shell::RunEngine( FML_LOG(ERROR) << "Could not launch engine with configuration."; } - // Enable platform channels for background isolates. - std::shared_ptr root_isolate = - weak_engine->GetRuntimeController()->GetRootIsolate().lock(); - FML_DCHECK(root_isolate); - root_isolate->GetIsolateGroupData().SetPlatformMessageHandler( - root_isolate->GetRootIsolateId(), platform_message_handler); - result(run_result); })); } diff --git a/shell/common/shell.h b/shell/common/shell.h index 5a8138e0ccc5b..431ae99437d23 100644 --- a/shell/common/shell.h +++ b/shell/common/shell.h @@ -397,11 +397,9 @@ class Shell final : public PlatformView::Delegate, /// @see `CreateCompatibleGenerator` void RegisterImageDecoder(ImageGeneratorFactory factory, int32_t priority); - //---------------------------------------------------------------------------- - /// @brief Returns the delegate object that handles PlatformMessage's from - /// Flutter to the host platform (and its responses). + // |Engine::Delegate| const std::shared_ptr& GetPlatformMessageHandler() - const; + const override; const std::weak_ptr GetVsyncWaiter() const; From a064bf293fd66fad6dde80dd294c0a416721b569 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 25 Aug 2022 16:02:26 -0700 Subject: [PATCH 07/19] added unit test for platform message response dart port --- ci/licenses_golden/licenses_flutter | 3 +- lib/ui/BUILD.gn | 1 + lib/ui/fixtures/ui_test.dart | 18 +++++ ...rm_message_response_dart_port_unittests.cc | 74 +++++++++++++++++++ 4 files changed, 95 insertions(+), 1 deletion(-) create mode 100644 lib/ui/window/platform_message_response_dart_port_unittests.cc diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index 285319b6df3de..0ad24986db20c 100644 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -1123,9 +1123,10 @@ FILE: ../../../flutter/lib/ui/window/platform_message_response.cc FILE: ../../../flutter/lib/ui/window/platform_message_response.h FILE: ../../../flutter/lib/ui/window/platform_message_response_dart.cc FILE: ../../../flutter/lib/ui/window/platform_message_response_dart.h -FILE: ../../../flutter/lib/ui/window/platform_message_response_dart_unittests.cc FILE: ../../../flutter/lib/ui/window/platform_message_response_dart_port.cc FILE: ../../../flutter/lib/ui/window/platform_message_response_dart_port.h +FILE: ../../../flutter/lib/ui/window/platform_message_response_dart_port_unittests.cc +FILE: ../../../flutter/lib/ui/window/platform_message_response_dart_unittests.cc FILE: ../../../flutter/lib/ui/window/pointer_data.cc FILE: ../../../flutter/lib/ui/window/pointer_data.h FILE: ../../../flutter/lib/ui/window/pointer_data_packet.cc diff --git a/lib/ui/BUILD.gn b/lib/ui/BUILD.gn index 25bb688666b75..fb2d801241927 100644 --- a/lib/ui/BUILD.gn +++ b/lib/ui/BUILD.gn @@ -233,6 +233,7 @@ if (enable_unittests) { "painting/single_frame_codec_unittests.cc", "semantics/semantics_update_builder_unittests.cc", "window/platform_configuration_unittests.cc", + "window/platform_message_response_dart_port_unittests.cc", "window/platform_message_response_dart_unittests.cc", "window/pointer_data_packet_converter_unittests.cc", ] diff --git a/lib/ui/fixtures/ui_test.dart b/lib/ui/fixtures/ui_test.dart index b04ef7be99fe4..b13cd3e3c8b6c 100644 --- a/lib/ui/fixtures/ui_test.dart +++ b/lib/ui/fixtures/ui_test.dart @@ -5,6 +5,8 @@ import 'dart:async'; import 'dart:typed_data'; import 'dart:ui'; +import 'dart:isolate' show ReceivePort, SendPort; +import 'dart:ffi'; void main() {} @@ -234,6 +236,21 @@ void frameCallback(_Image, int) { print('called back'); } +@pragma('vm:entry-point') +void platformMessagePortResponseTest() async { + ReceivePort receivePort = ReceivePort(); + _callPlatformMessageResponseDartPort(receivePort.sendPort.nativePort); + List resultList = await receivePort.first; + int identifier = resultList[0] as int; + Uint8List? bytes = resultList[1] as Uint8List?; + ByteData result = ByteData.sublistView(bytes!); + if (result.lengthInBytes == 100) { + _finishCallResponse(true); + } else { + _finishCallResponse(false); + } +} + @pragma('vm:entry-point') void platformMessageResponseTest() { _callPlatformMessageResponseDart((ByteData? result) { @@ -246,6 +263,7 @@ void platformMessageResponseTest() { }); } +void _callPlatformMessageResponseDartPort(int port) native 'CallPlatformMessageResponseDartPort'; void _callPlatformMessageResponseDart(void Function(ByteData? result) callback) native 'CallPlatformMessageResponseDart'; void _finishCallResponse(bool didPass) native 'FinishCallResponse'; diff --git a/lib/ui/window/platform_message_response_dart_port_unittests.cc b/lib/ui/window/platform_message_response_dart_port_unittests.cc new file mode 100644 index 0000000000000..c30309d6e0c09 --- /dev/null +++ b/lib/ui/window/platform_message_response_dart_port_unittests.cc @@ -0,0 +1,74 @@ +// 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/common/task_runners.h" +#include "flutter/fml/mapping.h" +#include "flutter/fml/synchronization/waitable_event.h" +#include "flutter/lib/ui/window/platform_message_response_dart_port.h" +#include "flutter/runtime/dart_vm.h" +#include "flutter/shell/common/shell_test.h" +#include "flutter/shell/common/thread_host.h" +#include "flutter/testing/testing.h" + +namespace flutter { +namespace testing { + +TEST_F(ShellTest, PlatformMessageResponseDartPort) { + bool did_pass = false; + auto message_latch = std::make_shared(); + TaskRunners task_runners("test", // label + GetCurrentTaskRunner(), // platform + CreateNewThread(), // raster + CreateNewThread(), // ui + CreateNewThread() // io + ); + + auto nativeCallPlatformMessageResponseDartPort = + [ui_task_runner = + task_runners.GetUITaskRunner()](Dart_NativeArguments args) { + auto dart_state = std::make_shared(); + auto response = fml::MakeRefCounted( + tonic::DartConverter::FromDart( + Dart_GetNativeArgument(args, 0)), + 123, "foobar"); + uint8_t* data = static_cast(malloc(100)); + auto mapping = std::make_unique(data, 100); + response->Complete(std::move(mapping)); + }; + + AddNativeCallback( + "CallPlatformMessageResponseDartPort", + CREATE_NATIVE_ENTRY(nativeCallPlatformMessageResponseDartPort)); + + auto nativeFinishCallResponse = [message_latch, + &did_pass](Dart_NativeArguments args) { + did_pass = + tonic::DartConverter::FromDart(Dart_GetNativeArgument(args, 0)); + message_latch->Signal(); + }; + + AddNativeCallback("FinishCallResponse", + CREATE_NATIVE_ENTRY(nativeFinishCallResponse)); + + Settings settings = CreateSettingsForFixture(); + + std::unique_ptr shell = + CreateShell(std::move(settings), std::move(task_runners)); + + ASSERT_TRUE(shell->IsSetup()); + auto configuration = RunConfiguration::InferFromSettings(settings); + configuration.SetEntrypoint("platformMessagePortResponseTest"); + + shell->RunEngine(std::move(configuration), [](auto result) { + ASSERT_EQ(result, Engine::RunStatus::Success); + }); + + message_latch->Wait(); + + ASSERT_TRUE(did_pass); + DestroyShell(std::move(shell), std::move(task_runners)); +} + +} // namespace testing +} // namespace flutter From d156d0ecbf632fc4bb406a39b3ee78e67553e4f5 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Fri, 26 Aug 2022 09:39:10 -0700 Subject: [PATCH 08/19] fixed analyzer error --- lib/ui/platform_dispatcher.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ui/platform_dispatcher.dart b/lib/ui/platform_dispatcher.dart index 80a9ea7f3fe4d..2811f482a9b6f 100644 --- a/lib/ui/platform_dispatcher.dart +++ b/lib/ui/platform_dispatcher.dart @@ -82,7 +82,7 @@ class RootIsolateToken { /// The token for the root isolate that is executing this Dart code. If this /// Dart code is not executing on a root isolate [instance] will be null. late final RootIsolateToken? instance = () { - int rootIsolateId = __getRootIsolateId(); + final int rootIsolateId = __getRootIsolateId(); return rootIsolateId == 0 ? null : RootIsolateToken._(rootIsolateId); }(); From a458d8de56a86467d0e14257c1349e754eb11dfa Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Fri, 26 Aug 2022 10:10:37 -0700 Subject: [PATCH 09/19] added a root isolate token test --- lib/ui/fixtures/ui_test.dart | 18 +++++++++++++++++- lib/ui/platform_dispatcher.dart | 2 +- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/lib/ui/fixtures/ui_test.dart b/lib/ui/fixtures/ui_test.dart index b13cd3e3c8b6c..b113f62a5cd2c 100644 --- a/lib/ui/fixtures/ui_test.dart +++ b/lib/ui/fixtures/ui_test.dart @@ -5,7 +5,7 @@ import 'dart:async'; import 'dart:typed_data'; import 'dart:ui'; -import 'dart:isolate' show ReceivePort, SendPort; +import 'dart:isolate'; import 'dart:ffi'; void main() {} @@ -902,9 +902,25 @@ void hooksTests() { expectEquals(result, true); }); + test('root isolate token', () async { + if (RootIsolateToken.instance == null) { + throw Exception('We should have a token on a root isolate.'); + } + ReceivePort receivePort = ReceivePort(); + Isolate.spawn(_backgroundRootIsolateTestMain, receivePort.sendPort); + bool didPass = await receivePort.first as bool; + if (!didPass) { + throw Exception('Background isolate found a root isolate id.'); + } + }); + _finish(); } +void _backgroundRootIsolateTestMain(SendPort port) { + port.send(RootIsolateToken.instance == null); +} + typedef _Callback = void Function(T result); typedef _Callbacker = String? Function(_Callback callback); diff --git a/lib/ui/platform_dispatcher.dart b/lib/ui/platform_dispatcher.dart index 2811f482a9b6f..bdd20bc8d02c1 100644 --- a/lib/ui/platform_dispatcher.dart +++ b/lib/ui/platform_dispatcher.dart @@ -81,7 +81,7 @@ class RootIsolateToken { /// The token for the root isolate that is executing this Dart code. If this /// Dart code is not executing on a root isolate [instance] will be null. - late final RootIsolateToken? instance = () { + static late final RootIsolateToken? instance = () { final int rootIsolateId = __getRootIsolateId(); return rootIsolateId == 0 ? null : RootIsolateToken._(rootIsolateId); }(); From d2a4429de37b3e1e36dd65855d78859a269a5a90 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Fri, 26 Aug 2022 10:29:34 -0700 Subject: [PATCH 10/19] added todo --- lib/ui/window/platform_message_response_dart_port.cc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/ui/window/platform_message_response_dart_port.cc b/lib/ui/window/platform_message_response_dart_port.cc index 17801733ca9b4..0cf96a045a529 100644 --- a/lib/ui/window/platform_message_response_dart_port.cc +++ b/lib/ui/window/platform_message_response_dart_port.cc @@ -40,6 +40,9 @@ void PlatformMessageResponseDartPort::Complete( Dart_CObject response_data = { .type = Dart_CObject_kExternalTypedData, }; + // TODO(https://github.com/dart-lang/sdk/issues/49827): Move to kTypedData + // when const values are accepted. Also consider using kExternalTypedData only + // when the payload is >= 1KB. uint8_t* copy = static_cast(malloc(data->GetSize())); memcpy(copy, data->GetMapping(), data->GetSize()); response_data.value.as_external_typed_data.type = Dart_TypedData_kUint8; From cf86d36f8ac473f206b60d7249d241c59fae2cb2 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Fri, 26 Aug 2022 11:22:12 -0700 Subject: [PATCH 11/19] turned everything off for other platforms --- lib/ui/platform_dispatcher.dart | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/lib/ui/platform_dispatcher.dart b/lib/ui/platform_dispatcher.dart index bdd20bc8d02c1..0be0332d6f3ba 100644 --- a/lib/ui/platform_dispatcher.dart +++ b/lib/ui/platform_dispatcher.dart @@ -81,7 +81,7 @@ class RootIsolateToken { /// The token for the root isolate that is executing this Dart code. If this /// Dart code is not executing on a root isolate [instance] will be null. - static late final RootIsolateToken? instance = () { + static final RootIsolateToken? instance = () { final int rootIsolateId = __getRootIsolateId(); return rootIsolateId == 0 ? null : RootIsolateToken._(rootIsolateId); }(); @@ -583,7 +583,13 @@ class PlatformDispatcher { /// Registers the current isolate with the isolate identified with by the /// [token]. This is required if platform channels are to be used on a /// background isolate. - void registerBackgroundIsolate(RootIsolateToken token) => __registerBackgroundIsolate(token._rootIsolateId); + void registerBackgroundIsolate(RootIsolateToken token) { + if (!Platform.isIOS) { + // Issue: https://github.com/flutter/flutter/issues/13937 + throw UnimplementedError('Platform doesn\'t yet support platform channels on background isolates.'); + } + __registerBackgroundIsolate(token._rootIsolateId); + } @FfiNative('PlatformConfigurationNativeApi::RegisterBackgroundIsolate') external static void __registerBackgroundIsolate(int rootIsolateId); From c88803536863ade5a9247e59778356e0786c64cb Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Fri, 26 Aug 2022 11:53:28 -0700 Subject: [PATCH 12/19] linter --- lib/ui/platform_dispatcher.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ui/platform_dispatcher.dart b/lib/ui/platform_dispatcher.dart index 0be0332d6f3ba..5d332bf2f0de9 100644 --- a/lib/ui/platform_dispatcher.dart +++ b/lib/ui/platform_dispatcher.dart @@ -586,7 +586,7 @@ class PlatformDispatcher { void registerBackgroundIsolate(RootIsolateToken token) { if (!Platform.isIOS) { // Issue: https://github.com/flutter/flutter/issues/13937 - throw UnimplementedError('Platform doesn\'t yet support platform channels on background isolates.'); + throw UnimplementedError("Platform doesn't yet support platform channels on background isolates."); } __registerBackgroundIsolate(token._rootIsolateId); } From fa2be9d1b77648e82b735247f97f0b6783cff280 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Fri, 26 Aug 2022 14:58:58 -0700 Subject: [PATCH 13/19] jason feedback --- lib/ui/ui_dart_state.cc | 3 +++ lib/ui/window/platform_message_response_dart_port.cc | 8 +++++--- lib/web_ui/lib/platform_dispatcher.dart | 12 +++++++++--- lib/web_ui/lib/src/engine/platform_dispatcher.dart | 7 +------ shell/common/shell.cc | 3 +-- 5 files changed, 19 insertions(+), 14 deletions(-) diff --git a/lib/ui/ui_dart_state.cc b/lib/ui/ui_dart_state.cc index 76e46708ac205..b72911ba9da05 100644 --- a/lib/ui/ui_dart_state.cc +++ b/lib/ui/ui_dart_state.cc @@ -231,6 +231,9 @@ void UIDartState::HandlePlatformMessage( platform_message_handler_.lock(); if (handler) { handler->HandlePlatformMessage(std::move(message)); + } else { + FML_DLOG(WARNING) << "Dropping background isolate platform message on " + << message->channel(); } } } diff --git a/lib/ui/window/platform_message_response_dart_port.cc b/lib/ui/window/platform_message_response_dart_port.cc index 0cf96a045a529..ffa4a0d79cc74 100644 --- a/lib/ui/window/platform_message_response_dart_port.cc +++ b/lib/ui/window/platform_message_response_dart_port.cc @@ -4,6 +4,7 @@ #include "flutter/lib/ui/window/platform_message_response_dart_port.h" +#include #include #include "flutter/common/task_runners.h" @@ -51,13 +52,14 @@ void PlatformMessageResponseDartPort::Complete( response_data.value.as_external_typed_data.peer = copy; response_data.value.as_external_typed_data.callback = FreeFinalizer; - Dart_CObject* response_values[2] = {&response_identifier, &response_data}; + std::array response_values = {&response_identifier, + &response_data}; Dart_CObject response = { .type = Dart_CObject_kArray, }; - response.value.as_array.length = 2; - response.value.as_array.values = response_values; + response.value.as_array.length = response_values.size(); + response.value.as_array.values = response_values.data(); bool did_send = Dart_PostCObject(send_port_, &response); FML_CHECK(did_send); diff --git a/lib/web_ui/lib/platform_dispatcher.dart b/lib/web_ui/lib/platform_dispatcher.dart index cf81b34c01965..7c67e7c3d5f4f 100644 --- a/lib/web_ui/lib/platform_dispatcher.dart +++ b/lib/web_ui/lib/platform_dispatcher.dart @@ -16,6 +16,14 @@ typedef PlatformMessageCallback = void Function( typedef PlatformConfigurationChangedCallback = void Function(PlatformConfiguration configuration); typedef ErrorCallback = bool Function(Object exception, StackTrace stackTrace); +// ignore: avoid_classes_with_only_static_members +/// A token that represents a root isolate. +class RootIsolateToken { + static RootIsolateToken? get instance { + throw UnsupportedError('Root isolate not identifiable on web.'); + } +} + abstract class PlatformDispatcher { static PlatformDispatcher get instance => engine.EnginePlatformDispatcher.instance; @@ -55,9 +63,7 @@ abstract class PlatformDispatcher { int identifier, Object port); - int registerRootIsolate(); - - void registerBackgroundIsolate(int rootIsolateId); + void registerBackgroundIsolate(RootIsolateToken token); PlatformMessageCallback? get onPlatformMessage; set onPlatformMessage(PlatformMessageCallback? callback); diff --git a/lib/web_ui/lib/src/engine/platform_dispatcher.dart b/lib/web_ui/lib/src/engine/platform_dispatcher.dart index 288e541acd4f8..1cb26a9e0bc5f 100644 --- a/lib/web_ui/lib/src/engine/platform_dispatcher.dart +++ b/lib/web_ui/lib/src/engine/platform_dispatcher.dart @@ -354,12 +354,7 @@ class EnginePlatformDispatcher extends ui.PlatformDispatcher { } @override - int registerRootIsolate() { - throw Exception("Isolates aren't supported in web."); - } - - @override - void registerBackgroundIsolate(int rootIsolateId) { + void registerBackgroundIsolate(RootIsolateToken token) { throw Exception("Isolates aren't supported in web."); } diff --git a/shell/common/shell.cc b/shell/common/shell.cc index f71d914978a14..72f4e52301418 100644 --- a/shell/common/shell.cc +++ b/shell/common/shell.cc @@ -593,8 +593,7 @@ void Shell::RunEngine( task_runners_.GetUITaskRunner(), fml::MakeCopyable( [run_configuration = std::move(run_configuration), - weak_engine = weak_engine_, result, - platform_message_handler = platform_message_handler_]() mutable { + weak_engine = weak_engine_, result]() mutable { if (!weak_engine) { FML_LOG(ERROR) << "Could not launch engine with configuration - no engine."; From ea073673eddddd79b9d4716d8e05a43bc3d0685f Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Fri, 26 Aug 2022 15:55:28 -0700 Subject: [PATCH 14/19] jason feedback (error send without registering) --- lib/ui/fixtures/ui_test.dart | 25 +++++++++++++++++++ lib/ui/ui_dart_state.cc | 8 +++--- lib/ui/ui_dart_state.h | 2 +- lib/ui/window/platform_configuration.cc | 9 +++---- .../lib/src/engine/platform_dispatcher.dart | 2 +- 5 files changed, 36 insertions(+), 10 deletions(-) diff --git a/lib/ui/fixtures/ui_test.dart b/lib/ui/fixtures/ui_test.dart index b113f62a5cd2c..fadf6db6a6fe4 100644 --- a/lib/ui/fixtures/ui_test.dart +++ b/lib/ui/fixtures/ui_test.dart @@ -914,6 +914,15 @@ void hooksTests() { } }); + test('send port message without registering', () async { + ReceivePort receivePort = ReceivePort(); + Isolate.spawn(_backgroundIsolateSendWithoutRegistering, receivePort.sendPort); + bool didError = await receivePort.first as bool; + if (!didError) { + throw Exception('Expected an error when not registering a root isolate and sending port messages.'); + } + }); + _finish(); } @@ -921,6 +930,22 @@ void _backgroundRootIsolateTestMain(SendPort port) { port.send(RootIsolateToken.instance == null); } +void _backgroundIsolateSendWithoutRegistering(SendPort port) { + bool didError = false; + ReceivePort messagePort = ReceivePort(); + try { + PlatformDispatcher.instance.sendPortPlatformMessage( + 'foo', + null, + 1, + messagePort.sendPort, + ); + } catch (_) { + didError = true; + } + port.send(didError); +} + typedef _Callback = void Function(T result); typedef _Callbacker = String? Function(_Callback callback); diff --git a/lib/ui/ui_dart_state.cc b/lib/ui/ui_dart_state.cc index b72911ba9da05..c3c344cb92193 100644 --- a/lib/ui/ui_dart_state.cc +++ b/lib/ui/ui_dart_state.cc @@ -221,7 +221,7 @@ bool UIDartState::enable_skparagraph() const { return enable_skparagraph_; } -void UIDartState::HandlePlatformMessage( +Dart_Handle UIDartState::HandlePlatformMessage( std::unique_ptr message) { if (platform_configuration_) { platform_configuration_->client()->HandlePlatformMessage( @@ -232,10 +232,12 @@ void UIDartState::HandlePlatformMessage( if (handler) { handler->HandlePlatformMessage(std::move(message)); } else { - FML_DLOG(WARNING) << "Dropping background isolate platform message on " - << message->channel(); + return tonic::ToDart( + "No platform channel handler registered for background isolate."); } } + + return Dart_Null(); } int64_t UIDartState::GetRootIsolateId() const { diff --git a/lib/ui/ui_dart_state.h b/lib/ui/ui_dart_state.h index 618645330b1b1..db3242bb36bb9 100644 --- a/lib/ui/ui_dart_state.h +++ b/lib/ui/ui_dart_state.h @@ -110,7 +110,7 @@ class UIDartState : public tonic::DartState { void SetPlatformMessageHandler(std::weak_ptr handler); - void HandlePlatformMessage(std::unique_ptr message); + Dart_Handle HandlePlatformMessage(std::unique_ptr message); const TaskRunners& GetTaskRunners() const; diff --git a/lib/ui/window/platform_configuration.cc b/lib/ui/window/platform_configuration.cc index e2c24e6e448c9..c6a1c0db3905b 100644 --- a/lib/ui/window/platform_configuration.cc +++ b/lib/ui/window/platform_configuration.cc @@ -286,18 +286,18 @@ void PlatformConfigurationNativeApi::SetNeedsReportTimings(bool value) { } namespace { -void HandlePlatformMessage( +Dart_Handle HandlePlatformMessage( UIDartState* dart_state, const std::string& name, Dart_Handle data_handle, const fml::RefPtr& response) { if (Dart_IsNull(data_handle)) { - dart_state->HandlePlatformMessage( + return dart_state->HandlePlatformMessage( std::make_unique(name, response)); } else { tonic::DartByteData data(data_handle); const uint8_t* buffer = static_cast(data.data()); - dart_state->HandlePlatformMessage(std::make_unique( + return dart_state->HandlePlatformMessage(std::make_unique( name, fml::MallocMapping::Copy(buffer, data.length_in_bytes()), response)); } @@ -320,9 +320,8 @@ Dart_Handle PlatformConfigurationNativeApi::SendPlatformMessage( tonic::DartPersistentValue(dart_state, callback), dart_state->GetTaskRunners().GetUITaskRunner(), name); } - HandlePlatformMessage(dart_state, name, data_handle, response); - return Dart_Null(); + return HandlePlatformMessage(dart_state, name, data_handle, response); } Dart_Handle PlatformConfigurationNativeApi::SendPortPlatformMessage( diff --git a/lib/web_ui/lib/src/engine/platform_dispatcher.dart b/lib/web_ui/lib/src/engine/platform_dispatcher.dart index 1cb26a9e0bc5f..5f59713698149 100644 --- a/lib/web_ui/lib/src/engine/platform_dispatcher.dart +++ b/lib/web_ui/lib/src/engine/platform_dispatcher.dart @@ -354,7 +354,7 @@ class EnginePlatformDispatcher extends ui.PlatformDispatcher { } @override - void registerBackgroundIsolate(RootIsolateToken token) { + void registerBackgroundIsolate(ui.RootIsolateToken token) { throw Exception("Isolates aren't supported in web."); } From 042114b3a8b234ebb7516cc8f71a3a311d34301e Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Mon, 29 Aug 2022 15:55:07 -0700 Subject: [PATCH 15/19] fixed new imports --- shell/platform/darwin/ios/BUILD.gn | 1 + shell/platform/darwin/ios/platform_message_handler_ios_test.mm | 2 ++ 2 files changed, 3 insertions(+) diff --git a/shell/platform/darwin/ios/BUILD.gn b/shell/platform/darwin/ios/BUILD.gn index e3a8339e22033..27fc1f7f4e196 100644 --- a/shell/platform/darwin/ios/BUILD.gn +++ b/shell/platform/darwin/ios/BUILD.gn @@ -210,6 +210,7 @@ source_set("ios_test_flutter_mrc") { deps = [ ":flutter_framework_source", "//flutter/common:common", + "//flutter/lib/ui:ui", "//flutter/shell/common:common", "//flutter/shell/platform/darwin/common:framework_shared", "//flutter/shell/platform/embedder:embedder_as_internal_library", diff --git a/shell/platform/darwin/ios/platform_message_handler_ios_test.mm b/shell/platform/darwin/ios/platform_message_handler_ios_test.mm index 376162ead5980..7249722c80624 100644 --- a/shell/platform/darwin/ios/platform_message_handler_ios_test.mm +++ b/shell/platform/darwin/ios/platform_message_handler_ios_test.mm @@ -9,6 +9,8 @@ #import "flutter/common/task_runners.h" #import "flutter/fml/message_loop.h" #import "flutter/fml/thread.h" +#import "flutter/lib/ui/window/platform_message.h" +#import "flutter/lib/ui/window/platform_message_response.h" #import "flutter/shell/common/thread_host.h" #import "flutter/shell/platform/darwin/common/framework/Headers/FlutterMacros.h" From 046fb081dd09a73b6b326792ca9d8b74225fe9d4 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Wed, 7 Sep 2022 10:27:33 -0700 Subject: [PATCH 16/19] dan feedback --- lib/ui/fixtures/ui_test.dart | 5 +++++ lib/ui/platform_dispatcher.dart | 9 +++++---- lib/ui/window/platform_configuration.cc | 13 ++++++++----- 3 files changed, 18 insertions(+), 9 deletions(-) diff --git a/lib/ui/fixtures/ui_test.dart b/lib/ui/fixtures/ui_test.dart index fadf6db6a6fe4..6ccd2388ce877 100644 --- a/lib/ui/fixtures/ui_test.dart +++ b/lib/ui/fixtures/ui_test.dart @@ -926,10 +926,15 @@ void hooksTests() { _finish(); } +/// Sends `true` on [port] if the isolate executing the function is not a root +/// isolate. void _backgroundRootIsolateTestMain(SendPort port) { port.send(RootIsolateToken.instance == null); } +/// Sends `true` on [port] if [PlatformDispatcher.sendPortPlatformMessage] +/// throws an exception without calling +/// [PlatformDispatcher.registerBackgroundIsolate]. void _backgroundIsolateSendWithoutRegistering(SendPort port) { bool didError = false; ReceivePort messagePort = ReceivePort(); diff --git a/lib/ui/platform_dispatcher.dart b/lib/ui/platform_dispatcher.dart index 5d332bf2f0de9..74f7a37412d26 100644 --- a/lib/ui/platform_dispatcher.dart +++ b/lib/ui/platform_dispatcher.dart @@ -558,10 +558,11 @@ class PlatformDispatcher { /// Sends a message to a platform-specific plugin via a [SendPort]. /// /// This operates similarly to [sendPlatformMessage] but is used when sending - /// messages from background isolates. [port] allows Flutter to know which - /// isolate to send the result to. [name] is the name of the channel - /// communication will happen on. [data] is the payload of the message. - /// [identifier] is a unique integer assigned to the message. + /// messages from background isolates. The [port] parameter allows Flutter to + /// know which isolate to send the result to. The [name] parameter is the name + /// of the channel communication will happen on. The [data] parameter is the + /// payload of the message. The [identifier] parameter is a unique integer + /// assigned to the message. void sendPortPlatformMessage( String name, ByteData? data, diff --git a/lib/ui/window/platform_configuration.cc b/lib/ui/window/platform_configuration.cc index c6a1c0db3905b..895a567d4476a 100644 --- a/lib/ui/window/platform_configuration.cc +++ b/lib/ui/window/platform_configuration.cc @@ -311,7 +311,9 @@ Dart_Handle PlatformConfigurationNativeApi::SendPlatformMessage( UIDartState* dart_state = UIDartState::Current(); if (!dart_state->platform_configuration()) { - return tonic::ToDart("SendPlatformMessage only works on the root isolate."); + return tonic::ToDart( + "SendPlatformMessage only works on the root isolate, see " + "SendPortPlatformMessage."); } fml::RefPtr response; @@ -437,11 +439,12 @@ void PlatformConfigurationNativeApi::RegisterBackgroundIsolate( int64_t isolate_id) { UIDartState* dart_state = UIDartState::Current(); FML_DCHECK(dart_state && !dart_state->IsRootIsolate()); - auto weak_platform_message_handler = + auto platform_message_handler = (*static_cast*>( - Dart_CurrentIsolateGroupData())) - ->GetPlatformMessageHandler(isolate_id); - FML_DCHECK(weak_platform_message_handler.lock()); + Dart_CurrentIsolateGroupData())); + FML_DCHECK(platform_message_handler); + auto weak_platform_message_handler = + platform_message_handler->GetPlatformMessageHandler(isolate_id); dart_state->SetPlatformMessageHandler(weak_platform_message_handler); } From 8b3d8904502cad6d787abd5d4ec759a56a903097 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Wed, 7 Sep 2022 11:03:22 -0700 Subject: [PATCH 17/19] dan feedback - renamed GetRootIsolateId --- lib/ui/dart_ui.cc | 2 +- lib/ui/platform_dispatcher.dart | 14 +++++++------- lib/ui/ui_dart_state.cc | 2 +- lib/ui/ui_dart_state.h | 4 +++- lib/ui/window/platform_configuration.cc | 4 ++-- lib/ui/window/platform_configuration.h | 2 +- runtime/runtime_controller.cc | 2 +- 7 files changed, 16 insertions(+), 14 deletions(-) diff --git a/lib/ui/dart_ui.cc b/lib/ui/dart_ui.cc index 8f33cbd70ec7d..7a1eea6bff344 100644 --- a/lib/ui/dart_ui.cc +++ b/lib/ui/dart_ui.cc @@ -100,7 +100,7 @@ typedef CanvasPath Path; V(PlatformConfigurationNativeApi::ComputePlatformResolvedLocale, 1) \ V(PlatformConfigurationNativeApi::SendPlatformMessage, 3) \ V(PlatformConfigurationNativeApi::RespondToPlatformMessage, 2) \ - V(PlatformConfigurationNativeApi::GetRootIsolateId, 0) \ + V(PlatformConfigurationNativeApi::GetRootIsolateToken, 0) \ V(PlatformConfigurationNativeApi::RegisterBackgroundIsolate, 1) \ V(PlatformConfigurationNativeApi::SendPortPlatformMessage, 4) \ V(DartRuntimeHooks::Logger_PrintDebugString, 1) \ diff --git a/lib/ui/platform_dispatcher.dart b/lib/ui/platform_dispatcher.dart index 74f7a37412d26..7af64131c2613 100644 --- a/lib/ui/platform_dispatcher.dart +++ b/lib/ui/platform_dispatcher.dart @@ -75,19 +75,19 @@ ByteData? _wrapUnmodifiableByteData(ByteData? byteData) => /// A token that represents a root isolate. class RootIsolateToken { - RootIsolateToken._(this._rootIsolateId); + RootIsolateToken._(this._token); - final int _rootIsolateId; + final int _token; /// The token for the root isolate that is executing this Dart code. If this /// Dart code is not executing on a root isolate [instance] will be null. static final RootIsolateToken? instance = () { - final int rootIsolateId = __getRootIsolateId(); - return rootIsolateId == 0 ? null : RootIsolateToken._(rootIsolateId); + final int token = __getRootIsolateToken(); + return token == 0 ? null : RootIsolateToken._(token); }(); - @FfiNative('PlatformConfigurationNativeApi::GetRootIsolateId') - external static int __getRootIsolateId(); + @FfiNative('PlatformConfigurationNativeApi::GetRootIsolateToken') + external static int __getRootIsolateToken(); } /// Platform event dispatcher singleton. @@ -589,7 +589,7 @@ class PlatformDispatcher { // Issue: https://github.com/flutter/flutter/issues/13937 throw UnimplementedError("Platform doesn't yet support platform channels on background isolates."); } - __registerBackgroundIsolate(token._rootIsolateId); + __registerBackgroundIsolate(token._token); } @FfiNative('PlatformConfigurationNativeApi::RegisterBackgroundIsolate') external static void __registerBackgroundIsolate(int rootIsolateId); diff --git a/lib/ui/ui_dart_state.cc b/lib/ui/ui_dart_state.cc index c3c344cb92193..208951c3238b6 100644 --- a/lib/ui/ui_dart_state.cc +++ b/lib/ui/ui_dart_state.cc @@ -240,7 +240,7 @@ Dart_Handle UIDartState::HandlePlatformMessage( return Dart_Null(); } -int64_t UIDartState::GetRootIsolateId() const { +int64_t UIDartState::GetRootIsolateToken() const { return IsRootIsolate() ? reinterpret_cast(this) : 0; } diff --git a/lib/ui/ui_dart_state.h b/lib/ui/ui_dart_state.h index db3242bb36bb9..95ccd9b3e43f4 100644 --- a/lib/ui/ui_dart_state.h +++ b/lib/ui/ui_dart_state.h @@ -159,7 +159,9 @@ class UIDartState : public tonic::DartState { return unhandled_exception_callback_; } - int64_t GetRootIsolateId() const; + /// Returns a enumeration that that uniquely represents this root isolate. + /// Returns `0` if called from a non-root isolate. + int64_t GetRootIsolateToken() const; protected: UIDartState(TaskObserverAdd add_callback, diff --git a/lib/ui/window/platform_configuration.cc b/lib/ui/window/platform_configuration.cc index 895a567d4476a..6704dbe44996a 100644 --- a/lib/ui/window/platform_configuration.cc +++ b/lib/ui/window/platform_configuration.cc @@ -429,10 +429,10 @@ std::string PlatformConfigurationNativeApi::DefaultRouteName() { ->DefaultRouteName(); } -int64_t PlatformConfigurationNativeApi::GetRootIsolateId() { +int64_t PlatformConfigurationNativeApi::GetRootIsolateToken() { UIDartState* dart_state = UIDartState::Current(); FML_DCHECK(dart_state); - return dart_state->GetRootIsolateId(); + return dart_state->GetRootIsolateToken(); } void PlatformConfigurationNativeApi::RegisterBackgroundIsolate( diff --git a/lib/ui/window/platform_configuration.h b/lib/ui/window/platform_configuration.h index 9ce0f7e46d845..dcf82d11f1013 100644 --- a/lib/ui/window/platform_configuration.h +++ b/lib/ui/window/platform_configuration.h @@ -515,7 +515,7 @@ class PlatformConfigurationNativeApi { /// static int RequestDartPerformanceMode(int mode); - static int64_t GetRootIsolateId(); + static int64_t GetRootIsolateToken(); static void RegisterBackgroundIsolate(int64_t isolate_id); }; diff --git a/runtime/runtime_controller.cc b/runtime/runtime_controller.cc index 6d16fae39fb82..292b32ade2342 100644 --- a/runtime/runtime_controller.cc +++ b/runtime/runtime_controller.cc @@ -390,7 +390,7 @@ bool RuntimeController::LaunchRootIsolate( // Enable platform channels for background isolates. strong_root_isolate->GetIsolateGroupData().SetPlatformMessageHandler( - strong_root_isolate->GetRootIsolateId(), + strong_root_isolate->GetRootIsolateToken(), client_.GetPlatformMessageHandler()); // The root isolate ivar is weak. From 9cc856763c24e9e5e6d7fde7d8adc14c83387121 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Wed, 7 Sep 2022 11:31:50 -0700 Subject: [PATCH 18/19] dan feedback --- lib/ui/platform_dispatcher.dart | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/ui/platform_dispatcher.dart b/lib/ui/platform_dispatcher.dart index 7af64131c2613..a24b2f60f8318 100644 --- a/lib/ui/platform_dispatcher.dart +++ b/lib/ui/platform_dispatcher.dart @@ -77,6 +77,7 @@ ByteData? _wrapUnmodifiableByteData(ByteData? byteData) => class RootIsolateToken { RootIsolateToken._(this._token); + /// An enumeration representing the root isolate (0 if not a root isolate). final int _token; /// The token for the root isolate that is executing this Dart code. If this From 9acbe3be61af3fd846687640459e3b2dccbdfc09 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Wed, 7 Sep 2022 15:29:10 -0700 Subject: [PATCH 19/19] removed `isolate_id` from variable names --- lib/ui/window/platform_configuration.cc | 4 ++-- lib/ui/window/platform_configuration.h | 6 +++--- runtime/dart_isolate_group_data.cc | 9 +++++---- runtime/dart_isolate_group_data.h | 4 ++-- 4 files changed, 12 insertions(+), 11 deletions(-) diff --git a/lib/ui/window/platform_configuration.cc b/lib/ui/window/platform_configuration.cc index 6704dbe44996a..0b61621ea9c17 100644 --- a/lib/ui/window/platform_configuration.cc +++ b/lib/ui/window/platform_configuration.cc @@ -436,7 +436,7 @@ int64_t PlatformConfigurationNativeApi::GetRootIsolateToken() { } void PlatformConfigurationNativeApi::RegisterBackgroundIsolate( - int64_t isolate_id) { + int64_t root_isolate_token) { UIDartState* dart_state = UIDartState::Current(); FML_DCHECK(dart_state && !dart_state->IsRootIsolate()); auto platform_message_handler = @@ -444,7 +444,7 @@ void PlatformConfigurationNativeApi::RegisterBackgroundIsolate( Dart_CurrentIsolateGroupData())); FML_DCHECK(platform_message_handler); auto weak_platform_message_handler = - platform_message_handler->GetPlatformMessageHandler(isolate_id); + platform_message_handler->GetPlatformMessageHandler(root_isolate_token); dart_state->SetPlatformMessageHandler(weak_platform_message_handler); } diff --git a/lib/ui/window/platform_configuration.h b/lib/ui/window/platform_configuration.h index dcf82d11f1013..d482c756984d3 100644 --- a/lib/ui/window/platform_configuration.h +++ b/lib/ui/window/platform_configuration.h @@ -450,11 +450,11 @@ class PlatformMessageHandlerStorage { public: virtual ~PlatformMessageHandlerStorage() = default; virtual void SetPlatformMessageHandler( - int64_t root_isolate_id, + int64_t root_isolate_token, std::weak_ptr handler) = 0; virtual std::weak_ptr GetPlatformMessageHandler( - int64_t root_isolate_id) const = 0; + int64_t root_isolate_token) const = 0; }; //---------------------------------------------------------------------------- @@ -517,7 +517,7 @@ class PlatformConfigurationNativeApi { static int64_t GetRootIsolateToken(); - static void RegisterBackgroundIsolate(int64_t isolate_id); + static void RegisterBackgroundIsolate(int64_t root_isolate_token); }; } // namespace flutter diff --git a/runtime/dart_isolate_group_data.cc b/runtime/dart_isolate_group_data.cc index 2c77ff1859c04..3aeff009b855e 100644 --- a/runtime/dart_isolate_group_data.cc +++ b/runtime/dart_isolate_group_data.cc @@ -65,16 +65,17 @@ void DartIsolateGroupData::SetChildIsolatePreparer( } void DartIsolateGroupData::SetPlatformMessageHandler( - int64_t root_isolate_id, + int64_t root_isolate_token, std::weak_ptr handler) { std::scoped_lock lock(platform_message_handlers_mutex_); - platform_message_handlers_[root_isolate_id] = handler; + platform_message_handlers_[root_isolate_token] = handler; } std::weak_ptr -DartIsolateGroupData::GetPlatformMessageHandler(int64_t root_isolate_id) const { +DartIsolateGroupData::GetPlatformMessageHandler( + int64_t root_isolate_token) const { std::scoped_lock lock(platform_message_handlers_mutex_); - auto it = platform_message_handlers_.find(root_isolate_id); + auto it = platform_message_handlers_.find(root_isolate_token); return it == platform_message_handlers_.end() ? std::weak_ptr() : it->second; diff --git a/runtime/dart_isolate_group_data.h b/runtime/dart_isolate_group_data.h index 2aacf6cf5ed19..8faef1abf8941 100644 --- a/runtime/dart_isolate_group_data.h +++ b/runtime/dart_isolate_group_data.h @@ -58,12 +58,12 @@ class DartIsolateGroupData : public PlatformMessageHandlerStorage { // |PlatformMessageHandlerStorage| void SetPlatformMessageHandler( - int64_t root_isolate_id, + int64_t root_isolate_token, std::weak_ptr handler) override; // |PlatformMessageHandlerStorage| std::weak_ptr GetPlatformMessageHandler( - int64_t root_isolate_id) const override; + int64_t root_isolate_token) const override; private: const Settings settings_;