Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions shell/platform/darwin/ios/framework/Source/FlutterEngine.mm
Original file line number Diff line number Diff line change
Expand Up @@ -942,10 +942,10 @@ - (FlutterEngine*)spawnWithEntrypoint:(/*nullable*/ NSString*)entrypoint
SetEntryPoint(&settings, entrypoint, libraryURI);

flutter::Shell::CreateCallback<flutter::PlatformView> on_create_platform_view =
[self](flutter::Shell& shell) {
[self recreatePlatformViewController];
[result](flutter::Shell& shell) {
[result recreatePlatformViewController];
return std::make_unique<flutter::PlatformViewIOS>(
shell, self->_renderingApi, self->_platformViewsController, shell.GetTaskRunners());
Comment on lines -945 to -948
Copy link
Member Author

Choose a reason for hiding this comment

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

This was the root of all the problems I experienced the last couple of days. I don't think we need the ScopedTaskRunner. I got confused..

Copy link
Member

Choose a reason for hiding this comment

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

This was the root of all the problems I experienced the last couple of days. I don't think we need the ScopedTaskRunner.

Wait rly? Ha, awesome.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yea, so I was getting crashes inside the raster thread after the spawner was killed. It was a crash talking to the external view embedder. So, it appeared that the raster event queue had a queued event talking to that deleted rasterizer. It turns out the rasterizer that was involved in the crash was actually the spawned engines rasterizer. So, the event was correctly going forward with a live rasterizer but it ultimately was referencing something that had been deleted, the platformViewsController. It's easy to overlook that that is a shared_ptr, even then I didn't expect there to be a crash inside. The shared_ptr is kind of pointless if it doesn't actually maintain ownership of its children (or use weak ptrs).

shell, result->_renderingApi, result->_platformViewsController, shell.GetTaskRunners());
};

