PlatformViewsController: clear composition_order_ in the beginning of each frame. (flutter#22574)

Author: Chris Yang

shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm

@@ -241,13 +241,13 @@
   gesture_recognizers_blocking_policies[idString] = gestureRecognizerBlockingPolicy;
 }
 
-void FlutterPlatformViewsController::SetFrameSize(SkISize frame_size) {
+void FlutterPlatformViewsController::BeginFrame(SkISize frame_size) {
+  ResetFrameState();
   frame_size_ = frame_size;
 }
 
 void FlutterPlatformViewsController::CancelFrame() {
-  picture_recorders_.clear();
-  composition_order_.clear();
+  ResetFrameState();
 }
 
 // TODO(cyanglaz): https://github.com/flutter/flutter/issues/56474
@@ -603,13 +603,6 @@
   }
 }
 
-void FlutterPlatformViewsController::EndFrame(
-    bool should_resubmit_frame,
-    fml::RefPtr<fml::RasterThreadMerger> raster_thread_merger) {
-  // Reset the composition order, so next frame starts empty.
-  composition_order_.clear();
-}
-
 std::shared_ptr<FlutterPlatformViewLayer> FlutterPlatformViewsController::GetLayer(
     GrDirectContext* gr_context,
     std::shared_ptr<IOSContext> ios_context,
@@ -705,6 +698,11 @@
   }
 }
 
+void FlutterPlatformViewsController::ResetFrameState() {
+  picture_recorders_.clear();
+  composition_order_.clear();
+}
+
 }  // namespace flutter
 
 // This recognizers delays touch events from being dispatched to the responder chain until it failed

shell/platform/darwin/ios/framework/Source/FlutterPlatformViewsTest.mm

@@ -204,8 +204,6 @@ - (void)testCanCreatePlatformViewWithoutFlutterView {
       result);
 
   XCTAssertNotNil(gMockPlatformView);
-
-  flutterPlatformViewsController->Reset();
 }
 
 - (void)testChildClippingViewHitTests {
@@ -281,7 +279,6 @@ - (void)testCompositePlatformView {
   CGRect platformViewRectInFlutterView = [gMockPlatformView convertRect:gMockPlatformView.bounds
                                                                  toView:mockFlutterView];
   XCTAssertTrue(CGRectEqualToRect(platformViewRectInFlutterView, CGRectMake(100, 100, 300, 300)));
-  flutterPlatformViewsController->Reset();
 }
 
 - (void)testChildClippingViewShouldBeTheBoundingRectOfPlatformView {
@@ -351,8 +348,6 @@ - (void)testChildClippingViewShouldBeTheBoundingRectOfPlatformView {
   XCTAssertLessThan(
       fabs(platformViewRectInFlutterView.size.height - childClippingView.frame.size.height),
       kFloatCompareEpsilon);
-
-  flutterPlatformViewsController->Reset();
 }
 
 - (void)testClipRect {
@@ -424,7 +419,6 @@ - (void)testClipRect {
       }
     }
   }
-  flutterPlatformViewsController->Reset();
 }
 
 - (void)testClipRRect {
@@ -496,7 +490,6 @@ - (void)testClipRRect {
       }
     }
   }
-  flutterPlatformViewsController->Reset();
 }
 
 - (void)testClipPath {
@@ -569,7 +562,6 @@ - (void)testClipPath {
       }
     }
   }
