[Impeller] Do not reference this in the submit callback for Metal GPU Surfaces

shell/gpu/gpu_surface_metal_impeller.h

@@ -47,7 +47,8 @@ class IMPELLER_CA_METAL_LAYER_AVAILABLE GPUSurfaceMetalImpeller
   bool disable_partial_repaint_ = false;
   // Accumulated damage for each framebuffer; Key is address of underlying
   // MTLTexture for each drawable
-  std::map<uintptr_t, SkIRect> damage_;
+  std::shared_ptr<std::map<uintptr_t, SkIRect>> damage_ =
+      std::make_shared<std::map<uintptr_t, SkIRect>>();
 
   // |Surface|
   std::unique_ptr<SurfaceFrame> AcquireFrame(

shell/gpu/gpu_surface_metal_impeller.mm

@@ -108,11 +108,12 @@
 
   id<MTLTexture> last_texture = static_cast<id<MTLTexture>>(last_texture_);
   SurfaceFrame::SubmitCallback submit_callback =
-      fml::MakeCopyable([this,                           //
-                         renderer = impeller_renderer_,  //
-                         aiks_context = aiks_context_,   //
-                         drawable,                       //
-                         last_texture                    //
+      fml::MakeCopyable([damage = damage_,
+                         disable_partial_repaint = disable_partial_repaint_,  //
+                         renderer = impeller_renderer_,                       //
+                         aiks_context = aiks_context_,                        //
+                         drawable,                                            //
+                         last_texture                                         //
   ](SurfaceFrame& surface_frame, DlCanvas* canvas) mutable -> bool {
         if (!aiks_context) {
           return false;
@@ -124,10 +125,10 @@
           return false;
         }
 
-        if (!disable_partial_repaint_) {
+        if (!disable_partial_repaint && damage) {
           uintptr_t texture = reinterpret_cast<uintptr_t>(last_texture);
 
-          for (auto& entry : damage_) {
+          for (auto& entry : *damage) {
             if (entry.first != texture) {
               // Accumulate damage for other framebuffers
               if (surface_frame.submit_info().frame_damage) {
@@ -136,7 +137,7 @@
             }
           }
           // Reset accumulated damage for current framebuffer
-          damage_[texture] = SkIRect::MakeEmpty();
+          (*damage)[texture] = SkIRect::MakeEmpty();
         }
 
         std::optional<impeller::IRect> clip_rect;
@@ -146,8 +147,8 @@
                                                 buffer_damage->width(), buffer_damage->height());
         }
 
-        auto surface = impeller::SurfaceMTL::MakeFromMetalLayerDrawable(
-            impeller_renderer_->GetContext(), drawable, clip_rect);
+        auto surface = impeller::SurfaceMTL::MakeFromMetalLayerDrawable(renderer->GetContext(),
+                                                                        drawable, clip_rect);
 
         if (clip_rect && clip_rect->IsEmpty()) {
           return surface->Present();
@@ -174,8 +175,8 @@
     // Provide accumulated damage to rasterizer (area in current framebuffer that lags behind
     // front buffer)
     uintptr_t texture = reinterpret_cast<uintptr_t>(drawable.texture);
-    auto i = damage_.find(texture);
-    if (i != damage_.end()) {
+    auto i = damage_->find(texture);
+    if (i != damage_->end()) {
       framebuffer_info.existing_damage = i->second;
     }
     framebuffer_info.supports_partial_repaint = true;
@@ -205,7 +206,8 @@
   }
 
   SurfaceFrame::SubmitCallback submit_callback =
-      fml::MakeCopyable([this,                           //
+      fml::MakeCopyable([disable_partial_repaint = disable_partial_repaint_,  //
+                         damage = damage_,
                          renderer = impeller_renderer_,  //
                          aiks_context = aiks_context_,   //
                          texture_info,                   //
@@ -222,10 +224,10 @@
           return false;
         }
 
-        if (!disable_partial_repaint_) {
+        if (!disable_partial_repaint && damage) {
           uintptr_t texture_ptr = reinterpret_cast<uintptr_t>(mtl_texture);
 
-          for (auto& entry : damage_) {
+          for (auto& entry : *damage) {
             if (entry.first != texture_ptr) {
               // Accumulate damage for other framebuffers
               if (surface_frame.submit_info().frame_damage) {
@@ -234,7 +236,7 @@
             }
           }
           // Reset accumulated damage for current framebuffer
-          damage_[texture_ptr] = SkIRect::MakeEmpty();
+          (*damage)[texture_ptr] = SkIRect::MakeEmpty();
         }
 
         std::optional<impeller::IRect> clip_rect;
@@ -278,8 +280,8 @@
     // Provide accumulated damage to rasterizer (area in current framebuffer that lags behind
     // front buffer)
     uintptr_t texture = reinterpret_cast<uintptr_t>(mtl_texture);
-    auto i = damage_.find(texture);
-    if (i != damage_.end()) {
+    auto i = damage_->find(texture);
+    if (i != damage_->end()) {
       framebuffer_info.existing_damage = i->second;
     }
     framebuffer_info.supports_partial_repaint = true;

shell/gpu/gpu_surface_metal_impeller_unittests.mm

@@ -24,6 +24,7 @@
   ~TestGPUSurfaceMetalDelegate() = default;
 
   GPUCAMetalLayerHandle GetCAMetalLayer(const SkISize& frame_info) const override {
+    layer_.drawableSize = CGSizeMake(frame_info.width(), frame_info.height());
     return (__bridge GPUCAMetalLayerHandle)(layer_);
   }
 
@@ -35,6 +36,8 @@ GPUCAMetalLayerHandle GetCAMetalLayer(const SkISize& frame_info) const override
 
   bool AllowsDrawingWhenGpuDisabled() const override { return true; }
 
+  void SetDevice() { layer_.device = ::MTLCreateSystemDefaultDevice(); }
+
  private:
   CAMetalLayer* layer_ = nil;
 };
@@ -77,5 +80,22 @@ GPUCAMetalLayerHandle GetCAMetalLayer(const SkISize& frame_info) const override
   ASSERT_EQ(frame, nullptr);
 }
 
+TEST(GPUSurfaceMetalImpeller, AcquireFrameFromCAMetalLayerDoesNotRetainThis) {
+  auto delegate = std::make_shared<TestGPUSurfaceMetalDelegate>();
+  delegate->SetDevice();
+  std::unique_ptr<Surface> surface =
+      std::make_unique<GPUSurfaceMetalImpeller>(delegate.get(), CreateImpellerContext());
+
+  ASSERT_TRUE(surface->IsValid());
+
+  auto frame = surface->AcquireFrame(SkISize::Make(100, 100));
+  ASSERT_TRUE(frame);
+
+  // Simulate a rasterizer teardown, e.g. due to going to the background.
+  surface.reset();
+
+  ASSERT_TRUE(frame->Submit());
+}
+
 }  // namespace testing
 }  // namespace flutter