From 783e34e13a32354ed7627b63586e8ed52af46520 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Wed, 10 May 2023 14:38:46 -0700 Subject: [PATCH 01/11] Impl --- .../macos/framework/Source/FlutterEngine.mm | 9 +++ .../framework/Source/FlutterEngineTest.mm | 11 +-- .../framework/Source/FlutterEngine_Internal.h | 2 + .../Source/FlutterThreadSynchronizer.h | 12 +++- .../Source/FlutterThreadSynchronizer.mm | 69 ++++++++++++++++--- .../macos/framework/Source/FlutterView.h | 13 +--- .../macos/framework/Source/FlutterView.mm | 25 +++---- .../framework/Source/FlutterViewController.mm | 9 ++- .../macos/framework/Source/FlutterViewTest.mm | 7 +- 9 files changed, 117 insertions(+), 40 deletions(-) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm b/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm index 16e8405a6fcdd..4f09c1a1b95e6 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm @@ -382,6 +382,8 @@ @implementation FlutterEngine { // A method channel for miscellaneous platform functionality. FlutterMethodChannel* _platformChannel; + FlutterThreadSynchronizer* _threadSynchronizer; + int _nextViewId; } @@ -421,6 +423,7 @@ - (instancetype)initWithName:(NSString*)labelPrefix object:nil]; _platformViewController = [[FlutterPlatformViewController alloc] init]; + _threadSynchronizer = [[FlutterThreadSynchronizer alloc] init]; [self setUpPlatformViewChannel]; [self setUpAccessibilityChannel]; [self setUpNotificationCenterListeners]; @@ -905,6 +908,8 @@ - (void)shutDownEngine { } NSEnumerator* viewControllerEnumerator = [_viewControllers objectEnumerator]; + [_threadSynchronizer shutdown]; + _threadSynchronizer = nil; FlutterViewController* nextViewController; while ((nextViewController = [viewControllerEnumerator nextObject])) { [nextViewController.flutterView shutdown]; @@ -1072,6 +1077,10 @@ - (NSPasteboard*)pasteboard { return flutter::GetSwitchesFromEnvironment(); } +- (FlutterThreadSynchronizer*)threadSynchronizer { + return _threadSynchronizer; +} + #pragma mark - FlutterBinaryMessenger - (void)sendOnChannel:(nonnull NSString*)channel message:(nullable NSData*)message { diff --git a/shell/platform/darwin/macos/framework/Source/FlutterEngineTest.mm b/shell/platform/darwin/macos/framework/Source/FlutterEngineTest.mm index d7aa813ca2fbf..8018c5f8d2bee 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterEngineTest.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterEngineTest.mm @@ -24,6 +24,8 @@ // CREATE_NATIVE_ENTRY and MOCK_ENGINE_PROC are leaky by design // NOLINTBEGIN(clang-analyzer-core.StackAddressEscape) +constexpr int64_t kDefaultViewId = 0ll; + @interface FlutterEngine (Test) /** * The FlutterCompositor object currently in use by the FlutterEngine. @@ -435,7 +437,7 @@ - (nonnull NSView*)createWithViewIdentifier:(int64_t)viewId arguments:(nullable result:^(id result){ }]; - [viewController.flutterView.threadSynchronizer blockUntilFrameAvailable]; + [engine.threadSynchronizer blockUntilFrameAvailable]; CALayer* rootLayer = viewController.flutterView.layer; @@ -626,9 +628,10 @@ - (nonnull NSView*)createWithViewIdentifier:(int64_t)viewId arguments:(nullable [threadSynchronizer shutdown]; std::thread rasterThread([&threadSynchronizer] { - [threadSynchronizer performCommit:CGSizeMake(100, 100) - notify:^{ - }]; + [threadSynchronizer performCommitForView:kDefaultViewId + size:CGSizeMake(100, 100) + notify:^{ + }]; }); rasterThread.join(); diff --git a/shell/platform/darwin/macos/framework/Source/FlutterEngine_Internal.h b/shell/platform/darwin/macos/framework/Source/FlutterEngine_Internal.h index e291094eaa342..e5b185c492b03 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterEngine_Internal.h +++ b/shell/platform/darwin/macos/framework/Source/FlutterEngine_Internal.h @@ -107,6 +107,8 @@ typedef NS_ENUM(NSInteger, FlutterAppExitResponse) { */ @property(nonatomic, readonly) FlutterEngineTerminationHandler* terminationHandler; +@property(nonatomic, readonly) FlutterThreadSynchronizer* threadSynchronizer; + /** * Attach a view controller to the engine as its default controller. * diff --git a/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.h b/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.h index 8eca9611f0f48..5cc399cb33279 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.h +++ b/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.h @@ -15,7 +15,9 @@ * Called from platform thread. Blocks until commit with given size (or empty) * is requested. */ -- (void)beginResize:(CGSize)size notify:(nonnull dispatch_block_t)notify; +- (void)beginResizeForView:(int64_t)viewId + size:(CGSize)size + notify:(nonnull dispatch_block_t)notify; /** * Called from raster thread. Schedules the given block on platform thread @@ -26,7 +28,13 @@ * * The notify block is guaranteed to be called within a core animation transaction. */ -- (void)performCommit:(CGSize)size notify:(nonnull dispatch_block_t)notify; +- (void)performCommitForView:(int64_t)viewId + size:(CGSize)size + notify:(nonnull dispatch_block_t)notify; + +- (void)registerView:(int64_t)viewId; + +- (void)deregisterView:(int64_t)viewId; /** * Called when shutting down. Unblocks everything and prevents any further synchronization. diff --git a/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.mm b/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.mm index cb442ea768ffb..82f65f932e61d 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.mm @@ -3,6 +3,7 @@ #import #include +#include #include #import "flutter/fml/logging.h" @@ -11,7 +12,7 @@ @interface FlutterThreadSynchronizer () { std::mutex _mutex; BOOL _shuttingDown; - CGSize _contentSize; + std::unordered_map _contentSizes; std::vector _scheduledBlocks; BOOL _beginResizeWaiting; @@ -20,10 +21,42 @@ @interface FlutterThreadSynchronizer () { std::condition_variable _condBlockBeginResize; } +/** + * Returns true if all existing views have a non-zero size. + * + * If there are no views, still returns true. + */ +- (BOOL)allViewsHaveFrame; + +/** + * Returns true if there are any views that have a non-zero size. + * + * If there are no views, returns false. + */ +- (BOOL)someViewsHaveFrame; + @end @implementation FlutterThreadSynchronizer +- (BOOL)allViewsHaveFrame { + for (auto const& [viewId, contentSize] : _contentSizes) { + if (CGSizeEqualToSize(contentSize, CGSizeZero)) { + return NO; + } + } + return YES; +} + +- (BOOL)someViewsHaveFrame { + for (auto const& [viewId, contentSize] : _contentSizes) { + if (!CGSizeEqualToSize(contentSize, CGSizeZero)) { + return YES; + } + } + return NO; +} + - (void)drain { FML_DCHECK([NSThread isMainThread]); @@ -41,7 +74,7 @@ - (void)blockUntilFrameAvailable { _beginResizeWaiting = YES; - while (CGSizeEqualToSize(_contentSize, CGSizeZero) && !_shuttingDown) { + while (![self someViewsHaveFrame] && !_shuttingDown) { _condBlockBeginResize.wait(lock); [self drain]; } @@ -49,10 +82,12 @@ - (void)blockUntilFrameAvailable { _beginResizeWaiting = NO; } -- (void)beginResize:(CGSize)size notify:(nonnull dispatch_block_t)notify { +- (void)beginResizeForView:(int64_t)viewId + size:(CGSize)size + notify:(nonnull dispatch_block_t)notify { std::unique_lock lock(_mutex); - if (CGSizeEqualToSize(_contentSize, CGSizeZero) || _shuttingDown) { + if (![self allViewsHaveFrame] || _shuttingDown) { // No blocking until framework produces at least one frame notify(); return; @@ -62,12 +97,18 @@ - (void)beginResize:(CGSize)size notify:(nonnull dispatch_block_t)notify { notify(); - _contentSize = CGSizeMake(-1, -1); + _contentSizes[viewId] = CGSizeMake(-1, -1); _beginResizeWaiting = YES; - while (!CGSizeEqualToSize(_contentSize, size) && // - !CGSizeEqualToSize(_contentSize, CGSizeZero) && !_shuttingDown) { + while (true) { + if (_shuttingDown) { + break; + } + const CGSize& contentSize = _contentSizes[viewId]; + if (CGSizeEqualToSize(contentSize, size) || CGSizeEqualToSize(contentSize, CGSizeZero)) { + break; + } _condBlockBeginResize.wait(lock); [self drain]; } @@ -75,7 +116,9 @@ - (void)beginResize:(CGSize)size notify:(nonnull dispatch_block_t)notify { _beginResizeWaiting = NO; } -- (void)performCommit:(CGSize)size notify:(nonnull dispatch_block_t)notify { +- (void)performCommitForView:(int64_t)viewId + size:(CGSize)size + notify:(nonnull dispatch_block_t)notify { fml::AutoResetWaitableEvent event; { std::unique_lock lock(_mutex); @@ -87,7 +130,7 @@ - (void)performCommit:(CGSize)size notify:(nonnull dispatch_block_t)notify { fml::AutoResetWaitableEvent& e = event; _scheduledBlocks.push_back(^{ notify(); - _contentSize = size; + _contentSizes[viewId] = size; e.Signal(); }); if (_beginResizeWaiting) { @@ -102,6 +145,14 @@ - (void)performCommit:(CGSize)size notify:(nonnull dispatch_block_t)notify { event.Wait(); } +- (void)registerView:(int64_t)viewId { + _contentSizes[viewId] = CGSizeZero; +} + +- (void)deregisterView:(int64_t)viewId { + _contentSizes.erase(viewId); +} + - (void)shutdown { std::unique_lock lock(_mutex); _shuttingDown = YES; diff --git a/shell/platform/darwin/macos/framework/Source/FlutterView.h b/shell/platform/darwin/macos/framework/Source/FlutterView.h index f581079574bdc..fe2d55aac554d 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterView.h +++ b/shell/platform/darwin/macos/framework/Source/FlutterView.h @@ -44,7 +44,8 @@ constexpr FlutterViewId kFlutterDefaultViewId = 0ll; - (nullable instancetype)initWithMTLDevice:(nonnull id)device commandQueue:(nonnull id)commandQueue reshapeListener:(nonnull id)reshapeListener - NS_DESIGNATED_INITIALIZER; + threadSynchronizer:(nonnull FlutterThreadSynchronizer*)threadSynchronizer + viewId:(int64_t)viewId NS_DESIGNATED_INITIALIZER; - (nullable instancetype)initWithFrame:(NSRect)frameRect pixelFormat:(nullable NSOpenGLPixelFormat*)format NS_UNAVAILABLE; @@ -74,13 +75,3 @@ constexpr FlutterViewId kFlutterDefaultViewId = 0ll; - (void)setBackgroundColor:(nonnull NSColor*)color; @end - -@interface FlutterView (FlutterViewPrivate) - -/** - * Returns FlutterThreadSynchronizer for this view. - * Used for FlutterEngineTest. - */ -- (nonnull FlutterThreadSynchronizer*)threadSynchronizer; - -@end diff --git a/shell/platform/darwin/macos/framework/Source/FlutterView.mm b/shell/platform/darwin/macos/framework/Source/FlutterView.mm index c3f802d43ed9e..db20790f07b57 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterView.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterView.mm @@ -10,6 +10,7 @@ #import @interface FlutterView () { + int64_t _viewId; __weak id _reshapeListener; FlutterThreadSynchronizer* _threadSynchronizer; FlutterSurfaceManager* _surfaceManager; @@ -21,40 +22,41 @@ @implementation FlutterView - (instancetype)initWithMTLDevice:(id)device commandQueue:(id)commandQueue - reshapeListener:(id)reshapeListener { + reshapeListener:(id)reshapeListener + threadSynchronizer:(FlutterThreadSynchronizer*)threadSynchronizer + viewId:(int64_t)viewId { self = [super initWithFrame:NSZeroRect]; if (self) { [self setWantsLayer:YES]; [self setBackgroundColor:[NSColor blackColor]]; [self setLayerContentsRedrawPolicy:NSViewLayerContentsRedrawDuringViewResize]; + _viewId = viewId; _reshapeListener = reshapeListener; - _threadSynchronizer = [[FlutterThreadSynchronizer alloc] init]; + _threadSynchronizer = threadSynchronizer; _surfaceManager = [[FlutterSurfaceManager alloc] initWithDevice:device commandQueue:commandQueue layer:self.layer delegate:self]; + [_threadSynchronizer registerView:viewId]; } return self; } - (void)onPresent:(CGSize)frameSize withBlock:(dispatch_block_t)block { - [_threadSynchronizer performCommit:frameSize notify:block]; + [_threadSynchronizer performCommitForView:_viewId size:frameSize notify:block]; } - (FlutterSurfaceManager*)surfaceManager { return _surfaceManager; } -- (FlutterThreadSynchronizer*)threadSynchronizer { - return _threadSynchronizer; -} - - (void)reshaped { CGSize scaledSize = [self convertSizeToBacking:self.bounds.size]; - [_threadSynchronizer beginResize:scaledSize - notify:^{ - [_reshapeListener viewDidReshape:self]; - }]; + [_threadSynchronizer beginResizeForView:_viewId + size:scaledSize + notify:^{ + [_reshapeListener viewDidReshape:self]; + }]; } - (void)setBackgroundColor:(NSColor*)color { @@ -113,7 +115,6 @@ - (BOOL)layer:(CALayer*)layer } - (void)shutdown { - [_threadSynchronizer shutdown]; } #pragma mark - NSAccessibility overrides diff --git a/shell/platform/darwin/macos/framework/Source/FlutterViewController.mm b/shell/platform/darwin/macos/framework/Source/FlutterViewController.mm index f6e9aa3eb957f..f7c082b694323 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterViewController.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterViewController.mm @@ -372,6 +372,10 @@ @implementation FlutterViewController { std::shared_ptr _bridge; FlutterViewId _id; + + // FlutterViewController does not actually uses the synchronizer, but only + // passes it to FlutterView. + __weak FlutterThreadSynchronizer* _weakViewThreadSynchronizer; } @synthesize viewId = _viewId; @@ -398,6 +402,7 @@ static void CommonInit(FlutterViewController* controller, FlutterEngine* engine) controller.engine, controller.viewId); controller->_mouseTrackingMode = FlutterMouseTrackingModeInKeyWindow; controller->_textInputPlugin = [[FlutterTextInputPlugin alloc] initWithViewController:controller]; + controller->_weakViewThreadSynchronizer = engine.threadSynchronizer; [controller initializeKeyboard]; [controller notifySemanticsEnabledChanged]; // macOS fires this message when changing IMEs. @@ -858,7 +863,9 @@ - (nonnull FlutterView*)createFlutterViewWithMTLDevice:(id)device commandQueue:(id)commandQueue { return [[FlutterView alloc] initWithMTLDevice:device commandQueue:commandQueue - reshapeListener:self]; + reshapeListener:self + threadSynchronizer:_weakViewThreadSynchronizer + viewId:_viewId]; } - (void)onKeyboardLayoutChanged { diff --git a/shell/platform/darwin/macos/framework/Source/FlutterViewTest.mm b/shell/platform/darwin/macos/framework/Source/FlutterViewTest.mm index 15ca078042fd8..7599b66e8b0ec 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterViewTest.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterViewTest.mm @@ -8,6 +8,8 @@ #import "flutter/testing/testing.h" +constexpr int64_t kDefaultViewId = 0ll; + @interface TestReshapeListener : NSObject @end @@ -23,8 +25,11 @@ - (void)viewDidReshape:(nonnull NSView*)view { id device = MTLCreateSystemDefaultDevice(); id queue = [device newCommandQueue]; TestReshapeListener* listener = [[TestReshapeListener alloc] init]; + FlutterThreadSynchronizer* threadSynchronizer = [[FlutterThreadSynchronizer alloc] init]; FlutterView* view = [[FlutterView alloc] initWithMTLDevice:device commandQueue:queue - reshapeListener:listener]; + reshapeListener:listener + threadSynchronizer:threadSynchronizer + viewId:kDefaultViewId]; EXPECT_EQ([view layer:view.layer shouldInheritContentsScale:3.0 fromWindow:view.window], YES); } From b9226c9ae7efebd80bbb52765a31c057956d917e Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Mon, 22 May 2023 15:18:09 -0700 Subject: [PATCH 02/11] Basic test --- ci/licenses_golden/licenses_flutter | 4 ++++ shell/platform/darwin/macos/BUILD.gn | 1 + .../Source/FlutterThreadSynchronizer.h | 13 ++++++++++ .../Source/FlutterThreadSynchronizer.mm | 24 +++++++++++++++++-- 4 files changed, 40 insertions(+), 2 deletions(-) diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index c673eb20437be..efbcf647cab2a 100644 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -2720,6 +2720,9 @@ ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterTex ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterTextInputSemanticsObjectTest.mm + ../../../flutter/LICENSE ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterTextureRegistrar.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterTextureRegistrar.mm + ../../../flutter/LICENSE +ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.h + ../../../flutter/LICENSE +ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.mm + ../../../flutter/LICENSE +ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizerTest.mm + ../../../flutter/LICENSE ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterUmbrellaImportTests.m + ../../../flutter/LICENSE ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterView.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterView.mm + ../../../flutter/LICENSE @@ -5362,6 +5365,7 @@ FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterTextu FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterTextureRegistrar.mm FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.h FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.mm +FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizerTest.mm FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterUmbrellaImportTests.m FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterView.h FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterView.mm diff --git a/shell/platform/darwin/macos/BUILD.gn b/shell/platform/darwin/macos/BUILD.gn index edebb4ebfd3b1..16ffb24cd3f19 100644 --- a/shell/platform/darwin/macos/BUILD.gn +++ b/shell/platform/darwin/macos/BUILD.gn @@ -182,6 +182,7 @@ executable("flutter_desktop_darwin_unittests") { "framework/Source/FlutterSurfaceManagerTest.mm", "framework/Source/FlutterTextInputPluginTest.mm", "framework/Source/FlutterTextInputSemanticsObjectTest.mm", + "framework/Source/FlutterThreadSynchronizerTest.mm", "framework/Source/FlutterViewControllerTest.mm", "framework/Source/FlutterViewControllerTestUtils.h", "framework/Source/FlutterViewControllerTestUtils.mm", diff --git a/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.h b/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.h index 5cc399cb33279..4f0e1f7ad14ef 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.h +++ b/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.h @@ -1,3 +1,7 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + #import /** @@ -5,6 +9,8 @@ */ @interface FlutterThreadSynchronizer : NSObject +- (nullable instancetype)init; + /** * Blocks current thread until there is frame available. * Used in FlutterEngineTest. @@ -42,3 +48,10 @@ - (void)shutdown; @end + +@interface FlutterThreadSynchronizer (Test) + +- (nullable instancetype)initWithMainQueue:(nonnull dispatch_queue_t)queue + mainThread:(nonnull NSThread*)thread; + +@end diff --git a/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.mm b/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.mm index 82f65f932e61d..43ba3853ff8fe 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.mm @@ -1,3 +1,7 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.h" #import @@ -10,6 +14,8 @@ #import "flutter/fml/synchronization/waitable_event.h" @interface FlutterThreadSynchronizer () { + dispatch_queue_t _queue; + __weak NSThread* _queueThread; std::mutex _mutex; BOOL _shuttingDown; std::unordered_map _contentSizes; @@ -39,6 +45,20 @@ - (BOOL)someViewsHaveFrame; @implementation FlutterThreadSynchronizer +- (instancetype)init { + return [self initWithMainQueue:dispatch_get_main_queue() mainThread:[NSThread mainThread]]; +} + +- (instancetype)initWithMainQueue:(dispatch_queue_t)queue + mainThread:(NSThread*)thread { + self = [super init]; + if (self != nil) { + _queue = queue; + _queueThread = thread; + } + return self; +} + - (BOOL)allViewsHaveFrame { for (auto const& [viewId, contentSize] : _contentSizes) { if (CGSizeEqualToSize(contentSize, CGSizeZero)) { @@ -58,7 +78,7 @@ - (BOOL)someViewsHaveFrame { } - (void)drain { - FML_DCHECK([NSThread isMainThread]); + FML_DCHECK([NSThread currentThread] == _queueThread); [CATransaction begin]; [CATransaction setDisableActions:YES]; @@ -136,7 +156,7 @@ - (void)performCommitForView:(int64_t)viewId if (_beginResizeWaiting) { _condBlockBeginResize.notify_all(); } else { - dispatch_async(dispatch_get_main_queue(), ^{ + dispatch_async(_queue, ^{ std::unique_lock lock(_mutex); [self drain]; }); From 57e159f9bde5557a87a1cbe2316ca3c9e2165161 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Mon, 22 May 2023 17:31:37 -0700 Subject: [PATCH 03/11] Use set specific to check main --- .../Source/FlutterThreadSynchronizer.h | 3 +-- .../Source/FlutterThreadSynchronizer.mm | 22 ++++++++++++++----- 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.h b/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.h index 4f0e1f7ad14ef..6c424bf5d9fb8 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.h +++ b/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.h @@ -51,7 +51,6 @@ @interface FlutterThreadSynchronizer (Test) -- (nullable instancetype)initWithMainQueue:(nonnull dispatch_queue_t)queue - mainThread:(nonnull NSThread*)thread; +- (nullable instancetype)initWithMainQueue:(nonnull dispatch_queue_t)queue; @end diff --git a/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.mm b/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.mm index 43ba3853ff8fe..4bdc672e79f5e 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.mm @@ -13,9 +13,10 @@ #import "flutter/fml/logging.h" #import "flutter/fml/synchronization/waitable_event.h" +static constexpr intptr_t _kMainQueueContext = 0x1234ABCD; + @interface FlutterThreadSynchronizer () { dispatch_queue_t _queue; - __weak NSThread* _queueThread; std::mutex _mutex; BOOL _shuttingDown; std::unordered_map _contentSizes; @@ -41,24 +42,29 @@ - (BOOL)allViewsHaveFrame; */ - (BOOL)someViewsHaveFrame; +- (const void*)mainQueueKey; + @end @implementation FlutterThreadSynchronizer - (instancetype)init { - return [self initWithMainQueue:dispatch_get_main_queue() mainThread:[NSThread mainThread]]; + return [self initWithMainQueue:dispatch_get_main_queue()]; } -- (instancetype)initWithMainQueue:(dispatch_queue_t)queue - mainThread:(NSThread*)thread { +- (instancetype)initWithMainQueue:(dispatch_queue_t)queue { self = [super init]; if (self != nil) { _queue = queue; - _queueThread = thread; + dispatch_queue_set_specific(_queue, [self mainQueueKey], (void*)_kMainQueueContext, NULL); } return self; } +- (void)dealloc { + dispatch_queue_set_specific(_queue, [self mainQueueKey], NULL, NULL); +} + - (BOOL)allViewsHaveFrame { for (auto const& [viewId, contentSize] : _contentSizes) { if (CGSizeEqualToSize(contentSize, CGSizeZero)) { @@ -78,7 +84,7 @@ - (BOOL)someViewsHaveFrame { } - (void)drain { - FML_DCHECK([NSThread currentThread] == _queueThread); + FML_DCHECK(dispatch_get_specific([self mainQueueKey]) == (void*)_kMainQueueContext); [CATransaction begin]; [CATransaction setDisableActions:YES]; @@ -180,4 +186,8 @@ - (void)shutdown { [self drain]; } +- (const void*)mainQueueKey { + return (__bridge const void*)self; +} + @end From 85b1262719917475f0b04632c0f0d3605783688c Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Mon, 22 May 2023 17:50:49 -0700 Subject: [PATCH 04/11] Use set specific to check main --- .../framework/Source/FlutterThreadSynchronizer.h | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.h b/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.h index 6c424bf5d9fb8..3141b7b001cb6 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.h +++ b/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.h @@ -12,13 +12,7 @@ - (nullable instancetype)init; /** - * Blocks current thread until there is frame available. - * Used in FlutterEngineTest. - */ -- (void)blockUntilFrameAvailable; - -/** - * Called from platform thread. Blocks until commit with given size (or empty) + * Called from platform thread. Blocks until a commit with given size (or empty) * is requested. */ - (void)beginResizeForView:(int64_t)viewId @@ -53,4 +47,10 @@ - (nullable instancetype)initWithMainQueue:(nonnull dispatch_queue_t)queue; +/** + * Blocks current thread until there is frame available. + * Used in FlutterEngineTest. + */ +- (void)blockUntilFrameAvailable; + @end From 23c3851c788ce2efd9640bb373f3d30acad3e246 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Tue, 23 May 2023 01:13:50 -0700 Subject: [PATCH 05/11] FinishResizingOnlyWhenCommittingMatchingSize --- .../Source/FlutterThreadSynchronizer.h | 2 + .../Source/FlutterThreadSynchronizer.mm | 22 +-- .../Source/FlutterThreadSynchronizerTest.mm | 161 ++++++++++++++++++ 3 files changed, 170 insertions(+), 15 deletions(-) create mode 100644 shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizerTest.mm diff --git a/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.h b/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.h index 3141b7b001cb6..6c9279e8d0e85 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.h +++ b/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.h @@ -47,6 +47,8 @@ - (nullable instancetype)initWithMainQueue:(nonnull dispatch_queue_t)queue; +- (BOOL)isWaitingWhenMutexIsAvailable; + /** * Blocks current thread until there is frame available. * Used in FlutterEngineTest. diff --git a/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.mm b/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.mm index 4bdc672e79f5e..82229bbd5fa5f 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.mm @@ -13,10 +13,8 @@ #import "flutter/fml/logging.h" #import "flutter/fml/synchronization/waitable_event.h" -static constexpr intptr_t _kMainQueueContext = 0x1234ABCD; - @interface FlutterThreadSynchronizer () { - dispatch_queue_t _queue; + dispatch_queue_t _mainQueue; std::mutex _mutex; BOOL _shuttingDown; std::unordered_map _contentSizes; @@ -42,8 +40,6 @@ - (BOOL)allViewsHaveFrame; */ - (BOOL)someViewsHaveFrame; -- (const void*)mainQueueKey; - @end @implementation FlutterThreadSynchronizer @@ -55,16 +51,11 @@ - (instancetype)init { - (instancetype)initWithMainQueue:(dispatch_queue_t)queue { self = [super init]; if (self != nil) { - _queue = queue; - dispatch_queue_set_specific(_queue, [self mainQueueKey], (void*)_kMainQueueContext, NULL); + _mainQueue = queue; } return self; } -- (void)dealloc { - dispatch_queue_set_specific(_queue, [self mainQueueKey], NULL, NULL); -} - - (BOOL)allViewsHaveFrame { for (auto const& [viewId, contentSize] : _contentSizes) { if (CGSizeEqualToSize(contentSize, CGSizeZero)) { @@ -84,7 +75,7 @@ - (BOOL)someViewsHaveFrame { } - (void)drain { - FML_DCHECK(dispatch_get_specific([self mainQueueKey]) == (void*)_kMainQueueContext); + dispatch_assert_queue(_mainQueue); [CATransaction begin]; [CATransaction setDisableActions:YES]; @@ -162,7 +153,7 @@ - (void)performCommitForView:(int64_t)viewId if (_beginResizeWaiting) { _condBlockBeginResize.notify_all(); } else { - dispatch_async(_queue, ^{ + dispatch_async(_mainQueue, ^{ std::unique_lock lock(_mutex); [self drain]; }); @@ -186,8 +177,9 @@ - (void)shutdown { [self drain]; } -- (const void*)mainQueueKey { - return (__bridge const void*)self; +- (BOOL)isWaitingWhenMutexIsAvailable { + std::unique_lock lock(_mutex); + return _beginResizeWaiting; } @end diff --git a/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizerTest.mm b/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizerTest.mm new file mode 100644 index 0000000000000..0f886c77ab3b7 --- /dev/null +++ b/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizerTest.mm @@ -0,0 +1,161 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.h" + +#import +#import "flutter/testing/testing.h" +#import "flutter/fml/synchronization/waitable_event.h" + +namespace flutter::testing { + +namespace { + +} // namespace + +} // namespace flutter::testing + +@interface FlutterThreadSynchronizerTestScaffold : NSObject + +@property(nonatomic, readonly, nonnull) FlutterThreadSynchronizer* synchronizer; + +- (nullable instancetype)init; +- (void)dispatchMainTask:(nonnull void(^)())task; +- (void)dispatchRenderTask:(nonnull void(^)())task; +- (void)joinMain; +- (void)joinRender; +@end + +@implementation FlutterThreadSynchronizerTestScaffold { + dispatch_queue_t _mainQueue; + std::shared_ptr _mainLatch; + + dispatch_queue_t _renderQueue; + std::shared_ptr _renderLatch; + + FlutterThreadSynchronizer* _synchronizer; +} + +@synthesize synchronizer = _synchronizer; + +- (nullable instancetype)init { + self = [super init]; + if (self != nil) { + _mainQueue = dispatch_queue_create("MAIN", DISPATCH_QUEUE_SERIAL); + _renderQueue = dispatch_queue_create("RENDER", DISPATCH_QUEUE_SERIAL); + _synchronizer = [[FlutterThreadSynchronizer alloc] initWithMainQueue:_mainQueue]; + } + return self; +} + +- (void)dispatchMainTask:(nonnull void(^)())task { + dispatch_async(_mainQueue, task); +} + +- (void)dispatchRenderTask:(nonnull void(^)())task { + dispatch_async(_renderQueue, task); +} + +- (void)joinMain { + fml::AutoResetWaitableEvent latch; + fml::AutoResetWaitableEvent* pLatch = &latch; + dispatch_async(_mainQueue, ^{ pLatch->Signal(); }); + latch.Wait(); +} + +- (void)joinRender { + fml::AutoResetWaitableEvent latch; + fml::AutoResetWaitableEvent* pLatch = &latch; + dispatch_async(_renderQueue, ^{ pLatch->Signal(); }); + latch.Wait(); +} + +@end + +TEST(FlutterThreadSynchronizerTest, FinishResizingImmediatelyWhenSizeMatches) { + FlutterThreadSynchronizerTestScaffold* scaffold = [[FlutterThreadSynchronizerTestScaffold alloc] init]; + FlutterThreadSynchronizer* synchronizer = scaffold.synchronizer; + + [synchronizer registerView:1]; + + // Initial resize: does not block until the first frame. + __block int notifiedResize = 0; + [scaffold dispatchMainTask:^{ + [synchronizer beginResizeForView:1 size:CGSize{5, 5} notify:^{ + notifiedResize += 1; + }]; + }]; + EXPECT_FALSE([synchronizer isWaitingWhenMutexIsAvailable]); + [scaffold joinMain]; + EXPECT_EQ(notifiedResize, 1); + + // Still does not block. + [scaffold dispatchMainTask:^{ + [synchronizer beginResizeForView:1 size:CGSize{7, 7} notify:^{ + notifiedResize += 1; + }]; + }]; + EXPECT_FALSE([synchronizer isWaitingWhenMutexIsAvailable]); + [scaffold joinMain]; + EXPECT_EQ(notifiedResize, 2); + + // First frame + __block int notifiedCommit = 0; + [scaffold dispatchRenderTask:^{ + [synchronizer performCommitForView:1 size:CGSize{7, 7} notify:^{ + notifiedCommit += 1; + }]; + }]; + EXPECT_FALSE([synchronizer isWaitingWhenMutexIsAvailable]); + [scaffold joinRender]; + EXPECT_EQ(notifiedCommit, 1); +} + +TEST(FlutterThreadSynchronizerTest, FinishResizingOnlyWhenCommittingMatchingSize) { + FlutterThreadSynchronizerTestScaffold* scaffold = [[FlutterThreadSynchronizerTestScaffold alloc] init]; + FlutterThreadSynchronizer* synchronizer = scaffold.synchronizer; + fml::AutoResetWaitableEvent begunResizingLatch; + __block fml::AutoResetWaitableEvent* begunResizing = &begunResizingLatch; + + [synchronizer registerView:1]; + + // Initial resize: does not block until the first frame. + [scaffold dispatchMainTask:^{ + [synchronizer beginResizeForView:1 size:CGSize{5, 5} notify:^{}]; + }]; + [scaffold joinMain]; + EXPECT_FALSE([synchronizer isWaitingWhenMutexIsAvailable]); + + // First frame. + [scaffold dispatchRenderTask:^{ + [synchronizer performCommitForView:1 size:CGSize{5, 5} notify:^{}]; + }]; + [scaffold joinRender]; + EXPECT_FALSE([synchronizer isWaitingWhenMutexIsAvailable]); + + // Resize to (7, 7): blocks since there's a frame until the next frame. + [scaffold dispatchMainTask:^{ + [synchronizer beginResizeForView:1 size:CGSize{7, 7} notify:^{ + begunResizing->Signal(); + }]; + }]; + begunResizing->Wait(); + EXPECT_TRUE([synchronizer isWaitingWhenMutexIsAvailable]); + + // Render with old size. + [scaffold dispatchRenderTask:^{ + [synchronizer performCommitForView:1 size:CGSize{5, 5} notify:^{}]; + }]; + [scaffold joinRender]; + EXPECT_TRUE([synchronizer isWaitingWhenMutexIsAvailable]); + + // Render with new size. + [scaffold dispatchRenderTask:^{ + [synchronizer performCommitForView:1 size:CGSize{7, 7} notify:^{}]; + }]; + [scaffold joinRender]; + EXPECT_FALSE([synchronizer isWaitingWhenMutexIsAvailable]); + + [scaffold joinMain]; +} From b3cbb05c4ac492aa621272e7ca12c83f76808436 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Tue, 23 May 2023 14:30:30 -0700 Subject: [PATCH 06/11] FinishResizingWhenShuttingDown --- .../Source/FlutterThreadSynchronizer.h | 8 +- .../Source/FlutterThreadSynchronizer.mm | 3 + .../Source/FlutterThreadSynchronizerTest.mm | 143 ++++++++++++++---- 3 files changed, 123 insertions(+), 31 deletions(-) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.h b/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.h index 6c9279e8d0e85..74653956354ed 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.h +++ b/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.h @@ -37,7 +37,9 @@ - (void)deregisterView:(int64_t)viewId; /** - * Called when shutting down. Unblocks everything and prevents any further synchronization. + * Called from platform thread when shutting down. + * + * Prevents any further synchronization and no longer blocks any threads. */ - (void)shutdown; @@ -47,6 +49,10 @@ - (nullable instancetype)initWithMainQueue:(nonnull dispatch_queue_t)queue; +/** + * Blocks current thread until the mutex is available, then return whether the + * synchronizer is waiting for a correct commit during resizing. + */ - (BOOL)isWaitingWhenMutexIsAvailable; /** diff --git a/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.mm b/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.mm index 82229bbd5fa5f..a0561e4d30e8e 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.mm @@ -102,6 +102,7 @@ - (void)blockUntilFrameAvailable { - (void)beginResizeForView:(int64_t)viewId size:(CGSize)size notify:(nonnull dispatch_block_t)notify { + dispatch_assert_queue(_mainQueue); std::unique_lock lock(_mutex); if (![self allViewsHaveFrame] || _shuttingDown) { @@ -136,6 +137,7 @@ - (void)beginResizeForView:(int64_t)viewId - (void)performCommitForView:(int64_t)viewId size:(CGSize)size notify:(nonnull dispatch_block_t)notify { + dispatch_assert_queue_not(_mainQueue); fml::AutoResetWaitableEvent event; { std::unique_lock lock(_mutex); @@ -171,6 +173,7 @@ - (void)deregisterView:(int64_t)viewId { } - (void)shutdown { + dispatch_assert_queue(_mainQueue); std::unique_lock lock(_mutex); _shuttingDown = YES; _condBlockBeginResize.notify_all(); diff --git a/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizerTest.mm b/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizerTest.mm index 0f886c77ab3b7..ef97b62a3785a 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizerTest.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizerTest.mm @@ -5,14 +5,12 @@ #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.h" #import -#import "flutter/testing/testing.h" #import "flutter/fml/synchronization/waitable_event.h" +#import "flutter/testing/testing.h" namespace flutter::testing { -namespace { - -} // namespace +namespace {} // namespace } // namespace flutter::testing @@ -21,8 +19,8 @@ @interface FlutterThreadSynchronizerTestScaffold : NSObject @property(nonatomic, readonly, nonnull) FlutterThreadSynchronizer* synchronizer; - (nullable instancetype)init; -- (void)dispatchMainTask:(nonnull void(^)())task; -- (void)dispatchRenderTask:(nonnull void(^)())task; +- (void)dispatchMainTask:(nonnull void (^)())task; +- (void)dispatchRenderTask:(nonnull void (^)())task; - (void)joinMain; - (void)joinRender; @end @@ -49,32 +47,37 @@ - (nullable instancetype)init { return self; } -- (void)dispatchMainTask:(nonnull void(^)())task { +- (void)dispatchMainTask:(nonnull void (^)())task { dispatch_async(_mainQueue, task); } -- (void)dispatchRenderTask:(nonnull void(^)())task { +- (void)dispatchRenderTask:(nonnull void (^)())task { dispatch_async(_renderQueue, task); } - (void)joinMain { fml::AutoResetWaitableEvent latch; fml::AutoResetWaitableEvent* pLatch = &latch; - dispatch_async(_mainQueue, ^{ pLatch->Signal(); }); + dispatch_async(_mainQueue, ^{ + pLatch->Signal(); + }); latch.Wait(); } - (void)joinRender { fml::AutoResetWaitableEvent latch; fml::AutoResetWaitableEvent* pLatch = &latch; - dispatch_async(_renderQueue, ^{ pLatch->Signal(); }); + dispatch_async(_renderQueue, ^{ + pLatch->Signal(); + }); latch.Wait(); } @end TEST(FlutterThreadSynchronizerTest, FinishResizingImmediatelyWhenSizeMatches) { - FlutterThreadSynchronizerTestScaffold* scaffold = [[FlutterThreadSynchronizerTestScaffold alloc] init]; + FlutterThreadSynchronizerTestScaffold* scaffold = + [[FlutterThreadSynchronizerTestScaffold alloc] init]; FlutterThreadSynchronizer* synchronizer = scaffold.synchronizer; [synchronizer registerView:1]; @@ -82,9 +85,11 @@ - (void)joinRender { // Initial resize: does not block until the first frame. __block int notifiedResize = 0; [scaffold dispatchMainTask:^{ - [synchronizer beginResizeForView:1 size:CGSize{5, 5} notify:^{ - notifiedResize += 1; - }]; + [synchronizer beginResizeForView:1 + size:CGSize{5, 5} + notify:^{ + notifiedResize += 1; + }]; }]; EXPECT_FALSE([synchronizer isWaitingWhenMutexIsAvailable]); [scaffold joinMain]; @@ -92,9 +97,11 @@ - (void)joinRender { // Still does not block. [scaffold dispatchMainTask:^{ - [synchronizer beginResizeForView:1 size:CGSize{7, 7} notify:^{ - notifiedResize += 1; - }]; + [synchronizer beginResizeForView:1 + size:CGSize{7, 7} + notify:^{ + notifiedResize += 1; + }]; }]; EXPECT_FALSE([synchronizer isWaitingWhenMutexIsAvailable]); [scaffold joinMain]; @@ -103,9 +110,11 @@ - (void)joinRender { // First frame __block int notifiedCommit = 0; [scaffold dispatchRenderTask:^{ - [synchronizer performCommitForView:1 size:CGSize{7, 7} notify:^{ - notifiedCommit += 1; - }]; + [synchronizer performCommitForView:1 + size:CGSize{7, 7} + notify:^{ + notifiedCommit += 1; + }]; }]; EXPECT_FALSE([synchronizer isWaitingWhenMutexIsAvailable]); [scaffold joinRender]; @@ -113,49 +122,123 @@ - (void)joinRender { } TEST(FlutterThreadSynchronizerTest, FinishResizingOnlyWhenCommittingMatchingSize) { - FlutterThreadSynchronizerTestScaffold* scaffold = [[FlutterThreadSynchronizerTestScaffold alloc] init]; + FlutterThreadSynchronizerTestScaffold* scaffold = + [[FlutterThreadSynchronizerTestScaffold alloc] init]; FlutterThreadSynchronizer* synchronizer = scaffold.synchronizer; + // A latch to ensure that a beginResizeForView: call has at least executed + // something, so that the isWaitingWhenMutexIsAvailable: call correctly stops + // at either when beginResizeForView: finishes or waits half way. fml::AutoResetWaitableEvent begunResizingLatch; - __block fml::AutoResetWaitableEvent* begunResizing = &begunResizingLatch; + fml::AutoResetWaitableEvent* begunResizing = &begunResizingLatch; [synchronizer registerView:1]; // Initial resize: does not block until the first frame. [scaffold dispatchMainTask:^{ - [synchronizer beginResizeForView:1 size:CGSize{5, 5} notify:^{}]; + [synchronizer beginResizeForView:1 + size:CGSize{5, 5} + notify:^{ + }]; }]; [scaffold joinMain]; EXPECT_FALSE([synchronizer isWaitingWhenMutexIsAvailable]); // First frame. [scaffold dispatchRenderTask:^{ - [synchronizer performCommitForView:1 size:CGSize{5, 5} notify:^{}]; + [synchronizer performCommitForView:1 + size:CGSize{5, 5} + notify:^{ + }]; }]; [scaffold joinRender]; EXPECT_FALSE([synchronizer isWaitingWhenMutexIsAvailable]); - // Resize to (7, 7): blocks since there's a frame until the next frame. + // Resize to (7, 7): blocks until the next frame. [scaffold dispatchMainTask:^{ - [synchronizer beginResizeForView:1 size:CGSize{7, 7} notify:^{ - begunResizing->Signal(); - }]; + [synchronizer beginResizeForView:1 + size:CGSize{7, 7} + notify:^{ + begunResizing->Signal(); + }]; }]; begunResizing->Wait(); EXPECT_TRUE([synchronizer isWaitingWhenMutexIsAvailable]); // Render with old size. [scaffold dispatchRenderTask:^{ - [synchronizer performCommitForView:1 size:CGSize{5, 5} notify:^{}]; + [synchronizer performCommitForView:1 + size:CGSize{5, 5} + notify:^{ + }]; }]; [scaffold joinRender]; EXPECT_TRUE([synchronizer isWaitingWhenMutexIsAvailable]); // Render with new size. [scaffold dispatchRenderTask:^{ - [synchronizer performCommitForView:1 size:CGSize{7, 7} notify:^{}]; + [synchronizer performCommitForView:1 + size:CGSize{7, 7} + notify:^{ + }]; }]; [scaffold joinRender]; EXPECT_FALSE([synchronizer isWaitingWhenMutexIsAvailable]); [scaffold joinMain]; } + +TEST(FlutterThreadSynchronizerTest, FinishResizingWhenShuttingDown) { + FlutterThreadSynchronizerTestScaffold* scaffold = + [[FlutterThreadSynchronizerTestScaffold alloc] init]; + FlutterThreadSynchronizer* synchronizer = scaffold.synchronizer; + fml::AutoResetWaitableEvent begunResizingLatch; + fml::AutoResetWaitableEvent* begunResizing = &begunResizingLatch; + + [synchronizer registerView:1]; + + // Initial resize + [scaffold dispatchMainTask:^{ + [synchronizer beginResizeForView:1 + size:CGSize{5, 5} + notify:^{ + }]; + }]; + [scaffold joinMain]; + EXPECT_FALSE([synchronizer isWaitingWhenMutexIsAvailable]); + + // Push a frame. + [scaffold dispatchRenderTask:^{ + [synchronizer performCommitForView:1 + size:CGSize{5, 5} + notify:^{ + }]; + }]; + [scaffold joinRender]; + EXPECT_FALSE([synchronizer isWaitingWhenMutexIsAvailable]); + + [scaffold dispatchMainTask:^{ + [synchronizer shutdown]; + }]; + + // Resize to (7, 7). Should not block any frames since it has shut down. + [scaffold dispatchMainTask:^{ + [synchronizer beginResizeForView:1 + size:CGSize{7, 7} + notify:^{ + begunResizing->Signal(); + }]; + }]; + begunResizing->Wait(); + EXPECT_FALSE([synchronizer isWaitingWhenMutexIsAvailable]); + [scaffold joinMain]; + + // All further calls should be unblocking. + [scaffold dispatchRenderTask:^{ + [synchronizer performCommitForView:1 + size:CGSize{9, 9} + notify:^{ + }]; + }]; + [scaffold joinRender]; + EXPECT_FALSE([synchronizer isWaitingWhenMutexIsAvailable]); +} From dc05a5a20715708f30ec6b25e3a20f811e6584d1 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Tue, 23 May 2023 16:33:20 -0700 Subject: [PATCH 07/11] multi view tests --- .../Source/FlutterThreadSynchronizer.h | 12 ++ .../Source/FlutterThreadSynchronizerTest.mm | 150 +++++++++++++++++- 2 files changed, 159 insertions(+), 3 deletions(-) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.h b/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.h index 74653956354ed..1cd4ee64dfa49 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.h +++ b/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.h @@ -52,6 +52,18 @@ /** * Blocks current thread until the mutex is available, then return whether the * synchronizer is waiting for a correct commit during resizing. + * + * After calling an operation of the thread synchronizer, call this method, + * and when it returns, the thread synchronizer can be at one of the following 3 + * states: + * + * 1. The operation has not started at all (with a return value FALSE.) + * 2. The operation has ended (with a return value FALSE.) + * 3. beginResizeForView: is in progress, waiting (with a return value TRUE.) + * + * By eliminating the 1st case (such as using the notify callback), we can use + * this return value to decide whether the synchronizer is in case 2 or case 3, + * that is whether the resizing is blocked by a mismatching commit. */ - (BOOL)isWaitingWhenMutexIsAvailable; diff --git a/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizerTest.mm b/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizerTest.mm index ef97b62a3785a..4e27103f88091 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizerTest.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizerTest.mm @@ -75,7 +75,7 @@ - (void)joinRender { @end -TEST(FlutterThreadSynchronizerTest, FinishResizingImmediatelyWhenSizeMatches) { +TEST(FlutterThreadSynchronizerTest, RegularCommit) { FlutterThreadSynchronizerTestScaffold* scaffold = [[FlutterThreadSynchronizerTestScaffold alloc] init]; FlutterThreadSynchronizer* synchronizer = scaffold.synchronizer; @@ -121,7 +121,7 @@ - (void)joinRender { EXPECT_EQ(notifiedCommit, 1); } -TEST(FlutterThreadSynchronizerTest, FinishResizingOnlyWhenCommittingMatchingSize) { +TEST(FlutterThreadSynchronizerTest, ResizingBlocksRenderingUntilSizeMatches) { FlutterThreadSynchronizerTestScaffold* scaffold = [[FlutterThreadSynchronizerTestScaffold alloc] init]; FlutterThreadSynchronizer* synchronizer = scaffold.synchronizer; @@ -187,7 +187,7 @@ - (void)joinRender { [scaffold joinMain]; } -TEST(FlutterThreadSynchronizerTest, FinishResizingWhenShuttingDown) { +TEST(FlutterThreadSynchronizerTest, ShutdownMakesEverythingNonBlocking) { FlutterThreadSynchronizerTestScaffold* scaffold = [[FlutterThreadSynchronizerTestScaffold alloc] init]; FlutterThreadSynchronizer* synchronizer = scaffold.synchronizer; @@ -242,3 +242,147 @@ - (void)joinRender { [scaffold joinRender]; EXPECT_FALSE([synchronizer isWaitingWhenMutexIsAvailable]); } + +TEST(FlutterThreadSynchronizerTest, RegularCommitForMultipleViews) { + FlutterThreadSynchronizerTestScaffold* scaffold = + [[FlutterThreadSynchronizerTestScaffold alloc] init]; + FlutterThreadSynchronizer* synchronizer = scaffold.synchronizer; + fml::AutoResetWaitableEvent begunResizingLatch; + fml::AutoResetWaitableEvent* begunResizing = &begunResizingLatch; + + [synchronizer registerView:1]; + [synchronizer registerView:2]; + + // Initial resize: does not block until the first frame. + [scaffold dispatchMainTask:^{ + [synchronizer beginResizeForView:1 + size:CGSize{5, 5} + notify:^{ + }]; + [synchronizer beginResizeForView:2 + size:CGSize{15, 15} + notify:^{ + begunResizing->Signal(); + }]; + }]; + begunResizing->Wait(); + EXPECT_FALSE([synchronizer isWaitingWhenMutexIsAvailable]); + [scaffold joinMain]; + + // Still does not block. + [scaffold dispatchMainTask:^{ + [synchronizer beginResizeForView:1 + size:CGSize{7, 7} + notify:^{ + begunResizing->Signal(); + }]; + }]; + begunResizing->Signal(); + EXPECT_FALSE([synchronizer isWaitingWhenMutexIsAvailable]); + [scaffold joinMain]; + + // First frame + [scaffold dispatchRenderTask:^{ + [synchronizer performCommitForView:1 + size:CGSize{7, 7} + notify:^{ + }]; + [synchronizer performCommitForView:2 + size:CGSize{15, 15} + notify:^{ + }]; + }]; + [scaffold joinRender]; + EXPECT_FALSE([synchronizer isWaitingWhenMutexIsAvailable]); +} + +TEST(FlutterThreadSynchronizerTest, ResizingForMultipleViews) { + FlutterThreadSynchronizerTestScaffold* scaffold = + [[FlutterThreadSynchronizerTestScaffold alloc] init]; + FlutterThreadSynchronizer* synchronizer = scaffold.synchronizer; + fml::AutoResetWaitableEvent begunResizingLatch; + fml::AutoResetWaitableEvent* begunResizing = &begunResizingLatch; + + [synchronizer registerView:1]; + [synchronizer registerView:2]; + + // Initial resize: does not block until the first frame. + [scaffold dispatchMainTask:^{ + [synchronizer beginResizeForView:1 + size:CGSize{5, 5} + notify:^{ + }]; + [synchronizer beginResizeForView:2 + size:CGSize{15, 15} + notify:^{ + }]; + }]; + [scaffold joinMain]; + EXPECT_FALSE([synchronizer isWaitingWhenMutexIsAvailable]); + + // First frame. + [scaffold dispatchRenderTask:^{ + [synchronizer performCommitForView:1 + size:CGSize{5, 5} + notify:^{ + }]; + [synchronizer performCommitForView:2 + size:CGSize{15, 15} + notify:^{ + }]; + }]; + [scaffold joinRender]; + EXPECT_FALSE([synchronizer isWaitingWhenMutexIsAvailable]); + + // Resize view 2 to (17, 17): blocks until the next frame. + [scaffold dispatchMainTask:^{ + [synchronizer beginResizeForView:2 + size:CGSize{17, 17} + notify:^{ + begunResizing->Signal(); + }]; + }]; + begunResizing->Wait(); + EXPECT_TRUE([synchronizer isWaitingWhenMutexIsAvailable]); + + // Render view 1 with the size. Still blocking. + [scaffold dispatchRenderTask:^{ + [synchronizer performCommitForView:1 + size:CGSize{5, 5} + notify:^{ + }]; + }]; + [scaffold joinRender]; + EXPECT_TRUE([synchronizer isWaitingWhenMutexIsAvailable]); + + // Render view 2 with the old size. Still blocking. + [scaffold dispatchRenderTask:^{ + [synchronizer performCommitForView:1 + size:CGSize{15, 15} + notify:^{ + }]; + }]; + [scaffold joinRender]; + EXPECT_TRUE([synchronizer isWaitingWhenMutexIsAvailable]); + + // Render view 1 with the size. + [scaffold dispatchRenderTask:^{ + [synchronizer performCommitForView:1 + size:CGSize{5, 5} + notify:^{ + }]; + }]; + [scaffold joinRender]; + EXPECT_TRUE([synchronizer isWaitingWhenMutexIsAvailable]); + + // Render view 2 with the new size. Unblocks. + [scaffold dispatchRenderTask:^{ + [synchronizer performCommitForView:2 + size:CGSize{17, 17} + notify:^{ + }]; + }]; + [scaffold joinRender]; + [scaffold joinMain]; + EXPECT_FALSE([synchronizer isWaitingWhenMutexIsAvailable]); +} From 98b4013fc988adc4f04c8c64238541e904aacd7e Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Tue, 23 May 2023 17:13:49 -0700 Subject: [PATCH 08/11] Docs and locs --- .../Source/FlutterThreadSynchronizer.h | 30 ++++++++++++++++--- .../Source/FlutterThreadSynchronizer.mm | 4 +++ 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.h b/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.h index 1cd4ee64dfa49..b1a2570159282 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.h +++ b/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.h @@ -6,14 +6,20 @@ /** * Takes care of synchronization between raster and platform thread. + * + * All methods of this class must be called from the platform thread, + * except for performCommitForView:size:notify:. */ @interface FlutterThreadSynchronizer : NSObject +/** + * Creates a FlutterThreadSynchronizer that uses the OS main thread as the + * platform thread. + */ - (nullable instancetype)init; /** - * Called from platform thread. Blocks until a commit with given size (or empty) - * is requested. + * Blocks until all views have a commit with their given sizes (or empty) is requested. */ - (void)beginResizeForView:(int64_t)viewId size:(CGSize)size @@ -32,12 +38,24 @@ size:(CGSize)size notify:(nonnull dispatch_block_t)notify; +/** + * Requests the synchronizer to track another view. + * + * A view must be registered before calling begineResizeForView: or + * performCommitForView:. It is typically done when the view controller is + * created. + */ - (void)registerView:(int64_t)viewId; +/** + * Requests the synchronizer to no longer track a view. + * + * It is typically done when the view controller is created. + */ - (void)deregisterView:(int64_t)viewId; /** - * Called from platform thread when shutting down. + * Called when the engine shuts down. * * Prevents any further synchronization and no longer blocks any threads. */ @@ -45,8 +63,12 @@ @end -@interface FlutterThreadSynchronizer (Test) +@interface FlutterThreadSynchronizer (TestUtils) +/** + * Creates a FlutterThreadSynchronizer that uses the specified queue as the + * platform thread. + */ - (nullable instancetype)initWithMainQueue:(nonnull dispatch_queue_t)queue; /** diff --git a/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.mm b/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.mm index a0561e4d30e8e..75a4ff5037210 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.mm @@ -165,10 +165,14 @@ - (void)performCommitForView:(int64_t)viewId } - (void)registerView:(int64_t)viewId { + dispatch_assert_queue(_mainQueue); + std::unique_lock lock(_mutex); _contentSizes[viewId] = CGSizeZero; } - (void)deregisterView:(int64_t)viewId { + dispatch_assert_queue(_mainQueue); + std::unique_lock lock(_mutex); _contentSizes.erase(viewId); } From 31b54305dc0be59144537fdaed7c7fc781036d4c Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Tue, 23 May 2023 17:27:12 -0700 Subject: [PATCH 09/11] Use better way, and fix tests --- .../macos/framework/Source/FlutterEngine.mm | 4 ++-- .../framework/Source/FlutterEngineTest.mm | 3 ++- .../framework/Source/FlutterEngine_Internal.h | 6 ++++-- .../Source/FlutterThreadSynchronizerTest.mm | 19 +++++++------------ .../framework/Source/FlutterViewController.mm | 6 ++++-- .../Source/FlutterViewController_Internal.h | 9 ++++++--- 6 files changed, 25 insertions(+), 22 deletions(-) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm b/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm index 41269b6e94dd7..ad8d3e870ce20 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm @@ -592,7 +592,7 @@ - (void)registerViewController:(FlutterViewController*)controller forId:(Flutter NSAssert(![controller attached], @"The incoming view controller is already attached to an engine."); NSAssert([_viewControllers objectForKey:@(viewId)] == nil, @"The requested view ID is occupied."); - [controller attachToEngine:self withId:viewId]; + [controller setUpWithEngine:self viewId:viewId threadSynchronizer:_threadSynchronizer]; NSAssert(controller.viewId == viewId, @"Failed to assign view ID."); [_viewControllers setObject:controller forKey:@(viewId)]; } @@ -1122,7 +1122,7 @@ - (NSPasteboard*)pasteboard { return flutter::GetSwitchesFromEnvironment(); } -- (FlutterThreadSynchronizer*)threadSynchronizer { +- (FlutterThreadSynchronizer*)testThreadSynchronizer { return _threadSynchronizer; } diff --git a/shell/platform/darwin/macos/framework/Source/FlutterEngineTest.mm b/shell/platform/darwin/macos/framework/Source/FlutterEngineTest.mm index 5b3e111e89086..cf81a8ad798c2 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterEngineTest.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterEngineTest.mm @@ -34,6 +34,7 @@ @interface FlutterEngine (Test) * May be nil if the compositor has not been initialized yet. */ @property(nonatomic, readonly, nullable) flutter::FlutterCompositor* macOSCompositor; + @end @interface TestPlatformViewFactory : NSObject @@ -440,7 +441,7 @@ - (nonnull NSView*)createWithViewIdentifier:(int64_t)viewId arguments:(nullable result:^(id result){ }]; - [engine.threadSynchronizer blockUntilFrameAvailable]; + [engine.testThreadSynchronizer blockUntilFrameAvailable]; CALayer* rootLayer = viewController.flutterView.layer; diff --git a/shell/platform/darwin/macos/framework/Source/FlutterEngine_Internal.h b/shell/platform/darwin/macos/framework/Source/FlutterEngine_Internal.h index ba11b19851478..be973222ca95d 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterEngine_Internal.h +++ b/shell/platform/darwin/macos/framework/Source/FlutterEngine_Internal.h @@ -108,8 +108,6 @@ typedef NS_ENUM(NSInteger, FlutterAppExitResponse) { */ @property(nonatomic, readonly) FlutterEngineTerminationHandler* terminationHandler; -@property(nonatomic, readonly) FlutterThreadSynchronizer* threadSynchronizer; - /** * Attach a view controller to the engine as its default controller. * @@ -197,4 +195,8 @@ typedef NS_ENUM(NSInteger, FlutterAppExitResponse) { @end +@interface FlutterEngine (Tests) +- (nonnull FlutterThreadSynchronizer*)testThreadSynchronizer; +@end + NS_ASSUME_NONNULL_END diff --git a/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizerTest.mm b/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizerTest.mm index 4e27103f88091..2541f8675576d 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizerTest.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizerTest.mm @@ -80,11 +80,10 @@ - (void)joinRender { [[FlutterThreadSynchronizerTestScaffold alloc] init]; FlutterThreadSynchronizer* synchronizer = scaffold.synchronizer; - [synchronizer registerView:1]; - // Initial resize: does not block until the first frame. __block int notifiedResize = 0; [scaffold dispatchMainTask:^{ + [synchronizer registerView:1]; [synchronizer beginResizeForView:1 size:CGSize{5, 5} notify:^{ @@ -131,10 +130,9 @@ - (void)joinRender { fml::AutoResetWaitableEvent begunResizingLatch; fml::AutoResetWaitableEvent* begunResizing = &begunResizingLatch; - [synchronizer registerView:1]; - // Initial resize: does not block until the first frame. [scaffold dispatchMainTask:^{ + [synchronizer registerView:1]; [synchronizer beginResizeForView:1 size:CGSize{5, 5} notify:^{ @@ -194,10 +192,9 @@ - (void)joinRender { fml::AutoResetWaitableEvent begunResizingLatch; fml::AutoResetWaitableEvent* begunResizing = &begunResizingLatch; - [synchronizer registerView:1]; - // Initial resize [scaffold dispatchMainTask:^{ + [synchronizer registerView:1]; [synchronizer beginResizeForView:1 size:CGSize{5, 5} notify:^{ @@ -250,11 +247,10 @@ - (void)joinRender { fml::AutoResetWaitableEvent begunResizingLatch; fml::AutoResetWaitableEvent* begunResizing = &begunResizingLatch; - [synchronizer registerView:1]; - [synchronizer registerView:2]; - // Initial resize: does not block until the first frame. [scaffold dispatchMainTask:^{ + [synchronizer registerView:1]; + [synchronizer registerView:2]; [synchronizer beginResizeForView:1 size:CGSize{5, 5} notify:^{ @@ -303,11 +299,10 @@ - (void)joinRender { fml::AutoResetWaitableEvent begunResizingLatch; fml::AutoResetWaitableEvent* begunResizing = &begunResizingLatch; - [synchronizer registerView:1]; - [synchronizer registerView:2]; - // Initial resize: does not block until the first frame. [scaffold dispatchMainTask:^{ + [synchronizer registerView:1]; + [synchronizer registerView:2]; [synchronizer beginResizeForView:1 size:CGSize{5, 5} notify:^{ diff --git a/shell/platform/darwin/macos/framework/Source/FlutterViewController.mm b/shell/platform/darwin/macos/framework/Source/FlutterViewController.mm index f7c082b694323..007a7559945f6 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterViewController.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterViewController.mm @@ -402,7 +402,6 @@ static void CommonInit(FlutterViewController* controller, FlutterEngine* engine) controller.engine, controller.viewId); controller->_mouseTrackingMode = FlutterMouseTrackingModeInKeyWindow; controller->_textInputPlugin = [[FlutterTextInputPlugin alloc] initWithViewController:controller]; - controller->_weakViewThreadSynchronizer = engine.threadSynchronizer; [controller initializeKeyboard]; [controller notifySemanticsEnabledChanged]; // macOS fires this message when changing IMEs. @@ -546,10 +545,13 @@ - (void)notifySemanticsEnabledChanged { return _bridge; } -- (void)attachToEngine:(nonnull FlutterEngine*)engine withId:(FlutterViewId)viewId { +- (void)setUpWithEngine:(FlutterEngine*)engine + viewId:(FlutterViewId)viewId + threadSynchronizer:(FlutterThreadSynchronizer*)threadSynchronizer { NSAssert(_engine == nil, @"Already attached to an engine %@.", _engine); _engine = engine; _viewId = viewId; + _weakViewThreadSynchronizer = threadSynchronizer; } - (void)detachFromEngine { diff --git a/shell/platform/darwin/macos/framework/Source/FlutterViewController_Internal.h b/shell/platform/darwin/macos/framework/Source/FlutterViewController_Internal.h index 62801f9bce739..4ffeffac4bb17 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterViewController_Internal.h +++ b/shell/platform/darwin/macos/framework/Source/FlutterViewController_Internal.h @@ -39,11 +39,14 @@ - (BOOL)isDispatchingKeyEvent:(nonnull NSEvent*)event; /** - * Set the `engine` and `id` of this controller. + * Set up the controller with `engine` and `id`, and other engine-level classes. * - * This method is called by FlutterEngine. + * This method is called by FlutterEngine. A view controller must be set up + * before being used, and must be set up only once until detachFromEngine:. */ -- (void)attachToEngine:(nonnull FlutterEngine*)engine withId:(FlutterViewId)viewId; +- (void)setUpWithEngine:(nonnull FlutterEngine*)engine + viewId:(FlutterViewId)viewId + threadSynchronizer:(nonnull FlutterThreadSynchronizer*)threadSynchronizer; /** * Reset the `engine` and `id` of this controller. From 9cb1f69b2e1b638ed99b70e680bf4baa66ed6ebb Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Thu, 25 May 2023 13:31:22 -0700 Subject: [PATCH 10/11] Make FVC reg/dereg --- .../macos/framework/Source/FlutterThreadSynchronizer.h | 2 +- .../darwin/macos/framework/Source/FlutterView.mm | 1 - .../macos/framework/Source/FlutterViewController.mm | 9 ++++++--- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.h b/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.h index b1a2570159282..8d8d248bdefdb 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.h +++ b/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.h @@ -50,7 +50,7 @@ /** * Requests the synchronizer to no longer track a view. * - * It is typically done when the view controller is created. + * It is typically done when the view controller is destroyed. */ - (void)deregisterView:(int64_t)viewId; diff --git a/shell/platform/darwin/macos/framework/Source/FlutterView.mm b/shell/platform/darwin/macos/framework/Source/FlutterView.mm index db20790f07b57..2c30a7457dc8a 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterView.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterView.mm @@ -37,7 +37,6 @@ - (instancetype)initWithMTLDevice:(id)device commandQueue:commandQueue layer:self.layer delegate:self]; - [_threadSynchronizer registerView:viewId]; } return self; } diff --git a/shell/platform/darwin/macos/framework/Source/FlutterViewController.mm b/shell/platform/darwin/macos/framework/Source/FlutterViewController.mm index 007a7559945f6..af56e910904fc 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterViewController.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterViewController.mm @@ -375,7 +375,7 @@ @implementation FlutterViewController { // FlutterViewController does not actually uses the synchronizer, but only // passes it to FlutterView. - __weak FlutterThreadSynchronizer* _weakViewThreadSynchronizer; + FlutterThreadSynchronizer* _threadSynchronizer; } @synthesize viewId = _viewId; @@ -551,11 +551,14 @@ - (void)setUpWithEngine:(FlutterEngine*)engine NSAssert(_engine == nil, @"Already attached to an engine %@.", _engine); _engine = engine; _viewId = viewId; - _weakViewThreadSynchronizer = threadSynchronizer; + _threadSynchronizer = threadSynchronizer; + [_threadSynchronizer registerView:_viewId]; } - (void)detachFromEngine { NSAssert(_engine != nil, @"Not attached to any engine."); + [_threadSynchronizer deregisterView:_viewId]; + _threadSynchronizer = nil; _engine = nil; } @@ -866,7 +869,7 @@ - (nonnull FlutterView*)createFlutterViewWithMTLDevice:(id)device return [[FlutterView alloc] initWithMTLDevice:device commandQueue:commandQueue reshapeListener:self - threadSynchronizer:_weakViewThreadSynchronizer + threadSynchronizer:_threadSynchronizer viewId:_viewId]; } From da1530460b463cd4c9ef8f82a5a729a549921f51 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Thu, 25 May 2023 13:33:03 -0700 Subject: [PATCH 11/11] Remove FV:shutdown --- .../platform/darwin/macos/framework/Source/FlutterEngine.mm | 5 ----- shell/platform/darwin/macos/framework/Source/FlutterView.h | 6 ------ shell/platform/darwin/macos/framework/Source/FlutterView.mm | 2 -- 3 files changed, 13 deletions(-) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm b/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm index ad8d3e870ce20..d4f194cfca01e 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm @@ -931,13 +931,8 @@ - (void)shutDownEngine { return; } - NSEnumerator* viewControllerEnumerator = [_viewControllers objectEnumerator]; [_threadSynchronizer shutdown]; _threadSynchronizer = nil; - FlutterViewController* nextViewController; - while ((nextViewController = [viewControllerEnumerator nextObject])) { - [nextViewController.flutterView shutdown]; - } FlutterEngineResult result = _embedderAPI.Deinitialize(_engine); if (result != kSuccess) { diff --git a/shell/platform/darwin/macos/framework/Source/FlutterView.h b/shell/platform/darwin/macos/framework/Source/FlutterView.h index fe2d55aac554d..0c468d4c58d08 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterView.h +++ b/shell/platform/darwin/macos/framework/Source/FlutterView.h @@ -59,12 +59,6 @@ constexpr FlutterViewId kFlutterDefaultViewId = 0ll; */ @property(readonly, nonatomic, nonnull) FlutterSurfaceManager* surfaceManager; -/** - * Must be called when shutting down. Unblocks raster thread and prevents any further - * synchronization. - */ -- (void)shutdown; - /** * By default, the `FlutterSurfaceManager` creates two layers to manage Flutter * content, the content layer and containing layer. To set the native background diff --git a/shell/platform/darwin/macos/framework/Source/FlutterView.mm b/shell/platform/darwin/macos/framework/Source/FlutterView.mm index 2c30a7457dc8a..89bbdb9153828 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterView.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterView.mm @@ -113,8 +113,6 @@ - (BOOL)layer:(CALayer*)layer return YES; } -- (void)shutdown { -} #pragma mark - NSAccessibility overrides - (BOOL)isAccessibilityElement {