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

Commit e3b2782

Browse files
dkwingsmtdkwingsmt
authored
[macOS] Formalize FlutterViewController's initialization flow, and prohibit replacing (#38981)
This PR adds FlutterViewController.id, and formalizes how FlutterViewController is initialized by FlutterEngine: When FlutterViewController is set to the engine, the engine calls FlutterViewController#attachToEngine:withId:. When FlutterViewController is unset from the engine, the engine calls FlutterViewController#detachFromEngine. This allows the ID to be generated by the engine. The default View ID is now 1 instead of 0. In this way, we can use 0 as an empty value, the value for FlutterViewController.id when the FVC is not attached to an engine. (IDs are meant to be opaque, and views should be a map, not an array anyway.) This PR moves almost all initialization code into FVC's CommonInit to ensure that all init path produces the expected result. With this PR, FlutterEngine no long supports replacing viewController (from non-null to a different non-null). Setting the view controller (from null to non-null) is intended to allow pre-warming an engine before displaying it in add-to-app, but replacing the view controller does not make much sense. If the developer wants to persist the state of the view between different windows, they could just attach the view controller to different windows. In fact, Flutter doesn't even have a way to create a FlutterViewController that is unattached to an engine. All initializers either require an engine or create an engine. As a result, unit tests that used to create FVC with the wrong initializer are fixed.
1 parent 308ce91 commit e3b2782

14 files changed

+282
-301
lines changed

‎shell/platform/darwin/macos/framework/Headers/FlutterEngine.h‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@
77

88
#import <Foundation/Foundation.h>
99

10+
#include <stdint.h>
11+
1012
#import "FlutterBinaryMessenger.h"
1113
#import "FlutterDartProject.h"
1214
#import "FlutterMacros.h"
@@ -15,6 +17,17 @@
1517

1618
// TODO: Merge this file with the iOS FlutterEngine.h.
1719

20+
/**
21+
* The view ID for APIs that don't support multi-view.
22+
*
23+
* Some single-view APIs will eventually be replaced by their multi-view
24+
* variant. During the deprecation period, the single-view APIs will coexist with
25+
* and work with the multi-view APIs as if the other views don't exist. For
26+
* backward compatibility, single-view APIs will always operate the view with
27+
* this ID. Also, the first view assigned to the engine will also have this ID.
28+
*/
29+
extern const uint64_t kFlutterDefaultViewId;
30+
1831
@class FlutterViewController;
1932

2033
/**
@@ -67,6 +80,15 @@ FLUTTER_DARWIN_EXPORT
6780
*
6881
* The default view always has ID kFlutterDefaultViewId, and is the view
6982
* operated by the APIs that do not have a view ID specified.
83+
*
84+
* Setting this field from nil to a non-nil view controller also updates
85+
* the view controller's engine and ID.
86+
*
87+
* Setting this field from non-nil to nil will terminate the engine if
88+
* allowHeadlessExecution is NO.
89+
*
90+
* Setting this field from non-nil to a different non-nil FlutterViewController
91+
* is prohibited and will throw an assertion error.
7092
*/
7193
@property(nonatomic, nullable, weak) FlutterViewController* viewController;
7294

