[Impeller] null check drawable. (#47488)

Speculative fix for the first crash noted in flutter/flutter#136628 (comment)

nextDrawble can return null if the method times out.

Fixes flutter/flutter#136525

Author: Jonah Williams

BUILD.gn

@@ -196,6 +196,7 @@ group("unittests") {
     if (is_mac) {
       public_deps += [
         "//flutter/impeller/golden_tests:impeller_golden_tests",
+        "//flutter/shell/gpu:gpu_surface_metal_unittests",
         "//flutter/shell/platform/darwin/common:framework_common_unittests",
         "//flutter/third_party/spring_animation:spring_animation_unittests",
       ]

ci/licenses_golden/excluded_files

@@ -256,6 +256,7 @@
 ../../../flutter/shell/common/switches_unittests.cc
 ../../../flutter/shell/common/variable_refresh_rate_display_unittests.cc
 ../../../flutter/shell/common/vsync_waiter_unittests.cc
+../../../flutter/shell/gpu/gpu_surface_metal_impeller_unittests.mm
 ../../../flutter/shell/platform/android/.gitignore
 ../../../flutter/shell/platform/android/android_context_gl_impeller_unittests.cc
 ../../../flutter/shell/platform/android/android_context_gl_unittests.cc

shell/gpu/BUILD.gn

@@ -88,3 +88,27 @@ source_set("gpu_surface_metal") {
     public_deps += [ "//flutter/impeller" ]
   }
 }
+
+if (is_mac) {
+  impeller_component("gpu_surface_metal_unittests") {
+    testonly = true
+    target_type = "executable"
+    sources = [ "gpu_surface_metal_impeller_unittests.mm" ]
+
+    frameworks = [
+      "AppKit.framework",
+      "QuartzCore.framework",
+    ]
+
+    deps = [
+             "//flutter/testing",
+             ":gpu_surface_metal",
+             "//flutter/impeller/fixtures",
+             "//flutter/fml",
+             "//flutter/fml/dart",
+             "//flutter/runtime",
+             "//flutter/runtime:libdart",
+             "//flutter/testing",
+           ] + gpu_common_deps
+  }
+}

shell/gpu/gpu_surface_metal_impeller.mm

@@ -51,7 +51,7 @@
 
 // |Surface|
 bool GPUSurfaceMetalImpeller::IsValid() {
-  return !!aiks_context_;
+  return !!aiks_context_ && aiks_context_->IsValid();
 }
 
 // |Surface|
@@ -99,6 +99,9 @@
 
   auto drawable = impeller::SurfaceMTL::GetMetalDrawableAndValidate(
       impeller_renderer_->GetContext(), mtl_layer);
+  if (!drawable) {
+    return nullptr;
+  }
   if (Settings::kSurfaceDataAccessible) {
     last_texture_.reset([drawable.texture retain]);
   }

shell/gpu/gpu_surface_metal_impeller_unittests.mm

@@ -0,0 +1,81 @@
+// Copyright 2013 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.
+
+#include <Foundation/Foundation.h>
+#include <QuartzCore/QuartzCore.h>
+
+#include "flutter/shell/gpu/gpu_surface_metal_impeller.h"
+#include "gtest/gtest.h"
+#include "impeller/entity/mtl/entity_shaders.h"
+#include "impeller/entity/mtl/framebuffer_blend_shaders.h"
+#include "impeller/entity/mtl/modern_shaders.h"
+#include "impeller/renderer/backend/metal/context_mtl.h"
+
+namespace flutter {
+namespace testing {
+
+class TestGPUSurfaceMetalDelegate : public GPUSurfaceMetalDelegate {
+ public:
+  TestGPUSurfaceMetalDelegate() : GPUSurfaceMetalDelegate(MTLRenderTargetType::kCAMetalLayer) {
+    layer_ = [[CAMetalLayer alloc] init];
+  }
+
+  ~TestGPUSurfaceMetalDelegate() = default;
+
+  GPUCAMetalLayerHandle GetCAMetalLayer(const SkISize& frame_info) const override {
+    return (__bridge GPUCAMetalLayerHandle)(layer_);
+  }
+
+  bool PresentDrawable(GrMTLHandle drawable) const override { return true; }
+
+  GPUMTLTextureInfo GetMTLTexture(const SkISize& frame_info) const override { return {}; }
+
+  bool PresentTexture(GPUMTLTextureInfo texture) const override { return true; }
+
+  bool AllowsDrawingWhenGpuDisabled() const override { return true; }
+
+ private:
+  CAMetalLayer* layer_ = nil;
+};
+
+static std::shared_ptr<impeller::ContextMTL> CreateImpellerContext() {
+  std::vector<std::shared_ptr<fml::Mapping>> shader_mappings = {
+      std::make_shared<fml::NonOwnedMapping>(impeller_entity_shaders_data,
+                                             impeller_entity_shaders_length),
+      std::make_shared<fml::NonOwnedMapping>(impeller_modern_shaders_data,
+                                             impeller_modern_shaders_length),
+      std::make_shared<fml::NonOwnedMapping>(impeller_framebuffer_blend_shaders_data,
+                                             impeller_framebuffer_blend_shaders_length),
+  };
+  auto sync_switch = std::make_shared<fml::SyncSwitch>(false);
+  return impeller::ContextMTL::Create(shader_mappings, sync_switch, "Impeller Library");
+}
+
+TEST(GPUSurfaceMetalImpeller, InvalidImpellerContextCreatesCausesSurfaceToBeInvalid) {
+  auto delegate = std::make_shared<TestGPUSurfaceMetalDelegate>();
+  auto surface = std::make_shared<GPUSurfaceMetalImpeller>(delegate.get(), nullptr);
+
+  ASSERT_FALSE(surface->IsValid());
+}
+
+TEST(GPUSurfaceMetalImpeller, CanCreateValidSurface) {
+  auto delegate = std::make_shared<TestGPUSurfaceMetalDelegate>();
+  auto surface = std::make_shared<GPUSurfaceMetalImpeller>(delegate.get(), CreateImpellerContext());
+
+  ASSERT_TRUE(surface->IsValid());
+}
+
+TEST(GPUSurfaceMetalImpeller, AcquireFrameFromCAMetalLayerNullChecksDrawable) {
+  auto delegate = std::make_shared<TestGPUSurfaceMetalDelegate>();
+  std::shared_ptr<Surface> surface =
+      std::make_shared<GPUSurfaceMetalImpeller>(delegate.get(), CreateImpellerContext());
+
+  ASSERT_TRUE(surface->IsValid());
+
+  auto frame = surface->AcquireFrame(SkISize::Make(100, 100));
+  ASSERT_EQ(frame, nullptr);
+}
+
+}  // namespace testing
+}  // namespace flutter

testing/run_tests.py

@@ -437,6 +437,7 @@ def make_test(name, flags=None, extra_env=None):
         make_test('accessibility_unittests'),
         make_test('framework_common_unittests'),
         make_test('spring_animation_unittests'),
+        make_test('gpu_surface_metal_unittests'),
     ]
 
   if is_linux():