From 002159e772f4f6e136ce2b7608a2234e95060a09 Mon Sep 17 00:00:00 2001 From: gaaclarke <30870216+gaaclarke@users.noreply.github.com> Date: Tue, 18 Jun 2019 17:58:45 -0700 Subject: [PATCH] Surrogate binary messenger (flutter/engine#9347) Created FlutterSurrogateBinaryMessenger to make sure that channels are holding onto engines and not viewcontrollers. This doesn't change the public API but makes clients do what we want them to be doing, using Engine for FlutterBinaryMessenger. --- .../ci/licenses_golden/licenses_flutter | 2 + .../flutter/shell/platform/darwin/BUILD.gn | 1 + .../framework/Source/FlutterChannels.mm | 16 ++- .../Source/flutter_channels_unittest.mm | 107 ++++++++++++++++++ .../Source/FlutterSurrogateBinaryMessenger.h | 9 ++ .../framework/Source/FlutterViewController.mm | 11 +- .../Source/FlutterViewController_Internal.h | 3 +- 7 files changed, 142 insertions(+), 7 deletions(-) create mode 100644 engine/src/flutter/shell/platform/darwin/common/framework/Source/flutter_channels_unittest.mm create mode 100644 engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterSurrogateBinaryMessenger.h diff --git a/engine/src/flutter/ci/licenses_golden/licenses_flutter b/engine/src/flutter/ci/licenses_golden/licenses_flutter index 9d186456700..d97c918a0dd 100644 --- a/engine/src/flutter/ci/licenses_golden/licenses_flutter +++ b/engine/src/flutter/ci/licenses_golden/licenses_flutter @@ -664,6 +664,7 @@ FILE: ../../../flutter/shell/platform/darwin/common/framework/Source/FlutterChan FILE: ../../../flutter/shell/platform/darwin/common/framework/Source/FlutterCodecs.mm FILE: ../../../flutter/shell/platform/darwin/common/framework/Source/FlutterStandardCodec.mm FILE: ../../../flutter/shell/platform/darwin/common/framework/Source/FlutterStandardCodec_Internal.h +FILE: ../../../flutter/shell/platform/darwin/common/framework/Source/flutter_channels_unittest.mm FILE: ../../../flutter/shell/platform/darwin/common/framework/Source/flutter_codecs_unittest.mm FILE: ../../../flutter/shell/platform/darwin/common/framework/Source/flutter_standard_codec_unittest.mm FILE: ../../../flutter/shell/platform/darwin/ios/framework/Flutter.podspec @@ -699,6 +700,7 @@ FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/FlutterPlatfor FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.mm FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/FlutterPluginAppLifeCycleDelegate.mm FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/FlutterPluginAppLifeCycleDelegate_internal.h +FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/FlutterSurrogateBinaryMessenger.h FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/FlutterTextInputDelegate.h FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/FlutterTextInputPlugin.h FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/FlutterTextInputPlugin.mm diff --git a/engine/src/flutter/shell/platform/darwin/BUILD.gn b/engine/src/flutter/shell/platform/darwin/BUILD.gn index efae1a9f197..dcb55233e87 100644 --- a/engine/src/flutter/shell/platform/darwin/BUILD.gn +++ b/engine/src/flutter/shell/platform/darwin/BUILD.gn @@ -53,6 +53,7 @@ executable("flutter_channels_unittests") { testonly = true sources = [ + "common/framework/Source/flutter_channels_unittest.mm", "common/framework/Source/flutter_codecs_unittest.mm", "common/framework/Source/flutter_standard_codec_unittest.mm", ] diff --git a/engine/src/flutter/shell/platform/darwin/common/framework/Source/FlutterChannels.mm b/engine/src/flutter/shell/platform/darwin/common/framework/Source/FlutterChannels.mm index a5142b11c0a..20ab80e559c 100644 --- a/engine/src/flutter/shell/platform/darwin/common/framework/Source/FlutterChannels.mm +++ b/engine/src/flutter/shell/platform/darwin/common/framework/Source/FlutterChannels.mm @@ -3,9 +3,19 @@ // found in the LICENSE file. #include "flutter/shell/platform/darwin/common/framework/Headers/FlutterChannels.h" +#import "flutter/shell/platform/darwin/ios/framework/Source/FlutterSurrogateBinaryMessenger.h" #pragma mark - Basic message channel +static NSObject* getSurrogate(NSObject* messenger) { + if ([messenger conformsToProtocol:@protocol(FlutterSurrogateBinaryMessenger)]) { + NSObject* surrogate = + (NSObject*)messenger; + return [surrogate surrogateBinaryMessenger]; + } + return messenger; +} + @implementation FlutterBasicMessageChannel { NSObject* _messenger; NSString* _name; @@ -32,7 +42,7 @@ self = [super init]; NSAssert(self, @"Super init cannot be nil"); _name = [name retain]; - _messenger = [messenger retain]; + _messenger = [getSurrogate(messenger) retain]; _codec = [codec retain]; return self; } @@ -172,7 +182,7 @@ NSObject const* FlutterMethodNotImplemented = [NSObject new]; self = [super init]; NSAssert(self, @"Super init cannot be nil"); _name = [name retain]; - _messenger = [messenger retain]; + _messenger = [getSurrogate(messenger) retain]; _codec = [codec retain]; return self; } @@ -251,7 +261,7 @@ NSObject const* FlutterEndOfEventStream = [NSObject new]; self = [super init]; NSAssert(self, @"Super init cannot be nil"); _name = [name retain]; - _messenger = [messenger retain]; + _messenger = [getSurrogate(messenger) retain]; _codec = [codec retain]; return self; } diff --git a/engine/src/flutter/shell/platform/darwin/common/framework/Source/flutter_channels_unittest.mm b/engine/src/flutter/shell/platform/darwin/common/framework/Source/flutter_channels_unittest.mm new file mode 100644 index 00000000000..4e71e682d16 --- /dev/null +++ b/engine/src/flutter/shell/platform/darwin/common/framework/Source/flutter_channels_unittest.mm @@ -0,0 +1,107 @@ +// 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/shell/platform/darwin/common/framework/Headers/FlutterChannels.h" +#import "flutter/shell/platform/darwin/ios/framework/Source/FlutterSurrogateBinaryMessenger.h" +#include "gtest/gtest.h" + +@interface MockBinaryMessenger : NSObject +@property(nonatomic, assign) int sendOnChannel_message_count; +@end + +@implementation MockBinaryMessenger +- (void)sendOnChannel:(NSString*)channel message:(NSData* _Nullable)message { + _sendOnChannel_message_count++; +} + +- (void)sendOnChannel:(NSString*)channel + message:(NSData* _Nullable)message + binaryReply:(FlutterBinaryReply _Nullable)callback { +} + +- (void)setMessageHandlerOnChannel:(NSString*)channel + binaryMessageHandler:(FlutterBinaryMessageHandler _Nullable)handler { +} +@end + +@interface MockSurrogateBinaryMessenger + : NSObject +@property(nonatomic, strong) MockBinaryMessenger* surrogate; +@property(nonatomic, assign) int sendOnChannel_message_count; +@end + +@implementation MockSurrogateBinaryMessenger +- (instancetype)init { + self = [super init]; + if (self) { + _surrogate = [[[MockBinaryMessenger alloc] init] retain]; + } + return self; +} + +- (void)dealloc { + [_surrogate release]; + [super dealloc]; +} + +- (void)sendOnChannel:(NSString*)channel message:(NSData* _Nullable)message { + _sendOnChannel_message_count++; +} + +- (void)sendOnChannel:(NSString*)channel + message:(NSData* _Nullable)message + binaryReply:(FlutterBinaryReply _Nullable)callback { +} + +- (void)setMessageHandlerOnChannel:(NSString*)channel + binaryMessageHandler:(FlutterBinaryMessageHandler _Nullable)handler { +} + +- (NSObject*)surrogateBinaryMessenger { + return _surrogate; +} + +@end + +TEST(FlutterChannels, BasicMessageChannelUsesSurrogate) { + MockSurrogateBinaryMessenger* binaryMessenger = + [[[MockSurrogateBinaryMessenger alloc] init] autorelease]; + NSObject* codec = [FlutterStandardMessageCodec sharedInstance]; + FlutterBasicMessageChannel* channel = + [[[FlutterBasicMessageChannel alloc] initWithName:@"channel-name" + binaryMessenger:binaryMessenger + codec:codec] autorelease]; + ASSERT_TRUE(channel != nil); + [channel sendMessage:nil]; + ASSERT_EQ(1, binaryMessenger.surrogate.sendOnChannel_message_count); + ASSERT_EQ(0, binaryMessenger.sendOnChannel_message_count); +} + +TEST(FlutterChannels, MethodChannelUsesSurrogate) { + MockSurrogateBinaryMessenger* binaryMessenger = + [[[MockSurrogateBinaryMessenger alloc] init] autorelease]; + NSObject* codec = [FlutterStandardMethodCodec sharedInstance]; + FlutterMethodChannel* channel = [[[FlutterMethodChannel alloc] initWithName:@"channel-name" + binaryMessenger:binaryMessenger + codec:codec] autorelease]; + ASSERT_TRUE(channel != nil); + [channel invokeMethod:@"foo" arguments:nil]; + ASSERT_EQ(1, binaryMessenger.surrogate.sendOnChannel_message_count); + ASSERT_EQ(0, binaryMessenger.sendOnChannel_message_count); +} + +TEST(FlutterChannels, EventChannelRetainsSurrogate) { + MockSurrogateBinaryMessenger* binaryMessenger = + [[[MockSurrogateBinaryMessenger alloc] init] autorelease]; + NSObject* codec = [FlutterStandardMethodCodec sharedInstance]; + NSUInteger binaryMessengerRetainCount = [binaryMessenger retainCount]; + NSUInteger surrogateRetainCount = [binaryMessenger.surrogate retainCount]; + + FlutterEventChannel* channel = [[[FlutterEventChannel alloc] initWithName:@"channel-name" + binaryMessenger:binaryMessenger + codec:codec] autorelease]; + ASSERT_TRUE(channel != nil); + ASSERT_EQ(binaryMessengerRetainCount, binaryMessenger.retainCount); + ASSERT_EQ(surrogateRetainCount + 1, binaryMessenger.surrogate.retainCount); +} diff --git a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterSurrogateBinaryMessenger.h b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterSurrogateBinaryMessenger.h new file mode 100644 index 00000000000..2133d982859 --- /dev/null +++ b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterSurrogateBinaryMessenger.h @@ -0,0 +1,9 @@ +// 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. + +@protocol FlutterBinaryMessenger; + +@protocol FlutterSurrogateBinaryMessenger +- (NSObject*)surrogateBinaryMessenger; +@end diff --git a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm index fc08125a18a..58eaf90fb70 100644 --- a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm +++ b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm @@ -992,20 +992,20 @@ constexpr CGFloat kStandardStatusBarHeight = 20.0; #pragma mark - FlutterBinaryMessenger - (void)sendOnChannel:(NSString*)channel message:(NSData*)message { - [_engine.get() sendOnChannel:channel message:message]; + [self.surrogateBinaryMessenger sendOnChannel:channel message:message]; } - (void)sendOnChannel:(NSString*)channel message:(NSData*)message binaryReply:(FlutterBinaryReply)callback { NSAssert(channel, @"The channel must not be null"); - [_engine.get() sendOnChannel:channel message:message binaryReply:callback]; + [self.surrogateBinaryMessenger sendOnChannel:channel message:message binaryReply:callback]; } - (void)setMessageHandlerOnChannel:(NSString*)channel binaryMessageHandler:(FlutterBinaryMessageHandler)handler { NSAssert(channel, @"The channel must not be null"); - [_engine.get() setMessageHandlerOnChannel:channel binaryMessageHandler:handler]; + [self.surrogateBinaryMessenger setMessageHandlerOnChannel:channel binaryMessageHandler:handler]; } #pragma mark - FlutterTextureRegistry @@ -1048,4 +1048,9 @@ constexpr CGFloat kStandardStatusBarHeight = 20.0; return [_engine.get() valuePublishedByPlugin:pluginKey]; } +#pragma mark - FlutterSurrogateBinaryMessenger +- (NSObject*)surrogateBinaryMessenger { + return _engine.get(); +} + @end diff --git a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterViewController_Internal.h b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterViewController_Internal.h index 5947a9327ff..5fd903a5520 100644 --- a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterViewController_Internal.h +++ b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterViewController_Internal.h @@ -11,8 +11,9 @@ #include "flutter/shell/common/shell.h" #include "flutter/shell/platform/darwin/ios/framework/Headers/FlutterViewController.h" #include "flutter/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h" +#import "flutter/shell/platform/darwin/ios/framework/Source/FlutterSurrogateBinaryMessenger.h" -@interface FlutterViewController () +@interface FlutterViewController () - (fml::WeakPtr)getWeakPtr; - (flutter::FlutterPlatformViewsController*)platformViewsController;