From 5dc3782c412593b1727398cbb9bcc497fa5805ef Mon Sep 17 00:00:00 2001
From: Chinmay Garde <chinmaygarde@google.com>
Date: Thu, 30 Apr 2020 18:54:27 -0700
Subject: [PATCH] Don't depend on an implicit transaction when presenting
 drawables on the raster thread.

The way transactions were added changed in
https://github.com/flutter/engine/commit/68fd8334889610af08cbfc2828f91cf14faf3f1d.
This broke rendering using both Metal and OpenGL when no implicit transaction
was present on the transaction stack. The failure models differ based on Metal
vs. OpenGL and iOS/device versions. On older versions of iOS, rendering would
consume memory till exhaustion. On newer iOS versions, rendering would be stuck
(till a timeout). This patch brings transaction management back in line with as
it was earlier and also makes the Metal backend resilient to transactions being
present on the transaction stack at all. Since this is still quite brittle,
transaction management must be moved to IOSSurface as a followup.

Fixes https://github.com/flutter/flutter/issues/55784.
---
 shell/gpu/gpu_surface_metal.mm           |  5 +++++
 shell/platform/darwin/ios/ios_surface.mm | 20 ++------------------
 2 files changed, 7 insertions(+), 18 deletions(-)

diff --git a/shell/gpu/gpu_surface_metal.mm b/shell/gpu/gpu_surface_metal.mm
index 5ca8b6b4a42d5..ec96de928c91e 100644
--- a/shell/gpu/gpu_surface_metal.mm
+++ b/shell/gpu/gpu_surface_metal.mm
@@ -58,6 +58,11 @@
 
   ReleaseUnusedDrawableIfNecessary();
 
+  // When there are platform views in the scene, the drawable needs to be presented in the same
+  // transaction as the one created for platform views. When the drawable are being presented from
+  // the raster thread, there is no such transaction.
+  layer_.get().presentsWithTransaction = [[NSThread currentThread] isMainThread];
+
   auto surface = SkSurface::MakeFromCAMetalLayer(context_.get(),            // context
                                                  layer_.get(),              // layer
                                                  kTopLeft_GrSurfaceOrigin,  // origin
diff --git a/shell/platform/darwin/ios/ios_surface.mm b/shell/platform/darwin/ios/ios_surface.mm
index 5f2725273e0dd..bb630c3d9be44 100644
--- a/shell/platform/darwin/ios/ios_surface.mm
+++ b/shell/platform/darwin/ios/ios_surface.mm
@@ -90,12 +90,7 @@ bool IsIosEmbeddedViewsPreviewEnabled() {
   platform_views_controller_->CancelFrame();
   // Committing the current transaction as |BeginFrame| will create a nested
   // CATransaction otherwise.
-  if ([[NSThread currentThread] isMainThread]) {
-    // The only time we need to commit the `CATranscation` is when
-    // there are platform views in the scene, which has to be run on the
-    // main thread.
-    [CATransaction commit];
-  }
+  [CATransaction commit];
 }
 
 // |ExternalViewEmbedder|
@@ -103,12 +98,7 @@ bool IsIosEmbeddedViewsPreviewEnabled() {
   TRACE_EVENT0("flutter", "IOSSurface::BeginFrame");
   FML_CHECK(platform_views_controller_ != nullptr);
   platform_views_controller_->SetFrameSize(frame_size);
-  if ([[NSThread currentThread] isMainThread]) {
-    // The only time we need to commit the `CATranscation` is when
-    // there are platform views in the scene, which has to be run on the
-    // main thread.
-    [CATransaction begin];
-  }
+  [CATransaction begin];
 }
 
 // |ExternalViewEmbedder|
@@ -160,12 +150,6 @@ bool IsIosEmbeddedViewsPreviewEnabled() {
 // |ExternalViewEmbedder|
 void IOSSurface::FinishFrame() {
   TRACE_EVENT0("flutter", "IOSSurface::DidSubmitFrame");
-  if (![[NSThread currentThread] isMainThread]) {
-    return;
-  }
-  // The only time we need to commit the `CATranscation` is when
-  // there are platform views in the scene, which has to be run on the
-  // main thread.
   [CATransaction commit];
 }
 }  // namespace flutter