-  flutterPlatformViewsController->Reset();
 }
 
 - (void)testSetFlutterViewControllerAfterCreateCanStillDispatchTouchEvents {
@@ -632,8 +624,6 @@ - (void)testSetFlutterViewControllerAfterCreateCanStillDispatchTouchEvents {
   flutterPlatformViewsController->SetFlutterViewController(mockFlutterViewContoller);
   [forwardGectureRecognizer touchesBegan:touches2 withEvent:event2];
   OCMVerify([mockFlutterViewContoller touchesBegan:touches2 withEvent:event2]);
-
-  flutterPlatformViewsController->Reset();
 }
 
 - (void)testSetFlutterViewControllerInTheMiddleOfTouchEventShouldStillAllowGesturesToBeHandled {
@@ -884,8 +874,6 @@ - (void)testFlutterPlatformViewControllerSubmitFrameWithoutFlutterViewNotCrashin
   gpu_is_disabled->SetSwitch(false);
   XCTAssertTrue(flutterPlatformViewsController->SubmitFrame(
       nullptr, nullptr, std::move(mock_surface_submit_false), gpu_is_disabled));
-
-  flutterPlatformViewsController->Reset();
 }
 
 - (void)
@@ -937,6 +925,59 @@ - (void)testFlutterPlatformViewControllerSubmitFrameWithoutFlutterViewNotCrashin
   XCTAssertNil(gMockPlatformView);
 }
 
+- (void)testFlutterPlatformViewControllerBeginFrameShouldResetCompisitionOrder {
+  flutter::FlutterPlatformViewsTestMockPlatformViewDelegate mock_delegate;
+  auto thread_task_runner = CreateNewThread("FlutterPlatformViewsTest");
+  flutter::TaskRunners runners(/*label=*/self.name.UTF8String,
+                               /*platform=*/thread_task_runner,
+                               /*raster=*/thread_task_runner,
+                               /*ui=*/thread_task_runner,
+                               /*io=*/thread_task_runner);
+  auto flutterPlatformViewsController = std::make_shared<flutter::FlutterPlatformViewsController>();
+  auto platform_view = std::make_unique<flutter::PlatformViewIOS>(
+      /*delegate=*/mock_delegate,
+      /*rendering_api=*/flutter::IOSRenderingAPI::kSoftware,
+      /*platform_views_controller=*/flutterPlatformViewsController,
+      /*task_runners=*/runners);
+
+  UIView* mockFlutterView = [[[UIView alloc] initWithFrame:CGRectMake(0, 0, 500, 500)] autorelease];
+  flutterPlatformViewsController->SetFlutterView(mockFlutterView);
+
+  FlutterPlatformViewsTestMockFlutterPlatformFactory* factory =
+      [[FlutterPlatformViewsTestMockFlutterPlatformFactory new] autorelease];
+  flutterPlatformViewsController->RegisterViewFactory(
+      factory, @"MockFlutterPlatformView",
+      FlutterPlatformViewGestureRecognizersBlockingPolicyEager);
+  FlutterResult result = ^(id result) {
+  };
+
+  flutterPlatformViewsController->OnMethodCall(
+      [FlutterMethodCall
+          methodCallWithMethodName:@"create"
+                         arguments:@{@"id" : @0, @"viewType" : @"MockFlutterPlatformView"}],
+      result);
+
+  // First frame, |GetCurrentCanvases| is not empty after composite.
+  flutterPlatformViewsController->BeginFrame(SkISize::Make(300, 300));
+  flutter::MutatorsStack stack;
+  SkMatrix finalMatrix;
+  auto embeddedViewParams1 =
+      std::make_unique<flutter::EmbeddedViewParams>(finalMatrix, SkSize::Make(300, 300), stack);
+  flutterPlatformViewsController->PrerollCompositeEmbeddedView(0, std::move(embeddedViewParams1));
+  flutterPlatformViewsController->CompositeEmbeddedView(0);
+  XCTAssertEqual(flutterPlatformViewsController->GetCurrentCanvases().size(), 1UL);
+
+  // Second frame, |GetCurrentCanvases| should be empty at the start
+  flutterPlatformViewsController->BeginFrame(SkISize::Make(300, 300));
+  XCTAssertTrue(flutterPlatformViewsController->GetCurrentCanvases().empty());
+
+  auto embeddedViewParams2 =
+      std::make_unique<flutter::EmbeddedViewParams>(finalMatrix, SkSize::Make(300, 300), stack);
+  flutterPlatformViewsController->PrerollCompositeEmbeddedView(0, std::move(embeddedViewParams2));
+  flutterPlatformViewsController->CompositeEmbeddedView(0);
+  XCTAssertEqual(flutterPlatformViewsController->GetCurrentCanvases().size(), 1UL);
+}
+
 - (int)alphaOfPoint:(CGPoint)point onView:(UIView*)view {
   unsigned char pixel[4] = {0};
 

shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h

@@ -141,7 +141,8 @@ class FlutterPlatformViewsController {
       NSString* factoryId,
       FlutterPlatformViewGestureRecognizersBlockingPolicy gestureRecognizerBlockingPolicy);
 
-  void SetFrameSize(SkISize frame_size);
+  // Called at the begining of each frame.
+  void BeginFrame(SkISize frame_size);
 
   // Indicates that we don't compisite any platform views or overlays during this frame.
   // Also reverts the composition_order_ to its original state at the begining of the frame.
@@ -175,12 +176,6 @@ class FlutterPlatformViewsController {
                    std::unique_ptr<SurfaceFrame> frame,
                    const std::shared_ptr<fml::SyncSwitch>& gpu_disable_sync_switch);
 
-  // Invoked at the very end of a frame.
-  // After invoking this method, nothing should happen on the current TaskRunner during the same
-  // frame.
-  void EndFrame(bool should_resubmit_frame,
-                fml::RefPtr<fml::RasterThreadMerger> raster_thread_merger);
-
   void OnMethodCall(FlutterMethodCall* call, FlutterResult& result);
 
  private:
@@ -309,6 +304,9 @@ class FlutterPlatformViewsController {
                           std::shared_ptr<IOSContext> ios_context,
                           std::unique_ptr<SurfaceFrame> frame);
 
+  // Resets the state of the frame.
+  void ResetFrameState();
+
   FML_DISALLOW_COPY_AND_ASSIGN(FlutterPlatformViewsController);
 };
 