‎shell/platform/darwin/macos/framework/Headers/FlutterViewController.h‎

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,25 @@ typedef NS_ENUM(NSInteger, FlutterMouseTrackingMode) {
2525

2626
/**
2727
* Controls a view that displays Flutter content and manages input.
28+
*
29+
* A FlutterViewController works with a FlutterEngine. Upon creation, the view
30+
* controller is always added to an engine, either a given engine, or it implicitly
31+
* creates an engine and add itself to that engine.
32+
*
33+
* The FlutterEngine assigns each view controller attached to it a unique ID.
34+
* Each view controller corresponds to a view, and the ID is used by the framework
35+
* to specify which view to operate.
36+
*
37+
* A FlutterViewController can also be unattached to an engine after it is manually
38+
* unset from the engine, or transiently during the initialization process.
39+
* An unattached view controller is invalid. Whether the view controller is attached
40+
* can be queried using FlutterViewController#attached.
41+
*
42+
* The FlutterViewController strongly references the FlutterEngine, while
43+
* the engine weakly the view controller. When a FlutterViewController is deallocated,
44+
* it automatically removes itself from its attached engine. When a FlutterEngine
45+
* has no FlutterViewControllers attached, it might shut down itself or not depending
46+
* on its configuration.
2847
*/
2948
FLUTTER_DARWIN_EXPORT
3049
@interface FlutterViewController : NSViewController <FlutterPluginRegistry>
@@ -34,6 +53,16 @@ FLUTTER_DARWIN_EXPORT
3453
*/
3554
@property(nonatomic, nonnull, readonly) FlutterEngine* engine;
3655

56+
/**
57+
* The identifier for this view controller.
58+
*
59+
* The ID is assigned by FlutterEngine when the view controller is attached.
60+
*
61+
* If the view controller is unattached (see FlutterViewController#attached),
62+
* reading this property throws an assertion.
63+
*/
64+
@property(nonatomic, readonly) uint64_t id;
65+
3766
/**
3867
* The style of mouse tracking to use for the view. Defaults to
3968
* FlutterMouseTrackingModeInKeyWindow.
@@ -43,6 +72,12 @@ FLUTTER_DARWIN_EXPORT
4372
/**
4473
* Initializes a controller that will run the given project.
4574
*
75+
* In this initializer, this controller creates an engine, and is attached to
76+
* that engine as the default controller. In this way, this controller can not
77+
* be set to other engines. This initializer is suitable for the first Flutter
78+
* view controller of the app. To use the controller with an existing engine,
79+
* use initWithEngine:nibName:bundle: instead.
80+
*
4681
* @param project The project to run in this view controller. If nil, a default `FlutterDartProject`
4782
* will be used.
4883
*/
@@ -56,7 +91,8 @@ FLUTTER_DARWIN_EXPORT
5691
/**
5792
* Initializes this FlutterViewController with the specified `FlutterEngine`.
5893
*
59-
* The initialized viewcontroller will attach itself to the engine as part of this process.
94+
* This initializer is suitable for both the first Flutter view controller and
95+
* the following ones of the app.
6096
*
6197
* @param engine The `FlutterEngine` instance to attach to. Cannot be nil.
6298
* @param nibName The NIB name to initialize this controller with.
@@ -65,6 +101,12 @@ FLUTTER_DARWIN_EXPORT
65101
- (nonnull instancetype)initWithEngine:(nonnull FlutterEngine*)engine
66102
nibName:(nullable NSString*)nibName
67103
bundle:(nullable NSBundle*)nibBundle NS_DESIGNATED_INITIALIZER;
104+
105+
/**
106+
* Return YES if the view controller is attached to an engine.
107+
*/
108+
- (BOOL)attached;
109+
68110
/**
69111
* Invoked by the engine right before the engine is restarted.
70112
*

‎shell/platform/darwin/macos/framework/Source/AccessibilityBridgeMacTest.mm‎

Lines changed: 11 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -51,26 +51,22 @@ @implementation AccessibilityBridgeTestEngine
5151
namespace {
5252

5353
// Returns an engine configured for the text fixture resource configuration.
54-
FlutterEngine* CreateTestEngine() {
54+
FlutterViewController* CreateTestViewController() {
5555
NSString* fixtures = @(testing::GetFixturesPath());
5656
FlutterDartProject* project = [[FlutterDartProject alloc]
5757
initWithAssetsPath:fixtures
5858
ICUDataPath:[fixtures stringByAppendingString:@"/icudtl.dat"]];
59-
return [[AccessibilityBridgeTestEngine alloc] initWithName:@"test"
60-
project:project
61-
allowHeadlessExecution:true];
59+
FlutterEngine* engine = [[AccessibilityBridgeTestEngine alloc] initWithName:@"test"
60+
project:project
61+
allowHeadlessExecution:true];
62+
return [[FlutterViewController alloc] initWithEngine:engine nibName:nil bundle:nil];
6263
}
6364
} // namespace
6465

6566
TEST(AccessibilityBridgeMacTest, sendsAccessibilityCreateNotificationToWindowOfFlutterView) {
66-
FlutterEngine* engine = CreateTestEngine();
67-
NSString* fixtures = @(testing::GetFixturesPath());
68-
FlutterDartProject* project = [[FlutterDartProject alloc]
69-
initWithAssetsPath:fixtures
70-
ICUDataPath:[fixtures stringByAppendingString:@"/icudtl.dat"]];
71-
FlutterViewController* viewController = [[FlutterViewController alloc] initWithProject:project];
67+
FlutterViewController* viewController = CreateTestViewController();
68+
FlutterEngine* engine = viewController.engine;
7269
[viewController loadView];
73-
[engine setViewController:viewController];
7470

7571
NSWindow* expectedTarget = [[NSWindow alloc] initWithContentRect:NSMakeRect(0, 0, 800, 600)
7672
styleMask:NSBorderlessWindowMask
@@ -122,14 +118,9 @@ @implementation AccessibilityBridgeTestEngine
122118
}
123119

124120
TEST(AccessibilityBridgeMacTest, doesNotSendAccessibilityCreateNotificationWhenHeadless) {
125-
FlutterEngine* engine = CreateTestEngine();
126-
NSString* fixtures = @(testing::GetFixturesPath());
127-
FlutterDartProject* project = [[FlutterDartProject alloc]
128-
initWithAssetsPath:fixtures
129-
ICUDataPath:[fixtures stringByAppendingString:@"/icudtl.dat"]];
130-
FlutterViewController* viewController = [[FlutterViewController alloc] initWithProject:project];
121+
FlutterViewController* viewController = CreateTestViewController();
122+
FlutterEngine* engine = viewController.engine;
131123
[viewController loadView];
132-
[engine setViewController:viewController];
133124
// Setting up bridge so that the AccessibilityBridgeMacDelegateSpy
134125
// can query semantics information from.
135126
engine.semanticsEnabled = YES;
@@ -173,15 +164,9 @@ @implementation AccessibilityBridgeTestEngine
173164
}
174165

175166
TEST(AccessibilityBridgeMacTest, doesNotSendAccessibilityCreateNotificationWhenNoWindow) {
176-
FlutterEngine* engine = CreateTestEngine();
177-
// Create a view controller without attaching it to a window.
178-
NSString* fixtures = @(testing::GetFixturesPath());
179-
FlutterDartProject* project = [[FlutterDartProject alloc]
180-
initWithAssetsPath:fixtures
181-
ICUDataPath:[fixtures stringByAppendingString:@"/icudtl.dat"]];
182-
FlutterViewController* viewController = [[FlutterViewController alloc] initWithProject:project];
167+
FlutterViewController* viewController = CreateTestViewController();
168+
FlutterEngine* engine = viewController.engine;
183169
[viewController loadView];
184-
[engine setViewController:viewController];
185170

186171
// Setting up bridge so that the AccessibilityBridgeMacDelegateSpy
187172
// can query semantics information from.

‎shell/platform/darwin/macos/framework/Source/FlutterCompositor.h‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010

1111
#include "flutter/fml/macros.h"
1212
#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterPlatformViewController.h"
13-
#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterView.h"
1413
#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterViewProvider.h"
1514
#include "flutter/shell/platform/embedder/embedder.h"
1615

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

Lines changed: 46 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@
1919
#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterViewEngineProvider.h"
2020
#include "flutter/shell/platform/embedder/embedder.h"
2121

22+
const uint64_t kFlutterDefaultViewId = 0;
23+
2224
/**
2325
* Constructs and returns a FlutterLocale struct corresponding to |locale|, which must outlive
2426
* the returned struct.
@@ -80,6 +82,8 @@ @interface FlutterEngine () <FlutterBinaryMessenger>
8082
*/
8183
@property(nonatomic, strong) NSMutableArray<NSNumber*>* isResponseValid;
8284

85+
- (nullable FlutterViewController*)viewControllerForId:(uint64_t)viewId;
86+
8387
/**
8488
* Sends the list of user-preferred locales to the Flutter engine.
8589
*/
@@ -213,8 +217,6 @@ @implementation FlutterEngine {
213217
// when the engine is destroyed.
214218
std::unique_ptr<flutter::FlutterCompositor> _macOSCompositor;
215219

216-
FlutterViewEngineProvider* _viewProvider;
217-
218220
// FlutterCompositor is copied and used in embedder.cc.
219221
FlutterCompositor _compositor;
220222

@@ -248,7 +250,6 @@ - (instancetype)initWithName:(NSString*)labelPrefix
248250
_currentMessengerConnection = 1;
249251
_allowHeadlessExecution = allowHeadlessExecution;
250252
_semanticsEnabled = NO;
251-
_viewProvider = [[FlutterViewEngineProvider alloc] initWithEngine:self];
252253
_isResponseValid = [[NSMutableArray alloc] initWithCapacity:1];
253254
[_isResponseValid addObject:@YES];
254255

@@ -411,29 +412,41 @@ - (void)loadAOTData:(NSString*)assetsDir {
411412
}
412413

413414
- (void)setViewController:(FlutterViewController*)controller {
414-
if (_viewController != controller) {
415+
if (_viewController == controller) {
416+
// From nil to nil, or from non-nil to the same controller.
417+
return;
418+
}
419+
if (_viewController == nil && controller != nil) {
420+
// From nil to non-nil.
421+
NSAssert(controller.engine == nil,
422+
@"Failed to set view controller to the engine: "
423+
@"The given FlutterViewController is already attached to an engine %@. "
424+
@"If you wanted to create an FlutterViewController and set it to an existing engine, "
425+
@"you should create it with init(engine:, nibName, bundle:) instead.",
426+
controller.engine);
415427
_viewController = controller;
416-
417-
if (_semanticsEnabled && _bridge) {
418-
_bridge->UpdateDefaultViewController(_viewController);
419-
}
420-
421-
if (!controller && !_allowHeadlessExecution) {
428+
[_viewController attachToEngine:self withId:kFlutterDefaultViewId];
429+
} else if (_viewController != nil && controller == nil) {
430+
// From non-nil to nil.
431+
[_viewController detachFromEngine];
432+
_viewController = nil;
433+
if (!_allowHeadlessExecution) {
422434
[self shutDownEngine];
423435
}
436+
} else {
437+
// From non-nil to a different non-nil view controller.
438+
NSAssert(NO,
439+
@"Failed to set view controller to the engine: "
440+
@"The engine already has a default view controller %@. "
441+
@"If you wanted to make the default view render in a different window, "
442+
@"you should attach the current view controller to the window instead.",
443+
_viewController);
424444
}
425445
}
426446

427447
- (FlutterCompositor*)createFlutterCompositor {
428-
// TODO(richardjcai): Add support for creating a FlutterCompositor
429-
// with a nil _viewController for headless engines.
430-
// https://github.com/flutter/flutter/issues/71606
431-
if (!_viewController) {
432-
return nil;
433-
}
434-
435-
_macOSCompositor =
436-
std::make_unique<flutter::FlutterCompositor>(_viewProvider, _platformViewController);
448+
_macOSCompositor = std::make_unique<flutter::FlutterCompositor>(
449+
[[FlutterViewEngineProvider alloc] initWithEngine:self], _platformViewController);
437450

438451
_compositor = {};
439452
_compositor.struct_size = sizeof(FlutterCompositor);
@@ -539,10 +552,10 @@ - (nonnull NSString*)executableName {
539552
}
540553

541554
- (void)updateWindowMetrics {
542-
if (!_engine || !_viewController.viewLoaded) {
555+
if (!_engine || !self.viewController.viewLoaded) {
543556
return;
544557
}
545-
NSView* view = _viewController.flutterView;
558+
NSView* view = self.viewController.flutterView;
546559
CGRect scaledBounds = [view convertRectToBacking:view.bounds];
547560
CGSize scaledSize = scaledBounds.size;
548561
double pixelRatio = view.bounds.size.width == 0 ? 1 : scaledSize.width / view.bounds.size.width;
@@ -603,6 +616,17 @@ - (FlutterPlatformViewController*)platformViewController {
603616

604617
#pragma mark - Private methods
605618

619+
- (FlutterViewController*)viewControllerForId:(uint64_t)viewId {
620+
// TODO(dkwingsmt): The engine only supports single-view, therefore it
621+
// only processes the default ID. After the engine supports multiple views,
622+
// this method should be able to return the view for any IDs.
623+
NSAssert(viewId == kFlutterDefaultViewId, @"Unexpected view ID %llu", viewId);
624+
if (viewId == kFlutterDefaultViewId) {
625+
return _viewController;
626+
}
627+
return nil;
628+
}
629+
606630
- (void)sendUserLocales {
607631
if (!self.running) {
608632
return;
@@ -679,9 +703,7 @@ - (void)shutDownEngine {
679703
return;
680704
}
681705

682-
if (_viewController && _viewController.flutterView) {
683-
[_viewController.flutterView shutdown];
684-
}
706+
[self.viewController.flutterView shutdown];
685707

686708
FlutterEngineResult result = _embedderAPI.Deinitialize(_engine);
687709
if (result != kSuccess) {

0 commit comments

Comments
 (0)