Skip to content

Commit

Permalink
Revert "[macOS] Formalize FlutterViewController's initialization flow…
Browse files Browse the repository at this point in the history
…, and prohibit replacing (#38981)"

This reverts commit e3b2782.
  • Loading branch information
dkwingsmt committed Jan 25, 2023
1 parent e3b2782 commit 4ba3381
Show file tree
Hide file tree
Showing 14 changed files with 301 additions and 282 deletions.
22 changes: 0 additions & 22 deletions shell/platform/darwin/macos/framework/Headers/FlutterEngine.h
Expand Up @@ -7,8 +7,6 @@

#import <Foundation/Foundation.h>

#include <stdint.h>

#import "FlutterBinaryMessenger.h"
#import "FlutterDartProject.h"
#import "FlutterMacros.h"
Expand All @@ -17,17 +15,6 @@

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

/**
* The view ID for APIs that don't support multi-view.
*
* Some single-view APIs will eventually be replaced by their multi-view
* variant. During the deprecation period, the single-view APIs will coexist with
* and work with the multi-view APIs as if the other views don't exist. For
* backward compatibility, single-view APIs will always operate the view with
* this ID. Also, the first view assigned to the engine will also have this ID.
*/
extern const uint64_t kFlutterDefaultViewId;

@class FlutterViewController;

/**
Expand Down Expand Up @@ -80,15 +67,6 @@ FLUTTER_DARWIN_EXPORT
*
* The default view always has ID kFlutterDefaultViewId, and is the view
* operated by the APIs that do not have a view ID specified.
*
* Setting this field from nil to a non-nil view controller also updates
* the view controller's engine and ID.
*
* Setting this field from non-nil to nil will terminate the engine if
* allowHeadlessExecution is NO.
*
* Setting this field from non-nil to a different non-nil FlutterViewController
* is prohibited and will throw an assertion error.
*/
@property(nonatomic, nullable, weak) FlutterViewController* viewController;

Expand Down
Expand Up @@ -25,25 +25,6 @@ typedef NS_ENUM(NSInteger, FlutterMouseTrackingMode) {

/**
* Controls a view that displays Flutter content and manages input.
*
* A FlutterViewController works with a FlutterEngine. Upon creation, the view
* controller is always added to an engine, either a given engine, or it implicitly
* creates an engine and add itself to that engine.
*
* The FlutterEngine assigns each view controller attached to it a unique ID.
* Each view controller corresponds to a view, and the ID is used by the framework
* to specify which view to operate.
*
* A FlutterViewController can also be unattached to an engine after it is manually
* unset from the engine, or transiently during the initialization process.
* An unattached view controller is invalid. Whether the view controller is attached
* can be queried using FlutterViewController#attached.
*
* The FlutterViewController strongly references the FlutterEngine, while
* the engine weakly the view controller. When a FlutterViewController is deallocated,
* it automatically removes itself from its attached engine. When a FlutterEngine
* has no FlutterViewControllers attached, it might shut down itself or not depending
* on its configuration.
*/
FLUTTER_DARWIN_EXPORT
@interface FlutterViewController : NSViewController <FlutterPluginRegistry>
Expand All @@ -53,16 +34,6 @@ FLUTTER_DARWIN_EXPORT
*/
@property(nonatomic, nonnull, readonly) FlutterEngine* engine;

/**
* The identifier for this view controller.
*
* The ID is assigned by FlutterEngine when the view controller is attached.
*
* If the view controller is unattached (see FlutterViewController#attached),
* reading this property throws an assertion.
*/
@property(nonatomic, readonly) uint64_t id;

/**
* The style of mouse tracking to use for the view. Defaults to
* FlutterMouseTrackingModeInKeyWindow.
Expand All @@ -72,12 +43,6 @@ FLUTTER_DARWIN_EXPORT
/**
* Initializes a controller that will run the given project.
*
* In this initializer, this controller creates an engine, and is attached to
* that engine as the default controller. In this way, this controller can not
* be set to other engines. This initializer is suitable for the first Flutter
* view controller of the app. To use the controller with an existing engine,
* use initWithEngine:nibName:bundle: instead.
*
* @param project The project to run in this view controller. If nil, a default `FlutterDartProject`
* will be used.
*/
Expand All @@ -91,8 +56,7 @@ FLUTTER_DARWIN_EXPORT
/**
* Initializes this FlutterViewController with the specified `FlutterEngine`.
*
* This initializer is suitable for both the first Flutter view controller and
* the following ones of the app.
* The initialized viewcontroller will attach itself to the engine as part of this process.
*
* @param engine The `FlutterEngine` instance to attach to. Cannot be nil.
* @param nibName The NIB name to initialize this controller with.
Expand All @@ -101,12 +65,6 @@ FLUTTER_DARWIN_EXPORT
- (nonnull instancetype)initWithEngine:(nonnull FlutterEngine*)engine
nibName:(nullable NSString*)nibName
bundle:(nullable NSBundle*)nibBundle NS_DESIGNATED_INITIALIZER;

/**
* Return YES if the view controller is attached to an engine.
*/
- (BOOL)attached;

/**
* Invoked by the engine right before the engine is restarted.
*
Expand Down
Expand Up @@ -51,22 +51,26 @@ @implementation AccessibilityBridgeTestEngine
namespace {

// Returns an engine configured for the text fixture resource configuration.
FlutterViewController* CreateTestViewController() {
FlutterEngine* CreateTestEngine() {
NSString* fixtures = @(testing::GetFixturesPath());
FlutterDartProject* project = [[FlutterDartProject alloc]
initWithAssetsPath:fixtures
ICUDataPath:[fixtures stringByAppendingString:@"/icudtl.dat"]];
FlutterEngine* engine = [[AccessibilityBridgeTestEngine alloc] initWithName:@"test"
project:project
allowHeadlessExecution:true];
return [[FlutterViewController alloc] initWithEngine:engine nibName:nil bundle:nil];
return [[AccessibilityBridgeTestEngine alloc] initWithName:@"test"
project:project
allowHeadlessExecution:true];
}
} // namespace

TEST(AccessibilityBridgeMacTest, sendsAccessibilityCreateNotificationToWindowOfFlutterView) {
FlutterViewController* viewController = CreateTestViewController();
FlutterEngine* engine = viewController.engine;
FlutterEngine* engine = CreateTestEngine();
NSString* fixtures = @(testing::GetFixturesPath());
FlutterDartProject* project = [[FlutterDartProject alloc]
initWithAssetsPath:fixtures
ICUDataPath:[fixtures stringByAppendingString:@"/icudtl.dat"]];
FlutterViewController* viewController = [[FlutterViewController alloc] initWithProject:project];
[viewController loadView];
[engine setViewController:viewController];

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

TEST(AccessibilityBridgeMacTest, doesNotSendAccessibilityCreateNotificationWhenHeadless) {
FlutterViewController* viewController = CreateTestViewController();
FlutterEngine* engine = viewController.engine;
FlutterEngine* engine = CreateTestEngine();
NSString* fixtures = @(testing::GetFixturesPath());
FlutterDartProject* project = [[FlutterDartProject alloc]
initWithAssetsPath:fixtures
ICUDataPath:[fixtures stringByAppendingString:@"/icudtl.dat"]];
FlutterViewController* viewController = [[FlutterViewController alloc] initWithProject:project];
[viewController loadView];
[engine setViewController:viewController];
// Setting up bridge so that the AccessibilityBridgeMacDelegateSpy
// can query semantics information from.
engine.semanticsEnabled = YES;
Expand Down Expand Up @@ -164,9 +173,15 @@ @implementation AccessibilityBridgeTestEngine
}

TEST(AccessibilityBridgeMacTest, doesNotSendAccessibilityCreateNotificationWhenNoWindow) {
FlutterViewController* viewController = CreateTestViewController();
FlutterEngine* engine = viewController.engine;
FlutterEngine* engine = CreateTestEngine();
// Create a view controller without attaching it to a window.
NSString* fixtures = @(testing::GetFixturesPath());
FlutterDartProject* project = [[FlutterDartProject alloc]
initWithAssetsPath:fixtures
ICUDataPath:[fixtures stringByAppendingString:@"/icudtl.dat"]];
FlutterViewController* viewController = [[FlutterViewController alloc] initWithProject:project];
[viewController loadView];
[engine setViewController:viewController];

// Setting up bridge so that the AccessibilityBridgeMacDelegateSpy
// can query semantics information from.
Expand Down
Expand Up @@ -10,6 +10,7 @@

#include "flutter/fml/macros.h"
#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterPlatformViewController.h"
#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterView.h"
#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterViewProvider.h"
#include "flutter/shell/platform/embedder/embedder.h"

Expand Down
70 changes: 24 additions & 46 deletions shell/platform/darwin/macos/framework/Source/FlutterEngine.mm
Expand Up @@ -19,8 +19,6 @@
#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterViewEngineProvider.h"
#include "flutter/shell/platform/embedder/embedder.h"

const uint64_t kFlutterDefaultViewId = 0;

/**
* Constructs and returns a FlutterLocale struct corresponding to |locale|, which must outlive
* the returned struct.
Expand Down Expand Up @@ -82,8 +80,6 @@ @interface FlutterEngine () <FlutterBinaryMessenger>
*/
@property(nonatomic, strong) NSMutableArray<NSNumber*>* isResponseValid;

- (nullable FlutterViewController*)viewControllerForId:(uint64_t)viewId;

/**
* Sends the list of user-preferred locales to the Flutter engine.
*/
Expand Down Expand Up @@ -217,6 +213,8 @@ @implementation FlutterEngine {
// when the engine is destroyed.
std::unique_ptr<flutter::FlutterCompositor> _macOSCompositor;

FlutterViewEngineProvider* _viewProvider;

// FlutterCompositor is copied and used in embedder.cc.
FlutterCompositor _compositor;

Expand Down Expand Up @@ -250,6 +248,7 @@ - (instancetype)initWithName:(NSString*)labelPrefix
_currentMessengerConnection = 1;
_allowHeadlessExecution = allowHeadlessExecution;
_semanticsEnabled = NO;
_viewProvider = [[FlutterViewEngineProvider alloc] initWithEngine:self];
_isResponseValid = [[NSMutableArray alloc] initWithCapacity:1];
[_isResponseValid addObject:@YES];

Expand Down Expand Up @@ -412,41 +411,29 @@ - (void)loadAOTData:(NSString*)assetsDir {
}

- (void)setViewController:(FlutterViewController*)controller {
if (_viewController == controller) {
// From nil to nil, or from non-nil to the same controller.
return;
}
if (_viewController == nil && controller != nil) {
// From nil to non-nil.
NSAssert(controller.engine == nil,
@"Failed to set view controller to the engine: "
@"The given FlutterViewController is already attached to an engine %@. "
@"If you wanted to create an FlutterViewController and set it to an existing engine, "
@"you should create it with init(engine:, nibName, bundle:) instead.",
controller.engine);
if (_viewController != controller) {
_viewController = controller;
[_viewController attachToEngine:self withId:kFlutterDefaultViewId];
} else if (_viewController != nil && controller == nil) {
// From non-nil to nil.
[_viewController detachFromEngine];
_viewController = nil;
if (!_allowHeadlessExecution) {

if (_semanticsEnabled && _bridge) {
_bridge->UpdateDefaultViewController(_viewController);
}

if (!controller && !_allowHeadlessExecution) {
[self shutDownEngine];
}
} else {
// From non-nil to a different non-nil view controller.
NSAssert(NO,
@"Failed to set view controller to the engine: "
@"The engine already has a default view controller %@. "
@"If you wanted to make the default view render in a different window, "
@"you should attach the current view controller to the window instead.",
_viewController);
}
}

- (FlutterCompositor*)createFlutterCompositor {
_macOSCompositor = std::make_unique<flutter::FlutterCompositor>(
[[FlutterViewEngineProvider alloc] initWithEngine:self], _platformViewController);
// TODO(richardjcai): Add support for creating a FlutterCompositor
// with a nil _viewController for headless engines.
// https://github.com/flutter/flutter/issues/71606
if (!_viewController) {
return nil;
}

_macOSCompositor =
std::make_unique<flutter::FlutterCompositor>(_viewProvider, _platformViewController);

_compositor = {};
_compositor.struct_size = sizeof(FlutterCompositor);
Expand Down Expand Up @@ -552,10 +539,10 @@ - (nonnull NSString*)executableName {
}

- (void)updateWindowMetrics {
if (!_engine || !self.viewController.viewLoaded) {
if (!_engine || !_viewController.viewLoaded) {
return;
}
NSView* view = self.viewController.flutterView;
NSView* view = _viewController.flutterView;
CGRect scaledBounds = [view convertRectToBacking:view.bounds];
CGSize scaledSize = scaledBounds.size;
double pixelRatio = view.bounds.size.width == 0 ? 1 : scaledSize.width / view.bounds.size.width;
Expand Down Expand Up @@ -616,17 +603,6 @@ - (FlutterPlatformViewController*)platformViewController {

#pragma mark - Private methods

- (FlutterViewController*)viewControllerForId:(uint64_t)viewId {
// TODO(dkwingsmt): The engine only supports single-view, therefore it
// only processes the default ID. After the engine supports multiple views,
// this method should be able to return the view for any IDs.
NSAssert(viewId == kFlutterDefaultViewId, @"Unexpected view ID %llu", viewId);
if (viewId == kFlutterDefaultViewId) {
return _viewController;
}
return nil;
}

- (void)sendUserLocales {
if (!self.running) {
return;
Expand Down Expand Up @@ -703,7 +679,9 @@ - (void)shutDownEngine {
return;
}

[self.viewController.flutterView shutdown];
if (_viewController && _viewController.flutterView) {
[_viewController.flutterView shutdown];
}

FlutterEngineResult result = _embedderAPI.Deinitialize(_engine);
if (result != kSuccess) {
Expand Down

0 comments on commit 4ba3381

Please sign in to comment.