shell/platform/darwin/ios/ios_external_view_embedder.mm

@@ -37,7 +37,7 @@
     fml::RefPtr<fml::RasterThreadMerger> raster_thread_merger) {
   TRACE_EVENT0("flutter", "IOSExternalViewEmbedder::BeginFrame");
   FML_CHECK(platform_views_controller_);
-  platform_views_controller_->SetFrameSize(frame_size);
+  platform_views_controller_->BeginFrame(frame_size);
 }
 
 // |ExternalViewEmbedder|
@@ -88,7 +88,6 @@
                                        fml::RefPtr<fml::RasterThreadMerger> raster_thread_merger) {
   TRACE_EVENT0("flutter", "IOSExternalViewEmbedder::EndFrame");
   FML_CHECK(platform_views_controller_);
-  return platform_views_controller_->EndFrame(should_resubmit_frame, raster_thread_merger);
 }
 
 // |ExternalViewEmbedder|

testing/scenario_app/ios/Scenarios/Scenarios.xcodeproj/project.pbxproj

@@ -57,6 +57,7 @@
 		6816DBAD2318696600A51400 /* golden_platform_view_cliprect_iPhone SE_simulator.png in Resources */ = {isa = PBXBuildFile; fileRef = 6816DBA82318696600A51400 /* golden_platform_view_cliprect_iPhone SE_simulator.png */; };
 		6816DBAE2318696600A51400 /* golden_platform_view_cliprrect_iPhone SE_simulator.png in Resources */ = {isa = PBXBuildFile; fileRef = 6816DBA92318696600A51400 /* golden_platform_view_cliprrect_iPhone SE_simulator.png */; };
 		68A5B63423EB71D300BDBCDB /* PlatformViewGestureRecognizerTests.m in Sources */ = {isa = PBXBuildFile; fileRef = 68A5B63323EB71D300BDBCDB /* PlatformViewGestureRecognizerTests.m */; };