flutter::Shell::CreateCallback<flutter::Rasterizer> on_create_rasterizer =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
0DB7820022EA2C9D00E9B371 /* App.framework in CopyFiles */ = {isa = PBXBuildFile; fileRef = 246B4E4122E3B5F700073EBF /* App.framework */; settings = {ATTRIBUTES = (CodeSignOnCopy, RemoveHeadersOnCopy, ); }; };
0DB7820122EA2CA500E9B371 /* App.framework in CopyFiles */ = {isa = PBXBuildFile; fileRef = 246B4E4122E3B5F700073EBF /* App.framework */; settings = {ATTRIBUTES = (CodeSignOnCopy, RemoveHeadersOnCopy, ); }; };
0DB7820222EA493B00E9B371 /* FlutterViewControllerTest.m in Sources */ = {isa = PBXBuildFile; fileRef = 0DB781FC22EA2C0300E9B371 /* FlutterViewControllerTest.m */; };
0DDEBC89258830B40065D0E8 /* SpawnEngineTest.m in Sources */ = {isa = PBXBuildFile; fileRef = 0DDEBC88258830B40065D0E8 /* SpawnEngineTest.m */; };
0DDEBC8B258839760065D0E8 /* golden_spawn_engine_works_iPhone 8_simulator.png in Resources */ = {isa = PBXBuildFile; fileRef = 0DDEBC8A258839760065D0E8 /* golden_spawn_engine_works_iPhone 8_simulator.png */; };
242F37A222E636DE001E83D4 /* Flutter.xcframework in Embed Frameworks */ = {isa = PBXBuildFile; fileRef = 246B4E4522E3B61000073EBF /* Flutter.xcframework */; settings = {ATTRIBUTES = (CodeSignOnCopy, RemoveHeadersOnCopy, ); }; };
242F37A322E636DE001E83D4 /* App.framework in Embed Frameworks */ = {isa = PBXBuildFile; fileRef = 246B4E4122E3B5F700073EBF /* App.framework */; settings = {ATTRIBUTES = (CodeSignOnCopy, RemoveHeadersOnCopy, ); }; };
244EA6D0230DBE8900B2D26E /* golden_platform_view_D21AP.png in Resources */ = {isa = PBXBuildFile; fileRef = 244EA6CF230DBE8900B2D26E /* golden_platform_view_D21AP.png */; };
Expand Down Expand Up @@ -124,6 +126,9 @@
0D8470A2240F0B1F0030B565 /* StatusBarTest.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = StatusBarTest.h; sourceTree = "<group>"; };
0D8470A3240F0B1F0030B565 /* StatusBarTest.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = StatusBarTest.m; sourceTree = "<group>"; };
0DB781FC22EA2C0300E9B371 /* FlutterViewControllerTest.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = FlutterViewControllerTest.m; sourceTree = "<group>"; };
0DDEBC87258830B40065D0E8 /* SpawnEngineTest.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = SpawnEngineTest.h; sourceTree = "<group>"; };
0DDEBC88258830B40065D0E8 /* SpawnEngineTest.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = SpawnEngineTest.m; sourceTree = "<group>"; };
0DDEBC8A258839760065D0E8 /* golden_spawn_engine_works_iPhone 8_simulator.png */ = {isa = PBXFileReference; lastKnownFileType = image.png; path = "golden_spawn_engine_works_iPhone 8_simulator.png"; sourceTree = "<group>"; };
244EA6CF230DBE8900B2D26E /* golden_platform_view_D21AP.png */ = {isa = PBXFileReference; lastKnownFileType = image.png; path = golden_platform_view_D21AP.png; sourceTree = "<group>"; };
246A6610252E693A00EAB0F3 /* RenderingSelectionTest.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = RenderingSelectionTest.m; sourceTree = "<group>"; };
246B4E4122E3B5F700073EBF /* App.framework */ = {isa = PBXFileReference; lastKnownFileType = wrapper.framework; path = App.framework; sourceTree = "<group>"; };
Expand Down Expand Up @@ -252,6 +257,7 @@
248D76ED22E388380012F0C1 /* ScenariosUITests */ = {
isa = PBXGroup;
children = (
0DDEBC8A258839760065D0E8 /* golden_spawn_engine_works_iPhone 8_simulator.png */,
4F06F1B124731F66000AF246 /* LocalizationInitializationTest.m */,
6402EBD024147BDA00987DCB /* UnobstructedPlatformViewTests.m */,
0AC83145256E534E00DAE6BE /* golden_bogus_font_text_iPhone 8_simulator.png */,
Expand Down Expand Up @@ -284,6 +290,8 @@
0A42BFB32447E179007E212E /* TextSemanticsFocusTest.m */,
0A42BFB52447E19F007E212E /* TextSemanticsFocusTest.h */,
246A6610252E693A00EAB0F3 /* RenderingSelectionTest.m */,
0DDEBC87258830B40065D0E8 /* SpawnEngineTest.h */,
0DDEBC88258830B40065D0E8 /* SpawnEngineTest.m */,
);
path = ScenariosUITests;
sourceTree = "<group>";
Expand Down Expand Up @@ -433,6 +441,7 @@
59A97FDA236B984300B4C066 /* golden_platform_view_multiple_background_foreground_iPhone SE_simulator.png in Resources */,
3DEF491923C3BE6500184216 /* golden_platform_view_rotate_iPhone 8_simulator.png in Resources */,
24D47D1B230C79840069DD5E /* golden_platform_view_D211AP.png in Resources */,
0DDEBC8B258839760065D0E8 /* golden_spawn_engine_works_iPhone 8_simulator.png in Resources */,
244EA6D0230DBE8900B2D26E /* golden_platform_view_D21AP.png in Resources */,
3DEF491A23C3BE6500184216 /* golden_platform_view_transform_iPhone 8_simulator.png in Resources */,
);
Expand Down Expand Up @@ -480,6 +489,7 @@
246A6611252E693A00EAB0F3 /* RenderingSelectionTest.m in Sources */,
4F06F1B32473296E000AF246 /* LocalizationInitializationTest.m in Sources */,
0A42BFB42447E179007E212E /* TextSemanticsFocusTest.m in Sources */,
0DDEBC89258830B40065D0E8 /* SpawnEngineTest.m in Sources */,
);
runOnlyForDeploymentPostprocessing = 0;
};
Expand Down
21 changes: 17 additions & 4 deletions testing/scenario_app/ios/Scenarios/Scenarios/AppDelegate.m
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,8 @@ - (BOOL)application:(UIApplication*)application
@"--text-semantics-focus" : @"text_semantics_focus",
@"--animated-color-square" : @"animated_color_square",
@"--platform-view-with-continuous-texture" : @"platform_view_with_continuous_texture",
@"--bogus-font-text" : @"bogus_font_text"
@"--bogus-font-text" : @"bogus_font_text",
@"--spawn-engine-works" : @"spawn_engine_works",
};
__block NSString* flutterViewControllerTestName = nil;
[launchArgsMap
Expand All @@ -80,6 +81,20 @@ - (BOOL)application:(UIApplication*)application
return [super application:application didFinishLaunchingWithOptions:launchOptions];
}

- (FlutterEngine*)engineForTest:(NSString*)scenarioIdentifier {
if ([scenarioIdentifier isEqualToString:@"spawn_engine_works"]) {
FlutterEngine* spawner = [[FlutterEngine alloc] initWithName:@"FlutterControllerTest"
project:nil];
[spawner run];
return [spawner spawnWithEntrypoint:nil libraryURI:nil];
} else {
FlutterEngine* engine = [[FlutterEngine alloc] initWithName:@"FlutterControllerTest"
project:nil];
[engine run];
return engine;
}
}

