Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[macOS] Change view ID's type to signed and a typedef #41653

Merged
merged 6 commits into from
May 5, 2023
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
11 changes: 0 additions & 11 deletions shell/platform/darwin/macos/framework/Headers/FlutterEngine.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,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 on 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
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,6 @@ FLUTTER_DARWIN_EXPORT
*/
@property(nullable, readonly) NSView* view;

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

/**
* Registers |delegate| to receive handleMethodCall:result: callbacks for the given |channel|.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,16 +53,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 viewId;

/**
* The style of mouse tracking to use for the view. Defaults to
* FlutterMouseTrackingModeInKeyWindow.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ class FlutterCompositor {

// Presents the FlutterLayers by updating the FlutterView specified by
// `view_id` using the layer content. Sets frame_started_ to false.
bool Present(uint64_t view_id, const FlutterLayer** layers, size_t layers_count);
bool Present(FlutterViewId view_id, const FlutterLayer** layers, size_t layers_count);

private:
void PresentPlatformViews(FlutterView* default_base_view,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
return true;
}

bool FlutterCompositor::Present(uint64_t view_id,
bool FlutterCompositor::Present(FlutterViewId view_id,
const FlutterLayer** layers,
size_t layers_count) {
FlutterView* view = [view_provider_ viewForId:view_id];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@
#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterViewProvider.h"
#import "flutter/testing/testing.h"

extern const uint64_t kFlutterDefaultViewId;

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

- (nullable FlutterView*)viewForId:(uint64_t)viewId {
- (nullable FlutterView*)viewForId:(FlutterViewId)viewId {
if (viewId == kFlutterDefaultViewId) {
return _defaultView;
}
Expand Down
18 changes: 9 additions & 9 deletions shell/platform/darwin/macos/framework/Source/FlutterEngine.mm
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@
#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterViewController_Internal.h"
#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterViewEngineProvider.h"

const uint64_t kFlutterDefaultViewId = 0;

NSString* const kFlutterPlatformChannel = @"flutter/platform";

/**
Expand Down Expand Up @@ -88,15 +86,15 @@ @interface FlutterEngine () <FlutterBinaryMessenger>
*/
@property(nonatomic, strong) NSMutableArray<NSNumber*>* isResponseValid;

