Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions ci/licenses_golden/licenses_flutter
Original file line number Diff line number Diff line change
Expand Up @@ -2511,6 +2511,9 @@ FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/FlutterTextInp
FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/FlutterTextInputPlugin.h
FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/FlutterTextInputPlugin.mm
FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/FlutterTextInputPluginTest.mm
FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/FlutterTextureRegistryRelay.h
FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/FlutterTextureRegistryRelay.mm
FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/FlutterTextureRegistryRelayTest.mm
FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/FlutterUIPressProxy.h
FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/FlutterUIPressProxy.mm
FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/FlutterUmbrellaImport.m
Expand Down
2 changes: 2 additions & 0 deletions shell/platform/darwin/ios/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ source_set("flutter_framework_source") {
"framework/Source/FlutterTextInputDelegate.h",
"framework/Source/FlutterTextInputPlugin.h",
"framework/Source/FlutterTextInputPlugin.mm",
"framework/Source/FlutterTextureRegistryRelay.mm",
"framework/Source/FlutterUIPressProxy.h",
"framework/Source/FlutterUIPressProxy.mm",
"framework/Source/FlutterUndoManagerDelegate.h",
Expand Down Expand Up @@ -259,6 +260,7 @@ shared_library("ios_test_flutter") {
"framework/Source/FlutterRestorationPluginTest.mm",
"framework/Source/FlutterSpellCheckPluginTest.mm",
"framework/Source/FlutterTextInputPluginTest.mm",
"framework/Source/FlutterTextureRegistryRelayTest.mm",
"framework/Source/FlutterUndoManagerPluginTest.mm",
"framework/Source/FlutterViewControllerTest.mm",
"framework/Source/SemanticsObjectTest.mm",
Expand Down
7 changes: 6 additions & 1 deletion shell/platform/darwin/ios/framework/Headers/FlutterEngine.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ extern NSString* const FlutterDefaultInitialRoute;
* One of these methods must be invoked before calling `-setViewController:`.
*/
FLUTTER_DARWIN_EXPORT
@interface FlutterEngine : NSObject <FlutterTextureRegistry, FlutterPluginRegistry>
@interface FlutterEngine : NSObject <FlutterPluginRegistry>

/**
* Default initializer for a FlutterEngine.
Expand Down Expand Up @@ -424,6 +424,11 @@ FLUTTER_DARWIN_EXPORT
*/
@property(nonatomic, readonly) NSObject<FlutterBinaryMessenger>* binaryMessenger;

/**
* The `FlutterTextureRegistry` associated with this FlutterEngine (used for register textures).
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* The `FlutterTextureRegistry` associated with this FlutterEngine (used for register textures).
* The `FlutterTextureRegistry` associated with this FlutterEngine (used to register textures).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done.

*/
@property(nonatomic, readonly) NSObject<FlutterTextureRegistry>* textureRegistry;

/**
* The UI Isolate ID of the engine.
*
Expand Down
14 changes: 12 additions & 2 deletions shell/platform/darwin/ios/framework/Source/FlutterEngine.mm
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
#import "flutter/shell/platform/darwin/ios/framework/Source/FlutterPlatformPlugin.h"
#import "flutter/shell/platform/darwin/ios/framework/Source/FlutterSpellCheckPlugin.h"
#import "flutter/shell/platform/darwin/ios/framework/Source/FlutterTextInputDelegate.h"
#import "flutter/shell/platform/darwin/ios/framework/Source/FlutterTextureRegistryRelay.h"
#import "flutter/shell/platform/darwin/ios/framework/Source/FlutterUndoManagerDelegate.h"
#import "flutter/shell/platform/darwin/ios/framework/Source/FlutterUndoManagerPlugin.h"
#import "flutter/shell/platform/darwin/ios/framework/Source/FlutterViewController_Internal.h"
Expand Down Expand Up @@ -87,7 +88,8 @@ - (instancetype)initWithPlugin:(NSString*)pluginKey flutterEngine:(FlutterEngine
@interface FlutterEngine () <FlutterIndirectScribbleDelegate,
FlutterUndoManagerDelegate,
FlutterTextInputDelegate,
FlutterBinaryMessenger>
FlutterBinaryMessenger,
FlutterTextureRegistry>
// Maintains a dictionary of plugin names that have registered with the engine. Used by
// FlutterEngineRegistrar to implement a FlutterPluginRegistrar.
@property(nonatomic, readonly) NSMutableDictionary* pluginPublications;
Expand Down Expand Up @@ -138,6 +140,7 @@ @implementation FlutterEngine {
BOOL _allowHeadlessExecution;
BOOL _restorationEnabled;
FlutterBinaryMessengerRelay* _binaryMessenger;
FlutterTextureRegistryRelay* _textureRegistry;
std::unique_ptr<flutter::ConnectionCollection> _connections;
}

Expand Down Expand Up @@ -197,6 +200,7 @@ - (instancetype)initWithName:(NSString*)labelPrefix
[self recreatePlatformViewController];

_binaryMessenger = [[FlutterBinaryMessengerRelay alloc] initWithParent:self];
_textureRegistry = [[FlutterTextureRegistryRelay alloc] initWithParent:self];
_connections.reset(new flutter::ConnectionCollection());

NSNotificationCenter* center = [NSNotificationCenter defaultCenter];
Expand Down Expand Up @@ -261,7 +265,9 @@ - (void)dealloc {
[_pluginPublications release];
[_registrars release];
_binaryMessenger.parent = nil;
_textureRegistry.parent = nil;
[_binaryMessenger release];
[_textureRegistry release];
Comment thread
cbracken marked this conversation as resolved.
[_isolateId release];

NSNotificationCenter* center = [NSNotificationCenter defaultCenter];
Expand Down Expand Up @@ -1073,6 +1079,10 @@ - (void)flutterViewAccessibilityDidCall {
return _binaryMessenger;
}

- (NSObject<FlutterTextureRegistry>*)textureRegistry {
return _textureRegistry;
}

// For test only. Ideally we should create a dependency injector for all dependencies and
// remove this.
- (void)setBinaryMessenger:(FlutterBinaryMessengerRelay*)binaryMessenger {
Expand Down Expand Up @@ -1333,7 +1343,7 @@ - (void)dealloc {
}

- (NSObject<FlutterTextureRegistry>*)textures {
return _flutterEngine;
return _flutterEngine.textureRegistry;
}

- (void)publish:(NSObject*)value {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// 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 "flutter/shell/platform/darwin/common/framework/Headers/FlutterMacros.h"
#import "flutter/shell/platform/darwin/common/framework/Headers/FlutterTexture.h"

#if FLUTTER_RUNTIME_MODE == FLUTTER_RUNTIME_MODE_DEBUG
FLUTTER_DARWIN_EXPORT
#endif
@interface FlutterTextureRegistryRelay : NSObject <FlutterTextureRegistry>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you add a comment explaining that this relay is used to solve the issue with plugin retaining the engine?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use /** */
comments as in the other headers.

From this comment I don't really understand when this would be used. Returned from what?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jmagman, are these comments ok?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about:

/**
 * Wrapper around a weakly held collection of registered textures.
 *
 * Avoids a retain cycle between plugins and the engine.
 */
@interface FlutterTextureRegistryRelay : NSObject <FlutterTextureRegistry>

/**
 * A weak reference to a FlutterEngine that will be passed texture registration.
 */
@property(nonatomic, assign) NSObject<FlutterTextureRegistry>* parent;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this would be better.

@property(nonatomic, assign) NSObject<FlutterTextureRegistry>* parent;
- (instancetype)initWithParent:(NSObject<FlutterTextureRegistry>*)parent;
@end
Comment thread
endless7 marked this conversation as resolved.
Outdated
Original file line number Diff line number Diff line change
@@ -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.

#import "flutter/shell/platform/darwin/ios/framework/Source/FlutterTextureRegistryRelay.h"

#include "flutter/fml/logging.h"

@implementation FlutterTextureRegistryRelay : NSObject

#pragma mark - FlutterTextureRegistry

- (instancetype)initWithParent:(NSObject<FlutterTextureRegistry>*)parent {
self = [super init];
if (self != nil) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

optional nits:

Suggested change
self = [super init];
if (self != nil) {
if (self = [super init]) {

this is following the pattern in the objc document: https://developer.apple.com/documentation/objectivec/nsobject/1418641-init?language=objc

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

self.parent = parent;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
self.parent = parent;
_parent = parent;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done.

}
return self;
}

- (int64_t)registerTexture:(NSObject<FlutterTexture>*)texture {
if (self.parent) {
return [self.parent registerTexture:texture];
} else {
FML_LOG(WARNING) << "Using on an empty registry.";
}
return 0;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

optional nits, escape early:

Suggested change
if (self.parent) {
return [self.parent registerTexture:texture];
} else {
FML_LOG(WARNING) << "Using on an empty registry.";
}
return 0;
if (!self.parent) {
FML_LOG(WARNING) << "Using on an empty registry.";
return 0;
}
return [self.parent registerTexture:texture];

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

}

- (void)textureFrameAvailable:(int64_t)textureId {
if (self.parent) {
return [self.parent textureFrameAvailable:textureId];
} else {
FML_LOG(WARNING) << "Using on an empty registry.";
}
}

- (void)unregisterTexture:(int64_t)textureId {
if (self.parent) {
return [self.parent unregisterTexture:textureId];
} else {
FML_LOG(WARNING) << "Using on an empty registry.";
}
}

Comment thread
endless7 marked this conversation as resolved.
@end
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
// 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 "flutter/shell/platform/darwin/ios/framework/Source/FlutterTextureRegistryRelay.h"

#import <OCMock/OCMock.h>
#import <XCTest/XCTest.h>

#import "flutter/shell/platform/darwin/common/framework/Headers/FlutterMacros.h"

FLUTTER_ASSERT_ARC

@interface FlutterTextureRegistryRelayTest : XCTestCase
@end

@implementation FlutterTextureRegistryRelayTest
Comment thread
endless7 marked this conversation as resolved.

- (void)testCreate {
id textureRegistry = OCMProtocolMock(@protocol(FlutterTextureRegistry));
FlutterTextureRegistryRelay* relay =
[[FlutterTextureRegistryRelay alloc] initWithParent:textureRegistry];
XCTAssertNotNil(relay);
XCTAssertEqual(textureRegistry, relay.parent);
}

- (void)testRegisterTexture {
id textureRegistry = OCMProtocolMock(@protocol(FlutterTextureRegistry));
FlutterTextureRegistryRelay* relay =
[[FlutterTextureRegistryRelay alloc] initWithParent:textureRegistry];
id texture = OCMProtocolMock(@protocol(FlutterTexture));
[relay registerTexture:texture];
OCMVerify([textureRegistry registerTexture:texture]);
}

- (void)testTextureFrameAvailable {
id textureRegistry = OCMProtocolMock(@protocol(FlutterTextureRegistry));
FlutterTextureRegistryRelay* relay =
[[FlutterTextureRegistryRelay alloc] initWithParent:textureRegistry];
[relay textureFrameAvailable:0];
OCMVerify([textureRegistry textureFrameAvailable:0]);
}

- (void)testUnregisterTexture {
id textureRegistry = OCMProtocolMock(@protocol(FlutterTextureRegistry));
FlutterTextureRegistryRelay* relay =
[[FlutterTextureRegistryRelay alloc] initWithParent:textureRegistry];
[relay unregisterTexture:0];
OCMVerify([textureRegistry unregisterTexture:0]);
}

@end
Original file line number Diff line number Diff line change
Expand Up @@ -1866,15 +1866,15 @@ - (void)cleanUpConnection:(FlutterBinaryMessengerConnection)connection {
#pragma mark - FlutterTextureRegistry

- (int64_t)registerTexture:(NSObject<FlutterTexture>*)texture {
return [_engine.get() registerTexture:texture];
return [_engine.get().textureRegistry registerTexture:texture];
}

- (void)unregisterTexture:(int64_t)textureId {
[_engine.get() unregisterTexture:textureId];
[_engine.get().textureRegistry unregisterTexture:textureId];
}

- (void)textureFrameAvailable:(int64_t)textureId {
[_engine.get() textureFrameAvailable:textureId];
[_engine.get().textureRegistry textureFrameAvailable:textureId];
}

- (NSString*)lookupKeyForAsset:(NSString*)asset {
Expand Down