- (FlutterViewController*)flutterViewControllerForTest:(NSString*)scenarioIdentifier
withEngine:(FlutterEngine*)engine {
if ([scenarioIdentifier isEqualToString:@"tap_status_bar"]) {
Expand All @@ -90,9 +105,7 @@ - (FlutterViewController*)flutterViewControllerForTest:(NSString*)scenarioIdenti
}

- (void)setupFlutterViewControllerTest:(NSString*)scenarioIdentifier {
FlutterEngine* engine = [[FlutterEngine alloc] initWithName:@"FlutterControllerTest" project:nil];
[engine run];

FlutterEngine* engine = [self engineForTest:scenarioIdentifier];
FlutterViewController* flutterViewController =
[self flutterViewControllerForTest:scenarioIdentifier withEngine:engine];

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,7 @@ NS_ASSUME_NONNULL_BEGIN
@interface FlutterEngine (ScenariosTest)
- (instancetype)initWithScenario:(NSString*)scenario
withCompletion:(nullable void (^)(void))engineRunCompletion;
- (FlutterEngine*)spawnWithEntrypoint:(nullable NSString*)entrypoint
libraryURI:(nullable NSString*)libraryURI;
@end
NS_ASSUME_NONNULL_END
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// Copyright 2020 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 <XCTest/XCTest.h>

NS_ASSUME_NONNULL_BEGIN

@interface SpawnEngineTest : XCTestCase
@property(nonatomic, strong) XCUIApplication* application;
@end

NS_ASSUME_NONNULL_END
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
// Copyright 2020 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 "SpawnEngineTest.h"
#import "GoldenImage.h"

Copy link
Member

Choose a reason for hiding this comment

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

might be simpler if you used GoldenTestManager.h

Copy link
Member Author

Choose a reason for hiding this comment

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

I had an eye out for this too. These tests are kind of a mess. You don't want to be in a situation where when you add tests you have to edit existing code, you should just be writing new code. That's why I don't like the GoldenTestManager. Anything that seemed to have references to platform view tests I tried to avoid, to help us try to get a perspective on what a test should really have. I didn't have a chance to refactor because my environment is broken.

Copy link
Member

Choose a reason for hiding this comment

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

GoldenTestManager isn't related to platform view anymore. It was previously but I refactored it a bit a few weeks ago.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is specifically what I'm talking about:
https://github.com/gaaclarke/engine/blob/788f879350fc48199415d0eecf708e33c3d2ebf5/testing/scenario_app/ios/Scenarios/ScenariosUITests/GoldenTestManager.m#L26:L26

This has to be edited to be used. Once I saw "platform view" I avoided it.

@implementation SpawnEngineTest

- (void)setUp {
[super setUp];
self.continueAfterFailure = NO;

self.application = [[XCUIApplication alloc] init];
self.application.launchArguments = @[ @"--spawn-engine-works", @"--enable-software-rendering" ];
[self.application launch];
}

- (void)testSpawnEngineWorks {
NSString* prefix = @"golden_spawn_engine_works_";
GoldenImage* golden = [[GoldenImage alloc] initWithGoldenNamePrefix:prefix];
if (!golden.image) {
XCTFail(@"unable to find golden image for: %@", prefix);
}
XCUIScreenshot* screenshot = [[XCUIScreen mainScreen] screenshot];
if (![golden compareGoldenToImage:screenshot.image]) {
XCTAttachment* screenshotAttachment = [XCTAttachment attachmentWithImage:screenshot.image];
screenshotAttachment.name = [golden.goldenName stringByAppendingString:@"_actual"];
screenshotAttachment.lifetime = XCTAttachmentLifetimeKeepAlways;
[self addAttachment:screenshotAttachment];

XCTFail(@"Goldens do not match. Follow the steps in the "
@"README to update golden named %@ if needed.",
golden.goldenName);
}
}

@end
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
1 change: 1 addition & 0 deletions testing/scenario_app/lib/src/scenarios.dart
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ Map<String, ScenarioFactory> _scenarios = <String, ScenarioFactory>{
'initial_route_reply': () => InitialRouteReply(PlatformDispatcher.instance),
'platform_view_with_continuous_texture': () => PlatformViewWithContinuousTexture(PlatformDispatcher.instance, 'Platform View', id: _viewId++),
'bogus_font_text': () => BogusFontText(PlatformDispatcher.instance),
'spawn_engine_works' : () => BogusFontText(PlatformDispatcher.instance),
Copy link
Member

Choose a reason for hiding this comment

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

You're compounding 2 tests. Maybe something generic like PoppableScreenScenario?

Copy link
Member Author

Choose a reason for hiding this comment

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

I planned on doing this. Unfortunately I spent hours yesterday trying to fix my scenario tests. I keep getting a bad kernel error. I can't figure it out so I can't work on this right now.

Copy link
Member Author

Choose a reason for hiding this comment

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

};

Map<String, dynamic> _currentScenarioParams = <String, dynamic>{};
Expand Down
2 changes: 1 addition & 1 deletion testing/scenario_app/run_ios_tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,6 @@ fi
cd ios/Scenarios
set -o pipefail && xcodebuild -sdk iphonesimulator \
-scheme Scenarios \
-destination 'platform=iOS Simulator,name=iPhone 8' \
-destination 'platform=iOS Simulator,OS=13.0,name=iPhone 8' \
Copy link
Member

Choose a reason for hiding this comment

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

what was the issue? Was it behaving different on Xcode 12?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yea, on CI their default OS is 13, if you run it with a recent version of Xcode it will use iOS 14 which will cause all the tests to fail (minor differences in the golden images).

test \
FLUTTER_ENGINE="$FLUTTER_ENGINE"