- (nullable FlutterViewController*)viewControllerForId:(uint64_t)viewId;
- (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.
*/
- (void)registerViewController:(FlutterViewController*)controller forId:(uint64_t)viewId;
- (void)registerViewController:(FlutterViewController*)controller forId:(FlutterViewId)viewId;

/**
* An internal method that removes the view controller with the given ID.
Expand All @@ -105,7 +103,7 @@ - (void)registerViewController:(FlutterViewController*)controller forId:(uint64_
* map. This is an no-op if the view ID is not associated with any view
* controllers.
*/
- (void)deregisterViewControllerForId:(uint64_t)viewId;
- (void)deregisterViewControllerForId:(FlutterViewId)viewId;

/**
* Shuts down the engine if view requirement is not met, and headless execution
Expand Down Expand Up @@ -259,6 +257,8 @@ - (void)requestApplicationTermination:(id)sender
@interface FlutterEngineRegistrar : NSObject <FlutterPluginRegistrar>
- (instancetype)initWithPlugin:(nonnull NSString*)pluginKey
flutterEngine:(nonnull FlutterEngine*)flutterEngine;

- (NSView*)viewForId:(FlutterViewId)viewId;
@end

@implementation FlutterEngineRegistrar {
Expand Down Expand Up @@ -291,7 +291,7 @@ - (NSView*)view {
return [self viewForId:kFlutterDefaultViewId];
}

- (NSView*)viewForId:(uint64_t)viewId {
- (NSView*)viewForId:(FlutterViewId)viewId {
FlutterViewController* controller = [_flutterEngine viewControllerForId:viewId];
if (controller == nil) {
return nil;
Expand Down Expand Up @@ -570,7 +570,7 @@ - (void)loadAOTData:(NSString*)assetsDir {
}
}

- (void)registerViewController:(FlutterViewController*)controller forId:(uint64_t)viewId {
- (void)registerViewController:(FlutterViewController*)controller forId:(FlutterViewId)viewId {
NSAssert(controller != nil, @"The controller must not be nil.");
NSAssert(![controller attached],
@"The incoming view controller is already attached to an engine.");
Expand All @@ -580,7 +580,7 @@ - (void)registerViewController:(FlutterViewController*)controller forId:(uint64_
[_viewControllers setObject:controller forKey:@(viewId)];
}

- (void)deregisterViewControllerForId:(uint64_t)viewId {
- (void)deregisterViewControllerForId:(FlutterViewId)viewId {
FlutterViewController* oldController = [self viewControllerForId:viewId];
if (oldController != nil) {
[oldController detachFromEngine];
Expand All @@ -594,7 +594,7 @@ - (void)shutDownIfNeeded {
}
}

- (FlutterViewController*)viewControllerForId:(uint64_t)viewId {
- (FlutterViewController*)viewControllerForId:(FlutterViewId)viewId {
FlutterViewController* controller = [_viewControllers objectForKey:@(viewId)];
NSAssert(controller == nil || controller.viewId == viewId,
@"The stored controller has unexpected view ID.");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -646,7 +646,7 @@ - (nonnull NSView*)createWithViewIdentifier:(int64_t)viewId arguments:(nullable
@autoreleasepool {
// Create FVC1.
viewController1 = [[FlutterViewController alloc] initWithProject:project];
EXPECT_EQ(viewController1.viewId, 0ull);
EXPECT_EQ(viewController1.viewId, 0ll);

engine = viewController1.engine;
engine.viewController = nil;
Expand All @@ -663,7 +663,7 @@ - (nonnull NSView*)createWithViewIdentifier:(int64_t)viewId arguments:(nullable

engine.viewController = viewController1;
EXPECT_EQ(engine.viewController, viewController1);
EXPECT_EQ(viewController1.viewId, 0ull);
EXPECT_EQ(viewController1.viewId, 0ll);
}

TEST_F(FlutterEngineTest, ManageControllersIfInitiatedByEngine) {
Expand All @@ -677,15 +677,15 @@ - (nonnull NSView*)createWithViewIdentifier:(int64_t)viewId arguments:(nullable

@autoreleasepool {
viewController1 = [[FlutterViewController alloc] initWithEngine:engine nibName:nil bundle:nil];
EXPECT_EQ(viewController1.viewId, 0ull);
EXPECT_EQ(viewController1.viewId, 0ll);
EXPECT_EQ(engine.viewController, viewController1);

engine.viewController = nil;

FlutterViewController* viewController2 = [[FlutterViewController alloc] initWithEngine:engine
nibName:nil
bundle:nil];
EXPECT_EQ(viewController2.viewId, 0ull);
EXPECT_EQ(viewController2.viewId, 0ll);
EXPECT_EQ(engine.viewController, viewController2);
}
// FVC2 is deallocated but FVC1 is retained.
Expand All @@ -694,7 +694,7 @@ - (nonnull NSView*)createWithViewIdentifier:(int64_t)viewId arguments:(nullable

engine.viewController = viewController1;
EXPECT_EQ(engine.viewController, viewController1);
EXPECT_EQ(viewController1.viewId, 0ull);
EXPECT_EQ(viewController1.viewId, 0ll);
}

TEST_F(FlutterEngineTest, HandlesTerminationRequest) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ typedef NS_ENUM(NSInteger, FlutterAppExitResponse) {
/**
* The |FlutterViewController| associated with the given view ID, if any.
*/
- (nullable FlutterViewController*)viewControllerForId:(uint64_t)viewId;
- (nullable FlutterViewController*)viewControllerForId:(FlutterViewId)viewId;

/**
* Informs the engine that the specified view controller's window metrics have changed.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,12 @@
/**
* Called by the engine when the given view's buffers should be swapped.
*/
- (BOOL)present:(uint64_t)viewId texture:(nonnull const FlutterMetalTexture*)texture;
- (BOOL)present:(FlutterViewId)viewId texture:(nonnull const FlutterMetalTexture*)texture;

/**
* Creates a Metal texture for the given view with the given size.
*/
- (FlutterMetalTexture)createTextureForView:(uint64_t)viewId size:(CGSize)size;
- (FlutterMetalTexture)createTextureForView:(FlutterViewId)viewId size:(CGSize)size;

/**
* Populates the texture registry with the provided metalTexture.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ static FlutterMetalTexture OnGetNextDrawableForDefaultView(FlutterEngine* engine
// TODO(dkwingsmt): This callback only supports single-view, therefore it only
// operates on the default view. To support multi-view, we need a new callback
// that also receives a view ID, or pass the ID via FlutterFrameInfo.
uint64_t viewId = kFlutterDefaultViewId;
FlutterViewId viewId = kFlutterDefaultViewId;
CGSize size = CGSizeMake(frameInfo->size.width, frameInfo->size.height);
return [engine.renderer createTextureForView:viewId size:size];
}
Expand All @@ -27,7 +27,7 @@ static bool OnPresentDrawableOfDefaultView(FlutterEngine* engine,
// TODO(dkwingsmt): This callback only supports single-view, therefore it only
// operates on the default view. To support multi-view, we need a new callback
// that also receives a view ID.
uint64_t viewId = kFlutterDefaultViewId;
FlutterViewId viewId = kFlutterDefaultViewId;
return [engine.renderer present:viewId texture:texture];
}

Expand Down Expand Up @@ -88,7 +88,7 @@ - (FlutterRendererConfig)createRendererConfig {

#pragma mark - Embedder callback implementations.

- (FlutterMetalTexture)createTextureForView:(uint64_t)viewId size:(CGSize)size {
- (FlutterMetalTexture)createTextureForView:(FlutterViewId)viewId size:(CGSize)size {
FlutterView* view = [_viewProvider viewForId:viewId];
NSAssert(view != nil, @"Can't create texture on a non-existent view 0x%llx.", viewId);
if (view == nil) {
Expand All @@ -98,7 +98,7 @@ - (FlutterMetalTexture)createTextureForView:(uint64_t)viewId size:(CGSize)size {
return [view.surfaceManager surfaceForSize:size].asFlutterMetalTexture;
}

- (BOOL)present:(uint64_t)viewId texture:(const FlutterMetalTexture*)texture {
- (BOOL)present:(FlutterViewId)viewId texture:(const FlutterMetalTexture*)texture {
FlutterView* view = [_viewProvider viewForId:viewId];
if (view == nil) {
return NO;
Expand Down
13 changes: 13 additions & 0 deletions shell/platform/darwin/macos/framework/Source/FlutterView.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,19 @@

#include <stdint.h>

typedef int64_t FlutterViewId;

/**
* 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 on the view with
* this ID. Also, the first view assigned to the engine will also have this ID.
*/
constexpr FlutterViewId kFlutterDefaultViewId = 0ll;

/**
* Listener for view resizing.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -370,6 +370,8 @@ @implementation FlutterViewController {
FlutterDartProject* _project;

std::shared_ptr<flutter::AccessibilityBridgeMac> _bridge;

FlutterViewId _id;
}

@synthesize viewId = _viewId;
Expand Down Expand Up @@ -510,7 +512,7 @@ - (void)setBackgroundColor:(NSColor*)color {
[_flutterView setBackgroundColor:_backgroundColor];
}

- (uint64_t)viewId {
- (FlutterViewId)viewId {
NSAssert([self attached], @"This view controller is not attched.");
return _viewId;
}
Expand Down Expand Up @@ -539,7 +541,7 @@ - (void)notifySemanticsEnabledChanged {
return _bridge;
}

- (void)attachToEngine:(nonnull FlutterEngine*)engine withId:(uint64_t)viewId {
- (void)attachToEngine:(nonnull FlutterEngine*)engine withId:(FlutterViewId)viewId {
NSAssert(_engine == nil, @"Already attached to an engine %@.", _engine);
_engine = engine;
_viewId = viewId;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,16 @@

@interface FlutterViewController () <FlutterKeyboardViewDelegate>

/**
* 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) FlutterViewId viewId;

// The FlutterView for this view controller.
@property(nonatomic, readonly, nullable) FlutterView* flutterView;

Expand All @@ -33,7 +43,7 @@
*
* This method is called by FlutterEngine.
*/
- (void)attachToEngine:(nonnull FlutterEngine*)engine withId:(uint64_t)viewId;
- (void)attachToEngine:(nonnull FlutterEngine*)engine withId:(FlutterViewId)viewId;

/**
* Reset the `engine` and `id` of this controller.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ - (instancetype)initWithEngine:(FlutterEngine*)engine {
return self;
}

- (nullable FlutterView*)viewForId:(uint64_t)viewId {
- (nullable FlutterView*)viewForId:(FlutterViewId)viewId {
return [_engine viewControllerForId:viewId].flutterView;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,9 @@
OCMStub([mockEngine viewControllerForId:0])
.ignoringNonObjectArgs()
.andDo(^(NSInvocation* invocation) {
uint64_t viewId;
FlutterViewId viewId;
[invocation getArgument:&viewId atIndex:2];
if (viewId == 0 /* kFlutterDefaultViewId */) {
if (viewId == kFlutterDefaultViewId) {
if (mockFlutterViewController != nil) {
[invocation setReturnValue:&mockFlutterViewController];
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@

#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterView.h"

extern const uint64_t kFlutterDefaultViewId;

/**
* An interface to query FlutterView.
*
Expand All @@ -20,6 +18,6 @@ extern const uint64_t kFlutterDefaultViewId;
*
* Returns nil if the ID is invalid.
*/
- (nullable FlutterView*)viewForId:(uint64_t)id;
- (nullable FlutterView*)viewForId:(FlutterViewId)id;

@end