Fix iOS crash when multiple platform views are in the scene (flutter#…

…13449) Having 2 or more platform views simultaneously in the layer tree was crashing immediately on iOS with GL backend. This regressed in flutter#11070 which passed gl_context to a function in a loop using std::move (which meant on the second iteration the caller is no longer the owner of the field). I added a scenarios_app test, though this test doesn't run on a physical device on CI so it would have only caught the problem when running locally (flutter/flutter#43852).
elancom · Oct 31, 2019 · c63aefd · c63aefd
1 parent 1765618
commit c63aefd
Show file tree

Hide file tree

Showing 8 changed files with 69 additions and 8 deletions.
diff --git a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm
@@ -368,7 +368,7 @@
 
   bool did_submit = true;
   for (int64_t view_id : composition_order_) {
-    EnsureOverlayInitialized(view_id, std::move(gl_context), gr_context);
+    EnsureOverlayInitialized(view_id, gl_context, gr_context);
     auto frame = overlays_[view_id]->surface->AcquireFrame(frame_size_);
     SkCanvas* canvas = frame->SkiaCanvas();
     canvas->drawPicture(picture_recorders_[view_id]->finishRecordingAsPicture());

diff --git a/testing/scenario_app/ios/Scenarios/Scenarios.xcodeproj/project.pbxproj b/testing/scenario_app/ios/Scenarios/Scenarios.xcodeproj/project.pbxproj
@@ -30,6 +30,7 @@
 		24D47D1B230C79840069DD5E /* golden_platform_view_D211AP.png in Resources */ = {isa = PBXBuildFile; fileRef = 24D47D1A230C79840069DD5E /* golden_platform_view_D211AP.png */; };
 		24D47D1D230CA2700069DD5E /* golden_platform_view_iPhone SE_simulator.png in Resources */ = {isa = PBXBuildFile; fileRef = 24D47D1C230CA2700069DD5E /* golden_platform_view_iPhone SE_simulator.png */; };
 		24F1FB89230B4579005ACE7C /* TextPlatformView.m in Sources */ = {isa = PBXBuildFile; fileRef = 24F1FB87230B4579005ACE7C /* TextPlatformView.m */; };
+		59A97FD8236A49D300B4C066 /* golden_platform_view_multiple_iPhone SE_simulator.png in Resources */ = {isa = PBXBuildFile; fileRef = 59A97FD7236A49D300B4C066 /* golden_platform_view_multiple_iPhone SE_simulator.png */; };
 		6816DB9E231750ED00A51400 /* GoldenPlatformViewTests.m in Sources */ = {isa = PBXBuildFile; fileRef = 6816DB9D231750ED00A51400 /* GoldenPlatformViewTests.m */; };
 		6816DBA12317573300A51400 /* GoldenImage.m in Sources */ = {isa = PBXBuildFile; fileRef = 6816DBA02317573300A51400 /* GoldenImage.m */; };
 		6816DBA42318358200A51400 /* PlatformViewGoldenTestManager.m in Sources */ = {isa = PBXBuildFile; fileRef = 6816DBA32318358200A51400 /* PlatformViewGoldenTestManager.m */; };
@@ -121,6 +122,7 @@
 		24D47D1E230CA4480069DD5E /* README.md */ = {isa = PBXFileReference; lastKnownFileType = net.daringfireball.markdown; path = README.md; sourceTree = "<group>"; };
 		24F1FB87230B4579005ACE7C /* TextPlatformView.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = TextPlatformView.m; sourceTree = "<group>"; };
 		24F1FB88230B4579005ACE7C /* TextPlatformView.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = TextPlatformView.h; sourceTree = "<group>"; };
+		59A97FD7236A49D300B4C066 /* golden_platform_view_multiple_iPhone SE_simulator.png */ = {isa = PBXFileReference; lastKnownFileType = image.png; path = "golden_platform_view_multiple_iPhone SE_simulator.png"; sourceTree = "<group>"; };
 		6816DB9C231750ED00A51400 /* GoldenPlatformViewTests.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = GoldenPlatformViewTests.h; sourceTree = "<group>"; };
 		6816DB9D231750ED00A51400 /* GoldenPlatformViewTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = GoldenPlatformViewTests.m; sourceTree = "<group>"; };
 		6816DB9F2317573300A51400 /* GoldenImage.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = GoldenImage.h; sourceTree = "<group>"; };
@@ -216,6 +218,7 @@
 		248D76ED22E388380012F0C1 /* ScenariosUITests */ = {
 			isa = PBXGroup;
 			children = (
+				59A97FD7236A49D300B4C066 /* golden_platform_view_multiple_iPhone SE_simulator.png */,
 				244EA6CF230DBE8900B2D26E /* golden_platform_view_D21AP.png */,
 				24D47D1C230CA2700069DD5E /* golden_platform_view_iPhone SE_simulator.png */,
 				24D47D1A230C79840069DD5E /* golden_platform_view_D211AP.png */,
@@ -375,6 +378,7 @@
 				6816DBAA2318696600A51400 /* golden_platform_view_clippath_iPhone SE_simulator.png in Resources */,
 				6816DBAD2318696600A51400 /* golden_platform_view_cliprect_iPhone SE_simulator.png in Resources */,
 				24D47D1B230C79840069DD5E /* golden_platform_view_D211AP.png in Resources */,
+				59A97FD8236A49D300B4C066 /* golden_platform_view_multiple_iPhone SE_simulator.png in Resources */,
 				24D47D1D230CA2700069DD5E /* golden_platform_view_iPhone SE_simulator.png in Resources */,
 				244EA6D0230DBE8900B2D26E /* golden_platform_view_D21AP.png in Resources */,
 				6816DBAC2318696600A51400 /* golden_platform_view_opacity_iPhone SE_simulator.png in Resources */,

diff --git a/testing/scenario_app/ios/Scenarios/Scenarios/AppDelegate.m b/testing/scenario_app/ios/Scenarios/Scenarios/AppDelegate.m
@@ -28,6 +28,7 @@ - (BOOL)application:(UIApplication*)application
   // The launchArgsMap should match the one in the `PlatformVieGoldenTestManager`.
   NSDictionary<NSString*, NSString*>* launchArgsMap = @{
     @"--platform-view" : @"platform_view",
+    @"--platform-view-multiple" : @"platform_view_multiple",
     @"--platform-view-cliprect" : @"platform_view_cliprect",
     @"--platform-view-cliprrect" : @"platform_view_cliprrect",
     @"--platform-view-clippath" : @"platform_view_clippath",

diff --git a/testing/scenario_app/ios/Scenarios/ScenariosUITests/PlatformViewGoldenTestManager.m b/testing/scenario_app/ios/Scenarios/ScenariosUITests/PlatformViewGoldenTestManager.m
@@ -23,6 +23,7 @@ - (instancetype)initWithLaunchArg:(NSString*)launchArg {
     dispatch_once(&onceToken, ^{
       launchArgsMap = @{
         @"--platform-view" : @"platform_view",
+        @"--platform-view-multiple" : @"platform_view_multiple",
         @"--platform-view-cliprect" : @"platform_view_cliprect",
         @"--platform-view-cliprrect" : @"platform_view_cliprrect",
         @"--platform-view-clippath" : @"platform_view_clippath",

diff --git a/testing/scenario_app/ios/Scenarios/ScenariosUITests/PlatformViewUITests.m b/testing/scenario_app/ios/Scenarios/ScenariosUITests/PlatformViewUITests.m
@@ -22,6 +22,24 @@ - (void)testPlatformView {
 
 @end
 
+@interface MultiplePlatformViewsTest : GoldenPlatformViewTests
+
+@end
+
+@implementation MultiplePlatformViewsTest
+
+- (instancetype)initWithInvocation:(NSInvocation*)invocation {
+  PlatformViewGoldenTestManager* manager =
+      [[PlatformViewGoldenTestManager alloc] initWithLaunchArg:@"--platform-view-multiple"];
+  return [super initWithManager:manager invocation:invocation];
+}
+
+- (void)testPlatformView {
+  [self checkGolden];
+}
+
+@end
+
 // Clip Rect Tests
 @interface PlatformViewMutationClipRectTests : GoldenPlatformViewTests
 

diff --git a/...cenarios/ScenariosUITests/golden_platform_view_multiple_iPhone SE_simulator.png b/...cenarios/ScenariosUITests/golden_platform_view_multiple_iPhone SE_simulator.png
diff --git a/testing/scenario_app/lib/main.dart b/testing/scenario_app/lib/main.dart
@@ -22,6 +22,7 @@ Map<String, Scenario> _scenarios = <String, Scenario>{
   'platform_view_clippath': PlatformViewClipPathScenario(window, 'PlatformViewClipPath', id: 3),
   'platform_view_transform': PlatformViewTransformScenario(window, 'PlatformViewTransform', id: 4),
   'platform_view_opacity': PlatformViewOpacityScenario(window, 'PlatformViewOpacity', id: 5),
+  'platform_view_multiple': MultiPlatformViewScenario(window, firstId: 6, secondId: 7),
   'poppable_screen': PoppableScreenScenario(window),
 };
 

diff --git a/testing/scenario_app/lib/src/platform_view.dart b/testing/scenario_app/lib/src/platform_view.dart
@@ -34,7 +34,7 @@ class PlatformViewScenario extends Scenario with _BasePlatformViewScenarioMixin
   PlatformViewScenario(Window window, String text, {int id = 0})
       : assert(window != null),
         super(window) {
-    constructScenario(window, text, id);
+    createPlatformView(window, text, id);
   }
 
   @override
@@ -47,13 +47,45 @@ class PlatformViewScenario extends Scenario with _BasePlatformViewScenarioMixin
   }
 }
 
+/// Builds a scene with 2 platform views.
+class MultiPlatformViewScenario extends Scenario with _BasePlatformViewScenarioMixin {
+  /// Creates the PlatformView scenario.
+  ///
+  /// The [window] parameter must not be null.
+  MultiPlatformViewScenario(Window window, {this.firstId, this.secondId})
+      : assert(window != null),
+        super(window) {
+    createPlatformView(window, 'platform view 1', firstId);
+    createPlatformView(window, 'platform view 2', secondId);
+  }
+
+  /// The platform view identifier to use for the first platform view.
+  final int firstId;
+
+  /// The platform view identifier to use for the second platform view.
+  final int secondId;
+
+  @override
+  void onBeginFrame(Duration duration) {
+    final SceneBuilder builder = SceneBuilder();
+
+    builder.pushOffset(0, 0);
+
+    builder.pushOffset(0, 600);
+    _addPlatformViewtoScene(builder, firstId, 500, 500);
+    builder.pop();
+
+    finishBuilderByAddingPlatformViewAndPicture(builder, secondId);
+  }
+}
+
 /// Platform view with clip rect.
 class PlatformViewClipRectScenario extends Scenario with _BasePlatformViewScenarioMixin {
   /// Constructs a platform view with clip rect scenario.
   PlatformViewClipRectScenario(Window window, String text, {int id = 0})
       : assert(window != null),
         super(window) {
-    constructScenario(window, text, id);
+    createPlatformView(window, text, id);
   }
 
   @override
@@ -165,7 +197,7 @@ mixin _BasePlatformViewScenarioMixin on Scenario {
   /// It prepare a TextPlatformView so it can be added to the SceneBuilder in `onBeginFrame`.
   /// Call this method in the constructor of the platform view related scenarios
   /// to perform necessary set up.
-  void constructScenario(Window window, String text, int id) {
+  void createPlatformView(Window window, String text, int id) {
     const int _valueInt32 = 3;
     const int _valueFloat64 = 6;
     const int _valueString = 7;
@@ -227,15 +259,19 @@ mixin _BasePlatformViewScenarioMixin on Scenario {
     );
   }
 
-  // Add a platform view and a picture to the scene, then finish the `sceneBuilder`.
-  void finishBuilderByAddingPlatformViewAndPicture(SceneBuilder sceneBuilder, int viewId) {
+  void _addPlatformViewtoScene(SceneBuilder sceneBuilder, int viewId, double width, double height) {
     if (Platform.isIOS) {
-      sceneBuilder.addPlatformView(viewId, width: 500, height: 500);
+      sceneBuilder.addPlatformView(viewId, width: width, height: height);
     } else if (Platform.isAndroid && _textureId != null) {
-      sceneBuilder.addTexture(_textureId, offset: const Offset(150, 300), width: 500, height: 500);
+      sceneBuilder.addTexture(_textureId, offset: const Offset(150, 300), width: width, height: height);
     } else {
       throw UnsupportedError('Platform ${Platform.operatingSystem} is not supported');
     }
+  }
+
+  // Add a platform view and a picture to the scene, then finish the `sceneBuilder`.
+  void finishBuilderByAddingPlatformViewAndPicture(SceneBuilder sceneBuilder, int viewId) {
+    _addPlatformViewtoScene(sceneBuilder, viewId, 500, 500);
     final PictureRecorder recorder = PictureRecorder();
     final Canvas canvas = Canvas(recorder);
     canvas.drawCircle(