[Impeller] null check drawable. (#47488)
Speculative fix for the first crash noted in https://github.com/flutter/flutter/issues/136628#issue-1944779964
nextDrawble can return null if the method times out.
Fixes https://github.com/flutter/flutter/issues/136525
diff --git a/BUILD.gn b/BUILD.gn
index 3a09a91..8a5bea6 100644
--- a/BUILD.gn
+++ b/BUILD.gn
@@ -196,6 +196,7 @@
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",
]
diff --git a/ci/licenses_golden/excluded_files b/ci/licenses_golden/excluded_files
index 18a25c8..4593402 100644
--- a/ci/licenses_golden/excluded_files
+++ b/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
diff --git a/shell/gpu/BUILD.gn b/shell/gpu/BUILD.gn
index 7440ff1..c7ccf87 100644
--- a/shell/gpu/BUILD.gn
+++ b/shell/gpu/BUILD.gn
@@ -88,3 +88,27 @@
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
+ }
+}
diff --git a/shell/gpu/gpu_surface_metal_impeller.mm b/shell/gpu/gpu_surface_metal_impeller.mm
index d76b138..967da21 100644
--- a/shell/gpu/gpu_surface_metal_impeller.mm
+++ b/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]);
}
diff --git a/shell/gpu/gpu_surface_metal_impeller_unittests.mm b/shell/gpu/gpu_surface_metal_impeller_unittests.mm
new file mode 100644
index 0000000..5d56f7f
--- /dev/null
+++ b/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
diff --git a/testing/run_tests.py b/testing/run_tests.py
index 8a139f9..dbeee78 100755
--- a/testing/run_tests.py
+++ b/testing/run_tests.py
@@ -437,6 +437,7 @@
make_test('accessibility_unittests'),
make_test('framework_common_unittests'),
make_test('spring_animation_unittests'),
+ make_test('gpu_surface_metal_unittests'),
]
if is_linux():
