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.
This commit is contained in:
gaaclarke 2020-07-08 13:26:29 -07:00 committed by GitHub
parent 40d3f7c64e
commit 41c503cc23
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
14 changed files with 287 additions and 18 deletions

View File

@ -934,6 +934,9 @@ FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/accessibility_
FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/accessibility_bridge_test.mm
FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/accessibility_text_entry.h
FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/accessibility_text_entry.mm
FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/connection_collection.cc
FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/connection_collection.h
FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/connection_collection_test.mm
FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/platform_message_response_darwin.h
FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/platform_message_response_darwin.mm
FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/platform_message_router.h

View File

@ -29,6 +29,8 @@ typedef void (^FlutterBinaryReply)(NSData* _Nullable reply);
*/
typedef void (^FlutterBinaryMessageHandler)(NSData* _Nullable message, FlutterBinaryReply reply);
typedef int64_t FlutterBinaryMessengerConnection;
/**
* A facility for communicating with the Flutter side using asynchronous message
* passing with binary messages.
@ -72,9 +74,20 @@ FLUTTER_EXPORT
*
* @param channel The channel name.
* @param handler The message handler.
* @return An identifier that represents the connection that was just created to the channel.
*/
- (void)setMessageHandlerOnChannel:(NSString*)channel
binaryMessageHandler:(FlutterBinaryMessageHandler _Nullable)handler;
- (FlutterBinaryMessengerConnection)setMessageHandlerOnChannel:(NSString*)channel
binaryMessageHandler:
(FlutterBinaryMessageHandler _Nullable)handler;
/**
* Clears out a channel's message handler if that handler is still the one that
* was created as a result of
* `setMessageHandlerOnChannel:binaryMessageHandler:`.
*
* @param connection The result from `setMessageHandlerOnChannel:binaryMessageHandler:`.
*/
- (void)cleanupConnection:(FlutterBinaryMessengerConnection)connection;
@end
NS_ASSUME_NONNULL_END
#endif // FLUTTER_FLUTTERBINARYMESSENGER_H_

View File

