Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Commit 41c503c

Browse files
gaaclarkegaaclarke
authored
Changed iOS channels to start cleaning up the accessibility handler when the bridge is deleted (#19556)
Started cleaning up the accessibility handler when the bridge is deleted and made nilling out channels safer by making sure they don't overwrite newly setup handlers.
1 parent 40d3f7c commit 41c503c

14 files changed

+287
-18
lines changed

‎ci/licenses_golden/licenses_flutter

+3
Original file line numberDiff line numberDiff line change
@@ -934,6 +934,9 @@ FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/accessibility_
934934
FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/accessibility_bridge_test.mm
935935
FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/accessibility_text_entry.h
936936
FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/accessibility_text_entry.mm
937+
FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/connection_collection.cc
938+
FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/connection_collection.h
939+
FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/connection_collection_test.mm
937940
FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/platform_message_response_darwin.h
938941
FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/platform_message_response_darwin.mm
939942
FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/platform_message_router.h

‎shell/platform/darwin/common/framework/Headers/FlutterBinaryMessenger.h

+15-2
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ typedef void (^FlutterBinaryReply)(NSData* _Nullable reply);
2929
*/
3030
typedef void (^FlutterBinaryMessageHandler)(NSData* _Nullable message, FlutterBinaryReply reply);
3131

32+
typedef int64_t FlutterBinaryMessengerConnection;
33+
3234
/**
3335
* A facility for communicating with the Flutter side using asynchronous message
3436
* passing with binary messages.
@@ -72,9 +74,20 @@ FLUTTER_EXPORT
7274
*
7375
* @param channel The channel name.
7476
* @param handler The message handler.
77+
* @return An identifier that represents the connection that was just created to the channel.
78+
*/
79+
- (FlutterBinaryMessengerConnection)setMessageHandlerOnChannel:(NSString*)channel
80+
binaryMessageHandler:
81+
(FlutterBinaryMessageHandler _Nullable)handler;
82+
83+
/**
84+
* Clears out a channel's message handler if that handler is still the one that
85+
* was created as a result of
86+
* `setMessageHandlerOnChannel:binaryMessageHandler:`.
87+
*
88+
* @param connection The result from `setMessageHandlerOnChannel:binaryMessageHandler:`.
7589
*/
76-
- (void)setMessageHandlerOnChannel:(NSString*)channel
77-
binaryMessageHandler:(FlutterBinaryMessageHandler _Nullable)handler;
90+
- (void)cleanupConnection:(FlutterBinaryMessengerConnection)connection;
7891
@end
7992
NS_ASSUME_NONNULL_END
8093
#endif // FLUTTER_FLUTTERBINARYMESSENGER_H_

‎shell/platform/darwin/common/framework/Source/FlutterChannels.mm

+16-4
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ @implementation FlutterBasicMessageChannel {
2020
NSObject<FlutterBinaryMessenger>* _messenger;
2121
NSString* _name;
2222
NSObject<FlutterMessageCodec>* _codec;
23+
FlutterBinaryMessengerConnection _connection;
2324
}
2425
+ (instancetype)messageChannelWithName:(NSString*)name
2526
binaryMessenger:(NSObject<FlutterBinaryMessenger>*)messenger {
@@ -68,7 +69,12 @@ - (void)sendMessage:(id)message reply:(FlutterReply)callback {
6869

6970
- (void)setMessageHandler:(FlutterMessageHandler)handler {
7071
if (!handler) {
71-
[_messenger setMessageHandlerOnChannel:_name binaryMessageHandler:nil];
72+
if (_connection > 0) {
73+
[_messenger cleanupConnection:_connection];
74+
_connection = 0;
75+
} else {
76+
[_messenger setMessageHandlerOnChannel:_name binaryMessageHandler:nil];
77+
}
7278
return;
7379
}
7480
// Grab reference to avoid retain on self.
@@ -78,7 +84,7 @@ - (void)setMessageHandler:(FlutterMessageHandler)handler {
7884
callback([codec encode:reply]);
7985
});
8086
};
81-
[_messenger setMessageHandlerOnChannel:_name binaryMessageHandler:messageHandler];
87+
_connection = [_messenger setMessageHandlerOnChannel:_name binaryMessageHandler:messageHandler];
8288
}
8389

8490
- (void)resizeChannelBuffer:(NSInteger)newSize {
@@ -168,6 +174,7 @@ @implementation FlutterMethodChannel {
168174
NSObject<FlutterBinaryMessenger>* _messenger;
169175
NSString* _name;
170176
NSObject<FlutterMethodCodec>* _codec;
177+
FlutterBinaryMessengerConnection _connection;
171178
}
172179

173180
+ (instancetype)methodChannelWithName:(NSString*)name
@@ -222,7 +229,12 @@ - (void)invokeMethod:(NSString*)method arguments:(id)arguments result:(FlutterRe
222229

223230
- (void)setMethodCallHandler:(FlutterMethodCallHandler)handler {
224231
if (!handler) {
225-
[_messenger setMessageHandlerOnChannel:_name binaryMessageHandler:nil];
232+
if (_connection > 0) {
233+
[_messenger cleanupConnection:_connection];
234+
_connection = 0;
235+
} else {
236+
[_messenger setMessageHandlerOnChannel:_name binaryMessageHandler:nil];
237+
}
226238
return;
227239
}
228240
// Make sure the block captures the codec, not self.
@@ -238,7 +250,7 @@ - (void)setMethodCallHandler:(FlutterMethodCallHandler)handler {
238250
callback([codec encodeSuccessEnvelope:result]);
239251
});
240252
};
241-
[_messenger setMessageHandlerOnChannel:_name binaryMessageHandler:messageHandler];
253+
_connection = [_messenger setMessageHandlerOnChannel:_name binaryMessageHandler:messageHandler];
242254
}
243255

244256
- (void)resizeChannelBuffer:(NSInteger)newSize {

‎shell/platform/darwin/common/framework/Source/FlutterChannelsTest.m

+55-2
Original file line numberDiff line numberDiff line change
@@ -35,10 +35,16 @@ - (void)sendOnChannel:(NSString*)channel
3535
self.message = message;
3636
}
3737

38-
- (void)setMessageHandlerOnChannel:(NSString*)channel
39-
binaryMessageHandler:(FlutterBinaryMessageHandler _Nullable)handler {
38+
- (FlutterBinaryMessengerConnection)setMessageHandlerOnChannel:(NSString*)channel
39+
binaryMessageHandler:
40+
(FlutterBinaryMessageHandler _Nullable)handler {
4041
[self.handlers setObject:handler forKey:channel];
42+
return 0;
4143
}
44+
45+
- (void)cleanupConnection:(FlutterBinaryMessengerConnection)connection {
46+
}
47+
4248
@end
4349

4450
@interface FlutterChannelsTest : XCTestCase
@@ -155,6 +161,53 @@ - (void)testResize {
155161
OCMExpect([binaryMessenger sendOnChannel:@"dev.flutter/channel-buffers" message:expectedMessage]);
156162
[channel resizeChannelBuffer:100];
157163
OCMVerifyAll(binaryMessenger);
164+
[binaryMessenger stopMocking];
165+
}
166+
167+
- (void)testBasicMessageChannelCleanup {
168+
NSString* channelName = @"foo";
169+
FlutterBinaryMessengerConnection connection = 123;
170+
id binaryMessenger = OCMProtocolMock(@protocol(FlutterBinaryMessenger));
171+
id codec = OCMProtocolMock(@protocol(FlutterMethodCodec));
172+
FlutterBasicMessageChannel* channel =
173+
[[FlutterBasicMessageChannel alloc] initWithName:channelName
174+
binaryMessenger:binaryMessenger
175+
codec:codec];
176+
FlutterMessageHandler handler = ^(id _Nullable message, FlutterReply callback) {
177+
NSLog(@"hey");
178+
};
179+
OCMStub([binaryMessenger setMessageHandlerOnChannel:channelName
180+
binaryMessageHandler:[OCMArg any]])
181+
.andReturn(connection);
182+
[channel setMessageHandler:handler];
183+
OCMVerify([binaryMessenger setMessageHandlerOnChannel:channelName
184+
binaryMessageHandler:[OCMArg isNotNil]]);
185+
[channel setMessageHandler:nil];
186+
OCMVerify([binaryMessenger cleanupConnection:connection]);
187+
}
188+
189+
- (void)testMethodChannelCleanup {
190+
NSString* channelName = @"foo";
191+
FlutterBinaryMessengerConnection connection = 123;
192+
id binaryMessenger = OCMProtocolMock(@protocol(FlutterBinaryMessenger));
193+
id codec = OCMProtocolMock(@protocol(FlutterMethodCodec));
194+
FlutterMethodChannel* channel = [[FlutterMethodChannel alloc] initWithName:channelName
195+
binaryMessenger:binaryMessenger
196+
codec:codec];
197+
XCTAssertNotNil(channel);
198+
199+
OCMStub([binaryMessenger setMessageHandlerOnChannel:channelName
200+
binaryMessageHandler:[OCMArg any]])
201+
.andReturn(connection);
202+
203+
FlutterMethodCallHandler handler =
204+
^(FlutterMethodCall* _Nonnull call, FlutterResult _Nonnull result) {
205+
};
206+
[channel setMethodCallHandler:handler];
207+
OCMVerify([binaryMessenger setMessageHandlerOnChannel:channelName
208+
binaryMessageHandler:[OCMArg isNotNil]]);
209+
[channel setMethodCallHandler:nil];
210+
OCMVerify([binaryMessenger cleanupConnection:connection]);
158211
}
159212

160213
@end

‎shell/platform/darwin/ios/BUILD.gn

+3
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,8 @@ source_set("flutter_framework_source") {
7474
"framework/Source/accessibility_bridge.mm",
7575
"framework/Source/accessibility_text_entry.h",
7676
"framework/Source/accessibility_text_entry.mm",
77+
"framework/Source/connection_collection.cc",
78+
"framework/Source/connection_collection.h",
7779
"framework/Source/platform_message_response_darwin.h",
7880
"framework/Source/platform_message_response_darwin.mm",
7981
"framework/Source/platform_message_router.h",
@@ -208,6 +210,7 @@ shared_library("ios_test_flutter") {
208210
"framework/Source/FlutterTextInputPluginTest.m",
209211
"framework/Source/FlutterViewControllerTest.mm",
210212
"framework/Source/SemanticsObjectTest.mm",
213+
"framework/Source/connection_collection_test.mm",
211214
]
212215
deps = [
213216
":flutter_framework_source",

‎shell/platform/darwin/ios/framework/Source/FlutterBinaryMessengerRelay.mm

+13-3
Original file line numberDiff line numberDiff line change
@@ -35,10 +35,20 @@ - (void)sendOnChannel:(NSString*)channel
3535
}
3636
}
3737

38-
- (void)setMessageHandlerOnChannel:(NSString*)channel
39-
binaryMessageHandler:(FlutterBinaryMessageHandler)handler {
38+
- (FlutterBinaryMessengerConnection)setMessageHandlerOnChannel:(NSString*)channel
39+
binaryMessageHandler:
40+
(FlutterBinaryMessageHandler)handler {
4041
if (self.parent) {
41-
[self.parent setMessageHandlerOnChannel:channel binaryMessageHandler:handler];
42+
return [self.parent setMessageHandlerOnChannel:channel binaryMessageHandler:handler];
43+
} else {
44+
FML_LOG(WARNING) << "Communicating on a dead channel.";
45+
return -1;
46+
}
47+
}
48+
49+
- (void)cleanupConnection:(FlutterBinaryMessengerConnection)connection {
50+
if (self.parent) {
51+
return [self.parent cleanupConnection:connection];
4252
} else {
4353
FML_LOG(WARNING) << "Communicating on a dead channel.";
4454
}

‎shell/platform/darwin/ios/framework/Source/FlutterEngine.mm

+17-2
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
#import "flutter/shell/platform/darwin/ios/framework/Source/FlutterPlatformPlugin.h"
2424
#import "flutter/shell/platform/darwin/ios/framework/Source/FlutterTextInputDelegate.h"
2525
#import "flutter/shell/platform/darwin/ios/framework/Source/FlutterViewController_Internal.h"
26+
#import "flutter/shell/platform/darwin/ios/framework/Source/connection_collection.h"
2627
#import "flutter/shell/platform/darwin/ios/framework/Source/platform_message_response_darwin.h"
2728
#import "flutter/shell/platform/darwin/ios/framework/Source/profiler_metrics_ios.h"
2829
#import "flutter/shell/platform/darwin/ios/ios_surface.h"
@@ -78,6 +79,7 @@ @implementation FlutterEngine {
7879

7980
BOOL _allowHeadlessExecution;
8081
FlutterBinaryMessengerRelay* _binaryMessenger;
82+
std::unique_ptr<flutter::ConnectionCollection> _connections;
8183
}
8284

8385
- (instancetype)initWithName:(NSString*)labelPrefix {
@@ -110,6 +112,7 @@ - (instancetype)initWithName:(NSString*)labelPrefix
110112
_platformViewsController.reset(new flutter::FlutterPlatformViewsController());
111113

112114
_binaryMessenger = [[FlutterBinaryMessengerRelay alloc] initWithParent:self];
115+
_connections.reset(new flutter::ConnectionCollection());
113116

114117
NSNotificationCenter* center = [NSNotificationCenter defaultCenter];
115118
[center addObserver:self
@@ -693,14 +696,26 @@ - (void)sendOnChannel:(NSString*)channel
693696
_shell->GetPlatformView()->DispatchPlatformMessage(platformMessage);
694697
}
695698

696-
- (void)setMessageHandlerOnChannel:(NSString*)channel
697-
binaryMessageHandler:(FlutterBinaryMessageHandler)handler {
699+
- (FlutterBinaryMessengerConnection)setMessageHandlerOnChannel:(NSString*)channel
700+
binaryMessageHandler:
701+
(FlutterBinaryMessageHandler)handler {
698702
NSParameterAssert(channel);
699703
if (_shell && _shell->IsSetup()) {
700704
self.iosPlatformView->GetPlatformMessageRouter().SetMessageHandler(channel.UTF8String, handler);
705+
return _connections->AquireConnection(channel.UTF8String);
701706
} else {
702707
NSAssert(!handler, @"Setting a message handler before the FlutterEngine has been run.");
703708
// Setting a handler to nil for a not setup channel is a noop.
709+
return flutter::ConnectionCollection::MakeErrorConnection(-1);
710+
}
711+
}
712+
713+
- (void)cleanupConnection:(FlutterBinaryMessengerConnection)connection {
714+
if (_shell && _shell->IsSetup()) {
715+
std::string channel = _connections->CleanupConnection(connection);
716+
if (!channel.empty()) {
717+
self.iosPlatformView->GetPlatformMessageRouter().SetMessageHandler(channel.c_str(), nil);
718+
}
704719
}
705720
}
706721

‎shell/platform/darwin/ios/framework/Source/FlutterViewController.mm

+9-3
Original file line numberDiff line numberDiff line change
@@ -1204,10 +1204,16 @@ - (void)sendOnChannel:(NSString*)channel
12041204
[_engine.get().binaryMessenger sendOnChannel:channel message:message binaryReply:callback];
12051205
}
12061206

1207-
- (void)setMessageHandlerOnChannel:(NSString*)channel
1208-
binaryMessageHandler:(FlutterBinaryMessageHandler)handler {
1207+
- (FlutterBinaryMessengerConnection)setMessageHandlerOnChannel:(NSString*)channel
1208+
binaryMessageHandler:
1209+
(FlutterBinaryMessageHandler)handler {
12091210
NSAssert(channel, @"The channel must not be null");
1210-
[_engine.get().binaryMessenger setMessageHandlerOnChannel:channel binaryMessageHandler:handler];
1211+
return [_engine.get().binaryMessenger setMessageHandlerOnChannel:channel
1212+
binaryMessageHandler:handler];
1213+
}
1214+
1215+
- (void)cleanupConnection:(FlutterBinaryMessengerConnection)connection {
1216+
[_engine.get().binaryMessenger cleanupConnection:connection];
12111217
}
12121218

12131219
#pragma mark - FlutterTextureRegistry

‎shell/platform/darwin/ios/framework/Source/accessibility_bridge.mm

+1
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ void PostAccessibilityNotification(UIAccessibilityNotifications notification,
7272
}
7373

7474
AccessibilityBridge::~AccessibilityBridge() {
75+
[accessibility_channel_.get() setMessageHandler:nil];
7576
clearState();
7677
view_.accessibilityElements = nil;
7778
}

‎shell/platform/darwin/ios/framework/Source/accessibility_bridge_test.mm

+43
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
#import <XCTest/XCTest.h>
66

7+
#import "flutter/shell/platform/darwin/common/framework/Headers/FlutterBinaryMessenger.h"
78
#import "flutter/shell/platform/darwin/common/framework/Headers/FlutterMacros.h"
89
#import "flutter/shell/platform/darwin/ios/framework/Headers/FlutterPlatformViews.h"
910
#import "flutter/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h"
@@ -368,4 +369,46 @@ - (void)testAnnouncesIgnoresRouteChangesWhenModal {
368369
XCTAssertEqual([accessibility_notifications count], 0ul);
369370
}
370371

372+
- (void)testAccessibilityMessageAfterDeletion {
373+
flutter::MockDelegate mock_delegate;
374+
auto thread = std::make_unique<fml::Thread>("AccessibilityBridgeTest");
375+
auto thread_task_runner = thread->GetTaskRunner();
376+
flutter::TaskRunners runners(/*label=*/self.name.UTF8String,
377+
/*platform=*/thread_task_runner,
378+
/*raster=*/thread_task_runner,
379+
/*ui=*/thread_task_runner,
380+
/*io=*/thread_task_runner);
381+
id messenger = OCMProtocolMock(@protocol(FlutterBinaryMessenger));
382+
id engine = OCMClassMock([FlutterEngine class]);
383+
id flutterViewController = OCMClassMock([FlutterViewController class]);
384+
385+
OCMStub([flutterViewController engine]).andReturn(engine);
386+
OCMStub([engine binaryMessenger]).andReturn(messenger);
387+
FlutterBinaryMessengerConnection connection = 123;
388+
OCMStub([messenger setMessageHandlerOnChannel:@"flutter/accessibility"
389+
binaryMessageHandler:[OCMArg any]])
390+
.andReturn(connection);
391+
392+
auto platform_view = std::make_unique<flutter::PlatformViewIOS>(
393+
/*delegate=*/mock_delegate,
394+
/*rendering_api=*/flutter::IOSRenderingAPI::kSoftware,
395+
/*task_runners=*/runners);
396+
fml::AutoResetWaitableEvent latch;
397+
thread_task_runner->PostTask([&] {
398+
auto weakFactory =
399+
std::make_unique<fml::WeakPtrFactory<FlutterViewController>>(flutterViewController);
400+
platform_view->SetOwnerViewController(weakFactory->GetWeakPtr());
401+
auto bridge =
402+
std::make_unique<flutter::AccessibilityBridge>(/*view=*/nil,
403+
/*platform_view=*/platform_view.get(),
404+
/*platform_views_controller=*/nil);
405+
XCTAssertTrue(bridge.get());
406+
OCMVerify([messenger setMessageHandlerOnChannel:@"flutter/accessibility"
407+
binaryMessageHandler:[OCMArg isNotNil]]);
408+
bridge.reset();
409+
latch.Signal();
410+
});
411+
latch.Wait();
412+
OCMVerify([messenger cleanupConnection:connection]);
413+
}
371414
@end

0 commit comments

Comments
 (0)