Add macOS support for plugin value publishing (flutter/engine#45502)

These directly copy the iOS APIs, to minimize the branching needed in plugins with shared implementation code, and to facilitate the long-term goal of merging the iOS and macOS plugin headers. This does mean replicating the unfortunately non-idiomatic behavior of having `valuePublishedByPlugin:` sometimes return `nil` and sometimes return `NSNull`, instead of distinguishing between `nil` cases (if that's actually even necessary here) via a more specific API. In isolation I would definitely not design the API with this behavior, but consistency with iOS is the more important factor.

(Eventually I think we'll need a sort of "v2" of iOS plugin APIs since there are a number of strange behaviors that we're currently stuck with, but migrating iOS and macOS together to a new set of APIs won't be any harder than doing just iOS, and in the short to medium term consistency will help the ecosystem more that trying to pre-create better APIs as macOS-only.)

Also fixes `FlutterEngineRegistrar` to have a weak pointer to the engine. This should really already have been the case since plugins can retain the registrar, creating a likely cycle; it's now a guaranteed cycle (and failed unit tests designed to find cycles) without that since the engine itself is now keeping references to them.

Fixes https://github.com/flutter/flutter/issues/124721

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
This commit is contained in:
stuartmorgan
2023-09-06 13:44:02 -07:00
committed by GitHub
parent 4a99ef108c
commit 5dc4e70477
4 changed files with 118 additions and 3 deletions

View File

@@ -71,6 +71,20 @@ FLUTTER_DARWIN_EXPORT
- (void)registerViewFactory:(nonnull NSObject<FlutterPlatformViewFactory>*)factory
withId:(nonnull NSString*)factoryId;
/**
* Publishes a value for external use of the plugin.
*
* Plugins may publish a single value, such as an instance of the
* plugin's main class, for situations where external control or
* interaction is needed.
*
* The published value will be available from the `FlutterPluginRegistry`.
* Repeated calls overwrite any previous publication.
*
* @param value The value to be published.
*/
- (void)publish:(nonnull NSObject*)value;
/**
* Returns the file name for the given asset.
* The returned file name can be used to access the asset in the application's main bundle.
@@ -119,4 +133,14 @@ FLUTTER_DARWIN_EXPORT
*/
- (nonnull id<FlutterPluginRegistrar>)registrarForPlugin:(nonnull NSString*)pluginKey;
/**
* Returns a value published by the specified plugin.
*
* @param pluginKey The unique key identifying the plugin.
* @return An object published by the plugin, if any. Will be `NSNull` if
* nothing has been published. Will be `nil` if the plugin has not been
* registered.
*/
- (nullable NSObject*)valuePublishedByPlugin:(nonnull NSString*)pluginKey;
@end

View File

@@ -25,6 +25,8 @@
#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterViewController_Internal.h"
#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterViewEngineProvider.h"
@class FlutterEngineRegistrar;
NSString* const kFlutterPlatformChannel = @"flutter/platform";
NSString* const kFlutterSettingsChannel = @"flutter/settings";
NSString* const kFlutterLifecycleChannel = @"flutter/lifecycle";
@@ -95,6 +97,12 @@ constexpr char kTextPlainFormat[] = "text/plain";
*/
@property(nonatomic, strong) NSPointerArray* pluginAppDelegates;
/**
* All registrars returned from registrarForPlugin:
*/
@property(nonatomic, readonly)
NSMutableDictionary<NSString*, FlutterEngineRegistrar*>* pluginRegistrars;
- (nullable FlutterViewController*)viewControllerForId:(FlutterViewId)viewId;
/**
@@ -274,12 +282,19 @@ constexpr char kTextPlainFormat[] = "text/plain";
- (instancetype)initWithPlugin:(nonnull NSString*)pluginKey
flutterEngine:(nonnull FlutterEngine*)flutterEngine;
- (NSView*)viewForId:(FlutterViewId)viewId;
- (nullable NSView*)viewForId:(FlutterViewId)viewId;
/**
* The value published by this plugin, or NSNull if nothing has been published.
*
* The unusual NSNull is for the documented behavior of valuePublishedByPlugin:.
*/
@property(nonatomic, readonly, nonnull) NSObject* publishedValue;
@end
@implementation FlutterEngineRegistrar {
NSString* _pluginKey;
FlutterEngine* _flutterEngine;
__weak FlutterEngine* _flutterEngine;
}
@dynamic view;
@@ -289,6 +304,7 @@ constexpr char kTextPlainFormat[] = "text/plain";
if (self) {
_pluginKey = [pluginKey copy];
_flutterEngine = flutterEngine;
_publishedValue = [NSNull null];
}
return self;
}
@@ -340,6 +356,10 @@ constexpr char kTextPlainFormat[] = "text/plain";
[[_flutterEngine platformViewController] registerViewFactory:factory withId:factoryId];
}
- (void)publish:(NSObject*)value {
_publishedValue = value;
}
- (NSString*)lookupKeyForAsset:(NSString*)asset {
return [FlutterDartProject lookupKeyForAsset:asset];
}
@@ -438,6 +458,7 @@ static void OnPlatformMessage(const FlutterPlatformMessage* message, FlutterEngi
_messengerHandlers = [[NSMutableDictionary alloc] init];
_binaryMessenger = [[FlutterBinaryMessengerRelay alloc] initWithParent:self];
_pluginAppDelegates = [NSPointerArray weakObjectsPointerArray];
_pluginRegistrars = [[NSMutableDictionary alloc] init];
_currentMessengerConnection = 1;
_allowHeadlessExecution = allowHeadlessExecution;
_semanticsEnabled = NO;
@@ -494,6 +515,11 @@ static void OnPlatformMessage(const FlutterPlatformMessage* message, FlutterEngi
}
}
}
// Clear any published values, just in case a plugin has created a retain cycle with the
// registrar.
for (NSString* pluginName in _pluginRegistrars) {
[_pluginRegistrars[pluginName] publish:[NSNull null]];
}
@synchronized(_isResponseValid) {
[_isResponseValid removeAllObjects];
[_isResponseValid addObject:@NO];
@@ -1334,7 +1360,18 @@ static void OnPlatformMessage(const FlutterPlatformMessage* message, FlutterEngi
#pragma mark - FlutterPluginRegistry
- (id<FlutterPluginRegistrar>)registrarForPlugin:(NSString*)pluginName {
return [[FlutterEngineRegistrar alloc] initWithPlugin:pluginName flutterEngine:self];
id<FlutterPluginRegistrar> registrar = self.pluginRegistrars[pluginName];
if (!registrar) {
FlutterEngineRegistrar* registrarImpl =
[[FlutterEngineRegistrar alloc] initWithPlugin:pluginName flutterEngine:self];
self.pluginRegistrars[pluginName] = registrarImpl;
registrar = registrarImpl;
}
return registrar;
}
- (nullable NSObject*)valuePublishedByPlugin:(NSString*)pluginName {
return self.pluginRegistrars[pluginName].publishedValue;
}
#pragma mark - FlutterTextureRegistrar

View File

@@ -608,6 +608,56 @@ TEST_F(FlutterEngineTest, FlutterTextureRegistryDoesNotReturnEngine) {
EXPECT_EQ(weakEngine, nil);
}
TEST_F(FlutterEngineTest, PublishedValueNilForUnknownPlugin) {
NSString* fixtures = @(flutter::testing::GetFixturesPath());
FlutterDartProject* project = [[FlutterDartProject alloc]
initWithAssetsPath:fixtures
ICUDataPath:[fixtures stringByAppendingString:@"/icudtl.dat"]];
FlutterEngine* engine = [[FlutterEngine alloc] initWithName:@"test"
project:project
allowHeadlessExecution:YES];
EXPECT_EQ([engine valuePublishedByPlugin:@"NoSuchPlugin"], nil);
}
TEST_F(FlutterEngineTest, PublishedValueNSNullIfNoPublishedValue) {
NSString* fixtures = @(flutter::testing::GetFixturesPath());
FlutterDartProject* project = [[FlutterDartProject alloc]
initWithAssetsPath:fixtures
ICUDataPath:[fixtures stringByAppendingString:@"/icudtl.dat"]];
FlutterEngine* engine = [[FlutterEngine alloc] initWithName:@"test"
project:project
allowHeadlessExecution:YES];
NSString* pluginName = @"MyPlugin";
// Request the registarar to register the plugin as existing.
[engine registrarForPlugin:pluginName];
// The documented behavior is that a plugin that exists but hasn't published
// anything returns NSNull, rather than nil, as on iOS.
EXPECT_EQ([engine valuePublishedByPlugin:pluginName], [NSNull null]);
}
TEST_F(FlutterEngineTest, PublishedValueReturnsLastPublished) {
NSString* fixtures = @(flutter::testing::GetFixturesPath());
FlutterDartProject* project = [[FlutterDartProject alloc]
initWithAssetsPath:fixtures
ICUDataPath:[fixtures stringByAppendingString:@"/icudtl.dat"]];
FlutterEngine* engine = [[FlutterEngine alloc] initWithName:@"test"
project:project
allowHeadlessExecution:YES];
NSString* pluginName = @"MyPlugin";
id<FlutterPluginRegistrar> registrar = [engine registrarForPlugin:pluginName];
NSString* firstValue = @"A published value";
NSArray* secondValue = @[ @"A different published value" ];
[registrar publish:firstValue];
EXPECT_EQ([engine valuePublishedByPlugin:pluginName], firstValue);
[registrar publish:secondValue];
EXPECT_EQ([engine valuePublishedByPlugin:pluginName], secondValue);
}
// If a channel overrides a previous channel with the same name, cleaning
// the previous channel should not affect the new channel.
//

View File

@@ -903,6 +903,10 @@ static void CommonInit(FlutterViewController* controller, FlutterEngine* engine)
return [_engine registrarForPlugin:pluginName];
}
- (NSObject*)valuePublishedByPlugin:(NSString*)pluginKey {
return [_engine valuePublishedByPlugin:pluginKey];
}
#pragma mark - FlutterKeyboardViewDelegate
- (void)sendKeyEvent:(const FlutterKeyEvent&)event