@ -20,6 +20,7 @@ static void ResizeChannelBuffer(NSObject<FlutterBinaryMessenger>* binaryMessenge
NSObject<FlutterBinaryMessenger>* _messenger;
NSString* _name;
NSObject<FlutterMessageCodec>* _codec;
FlutterBinaryMessengerConnection _connection;
}
+ (instancetype)messageChannelWithName:(NSString*)name
binaryMessenger:(NSObject<FlutterBinaryMessenger>*)messenger {
@ -68,7 +69,12 @@ static void ResizeChannelBuffer(NSObject<FlutterBinaryMessenger>* binaryMessenge
- (void)setMessageHandler:(FlutterMessageHandler)handler {
if (!handler) {
[_messenger setMessageHandlerOnChannel:_name binaryMessageHandler:nil];
if (_connection > 0) {
[_messenger cleanupConnection:_connection];
_connection = 0;
} else {
[_messenger setMessageHandlerOnChannel:_name binaryMessageHandler:nil];
}
return;
}
// Grab reference to avoid retain on self.
@ -78,7 +84,7 @@ static void ResizeChannelBuffer(NSObject<FlutterBinaryMessenger>* binaryMessenge
callback([codec encode:reply]);
});
};
[_messenger setMessageHandlerOnChannel:_name binaryMessageHandler:messageHandler];
_connection = [_messenger setMessageHandlerOnChannel:_name binaryMessageHandler:messageHandler];
}
- (void)resizeChannelBuffer:(NSInteger)newSize {
@ -168,6 +174,7 @@ NSObject const* FlutterMethodNotImplemented = [NSObject new];
NSObject<FlutterBinaryMessenger>* _messenger;
NSString* _name;
NSObject<FlutterMethodCodec>* _codec;
FlutterBinaryMessengerConnection _connection;
}
+ (instancetype)methodChannelWithName:(NSString*)name
@ -222,7 +229,12 @@ NSObject const* FlutterMethodNotImplemented = [NSObject new];
- (void)setMethodCallHandler:(FlutterMethodCallHandler)handler {
if (!handler) {
[_messenger setMessageHandlerOnChannel:_name binaryMessageHandler:nil];
if (_connection > 0) {
[_messenger cleanupConnection:_connection];
_connection = 0;
} else {
[_messenger setMessageHandlerOnChannel:_name binaryMessageHandler:nil];
}
return;
}
// Make sure the block captures the codec, not self.
@ -238,7 +250,7 @@ NSObject const* FlutterMethodNotImplemented = [NSObject new];
callback([codec encodeSuccessEnvelope:result]);
});
};
[_messenger setMessageHandlerOnChannel:_name binaryMessageHandler:messageHandler];
_connection = [_messenger setMessageHandlerOnChannel:_name binaryMessageHandler:messageHandler];
}
- (void)resizeChannelBuffer:(NSInteger)newSize {

View File

@ -35,10 +35,16 @@
self.message = message;
}
- (void)setMessageHandlerOnChannel:(NSString*)channel
binaryMessageHandler:(FlutterBinaryMessageHandler _Nullable)handler {
- (FlutterBinaryMessengerConnection)setMessageHandlerOnChannel:(NSString*)channel
binaryMessageHandler:
(FlutterBinaryMessageHandler _Nullable)handler {
[self.handlers setObject:handler forKey:channel];
return 0;
}
- (void)cleanupConnection:(FlutterBinaryMessengerConnection)connection {
}
@end
@interface FlutterChannelsTest : XCTestCase
@ -155,6 +161,53 @@
OCMExpect([binaryMessenger sendOnChannel:@"dev.flutter/channel-buffers" message:expectedMessage]);
[channel resizeChannelBuffer:100];
OCMVerifyAll(binaryMessenger);
[binaryMessenger stopMocking];
}
- (void)testBasicMessageChannelCleanup {
NSString* channelName = @"foo";
FlutterBinaryMessengerConnection connection = 123;
id binaryMessenger = OCMProtocolMock(@protocol(FlutterBinaryMessenger));
id codec = OCMProtocolMock(@protocol(FlutterMethodCodec));
FlutterBasicMessageChannel* channel =
[[FlutterBasicMessageChannel alloc] initWithName:channelName
binaryMessenger:binaryMessenger
codec:codec];
FlutterMessageHandler handler = ^(id _Nullable message, FlutterReply callback) {
NSLog(@"hey");
};
OCMStub([binaryMessenger setMessageHandlerOnChannel:channelName
binaryMessageHandler:[OCMArg any]])
.andReturn(connection);
[channel setMessageHandler:handler];
OCMVerify([binaryMessenger setMessageHandlerOnChannel:channelName
binaryMessageHandler:[OCMArg isNotNil]]);
[channel setMessageHandler:nil];
OCMVerify([binaryMessenger cleanupConnection:connection]);
}
- (void)testMethodChannelCleanup {
NSString* channelName = @"foo";
FlutterBinaryMessengerConnection connection = 123;
id binaryMessenger = OCMProtocolMock(@protocol(FlutterBinaryMessenger));
id codec = OCMProtocolMock(@protocol(FlutterMethodCodec));
FlutterMethodChannel* channel = [[FlutterMethodChannel alloc] initWithName:channelName
binaryMessenger:binaryMessenger
codec:codec];
XCTAssertNotNil(channel);
OCMStub([binaryMessenger setMessageHandlerOnChannel:channelName
binaryMessageHandler:[OCMArg any]])
.andReturn(connection);
FlutterMethodCallHandler handler =
^(FlutterMethodCall* _Nonnull call, FlutterResult _Nonnull result) {
};
[channel setMethodCallHandler:handler];
OCMVerify([binaryMessenger setMessageHandlerOnChannel:channelName
binaryMessageHandler:[OCMArg isNotNil]]);
[channel setMethodCallHandler:nil];
OCMVerify([binaryMessenger cleanupConnection:connection]);
}
@end

View File

