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

Commit f4dc96a

Browse files
authored
[macOS] Multi-view thread synchronizer (#41915)
This PR adapts `FlutterThreadSynchronizer` to multi-view. ### Problem The `FlutterThreadSynchronizer` is a macOS engine class to ensure that, if the current window is resized, nothing will not be presented until the layer tree is drawn with the up-to-date size. This class is not compatible with multiview. A simple way to realize it is: while the class is owned by `FlutterView`, it blocks the _global_ platform thread and rasterizer thread - there is got to be some problems. Indeed, when I was testing with multiple windows (#40399), the app freezes as soon as I resize a window. The problem is because Flutter only have one rasterizer thread. When I'm resizing window A, A's synchronizer detects that the size mismatches, so it blocks the rasterizer thread to wait for an updated content with the updated size. However, window B is to be rendered next, and B's size matches and will try to rasterize, and will be blocked due to the blocked rasterizer thread, window A's updated content will never arrive, causing a deadlock. ### Changes This PR removes the single-view assumption of `FlutterThreadSynchronizer` by making it created by `FlutterEngine` and shared by all `FlutterView`s, and that it prevents rasterization for all views if any view has a mismatched size. The synchronizer is assigned to the view controller in the `attachToEngine:withId` method (now renamed to `setUpWithEngine:viewId:threadSynchronizer:`. This PR also adds unit tests for `FlutterThreadSynchronizer`, which didn't have any unit tests at all. * To achieve this, the `beginResizeForView:` method no longer checks whether is called on the main thread, but whether it's called on the main queue. These are equivalent for the real main queue, since the documentation promises that the main queue always executes on the main thread. However, this change allows substituting the queue with a custom one for unit tests. [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
1 parent f46ea7e commit f4dc96a

13 files changed

+611
-66
lines changed

ci/licenses_golden/licenses_flutter

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2746,6 +2746,9 @@ ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterTex
27462746
ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterTextInputSemanticsObjectTest.mm + ../../../flutter/LICENSE
27472747
ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterTextureRegistrar.h + ../../../flutter/LICENSE
27482748
ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterTextureRegistrar.mm + ../../../flutter/LICENSE
2749+
ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.h + ../../../flutter/LICENSE
2750+
ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.mm + ../../../flutter/LICENSE
2751+
ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizerTest.mm + ../../../flutter/LICENSE
27492752
ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterUmbrellaImportTests.m + ../../../flutter/LICENSE
27502753
ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterView.h + ../../../flutter/LICENSE
27512754
ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterView.mm + ../../../flutter/LICENSE
@@ -5414,6 +5417,7 @@ FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterTextu
54145417
FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterTextureRegistrar.mm
54155418
FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.h
54165419
FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizer.mm
5420+
FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterThreadSynchronizerTest.mm
54175421
FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterUmbrellaImportTests.m
54185422
FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterView.h
54195423
FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterView.mm

shell/platform/darwin/macos/BUILD.gn

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,7 @@ executable("flutter_desktop_darwin_unittests") {
182182
"framework/Source/FlutterSurfaceManagerTest.mm",
183183
"framework/Source/FlutterTextInputPluginTest.mm",
184184
"framework/Source/FlutterTextInputSemanticsObjectTest.mm",
185+
"framework/Source/FlutterThreadSynchronizerTest.mm",
185186
"framework/Source/FlutterViewControllerTest.mm",
186187
"framework/Source/FlutterViewControllerTestUtils.h",
187188
"framework/Source/FlutterViewControllerTestUtils.mm",

shell/platform/darwin/macos/framework/Source/FlutterEngine.mm

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -388,6 +388,8 @@ @implementation FlutterEngine {
388388
// A method channel for miscellaneous platform functionality.
389389
FlutterMethodChannel* _platformChannel;
390390

391+
FlutterThreadSynchronizer* _threadSynchronizer;
392+
391393
int _nextViewId;
392394
}
393395

@@ -427,6 +429,7 @@ - (instancetype)initWithName:(NSString*)labelPrefix
427429
object:nil];
428430

429431
_platformViewController = [[FlutterPlatformViewController alloc] init];
432+
_threadSynchronizer = [[FlutterThreadSynchronizer alloc] init];
430433
[self setUpPlatformViewChannel];
431434
[self setUpAccessibilityChannel];
432435
[self setUpNotificationCenterListeners];
@@ -589,7 +592,7 @@ - (void)registerViewController:(FlutterViewController*)controller forId:(Flutter
589592
NSAssert(![controller attached],
590593
@"The incoming view controller is already attached to an engine.");
591594
NSAssert([_viewControllers objectForKey:@(viewId)] == nil, @"The requested view ID is occupied.");
592-
[controller attachToEngine:self withId:viewId];
595+
[controller setUpWithEngine:self viewId:viewId threadSynchronizer:_threadSynchronizer];
593596
NSAssert(controller.viewId == viewId, @"Failed to assign view ID.");
594597
[_viewControllers setObject:controller forKey:@(viewId)];
595598
}
@@ -928,11 +931,8 @@ - (void)shutDownEngine {
928931
return;
929932
}
930933

931-
NSEnumerator* viewControllerEnumerator = [_viewControllers objectEnumerator];
932-
FlutterViewController* nextViewController;
933-
while ((nextViewController = [viewControllerEnumerator nextObject])) {
934-
[nextViewController.flutterView shutdown];
935-
}
934+
[_threadSynchronizer shutdown];
935+
_threadSynchronizer = nil;
936936

937937
FlutterEngineResult result = _embedderAPI.Deinitialize(_engine);
938938
if (result != kSuccess) {
@@ -1117,6 +1117,10 @@ - (NSPasteboard*)pasteboard {
11171117
return flutter::GetSwitchesFromEnvironment();
11181118
}
11191119

1120+
- (FlutterThreadSynchronizer*)testThreadSynchronizer {
1121+
return _threadSynchronizer;
1122+
}
1123+
11201124
#pragma mark - FlutterBinaryMessenger
11211125

11221126
- (void)sendOnChannel:(nonnull NSString*)channel message:(nullable NSData*)message {

shell/platform/darwin/macos/framework/Source/FlutterEngineTest.mm

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,13 +25,16 @@
2525
// CREATE_NATIVE_ENTRY and MOCK_ENGINE_PROC are leaky by design
2626
// NOLINTBEGIN(clang-analyzer-core.StackAddressEscape)
2727

28+
constexpr int64_t kDefaultViewId = 0ll;
29+
2830
@interface FlutterEngine (Test)
2931
/**
3032
* The FlutterCompositor object currently in use by the FlutterEngine.
3133
*
3234
* May be nil if the compositor has not been initialized yet.
3335
*/
3436
@property(nonatomic, readonly, nullable) flutter::FlutterCompositor* macOSCompositor;
37+
3538
@end
3639

3740
@interface TestPlatformViewFactory : NSObject <FlutterPlatformViewFactory>
@@ -438,7 +441,7 @@ - (nonnull NSView*)createWithViewIdentifier:(int64_t)viewId arguments:(nullable
438441
result:^(id result){
439442
}];
440443

441-
[viewController.flutterView.threadSynchronizer blockUntilFrameAvailable];
444+
[engine.testThreadSynchronizer blockUntilFrameAvailable];
442445

443446
CALayer* rootLayer = viewController.flutterView.layer;
444447

@@ -629,9 +632,10 @@ - (nonnull NSView*)createWithViewIdentifier:(int64_t)viewId arguments:(nullable
629632
[threadSynchronizer shutdown];
630633

631634
std::thread rasterThread([&threadSynchronizer] {
632-
[threadSynchronizer performCommit:CGSizeMake(100, 100)
633-
notify:^{
634-
}];
635+
[threadSynchronizer performCommitForView:kDefaultViewId
636+
size:CGSizeMake(100, 100)
637+
notify:^{
638+
}];
635639
});
636640

637641
rasterThread.join();

shell/platform/darwin/macos/framework/Source/FlutterEngine_Internal.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,4 +195,8 @@ typedef NS_ENUM(NSInteger, FlutterAppExitResponse) {
195195

196196
@end
197197

198+
@interface FlutterEngine (Tests)
199+
- (nonnull FlutterThreadSynchronizer*)testThreadSynchronizer;
200+
@end
201+
198202
NS_ASSUME_NONNULL_END
Lines changed: 70 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,29 @@
1+
// Copyright 2013 The Flutter Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
15
#import <Cocoa/Cocoa.h>
26

37
/**
48
* Takes care of synchronization between raster and platform thread.
9+
*
10+
* All methods of this class must be called from the platform thread,
11+
* except for performCommitForView:size:notify:.
512
*/
613
@interface FlutterThreadSynchronizer : NSObject
714

815
/**
9-
* Blocks current thread until there is frame available.
10-
* Used in FlutterEngineTest.
16+
* Creates a FlutterThreadSynchronizer that uses the OS main thread as the
17+
* platform thread.
1118
*/
12-
- (void)blockUntilFrameAvailable;
19+
- (nullable instancetype)init;
1320

1421
/**
15-
* Called from platform thread. Blocks until commit with given size (or empty)
16-
* is requested.
22+
* Blocks until all views have a commit with their given sizes (or empty) is requested.
1723
*/
18-
- (void)beginResize:(CGSize)size notify:(nonnull dispatch_block_t)notify;
24+
- (void)beginResizeForView:(int64_t)viewId
25+
size:(CGSize)size
26+
notify:(nonnull dispatch_block_t)notify;
1927

2028
/**
2129
* Called from raster thread. Schedules the given block on platform thread
@@ -26,11 +34,65 @@
2634
*
2735
* The notify block is guaranteed to be called within a core animation transaction.
2836
*/
29-
- (void)performCommit:(CGSize)size notify:(nonnull dispatch_block_t)notify;
37+
- (void)performCommitForView:(int64_t)viewId
38+
size:(CGSize)size
39+
notify:(nonnull dispatch_block_t)notify;
40+
41+
/**
42+
* Requests the synchronizer to track another view.
43+
*
44+
* A view must be registered before calling begineResizeForView: or
45+
* performCommitForView:. It is typically done when the view controller is
46+
* created.
47+
*/
48+
- (void)registerView:(int64_t)viewId;
49+
50+
/**
51+
* Requests the synchronizer to no longer track a view.
52+
*
53+
* It is typically done when the view controller is destroyed.
54+
*/
55+
- (void)deregisterView:(int64_t)viewId;
3056

3157
/**
32-
* Called when shutting down. Unblocks everything and prevents any further synchronization.
58+
* Called when the engine shuts down.
59+
*
60+
* Prevents any further synchronization and no longer blocks any threads.
3361
*/
3462
- (void)shutdown;
3563

3664
@end
65+
66+
@interface FlutterThreadSynchronizer (TestUtils)
67+
68+
/**
69+
* Creates a FlutterThreadSynchronizer that uses the specified queue as the
70+
* platform thread.
71+
*/
72+
- (nullable instancetype)initWithMainQueue:(nonnull dispatch_queue_t)queue;
73+
74+
/**
75+
* Blocks current thread until the mutex is available, then return whether the
76+
* synchronizer is waiting for a correct commit during resizing.
77+
*
78+
* After calling an operation of the thread synchronizer, call this method,
79+
* and when it returns, the thread synchronizer can be at one of the following 3
80+
* states:
81+
*
82+
* 1. The operation has not started at all (with a return value FALSE.)
83+
* 2. The operation has ended (with a return value FALSE.)
84+
* 3. beginResizeForView: is in progress, waiting (with a return value TRUE.)
85+
*
86+
* By eliminating the 1st case (such as using the notify callback), we can use
87+
* this return value to decide whether the synchronizer is in case 2 or case 3,
88+
* that is whether the resizing is blocked by a mismatching commit.
89+
*/
90+
- (BOOL)isWaitingWhenMutexIsAvailable;
91+
92+
/**
93+
* Blocks current thread until there is frame available.
94+
* Used in FlutterEngineTest.
95+
*/
96+
- (void)blockUntilFrameAvailable;
97+
98+
@end

0 commit comments

Comments
 (0)