+		68D4017D2564859300ECD91A /* ContinuousTexture.m in Sources */ = {isa = PBXBuildFile; fileRef = 68D4017C2564859300ECD91A /* ContinuousTexture.m */; };
 /* End PBXBuildFile section */
 
 /* Begin PBXContainerItemProxy section */
@@ -172,6 +173,8 @@
 		6816DBA82318696600A51400 /* golden_platform_view_cliprect_iPhone SE_simulator.png */ = {isa = PBXFileReference; lastKnownFileType = image.png; path = "golden_platform_view_cliprect_iPhone SE_simulator.png"; sourceTree = "<group>"; };
 		6816DBA92318696600A51400 /* golden_platform_view_cliprrect_iPhone SE_simulator.png */ = {isa = PBXFileReference; lastKnownFileType = image.png; path = "golden_platform_view_cliprrect_iPhone SE_simulator.png"; sourceTree = "<group>"; };
 		68A5B63323EB71D300BDBCDB /* PlatformViewGestureRecognizerTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = PlatformViewGestureRecognizerTests.m; sourceTree = "<group>"; };
+		68D4017B2564859300ECD91A /* ContinuousTexture.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = ContinuousTexture.h; sourceTree = "<group>"; };
+		68D4017C2564859300ECD91A /* ContinuousTexture.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = ContinuousTexture.m; sourceTree = "<group>"; };
 /* End PBXFileReference section */
 
 /* Begin PBXFrameworksBuildPhase section */
@@ -238,6 +241,8 @@
 				0A57B3BC2323C4BD00DD9521 /* ScreenBeforeFlutter.m */,
 				0A57B3BE2323C74200DD9521 /* FlutterEngine+ScenariosTest.m */,
 				0A57B3C02323C74D00DD9521 /* FlutterEngine+ScenariosTest.h */,
+				68D4017B2564859300ECD91A /* ContinuousTexture.h */,
+				68D4017C2564859300ECD91A /* ContinuousTexture.m */,
 			);
 			path = Scenarios;
 			sourceTree = "<group>";
@@ -462,6 +467,7 @@
 			buildActionMask = 2147483647;
 			files = (
 				248D76DA22E388380012F0C1 /* main.m in Sources */,
+				68D4017D2564859300ECD91A /* ContinuousTexture.m in Sources */,
 				24F1FB89230B4579005ACE7C /* TextPlatformView.m in Sources */,
 				248D76CC22E388370012F0C1 /* AppDelegate.m in Sources */,
 				0A57B3BF2323C74200DD9521 /* FlutterEngine+ScenariosTest.m in Sources */,

testing/scenario_app/ios/Scenarios/Scenarios/AppDelegate.m

@@ -4,6 +4,7 @@
 
 #import "AppDelegate.h"
 
+#import "ContinuousTexture.h"
 #import "FlutterEngine+ScenariosTest.h"
 #import "ScreenBeforeFlutter.h"
 #import "TextPlatformView.h"
@@ -52,6 +53,7 @@ - (BOOL)application:(UIApplication*)application
     @"--tap-status-bar" : @"tap_status_bar",
     @"--text-semantics-focus" : @"text_semantics_focus",
     @"--animated-color-square" : @"animated_color_square",
+    @"--platform-view-with-continuous-texture" : @"platform_view_with_continuous_texture"
   };
   __block NSString* flutterViewControllerTestName = nil;
   [launchArgsMap
@@ -70,6 +72,10 @@ - (BOOL)application:(UIApplication*)application
   }
 
   [self.window makeKeyAndVisible];
+  if ([[[NSProcessInfo processInfo] arguments] containsObject:@"--with-continuous-texture"]) {
+    [ContinuousTexture
+        registerWithRegistrar:[self registrarForPlugin:@"com.constant.firing.texture"]];
+  }
   return [super application:application didFinishLaunchingWithOptions:launchOptions];
 }
 

testing/scenario_app/ios/Scenarios/Scenarios/ContinuousTexture.h

