From 5d01db9ce8b668c4a39b8733df77f1a4846d223c Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Wed, 21 Apr 2021 16:43:17 -0700 Subject: [PATCH 1/5] Removed superfluous copy operations in iOS flutter platform messages. --- .../darwin/common/buffer_conversions.h | 8 +--- .../darwin/common/buffer_conversions.mm | 14 ++---- .../ios/framework/Source/FlutterEngine.mm | 2 +- .../platform_message_response_darwin.mm | 44 +++++++++++++++++- .../Source/platform_message_router.mm | 46 ++++++++++++++++++- 5 files changed, 93 insertions(+), 21 deletions(-) diff --git a/shell/platform/darwin/common/buffer_conversions.h b/shell/platform/darwin/common/buffer_conversions.h index 60559b2dfe230..0e383bc37ac15 100644 --- a/shell/platform/darwin/common/buffer_conversions.h +++ b/shell/platform/darwin/common/buffer_conversions.h @@ -13,13 +13,9 @@ namespace flutter { -std::vector GetVectorFromNSData(NSData* data); +std::vector CopyNSDataToVector(NSData* data); -NSData* GetNSDataFromVector(const std::vector& buffer); - -std::unique_ptr GetMappingFromNSData(NSData* data); - -NSData* GetNSDataFromMapping(std::unique_ptr mapping); +std::unique_ptr CopyNSDataToMapping(NSData* data); } // namespace flutter diff --git a/shell/platform/darwin/common/buffer_conversions.mm b/shell/platform/darwin/common/buffer_conversions.mm index 69e81de46592b..f39fb88e03a2b 100644 --- a/shell/platform/darwin/common/buffer_conversions.mm +++ b/shell/platform/darwin/common/buffer_conversions.mm @@ -6,21 +6,13 @@ namespace flutter { -std::vector GetVectorFromNSData(NSData* data) { +std::vector CopyNSDataToVector(NSData* data) { const uint8_t* bytes = reinterpret_cast(data.bytes); return std::vector(bytes, bytes + data.length); } -NSData* GetNSDataFromVector(const std::vector& buffer) { - return [NSData dataWithBytes:buffer.data() length:buffer.size()]; -} - -std::unique_ptr GetMappingFromNSData(NSData* data) { - return std::make_unique(GetVectorFromNSData(data)); -} - -NSData* GetNSDataFromMapping(std::unique_ptr mapping) { - return [NSData dataWithBytes:mapping->GetMapping() length:mapping->GetSize()]; +std::unique_ptr CopyNSDataToMapping(NSData* data) { + return std::make_unique(CopyNSDataToVector(data)); } } // namespace flutter diff --git a/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm b/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm index d5e3564ecffca..81ee1f3754f57 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm @@ -807,7 +807,7 @@ - (void)sendOnChannel:(NSString*)channel fml::RefPtr platformMessage = (message == nil) ? fml::MakeRefCounted(channel.UTF8String, response) : fml::MakeRefCounted( - channel.UTF8String, flutter::GetVectorFromNSData(message), response); + channel.UTF8String, flutter::CopyNSDataToVector(message), response); _shell->GetPlatformView()->DispatchPlatformMessage(platformMessage); } diff --git a/shell/platform/darwin/ios/framework/Source/platform_message_response_darwin.mm b/shell/platform/darwin/ios/framework/Source/platform_message_response_darwin.mm index e7e09855c4625..e1aa651119ea8 100644 --- a/shell/platform/darwin/ios/framework/Source/platform_message_response_darwin.mm +++ b/shell/platform/darwin/ios/framework/Source/platform_message_response_darwin.mm @@ -4,6 +4,47 @@ #import "flutter/shell/platform/darwin/ios/framework/Source/platform_message_response_darwin.h" +/// A proxy object that behaves like NSData represented in a Mapping. +/// This isn't a subclass of NSData because NSData is in a class cluster. +@interface FlutterMappingData : NSObject +@end + +@implementation FlutterMappingData { + NSData* _data; + std::unique_ptr _mapping; +} + ++ (NSData*)dataWithMapping:(std::unique_ptr)mapping { + return (NSData*)[[[FlutterMappingData alloc] initWithMapping:std::move(mapping)] autorelease]; +} + +- (instancetype)initWithMapping:(std::unique_ptr)mapping { + self = [super init]; + if (self) { + const void* rawData = mapping->GetMapping(); + + // Const cast is required because the NSData API requires it despite + // guarentees that the buffer won't be deleted or modified. + _data = [[NSData alloc] initWithBytesNoCopy:const_cast(rawData) + length:mapping->GetSize() + freeWhenDone:NO]; + + _mapping = std::move(mapping); + } + return self; +} + +- (void)dealloc { + [_data release]; + [super dealloc]; +} + +- (id)forwardingTargetForSelector:(SEL)aSelector { + return _data; +} + +@end + namespace flutter { PlatformMessageResponseDarwin::PlatformMessageResponseDarwin( @@ -17,7 +58,8 @@ void PlatformMessageResponseDarwin::Complete(std::unique_ptr data) { fml::RefPtr self(this); platform_task_runner_->PostTask(fml::MakeCopyable([self, data = std::move(data)]() mutable { - self->callback_.get()(GetNSDataFromMapping(std::move(data))); + NSData* callbackData = [FlutterMappingData dataWithMapping:std::move(data)]; + self->callback_.get()(callbackData); })); } diff --git a/shell/platform/darwin/ios/framework/Source/platform_message_router.mm b/shell/platform/darwin/ios/framework/Source/platform_message_router.mm index 8fb81cc4a9a28..749df11f90060 100644 --- a/shell/platform/darwin/ios/framework/Source/platform_message_router.mm +++ b/shell/platform/darwin/ios/framework/Source/platform_message_router.mm @@ -8,6 +8,48 @@ #import "flutter/shell/platform/darwin/common/buffer_conversions.h" +/// A proxy object that behaves like NSData represented in a PlatformMessage. +/// This isn't a subclass of NSData because NSData is in a class cluster. +@interface FlutterMessageData : NSObject +@end + +@implementation FlutterMessageData { + NSData* _data; + fml::RefPtr _platformMessage; +} + ++ (NSData*)dataWithMessage:(fml::RefPtr)platformMessage { + return (NSData*)[[[FlutterMessageData alloc] initWithMessage:std::move(platformMessage)] + autorelease]; +} + +- (instancetype)initWithMessage:(fml::RefPtr)platformMessage { + self = [super init]; + if (self) { + const void* rawData = platformMessage->data().data(); + + // Const cast is required because the NSData API requires it despite + // guarentees that the buffer won't be deleted or modified. + _data = [[NSData alloc] initWithBytesNoCopy:const_cast(rawData) + length:platformMessage->data().size() + freeWhenDone:NO]; + + _platformMessage = std::move(platformMessage); + } + return self; +} + +- (void)dealloc { + [_data release]; + [super dealloc]; +} + +- (id)forwardingTargetForSelector:(SEL)aSelector { + return _data; +} + +@end + namespace flutter { PlatformMessageRouter::PlatformMessageRouter() = default; @@ -22,12 +64,12 @@ FlutterBinaryMessageHandler handler = it->second; NSData* data = nil; if (message->hasData()) { - data = GetNSDataFromVector(message->data()); + data = [FlutterMessageData dataWithMessage:std::move(message)]; } handler(data, ^(NSData* reply) { if (completer) { if (reply) { - completer->Complete(GetMappingFromNSData(reply)); + completer->Complete(CopyNSDataToMapping(reply)); } else { completer->CompleteEmpty(); } From b6c339f35ba8a7b232d98016827dd5fad78da627 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Mon, 3 May 2021 16:42:33 -0700 Subject: [PATCH 2/5] some cleanup and moved the objc classes to buffer_conversions --- .../darwin/common/buffer_conversions.h | 5 + .../darwin/common/buffer_conversions.mm | 97 +++++++++++++++++++ .../platform_message_response_darwin.mm | 43 +------- .../Source/platform_message_router.mm | 44 +-------- 4 files changed, 104 insertions(+), 85 deletions(-) diff --git a/shell/platform/darwin/common/buffer_conversions.h b/shell/platform/darwin/common/buffer_conversions.h index 0e383bc37ac15..1273ffb1f524a 100644 --- a/shell/platform/darwin/common/buffer_conversions.h +++ b/shell/platform/darwin/common/buffer_conversions.h @@ -10,6 +10,7 @@ #include #include "flutter/fml/mapping.h" +#import "flutter/lib/ui/window/platform_message.h" namespace flutter { @@ -17,6 +18,10 @@ std::vector CopyNSDataToVector(NSData* data); std::unique_ptr CopyNSDataToMapping(NSData* data); +NSData* ConvertMessageToNSData(fml::RefPtr message); + +NSData* ConvertMappingToNSData(std::unique_ptr mapping); + } // namespace flutter #endif // FLUTTER_SHELL_PLATFORM_DARWIN_COMMON_BUFFER_CONVERSIONS_H_ diff --git a/shell/platform/darwin/common/buffer_conversions.mm b/shell/platform/darwin/common/buffer_conversions.mm index f39fb88e03a2b..b71a48b5ccaae 100644 --- a/shell/platform/darwin/common/buffer_conversions.mm +++ b/shell/platform/darwin/common/buffer_conversions.mm @@ -4,6 +4,95 @@ #import "flutter/shell/platform/darwin/common/buffer_conversions.h" +/// A proxy object that behaves like NSData represented in a Mapping. +/// This isn't a subclass of NSData because NSData is in a class cluster. +@interface FlutterMappingData : NSObject +@end + +@implementation FlutterMappingData { + NSData* _data; + std::unique_ptr _mapping; +} + ++ (NSData*)dataWithMapping:(std::unique_ptr)mapping { + return (NSData*)[[[FlutterMappingData alloc] initWithMapping:std::move(mapping)] autorelease]; +} + +- (instancetype)initWithMapping:(std::unique_ptr)mapping { + self = [super init]; + if (self) { + const void* rawData = mapping->GetMapping(); + + // Const cast is required because the NSData API requires it despite + // guarentees that the buffer won't be deleted or modified. + _data = [[NSData alloc] initWithBytesNoCopy:const_cast(rawData) + length:mapping->GetSize() + freeWhenDone:NO]; + + _mapping = std::move(mapping); + } + return self; +} + +- (void)dealloc { + [_data release]; + [super dealloc]; +} + +- (id)forwardingTargetForSelector:(SEL)aSelector { + return _data; +} + +- (BOOL)respondsToSelector:(SEL)aSelector { + return [NSData instancesRespondToSelector:aSelector]; +} +@end + +/// A proxy object that behaves like NSData represented in a PlatformMessage. +/// This isn't a subclass of NSData because NSData is in a class cluster. +@interface FlutterMessageData : NSObject +@end + +@implementation FlutterMessageData { + NSData* _data; + fml::RefPtr _platformMessage; +} + ++ (NSData*)dataWithMessage:(fml::RefPtr)platformMessage { + return (NSData*)[[[FlutterMessageData alloc] initWithMessage:std::move(platformMessage)] + autorelease]; +} + +- (instancetype)initWithMessage:(fml::RefPtr)platformMessage { + self = [super init]; + if (self) { + const void* rawData = platformMessage->data().data(); + + // Const cast is required because the NSData API requires it despite + // guarentees that the buffer won't be deleted or modified. + _data = [[NSData alloc] initWithBytesNoCopy:const_cast(rawData) + length:platformMessage->data().size() + freeWhenDone:NO]; + + _platformMessage = std::move(platformMessage); + } + return self; +} + +- (void)dealloc { + [_data release]; + [super dealloc]; +} + +- (id)forwardingTargetForSelector:(SEL)aSelector { + return _data; +} + +- (BOOL)respondsToSelector:(SEL)aSelector { + return [NSData instancesRespondToSelector:aSelector]; +} +@end + namespace flutter { std::vector CopyNSDataToVector(NSData* data) { @@ -15,4 +104,12 @@ return std::make_unique(CopyNSDataToVector(data)); } +NSData* ConvertMessageToNSData(fml::RefPtr message) { + return [FlutterMessageData dataWithMessage:std::move(message)]; +} + +NSData* ConvertMappingToNSData(std::unique_ptr mapping) { + return [FlutterMappingData dataWithMapping:std::move(mapping)]; +} + } // namespace flutter diff --git a/shell/platform/darwin/ios/framework/Source/platform_message_response_darwin.mm b/shell/platform/darwin/ios/framework/Source/platform_message_response_darwin.mm index e1aa651119ea8..4cfa1e2aad734 100644 --- a/shell/platform/darwin/ios/framework/Source/platform_message_response_darwin.mm +++ b/shell/platform/darwin/ios/framework/Source/platform_message_response_darwin.mm @@ -4,47 +4,6 @@ #import "flutter/shell/platform/darwin/ios/framework/Source/platform_message_response_darwin.h" -/// A proxy object that behaves like NSData represented in a Mapping. -/// This isn't a subclass of NSData because NSData is in a class cluster. -@interface FlutterMappingData : NSObject -@end - -@implementation FlutterMappingData { - NSData* _data; - std::unique_ptr _mapping; -} - -+ (NSData*)dataWithMapping:(std::unique_ptr)mapping { - return (NSData*)[[[FlutterMappingData alloc] initWithMapping:std::move(mapping)] autorelease]; -} - -- (instancetype)initWithMapping:(std::unique_ptr)mapping { - self = [super init]; - if (self) { - const void* rawData = mapping->GetMapping(); - - // Const cast is required because the NSData API requires it despite - // guarentees that the buffer won't be deleted or modified. - _data = [[NSData alloc] initWithBytesNoCopy:const_cast(rawData) - length:mapping->GetSize() - freeWhenDone:NO]; - - _mapping = std::move(mapping); - } - return self; -} - -- (void)dealloc { - [_data release]; - [super dealloc]; -} - -- (id)forwardingTargetForSelector:(SEL)aSelector { - return _data; -} - -@end - namespace flutter { PlatformMessageResponseDarwin::PlatformMessageResponseDarwin( @@ -58,7 +17,7 @@ - (id)forwardingTargetForSelector:(SEL)aSelector { void PlatformMessageResponseDarwin::Complete(std::unique_ptr data) { fml::RefPtr self(this); platform_task_runner_->PostTask(fml::MakeCopyable([self, data = std::move(data)]() mutable { - NSData* callbackData = [FlutterMappingData dataWithMapping:std::move(data)]; + NSData* callbackData = ConvertMappingToNSData(std::move(data)); self->callback_.get()(callbackData); })); } diff --git a/shell/platform/darwin/ios/framework/Source/platform_message_router.mm b/shell/platform/darwin/ios/framework/Source/platform_message_router.mm index 749df11f90060..9d9710924f1b6 100644 --- a/shell/platform/darwin/ios/framework/Source/platform_message_router.mm +++ b/shell/platform/darwin/ios/framework/Source/platform_message_router.mm @@ -8,48 +8,6 @@ #import "flutter/shell/platform/darwin/common/buffer_conversions.h" -/// A proxy object that behaves like NSData represented in a PlatformMessage. -/// This isn't a subclass of NSData because NSData is in a class cluster. -@interface FlutterMessageData : NSObject -@end - -@implementation FlutterMessageData { - NSData* _data; - fml::RefPtr _platformMessage; -} - -+ (NSData*)dataWithMessage:(fml::RefPtr)platformMessage { - return (NSData*)[[[FlutterMessageData alloc] initWithMessage:std::move(platformMessage)] - autorelease]; -} - -- (instancetype)initWithMessage:(fml::RefPtr)platformMessage { - self = [super init]; - if (self) { - const void* rawData = platformMessage->data().data(); - - // Const cast is required because the NSData API requires it despite - // guarentees that the buffer won't be deleted or modified. - _data = [[NSData alloc] initWithBytesNoCopy:const_cast(rawData) - length:platformMessage->data().size() - freeWhenDone:NO]; - - _platformMessage = std::move(platformMessage); - } - return self; -} - -- (void)dealloc { - [_data release]; - [super dealloc]; -} - -- (id)forwardingTargetForSelector:(SEL)aSelector { - return _data; -} - -@end - namespace flutter { PlatformMessageRouter::PlatformMessageRouter() = default; @@ -64,7 +22,7 @@ - (id)forwardingTargetForSelector:(SEL)aSelector { FlutterBinaryMessageHandler handler = it->second; NSData* data = nil; if (message->hasData()) { - data = [FlutterMessageData dataWithMessage:std::move(message)]; + data = ConvertMessageToNSData(std::move(message)); } handler(data, ^(NSData* reply) { if (completer) { From bf68af730e4f1cdcee1f7e24876033c43cb085c2 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Mon, 3 May 2021 16:54:49 -0700 Subject: [PATCH 3/5] removed another copy when going from NSData to Mapping --- .../darwin/common/buffer_conversions.h | 2 +- .../darwin/common/buffer_conversions.mm | 21 +++++++++++++++++-- .../Source/platform_message_router.mm | 2 +- 3 files changed, 21 insertions(+), 4 deletions(-) diff --git a/shell/platform/darwin/common/buffer_conversions.h b/shell/platform/darwin/common/buffer_conversions.h index 1273ffb1f524a..6d0616f1c50d2 100644 --- a/shell/platform/darwin/common/buffer_conversions.h +++ b/shell/platform/darwin/common/buffer_conversions.h @@ -16,7 +16,7 @@ namespace flutter { std::vector CopyNSDataToVector(NSData* data); -std::unique_ptr CopyNSDataToMapping(NSData* data); +std::unique_ptr CovertNSDataToMapping(NSData* data); NSData* ConvertMessageToNSData(fml::RefPtr message); diff --git a/shell/platform/darwin/common/buffer_conversions.mm b/shell/platform/darwin/common/buffer_conversions.mm index b71a48b5ccaae..c0a1c918dda38 100644 --- a/shell/platform/darwin/common/buffer_conversions.mm +++ b/shell/platform/darwin/common/buffer_conversions.mm @@ -3,6 +3,23 @@ // found in the LICENSE file. #import "flutter/shell/platform/darwin/common/buffer_conversions.h" +#import "flutter/fml/platform/darwin/scoped_nsobject.h" + +namespace { +class NSDataMapping : public fml::Mapping { + public: + NSDataMapping(NSData* data) : data_([data retain]) {} + + size_t GetSize() const override { return [data_.get() length]; } + + const uint8_t* GetMapping() const override { + return static_cast([data_.get() bytes]); + } + + private: + fml::scoped_nsobject data_; +}; +} /// A proxy object that behaves like NSData represented in a Mapping. /// This isn't a subclass of NSData because NSData is in a class cluster. @@ -100,8 +117,8 @@ - (BOOL)respondsToSelector:(SEL)aSelector { return std::vector(bytes, bytes + data.length); } -std::unique_ptr CopyNSDataToMapping(NSData* data) { - return std::make_unique(CopyNSDataToVector(data)); +std::unique_ptr CovertNSDataToMapping(NSData* data) { + return std::make_unique(data); } NSData* ConvertMessageToNSData(fml::RefPtr message) { diff --git a/shell/platform/darwin/ios/framework/Source/platform_message_router.mm b/shell/platform/darwin/ios/framework/Source/platform_message_router.mm index 9d9710924f1b6..ec47ca9aade96 100644 --- a/shell/platform/darwin/ios/framework/Source/platform_message_router.mm +++ b/shell/platform/darwin/ios/framework/Source/platform_message_router.mm @@ -27,7 +27,7 @@ handler(data, ^(NSData* reply) { if (completer) { if (reply) { - completer->Complete(CopyNSDataToMapping(reply)); + completer->Complete(CovertNSDataToMapping(reply)); } else { completer->CompleteEmpty(); } From 201744300b123ac585c46434e5d5ace253135aa8 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Mon, 3 May 2021 17:24:09 -0700 Subject: [PATCH 4/5] gave buffer_conversions access to platform messages --- shell/platform/darwin/BUILD.gn | 1 + shell/platform/darwin/common/BUILD.gn | 1 + 2 files changed, 2 insertions(+) diff --git a/shell/platform/darwin/BUILD.gn b/shell/platform/darwin/BUILD.gn index 23c8e4055acf7..b44fa982b890c 100644 --- a/shell/platform/darwin/BUILD.gn +++ b/shell/platform/darwin/BUILD.gn @@ -40,6 +40,7 @@ source_set("flutter_channels") { "//flutter/common", "//flutter/flow", "//flutter/fml", + "//flutter/lib/ui:ui", "//flutter/runtime", "//flutter/shell/common", "//third_party/skia", diff --git a/shell/platform/darwin/common/BUILD.gn b/shell/platform/darwin/common/BUILD.gn index 45a9e3092b2bc..cda3ca70c639b 100644 --- a/shell/platform/darwin/common/BUILD.gn +++ b/shell/platform/darwin/common/BUILD.gn @@ -20,6 +20,7 @@ source_set("common") { "//flutter/common", "//flutter/flow", "//flutter/fml", + "//flutter/lib/ui:ui", "//flutter/runtime", "//flutter/shell/common", "//third_party/dart/runtime:dart_api", From 44edb685c354af7eec26bab0c61c631bce113f72 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Tue, 4 May 2021 15:33:59 -0700 Subject: [PATCH 5/5] removed assert --- shell/platform/darwin/common/framework/Source/FlutterCodecs.mm | 1 - 1 file changed, 1 deletion(-) diff --git a/shell/platform/darwin/common/framework/Source/FlutterCodecs.mm b/shell/platform/darwin/common/framework/Source/FlutterCodecs.mm index 27bbbce5c9435..46a5293fe2d58 100644 --- a/shell/platform/darwin/common/framework/Source/FlutterCodecs.mm +++ b/shell/platform/darwin/common/framework/Source/FlutterCodecs.mm @@ -16,7 +16,6 @@ + (instancetype)sharedInstance { } - (NSData*)encode:(id)message { - NSAssert(!message || [message isKindOfClass:[NSData class]], @""); return message; }