@ -74,6 +74,8 @@ source_set("flutter_framework_source") {
"framework/Source/accessibility_bridge.mm",
"framework/Source/accessibility_text_entry.h",
"framework/Source/accessibility_text_entry.mm",
"framework/Source/connection_collection.cc",
"framework/Source/connection_collection.h",
"framework/Source/platform_message_response_darwin.h",
"framework/Source/platform_message_response_darwin.mm",
"framework/Source/platform_message_router.h",
@ -208,6 +210,7 @@ shared_library("ios_test_flutter") {
"framework/Source/FlutterTextInputPluginTest.m",
"framework/Source/FlutterViewControllerTest.mm",
"framework/Source/SemanticsObjectTest.mm",
"framework/Source/connection_collection_test.mm",
]
deps = [
":flutter_framework_source",

View File

@ -35,10 +35,20 @@
}
}
- (void)setMessageHandlerOnChannel:(NSString*)channel
binaryMessageHandler:(FlutterBinaryMessageHandler)handler {
- (FlutterBinaryMessengerConnection)setMessageHandlerOnChannel:(NSString*)channel
binaryMessageHandler:
(FlutterBinaryMessageHandler)handler {
if (self.parent) {
[self.parent setMessageHandlerOnChannel:channel binaryMessageHandler:handler];
return [self.parent setMessageHandlerOnChannel:channel binaryMessageHandler:handler];
} else {
FML_LOG(WARNING) << "Communicating on a dead channel.";
return -1;
}
}
- (void)cleanupConnection:(FlutterBinaryMessengerConnection)connection {
if (self.parent) {
return [self.parent cleanupConnection:connection];
} else {
FML_LOG(WARNING) << "Communicating on a dead channel.";
}

View File

@ -23,6 +23,7 @@
#import "flutter/shell/platform/darwin/ios/framework/Source/FlutterPlatformPlugin.h"
#import "flutter/shell/platform/darwin/ios/framework/Source/FlutterTextInputDelegate.h"
#import "flutter/shell/platform/darwin/ios/framework/Source/FlutterViewController_Internal.h"
#import "flutter/shell/platform/darwin/ios/framework/Source/connection_collection.h"
#import "flutter/shell/platform/darwin/ios/framework/Source/platform_message_response_darwin.h"
#import "flutter/shell/platform/darwin/ios/framework/Source/profiler_metrics_ios.h"
#import "flutter/shell/platform/darwin/ios/ios_surface.h"
@ -78,6 +79,7 @@ static constexpr int kNumProfilerSamplesPerSec = 5;
BOOL _allowHeadlessExecution;
FlutterBinaryMessengerRelay* _binaryMessenger;
std::unique_ptr<flutter::ConnectionCollection> _connections;
}
- (instancetype)initWithName:(NSString*)labelPrefix {
@ -110,6 +112,7 @@ static constexpr int kNumProfilerSamplesPerSec = 5;
_platformViewsController.reset(new flutter::FlutterPlatformViewsController());
_binaryMessenger = [[FlutterBinaryMessengerRelay alloc] initWithParent:self];
_connections.reset(new flutter::ConnectionCollection());
NSNotificationCenter* center = [NSNotificationCenter defaultCenter];
[center addObserver:self
@ -693,14 +696,26 @@ static constexpr int kNumProfilerSamplesPerSec = 5;
_shell->GetPlatformView()->DispatchPlatformMessage(platformMessage);
}
- (void)setMessageHandlerOnChannel:(NSString*)channel
binaryMessageHandler:(FlutterBinaryMessageHandler)handler {
- (FlutterBinaryMessengerConnection)setMessageHandlerOnChannel:(NSString*)channel
binaryMessageHandler:
(FlutterBinaryMessageHandler)handler {
NSParameterAssert(channel);
if (_shell && _shell->IsSetup()) {
self.iosPlatformView->GetPlatformMessageRouter().SetMessageHandler(channel.UTF8String, handler);
return _connections->AquireConnection(channel.UTF8String);
} else {
NSAssert(!handler, @"Setting a message handler before the FlutterEngine has been run.");
// Setting a handler to nil for a not setup channel is a noop.
return flutter::ConnectionCollection::MakeErrorConnection(-1);
}
}
- (void)cleanupConnection:(FlutterBinaryMessengerConnection)connection {
if (_shell && _shell->IsSetup()) {
std::string channel = _connections->CleanupConnection(connection);
if (!channel.empty()) {
self.iosPlatformView->GetPlatformMessageRouter().SetMessageHandler(channel.c_str(), nil);
}
}
}

View File

@ -1204,10 +1204,16 @@ static flutter::PointerData::DeviceKind DeviceKindFromTouchType(UITouch* touch)
[_engine.get().binaryMessenger sendOnChannel:channel message:message binaryReply:callback];
}
- (void)setMessageHandlerOnChannel:(NSString*)channel
binaryMessageHandler:(FlutterBinaryMessageHandler)handler {
- (FlutterBinaryMessengerConnection)setMessageHandlerOnChannel:(NSString*)channel
binaryMessageHandler:
(FlutterBinaryMessageHandler)handler {
NSAssert(channel, @"The channel must not be null");
[_engine.get().binaryMessenger setMessageHandlerOnChannel:channel binaryMessageHandler:handler];
return [_engine.get().binaryMessenger setMessageHandlerOnChannel:channel
binaryMessageHandler:handler];
}
- (void)cleanupConnection:(FlutterBinaryMessengerConnection)connection {
[_engine.get().binaryMessenger cleanupConnection:connection];
}
#pragma mark - FlutterTextureRegistry

View File

@ -72,6 +72,7 @@ AccessibilityBridge::AccessibilityBridge(UIView* view,
}
AccessibilityBridge::~AccessibilityBridge() {
[accessibility_channel_.get() setMessageHandler:nil];
clearState();
view_.accessibilityElements = nil;
}

View File

@ -4,6 +4,7 @@
#import <XCTest/XCTest.h>
#import "flutter/shell/platform/darwin/common/framework/Headers/FlutterBinaryMessenger.h"
#import "flutter/shell/platform/darwin/common/framework/Headers/FlutterMacros.h"
#import "flutter/shell/platform/darwin/ios/framework/Headers/FlutterPlatformViews.h"
#import "flutter/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h"
@ -368,4 +369,46 @@ fml::RefPtr<fml::TaskRunner> CreateNewThread(std::string name) {
XCTAssertEqual([accessibility_notifications count], 0ul);
}
- (void)testAccessibilityMessageAfterDeletion {
flutter::MockDelegate mock_delegate;
auto thread = std::make_unique<fml::Thread>("AccessibilityBridgeTest");
auto thread_task_runner = thread->GetTaskRunner();
flutter::TaskRunners runners(/*label=*/self.name.UTF8String,
/*platform=*/thread_task_runner,
/*raster=*/thread_task_runner,
/*ui=*/thread_task_runner,
/*io=*/thread_task_runner);
id messenger = OCMProtocolMock(@protocol(FlutterBinaryMessenger));
id engine = OCMClassMock([FlutterEngine class]);
id flutterViewController = OCMClassMock([FlutterViewController class]);
OCMStub([flutterViewController engine]).andReturn(engine);
OCMStub([engine binaryMessenger]).andReturn(messenger);
FlutterBinaryMessengerConnection connection = 123;
OCMStub([messenger setMessageHandlerOnChannel:@"flutter/accessibility"
binaryMessageHandler:[OCMArg any]])
.andReturn(connection);
auto platform_view = std::make_unique<flutter::PlatformViewIOS>(
/*delegate=*/mock_delegate,
/*rendering_api=*/flutter::IOSRenderingAPI::kSoftware,
/*task_runners=*/runners);
fml::AutoResetWaitableEvent latch;
thread_task_runner->PostTask([&] {
auto weakFactory =
std::make_unique<fml::WeakPtrFactory<FlutterViewController>>(flutterViewController);
platform_view->SetOwnerViewController(weakFactory->GetWeakPtr());
auto bridge =
std::make_unique<flutter::AccessibilityBridge>(/*view=*/nil,
/*platform_view=*/platform_view.get(),
/*platform_views_controller=*/nil);
XCTAssertTrue(bridge.get());
OCMVerify([messenger setMessageHandlerOnChannel:@"flutter/accessibility"
binaryMessageHandler:[OCMArg isNotNil]]);
bridge.reset();
latch.Signal();
});
latch.Wait();
OCMVerify([messenger cleanupConnection:connection]);
}
@end

View File

@ -0,0 +1,46 @@
// 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/ios/framework/Source/connection_collection.h"
namespace flutter {
ConnectionCollection::Connection ConnectionCollection::AquireConnection(
const std::string& name) {
Connection nextConnection = ++counter_;
connections_[name] = nextConnection;
return nextConnection;
}
std::string ConnectionCollection::CleanupConnection(
ConnectionCollection::Connection connection) {
if (connection > 0) {
std::string channel;
for (auto& keyValue : connections_) {
if (keyValue.second == connection) {
channel = keyValue.first;
break;
}
}
if (channel.length() > 0) {
connections_.erase(channel);
return channel;
}
}
return "";
}
bool ConnectionCollection::IsValidConnection(
ConnectionCollection::Connection connection) {
return connection > 0;
}
ConnectionCollection::Connection ConnectionCollection::MakeErrorConnection(
int errCode) {
if (errCode < 0) {
return -1 * errCode;
}
return errCode;
}
} // namespace flutter

View File

@ -0,0 +1,36 @@
// 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 SHELL_PLATFORM_IOS_FRAMEWORK_SOURCE_CONNECTION_COLLECTION_H_
#define SHELL_PLATFORM_IOS_FRAMEWORK_SOURCE_CONNECTION_COLLECTION_H_
#include <cstdint>
#include <map>
#include <string>
namespace flutter {
/// Maintains a current integer assigned to a name (connections).
class ConnectionCollection {
public:
typedef int64_t Connection;
static const Connection kInvalidConnection = 0;
Connection AquireConnection(const std::string& name);
///\returns the name of the channel when cleanup is successful, otherwise
/// the empty string.
std::string CleanupConnection(Connection connection);
static bool IsValidConnection(Connection connection);
static Connection MakeErrorConnection(int errCode);
private:
std::map<std::string, Connection> connections_;
Connection counter_ = 0;
};
} // namespace flutter
#endif // SHELL_PLATFORM_IOS_FRAMEWORK_SOURCE_CONNECTION_COLLECTION_H_

View File

@ -0,0 +1,22 @@
// 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.
#import <OCMock/OCMock.h>
#import <XCTest/XCTest.h>
#import "flutter/shell/platform/darwin/ios/framework/Source/connection_collection.h"
@interface ConnectionCollectionTest : XCTestCase
@end
@implementation ConnectionCollectionTest
- (void)testSimple {
auto connections = std::make_unique<flutter::ConnectionCollection>();
flutter::ConnectionCollection::Connection connection = connections->AquireConnection("foo");
XCTAssertTrue(connections->CleanupConnection(connection) == "foo");
XCTAssertTrue(connections->CleanupConnection(connection).empty());
}
@end

View File

@ -447,9 +447,15 @@ static bool OnAcquireExternalTexture(FlutterEngine* engine,
}
}
- (void)setMessageHandlerOnChannel:(nonnull NSString*)channel
binaryMessageHandler:(nullable FlutterBinaryMessageHandler)handler {
- (FlutterBinaryMessengerConnection)setMessageHandlerOnChannel:(nonnull NSString*)channel
binaryMessageHandler:
(nullable FlutterBinaryMessageHandler)handler {
_messageHandlers[channel] = [handler copy];
return 0;
}
- (void)cleanupConnection:(FlutterBinaryMessengerConnection)connection {
// There hasn't been a need to implement this yet for macOS.
}
#pragma mark - FlutterPluginRegistry