@@ -0,0 +1,20 @@
+// Copyright 2019 The Chromium 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/Flutter.h>
+#import <Foundation/Foundation.h>
+
+NS_ASSUME_NONNULL_BEGIN
+
+// A texture plugin that ready textures continuously.
+@interface ContinuousTexture : NSObject <FlutterPlugin>
+
+@end
+
+// The testing texture used by |ContinuousTexture|
+@interface FlutterScenarioTestTexture : NSObject <FlutterTexture>
+
+@end
+
+NS_ASSUME_NONNULL_END

testing/scenario_app/ios/Scenarios/Scenarios/ContinuousTexture.m

@@ -0,0 +1,43 @@
+// Copyright 2019 The Chromium 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 "ContinuousTexture.h"
+
+@implementation ContinuousTexture
+
++ (void)registerWithRegistrar:(nonnull NSObject<FlutterPluginRegistrar>*)registrar {
+  NSObject<FlutterTextureRegistry>* textureRegistry = [registrar textures];
+  FlutterScenarioTestTexture* texture = [[FlutterScenarioTestTexture alloc] init];
+  int64_t textureId = [textureRegistry registerTexture:texture];
+  [NSTimer scheduledTimerWithTimeInterval:0.05
+                                  repeats:YES
+                                    block:^(NSTimer* _Nonnull timer) {
+                                      [textureRegistry textureFrameAvailable:textureId];
+                                    }];
+}
+
+@end
+
+@implementation FlutterScenarioTestTexture
+
+- (CVPixelBufferRef _Nullable)copyPixelBuffer {
+  return [self pixelBuffer];
+}
+
+- (CVPixelBufferRef)pixelBuffer {
+  NSDictionary* options = @{
+    // This key is required to generate SKPicture with CVPixelBufferRef in metal.
+    (NSString*)kCVPixelBufferMetalCompatibilityKey : @YES
+  };
+  CVPixelBufferRef pxbuffer = NULL;
+  CVReturn status = CVPixelBufferCreate(kCFAllocatorDefault, 200, 200, kCVPixelFormatType_32BGRA,
+                                        (__bridge CFDictionaryRef)options, &pxbuffer);
+
+  NSParameterAssert(status == kCVReturnSuccess && pxbuffer != NULL);
+
+  CVPixelBufferLockBaseAddress(pxbuffer, 0);
+  return pxbuffer;
+}
+
+@end

testing/scenario_app/ios/Scenarios/ScenariosUITests/PlatformViewUITests.m

@@ -4,6 +4,8 @@
 
 #import "GoldenPlatformViewTests.h"
 
+static const NSInteger kSecondsToWaitForPlatformView = 30;
+
 @interface PlatformViewUITests : GoldenPlatformViewTests
 
 @end
@@ -170,4 +172,35 @@ - (void)testPlatformView {
   XCUIDevice.sharedDevice.orientation = UIDeviceOrientationLandscapeLeft;
   [self checkGolden];
 }
+
+@end
+
+@interface PlatformViewWithContinuousTexture : XCTestCase
+
+@end
+
+@implementation PlatformViewWithContinuousTexture
+
+- (void)setUp {
+  self.continueAfterFailure = NO;
+}
+
+- (void)testPlatformViewWithContinuousTexture {
+  XCUIApplication* app = [[XCUIApplication alloc] init];
+  app.launchArguments =
+      @[ @"--platform-view-with-continuous-texture", @"--with-continuous-texture" ];
+  [app launch];
+
+  XCUIElement* platformView = app.textViews.firstMatch;
+  BOOL exists = [platformView waitForExistenceWithTimeout:kSecondsToWaitForPlatformView];
+  if (!exists) {
+    XCTFail(@"It took longer than %@ second to find the platform view."
+            @"There might be issues with the platform view's construction,"
+            @"or with how the scenario is built.",
+            @(kSecondsToWaitForPlatformView));
+  }
+
+  XCTAssertNotNil(platformView);
+}
+
 @end