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 4 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
4 changes: 2 additions & 2 deletions runtime/runtime_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@

namespace flutter {

const uint64_t kFlutterDefaultViewId = 0llu;
const uint64_t kFlutterImplicitViewId = 0llu;

RuntimeController::RuntimeController(RuntimeDelegate& p_client,
const TaskRunners& task_runners)
Expand Down Expand Up @@ -320,7 +320,7 @@ void RuntimeController::ScheduleFrame() {
// |PlatformConfigurationClient|
void RuntimeController::Render(Scene* scene) {
// TODO(dkwingsmt): Currently only supports a single window.
int64_t view_id = kFlutterDefaultViewId;
int64_t view_id = kFlutterImplicitViewId;
auto window =
UIDartState::Current()->platform_configuration()->get_window(view_id);
if (window == nullptr) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,30 @@ FLUTTER_DARWIN_EXPORT
@property(nonnull, readonly) id<FlutterTextureRegistry> textures;

/**
* The default view displaying Flutter content.
* The view displaying Flutter content.
*
* This method may return |nil|, for instance in a headless environment.
* Currently Flutter only supports one view, and this is the view.
*
* The default view is a special view operated by single-view APIs.
* Flutter plans to support multiple views in the future. Although single-view
Copy link
Member

Choose a reason for hiding this comment

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

I feel a bit uncomfortable about making sooo many concrete promises about how this will work in the future without having an actual implementation to back this up. I would keep this a little lighter and stick to what this does today, maybe with just a one-sentence hint that at some point flutter will support multiple views and at some point after that people will eventually have to migrate away from this API after the proper deprecation period. As we implement more stuff, the doc can evolve then.

Spending 90% of the doc on describing what this may do at some unknown point in the future seems odd.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, that's why my initial PR included nothing but the current status of this property.

/**
 * The view displaying Flutter content.
 *
 * This method may return |nil|, for instance in a headless environment.
 */

What do you think of this one?

Copy link
Member

Choose a reason for hiding this comment

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

I like that more than making promises about the future 😝

Copy link
Member

Choose a reason for hiding this comment

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

I still think we should nudge users away from this property though. If we want to give minimal explanation, perhaps we could reconsider this suggestion: #43364 (comment)

The implicit view that displays Flutter content.

This property is provided for backwards compatibility for apps
that assume a single view. This will eventually be replaced by
a multi-view API variant.

This method may return |nil|, for instance in a headless environment
or for apps that do not assume a single view.

Copy link
Contributor Author

@dkwingsmt dkwingsmt Jul 7, 2023

Choose a reason for hiding this comment

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

@loic-sharma Currently there's nowhere to nudge users to. It's not about being minimal, but that everything it talks about is not there yet. Imagine if you're a user, you see this concerning comment that announces an upcoming deprecation, what can you do? You can not migrate, you just keep worrying until the official support is here?

Copy link
Contributor Author

@dkwingsmt dkwingsmt Jul 7, 2023

Choose a reason for hiding this comment

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

The idea is that we should only document the current state, not predictions or promises.

Copy link
Member

Choose a reason for hiding this comment

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

Loïc's suggestion in #43364 (comment) also seems fine. It doesn't make any promises about HOW this may work in the future.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I'll use that one.

* APIs will eventually be replaced by their multi-view variants, 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. To achieve this,
* all behaviors of "the single view" (which is called "the implicit view") are
* preserved, allowing legacy single-view APIs to continue working, while
* new-style views created by new ways must be operated by the upcoming
* multi-view APIs.
*
* Plugins written for a single view can keep operating on this view and expect
* unchanged behavior for the implicit view. This includes that:
*
* - The first view controller attached to the engine will be linked to the
* implicit view.
* - Single-view Flutter APIs will operate the implicit view.
*
* This method may return |nil| if the view is not assigned. In single-view
* apps, this means that the app is running headlessly. In multi-view apps,
* this means that the compatible mode is disabled, or that the compatible mode
* is on but no view controller has been attached to the engine yet.
*/
@property(nullable, readonly) NSView* view;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,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_ viewForId:kFlutterDefaultViewId];
FlutterView* view = [view_provider_ viewForId:kFlutterImplicitViewId];
if (!view) {
return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,27 +12,27 @@
#import "flutter/testing/testing.h"

@interface FlutterViewMockProvider : NSObject <FlutterViewProvider> {
FlutterView* _defaultView;
FlutterView* _implicitView;
}
/**
* Create a FlutterViewMockProvider with the provided view as the default view.
* Create a FlutterViewMockProvider with the provided view as the implicit view.
*/
- (nonnull instancetype)initWithDefaultView:(nonnull FlutterView*)view;
- (nonnull instancetype)initWithImplicitView:(nonnull FlutterView*)view;
@end

@implementation FlutterViewMockProvider

- (nonnull instancetype)initWithDefaultView:(nonnull FlutterView*)view {
- (nonnull instancetype)initWithImplicitView:(nonnull FlutterView*)view {
self = [super init];
if (self != nil) {
_defaultView = view;
_implicitView = view;
}
return self;
}

- (nullable FlutterView*)viewForId:(FlutterViewId)viewId {
if (viewId == kFlutterDefaultViewId) {
return _defaultView;
if (viewId == kFlutterImplicitViewId) {
return _implicitView;
}
return nil;
}
Expand Down Expand Up @@ -81,7 +81,7 @@ - (nullable FlutterView*)viewForId:(FlutterViewId)viewId {

OCMStub([surfaceMock asFlutterMetalTexture]).andReturn(texture);

return [[FlutterViewMockProvider alloc] initWithDefaultView:viewMock];
return [[FlutterViewMockProvider alloc] initWithImplicitView:viewMock];
}
} // namespace

Expand Down Expand Up @@ -131,7 +131,7 @@ - (nullable FlutterView*)viewForId:(FlutterViewId)viewId {
}};
const FlutterLayer* layers_ptr = layers;

macos_compositor->Present(kFlutterDefaultViewId, &layers_ptr, 1);
macos_compositor->Present(kFlutterImplicitViewId, &layers_ptr, 1);

ASSERT_EQ(presentedSurfaces.count, 1ul);
}
Expand Down
38 changes: 19 additions & 19 deletions shell/platform/darwin/macos/framework/Source/FlutterEngine.mm
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ - (nullable FlutterViewController*)viewControllerForId:(FlutterViewId)viewId;
* An internal method that adds the view controller with the given ID.
*
* This method assigns the controller with the ID, puts the controller into the
* map, and does assertions related to the default view ID.
* map, and does assertions related to the implicit view ID.
*/
- (void)registerViewController:(FlutterViewController*)controller forId:(FlutterViewId)viewId;

Expand Down Expand Up @@ -296,7 +296,7 @@ - (instancetype)initWithPlugin:(NSString*)pluginKey flutterEngine:(FlutterEngine
}

- (NSView*)view {
return [self viewForId:kFlutterDefaultViewId];
return [self viewForId:kFlutterImplicitViewId];
}

- (NSView*)viewForId:(FlutterViewId)viewId {
Expand Down Expand Up @@ -422,9 +422,9 @@ - (instancetype)initWithName:(NSString*)labelPrefix
[_isResponseValid addObject:@YES];
_terminationHandler = [[FlutterEngineTerminationHandler alloc] initWithEngine:self
terminator:nil];
// kFlutterDefaultViewId is reserved for the default view.
// kFlutterImplicitViewId is reserved for the implicit view.
// All IDs above it are for regular views.
_nextViewId = kFlutterDefaultViewId + 1;
_nextViewId = kFlutterImplicitViewId + 1;

_embedderAPI.struct_size = sizeof(FlutterEngineProcTable);
FlutterEngineGetProcAddresses(&_embedderAPI);
Expand Down Expand Up @@ -506,10 +506,10 @@ - (BOOL)runWithEntrypoint:(NSString*)entrypoint {
flutterArguments.update_semantics_callback2 = [](const FlutterSemanticsUpdate2* update,
void* user_data) {
// TODO(dkwingsmt): This callback only supports single-view, therefore it
// only operates on the default view. To support multi-view, we need a
// only operates on the implicit view. To support multi-view, we need a
// way to pass in the ID (probably through FlutterSemanticsUpdate).
FlutterEngine* engine = (__bridge FlutterEngine*)user_data;
[[engine viewControllerForId:kFlutterDefaultViewId] updateSemantics:update];
[[engine viewControllerForId:kFlutterImplicitViewId] updateSemantics:update];
};
flutterArguments.custom_dart_entrypoint = entrypoint.UTF8String;
flutterArguments.shutdown_dart_vm_when_done = true;
Expand Down Expand Up @@ -645,7 +645,7 @@ - (FlutterViewController*)viewControllerForId:(FlutterViewId)viewId {

- (void)setViewController:(FlutterViewController*)controller {
FlutterViewController* currentController =
[_viewControllers objectForKey:@(kFlutterDefaultViewId)];
[_viewControllers objectForKey:@(kFlutterImplicitViewId)];
if (currentController == controller) {
// From nil to nil, or from non-nil to the same controller.
return;
Expand All @@ -658,26 +658,26 @@ - (void)setViewController:(FlutterViewController*)controller {
@"If you wanted to create an FlutterViewController and set it to an existing engine, "
@"you should use FlutterViewController#init(engine:, nibName, bundle:) instead.",
controller.engine);
[self registerViewController:controller forId:kFlutterDefaultViewId];
[self registerViewController:controller forId:kFlutterImplicitViewId];
} else if (currentController != nil && controller == nil) {
NSAssert(currentController.viewId == kFlutterDefaultViewId,
NSAssert(currentController.viewId == kFlutterImplicitViewId,
@"The default controller has an unexpected ID %llu", currentController.viewId);
// From non-nil to nil.
[self deregisterViewControllerForId:kFlutterDefaultViewId];
[self deregisterViewControllerForId:kFlutterImplicitViewId];
[self shutDownIfNeeded];
} 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, "
@"The engine already has an implicit view controller %@. "
@"If you wanted to make the implicit view render in a different window, "
@"you should attach the current view controller to the window instead.",
[_viewControllers objectForKey:@(kFlutterDefaultViewId)]);
[_viewControllers objectForKey:@(kFlutterImplicitViewId)]);
}
}

- (FlutterViewController*)viewController {
return [self viewControllerForId:kFlutterDefaultViewId];
return [self viewControllerForId:kFlutterImplicitViewId];
}

- (FlutterCompositor*)createFlutterCompositor {
Expand Down Expand Up @@ -705,9 +705,9 @@ - (FlutterCompositor*)createFlutterCompositor {
void* user_data //
) {
// TODO(dkwingsmt): This callback only supports single-view, therefore it
// only operates on the default view. To support multi-view, we need a new
// only operates on the implicit view. To support multi-view, we need a new
// callback that also receives a view ID.
return reinterpret_cast<flutter::FlutterCompositor*>(user_data)->Present(kFlutterDefaultViewId,
return reinterpret_cast<flutter::FlutterCompositor*>(user_data)->Present(kFlutterImplicitViewId,
layers, layers_count);
};

Expand All @@ -725,7 +725,7 @@ - (FlutterCompositor*)createFlutterCompositor {
#pragma mark - Framework-internal methods

- (void)addViewController:(FlutterViewController*)controller {
[self registerViewController:controller forId:kFlutterDefaultViewId];
[self registerViewController:controller forId:kFlutterImplicitViewId];
}

- (void)removeViewController:(nonnull FlutterViewController*)viewController {
Expand Down Expand Up @@ -812,7 +812,7 @@ - (nonnull NSString*)executableName {
}

- (void)updateWindowMetricsForViewController:(FlutterViewController*)viewController {
if (viewController.viewId != kFlutterDefaultViewId) {
if (viewController.viewId != kFlutterImplicitViewId) {
// TODO(dkwingsmt): The embedder API only supports single-view for now. As
// embedder APIs are converted to multi-view, this method should support any
// views.
Expand Down Expand Up @@ -1078,7 +1078,7 @@ - (void)handleAccessibilityEvent:(NSDictionary<NSString*, id>*)annotatedEvent {
- (void)announceAccessibilityMessage:(NSString*)message
withPriority:(NSAccessibilityPriorityLevel)priority {
NSAccessibilityPostNotificationWithUserInfo(
[self viewControllerForId:kFlutterDefaultViewId].flutterView,
[self viewControllerForId:kFlutterImplicitViewId].flutterView,
NSAccessibilityAnnouncementRequestedNotification,
@{NSAccessibilityAnnouncementKey : message, NSAccessibilityPriorityKey : @(priority)});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
// CREATE_NATIVE_ENTRY and MOCK_ENGINE_PROC are leaky by design
// NOLINTBEGIN(clang-analyzer-core.StackAddressEscape)

constexpr int64_t kDefaultViewId = 0ll;
constexpr int64_t kImplicitViewId = 0ll;

@interface FlutterEngine (Test)
/**
Expand Down Expand Up @@ -354,7 +354,7 @@ - (nonnull NSView*)createWithViewIdentifier:(int64_t)viewId arguments:(nullable
FlutterSemanticsNode2* nodes[] = {&root, &child1};
update.nodes = nodes;
update.custom_action_count = 0;
// This call updates semantics for the default view, which does not exist,
// This call updates semantics for the implicit view, which does not exist,
// and therefore this call is invalid. But the engine should not crash.
update_semantics_callback(&update, (__bridge void*)engine);

Expand Down Expand Up @@ -632,7 +632,7 @@ - (nonnull NSView*)createWithViewIdentifier:(int64_t)viewId arguments:(nullable
[threadSynchronizer shutdown];

std::thread rasterThread([&threadSynchronizer] {
[threadSynchronizer performCommitForView:kDefaultViewId
[threadSynchronizer performCommitForView:kImplicitViewId
size:CGSizeMake(100, 100)
notify:^{
}];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ typedef NS_ENUM(NSInteger, FlutterAppExitResponse) {
*
* Practically, since FlutterEngine can only be attached with one controller,
* the given controller, if successfully attached, will always have the default
* view ID kFlutterDefaultViewId.
* view ID kFlutterImplicitViewId.
*
* The engine holds a weak reference to the attached view controller.
*
Expand All @@ -127,11 +127,11 @@ typedef NS_ENUM(NSInteger, FlutterAppExitResponse) {
/**
* Dissociate the given view controller from this engine.
*
* Practically, since FlutterEngine can only be attached with one controller,
* the given controller must be the default view controller.
*
* If the view controller is not associated with this engine, this call throws an
* assertion.
*
* Practically, since FlutterEngine can only be attached with one controller for
* now, the given controller must be the current view controller.
*/
- (void)removeViewController:(FlutterViewController*)viewController;

Expand Down
2 changes: 1 addition & 1 deletion shell/platform/darwin/macos/framework/Source/FlutterView.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ typedef int64_t FlutterViewId;
* backward compatibility, single-view APIs will always operate on the view with
* this ID. Also, the first view assigned to the engine will also have this ID.
*/
constexpr FlutterViewId kFlutterDefaultViewId = 0ll;
constexpr FlutterViewId kFlutterImplicitViewId = 0ll;

/**
* Listener for view resizing.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
.andDo(^(NSInvocation* invocation) {
FlutterViewId viewId;
[invocation getArgument:&viewId atIndex:2];
if (viewId == kFlutterDefaultViewId) {
if (viewId == kFlutterImplicitViewId) {
if (mockFlutterViewController != nil) {
[invocation setReturnValue:&mockFlutterViewController];
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

#import "flutter/testing/testing.h"

constexpr int64_t kDefaultViewId = 0ll;
constexpr int64_t kImplicitViewId = 0ll;

@interface TestReshapeListener : NSObject <FlutterViewReshapeListener>

Expand All @@ -30,6 +30,6 @@ - (void)viewDidReshape:(nonnull NSView*)view {
commandQueue:queue
reshapeListener:listener
threadSynchronizer:threadSynchronizer
viewId:kDefaultViewId];
viewId:kImplicitViewId];
EXPECT_EQ([view layer:view.layer shouldInheritContentsScale:3.0 fromWindow:view.window], YES);
}