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

[macOS] Make FlutterEngine support multiple views #37976

Merged
merged 14 commits into from
Feb 9, 2023
Merged

[macOS] Make FlutterEngine support multiple views #37976

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
74503c6
Rebase all
dkwingsmt Jan 26, 2023
e249299
Do not remove views on hot restart
dkwingsmt Jan 26, 2023
f5335ea
Remove log, fix doc
dkwingsmt Jan 26, 2023
fd7d3d9
Apply suggestions from code review
dkwingsmt Feb 2, 2023
d705c82
Merge remote-tracking branch 'origin/main' into multiview-engine
dkwingsmt Feb 2, 2023
3ac89d9
getDefaultView
dkwingsmt Feb 2, 2023
0f44c6d
postpone deprecation. Add TODO
dkwingsmt Feb 3, 2023
9de9616
Apply suggestions from code review
dkwingsmt Feb 6, 2023
ad69a76
Merge remote-tracking branch 'origin/main' into multiview-engine
dkwingsmt Feb 7, 2023
c05f0e8
Change to single
dkwingsmt Feb 7, 2023
c2ebe66
Update FlutterEngine.h
dkwingsmt Feb 9, 2023
0625db7
Update FlutterEngine_Internal.h
dkwingsmt Feb 9, 2023
63a41c9
Merge remote-tracking branch 'origin/main' into multiview-engine
dkwingsmt Feb 9, 2023
d063600
Merge remote-tracking branch 'dkwingsmt/multiview-engine' into multiv…
dkwingsmt Feb 9, 2023
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
8 changes: 5 additions & 3 deletions shell/platform/darwin/macos/framework/Headers/FlutterEngine.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@ extern const uint64_t kFlutterDefaultViewId;

/**
* Coordinates a single instance of execution of a Flutter engine.
*
* A FlutterEngine can only be attached with one controller from the native
* code.
*/
FLUTTER_DARWIN_EXPORT
@interface FlutterEngine : NSObject <FlutterTextureRegistry, FlutterPluginRegistry>
Expand Down Expand Up @@ -76,10 +79,9 @@ FLUTTER_DARWIN_EXPORT
- (BOOL)runWithEntrypoint:(nullable NSString*)entrypoint;

/**
* The default `FlutterViewController` associated with this engine, if any.
* The `FlutterViewController` of this engine, if any.
*
* The default view always has ID kFlutterDefaultViewId, and is the view
* operated by the APIs that do not have a view ID specified.
* This view is used by legacy APIs that assume a single view.
*
* Setting this field from nil to a non-nil view controller also updates
* the view controller's engine and ID.
Expand Down
14 changes: 10 additions & 4 deletions shell/platform/darwin/macos/framework/Headers/FlutterPluginRegistrarMacOS.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,18 @@ FLUTTER_DARWIN_EXPORT
@property(nonnull, readonly) id<FlutterTextureRegistry> textures;

/**
* The view displaying Flutter content. May return |nil|, for instance in a headless environment.
* The default view displaying Flutter content.
*
* WARNING: If/when multiple Flutter views within the same application are supported (#30701), this
* API will change.
* This method may return |nil|, for instance in a headless environment.
*
* The default view is a special view operated by single-view APIs.
*/
- (nullable NSView*)view;
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know if it's the same as the internal breakage, but this broke file_selector_macos, and would potentially break any other Swift plugin as well. In Obj-C the same syntax can be used to call methods and access properties, but that's not the case in Swift, so changing a property to a method is a breaking change, which would make supporting both master and stable with a single plugin impossible. We should not change this without a very compelling reason.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this is exactly why this PR was reverted. I did this only because I thought it was equivalent (for objC). I'll change it back to proprty syntax.


/**
* The `NSView` associated with the given view ID, if any.
*/
@property(nullable, readonly) NSView* view;
- (nullable NSView*)viewForId:(uint64_t)viewId;

/**
* Registers |delegate| to receive handleMethodCall:result: callbacks for the given |channel|.
Expand Down
4 changes: 3 additions & 1 deletion shell/platform/darwin/macos/framework/Headers/FlutterViewController.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,9 @@ FLUTTER_DARWIN_EXPORT
NS_DESIGNATED_INITIALIZER;
- (nonnull instancetype)initWithCoder:(nonnull NSCoder*)nibNameOrNil NS_DESIGNATED_INITIALIZER;
/**
* Initializes this FlutterViewController with the specified `FlutterEngine`.
* Initializes this FlutterViewController with an existing `FlutterEngine`.
Copy link
Member

Choose a reason for hiding this comment

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

Should there be an empty line between this comment and the previous method?

*
* The initialized view controller will add itself to the engine as part of this process.
*
* This initializer is suitable for both the first Flutter view controller and
* the following ones of the app.
Expand Down
4 changes: 2 additions & 2 deletions shell/platform/darwin/macos/framework/Source/FlutterCompositor.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
// TODO(dkwingsmt): This class only supports single-view for now. As more
// classes are gradually converted to multi-view, it should get the view ID
// from somewhere.
FlutterView* view = [view_provider_ getView:kFlutterDefaultViewId];
FlutterView* view = [view_provider_ viewForId:kFlutterDefaultViewId];
if (!view) {
return false;
}
Expand All @@ -37,7 +37,7 @@
bool FlutterCompositor::Present(uint64_t view_id,
const FlutterLayer** layers,
size_t layers_count) {
FlutterView* view = [view_provider_ getView:view_id];
FlutterView* view = [view_provider_ viewForId:view_id];
if (!view) {
return false;
}
Expand Down
4 changes: 3 additions & 1 deletion shell/platform/darwin/macos/framework/Source/FlutterCompositorTest.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterViewProvider.h"
#import "flutter/testing/testing.h"

extern const uint64_t kFlutterDefaultViewId;
Copy link
Member

Choose a reason for hiding this comment

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

I'm assuming the reason this is a const global rather than constexpr is that it's part of our public headers and therefore we need it to be pure Obj-C and not Obj-C++?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure which header to put the constexpr to, because the files that rely on this value might not all import the same header. In this way I can put the declaration in any header file and just use it. Also, you can say in this way we're hiding the actual value of this magic value.

Do you think I should try to change it?


@interface FlutterViewMockProvider : NSObject <FlutterViewProvider> {
FlutterView* _defaultView;
}
Expand All @@ -30,7 +32,7 @@ - (nonnull instancetype)initWithDefaultView:(nonnull FlutterView*)view {
return self;
}

- (nullable FlutterView*)getView:(uint64_t)viewId {
- (nullable FlutterView*)viewForId:(uint64_t)viewId {
if (viewId == kFlutterDefaultViewId) {
return _defaultView;
}
Expand Down
Loading