From 2ae4942db04a6cf760ebbef6c34a2d0d4a57ea02 Mon Sep 17 00:00:00 2001 From: Jason Simmons Date: Tue, 8 Aug 2023 10:40:03 -0700 Subject: [PATCH] Use the Clang unreachable code warning flag in the engine tree (flutter/engine#44458) --- engine/src/flutter/BUILD.gn | 6 ++- .../testing/dl_rendering_unittests.cc | 3 +- .../performance_overlay_layer_unittests.cc | 3 +- .../src/flutter/fml/message_loop_unittests.cc | 12 +++-- .../contents/runtime_effect_contents.cc | 3 +- .../renderer/backend/metal/context_mtl.mm | 3 +- .../renderer/backend/metal/render_pass_mtl.mm | 7 +-- engine/src/flutter/lib/ui/painting/path.cc | 5 +- .../shell/common/rasterizer_unittests.cc | 4 ++ .../shell/common/shell_fuchsia_unittests.cc | 6 ++- .../flutter/shell/common/shell_unittests.cc | 48 ++++++++++++------- .../shell/gpu/gpu_surface_metal_impeller.mm | 18 ++++--- .../android/surface/android_native_window.cc | 2 +- .../embedder/tests/embedder_a11y_unittests.cc | 6 ++- .../embedder/tests/embedder_unittests.cc | 6 ++- .../shell/platform/glfw/platform_handler.cc | 1 - engine/src/flutter/testing/elf_loader.cc | 22 +++++---- .../ax/ax_node_position_unittest.cc | 9 ++++ .../accessibility/base/win/variant_vector.cc | 2 +- 19 files changed, 111 insertions(+), 55 deletions(-) diff --git a/engine/src/flutter/BUILD.gn b/engine/src/flutter/BUILD.gn index 8bb4183c6ff..05fbbb09336 100644 --- a/engine/src/flutter/BUILD.gn +++ b/engine/src/flutter/BUILD.gn @@ -20,11 +20,15 @@ declare_args() { config("config") { include_dirs = [ ".." ] + cflags = [] if (is_win) { if (current_cpu != "x86") { - cflags = [ "/WX" ] # Treat warnings as errors. + cflags += [ "/WX" ] # Treat warnings as errors. } } + if (is_clang) { + cflags += [ "-Wunreachable-code" ] + } } config("export_dynamic_symbols") { diff --git a/engine/src/flutter/display_list/testing/dl_rendering_unittests.cc b/engine/src/flutter/display_list/testing/dl_rendering_unittests.cc index 97282a70340..491c82a0f9f 100644 --- a/engine/src/flutter/display_list/testing/dl_rendering_unittests.cc +++ b/engine/src/flutter/display_list/testing/dl_rendering_unittests.cc @@ -3363,7 +3363,7 @@ TEST_F(DisplayListCanvas, DrawTextBlob) { // default. #if defined(OS_FUCHSIA) GTEST_SKIP() << "Rendering comparisons require a valid default font manager"; -#endif // OS_FUCHSIA +#else sk_sp blob = CanvasCompareTester::MakeTextBlob("Testing", kRenderHeight * 0.33f); SkScalar render_y_1_3 = kRenderTop + kRenderHeight * 0.3; @@ -3388,6 +3388,7 @@ TEST_F(DisplayListCanvas, DrawTextBlob) { // padding to the tolerance CanvasCompareTester::DefaultTolerance.addBoundsPadding(33, 13)); EXPECT_TRUE(blob->unique()); +#endif // OS_FUCHSIA } TEST_F(DisplayListCanvas, DrawShadow) { diff --git a/engine/src/flutter/flow/layers/performance_overlay_layer_unittests.cc b/engine/src/flutter/flow/layers/performance_overlay_layer_unittests.cc index b98a248d450..b41143b863f 100644 --- a/engine/src/flutter/flow/layers/performance_overlay_layer_unittests.cc +++ b/engine/src/flutter/flow/layers/performance_overlay_layer_unittests.cc @@ -104,7 +104,7 @@ static void TestPerformanceOverlayLayerGold(int refresh_rate) { // platforms. #if !defined(FML_OS_LINUX) GTEST_SKIP() << "Skipping golden tests on non-Linux OSes"; -#endif // FML_OS_LINUX +#else const bool golden_data_matches = golden_data->equals(snapshot_data.get()); if (!golden_data_matches) { SkFILEWStream wstream(new_golden_file_path.c_str()); @@ -126,6 +126,7 @@ static void TestPerformanceOverlayLayerGold(int refresh_rate) { << "See also the base64 encoded " << new_golden_file_path << ":\n" << b64_char; } +#endif // FML_OS_LINUX } } // namespace diff --git a/engine/src/flutter/fml/message_loop_unittests.cc b/engine/src/flutter/fml/message_loop_unittests.cc index 4faad1db3b3..222810d4060 100644 --- a/engine/src/flutter/fml/message_loop_unittests.cc +++ b/engine/src/flutter/fml/message_loop_unittests.cc @@ -159,7 +159,7 @@ TEST(MessageLoop, TIMESENSITIVE(SingleDelayedTaskByDelta)) { #if defined(OS_FUCHSIA) GTEST_SKIP() << "This test does not work on Fuchsia. https://fxbug.dev/110020 "; -#endif // OS_FUCHSIA +#else bool checked = false; std::thread thread([&checked]() { @@ -180,13 +180,14 @@ TEST(MessageLoop, TIMESENSITIVE(SingleDelayedTaskByDelta)) { }); thread.join(); ASSERT_TRUE(checked); +#endif // OS_FUCHSIA } TEST(MessageLoop, TIMESENSITIVE(SingleDelayedTaskForTime)) { #if defined(OS_FUCHSIA) GTEST_SKIP() << "This test does not work on Fuchsia. https://fxbug.dev/110020 "; -#endif // OS_FUCHSIA +#else bool checked = false; std::thread thread([&checked]() { @@ -207,13 +208,14 @@ TEST(MessageLoop, TIMESENSITIVE(SingleDelayedTaskForTime)) { }); thread.join(); ASSERT_TRUE(checked); +#endif // OS_FUCHSIA } TEST(MessageLoop, TIMESENSITIVE(MultipleDelayedTasksWithIncreasingDeltas)) { #if defined(OS_FUCHSIA) GTEST_SKIP() << "This test does not work on Fuchsia. https://fxbug.dev/110020 "; -#endif // OS_FUCHSIA +#else const auto count = 10; int checked = false; @@ -239,13 +241,14 @@ TEST(MessageLoop, TIMESENSITIVE(MultipleDelayedTasksWithIncreasingDeltas)) { }); thread.join(); ASSERT_EQ(checked, count); +#endif // OS_FUCHSIA } TEST(MessageLoop, TIMESENSITIVE(MultipleDelayedTasksWithDecreasingDeltas)) { #if defined(OS_FUCHSIA) GTEST_SKIP() << "This test does not work on Fuchsia. https://fxbug.dev/110020 "; -#endif // OS_FUCHSIA +#else const auto count = 10; int checked = false; @@ -271,6 +274,7 @@ TEST(MessageLoop, TIMESENSITIVE(MultipleDelayedTasksWithDecreasingDeltas)) { }); thread.join(); ASSERT_EQ(checked, count); +#endif // OS_FUCHSIA } TEST(MessageLoop, TaskObserverFire) { diff --git a/engine/src/flutter/impeller/entity/contents/runtime_effect_contents.cc b/engine/src/flutter/impeller/entity/contents/runtime_effect_contents.cc index ba2dff69c4e..2d613d1a86b 100644 --- a/engine/src/flutter/impeller/entity/contents/runtime_effect_contents.cc +++ b/engine/src/flutter/impeller/entity/contents/runtime_effect_contents.cc @@ -49,7 +49,7 @@ bool RuntimeEffectContents::Render(const ContentContext& renderer, // run m3 applications. #ifdef FML_OS_ANDROID return true; -#endif +#else auto context = renderer.GetContext(); auto library = context->GetShaderLibrary(); @@ -261,6 +261,7 @@ bool RuntimeEffectContents::Render(const ContentContext& renderer, return restore.Render(renderer, entity, pass); } return true; +#endif // FML_OS_ANDROID } } // namespace impeller diff --git a/engine/src/flutter/impeller/renderer/backend/metal/context_mtl.mm b/engine/src/flutter/impeller/renderer/backend/metal/context_mtl.mm index 566dd3a17e0..e92a4bb384f 100644 --- a/engine/src/flutter/impeller/renderer/backend/metal/context_mtl.mm +++ b/engine/src/flutter/impeller/renderer/backend/metal/context_mtl.mm @@ -21,7 +21,7 @@ static bool DeviceSupportsFramebufferFetch(id device) { // The iOS simulator lies about supporting framebuffer fetch. #if FML_OS_IOS_SIMULATOR return false; -#endif // FML_OS_IOS_SIMULATOR +#else // FML_OS_IOS_SIMULATOR if (@available(macOS 10.15, iOS 13, tvOS 13, *)) { return [device supportsFamily:MTLGPUFamilyApple2]; @@ -34,6 +34,7 @@ static bool DeviceSupportsFramebufferFetch(id device) { #else return false; #endif // FML_OS_IOS +#endif // FML_OS_IOS_SIMULATOR } static bool DeviceSupportsComputeSubgroups(id device) { diff --git a/engine/src/flutter/impeller/renderer/backend/metal/render_pass_mtl.mm b/engine/src/flutter/impeller/renderer/backend/metal/render_pass_mtl.mm index dc5e4fa6216..8aed2648917 100644 --- a/engine/src/flutter/impeller/renderer/backend/metal/render_pass_mtl.mm +++ b/engine/src/flutter/impeller/renderer/backend/metal/render_pass_mtl.mm @@ -498,13 +498,13 @@ bool RenderPassMTL::EncodeCommands(const std::shared_ptr& allocator, #if TARGET_OS_SIMULATOR VALIDATION_LOG << "iOS Simulator does not support instanced rendering."; return false; -#endif +#else // TARGET_OS_SIMULATOR [encoder drawPrimitives:ToMTLPrimitiveType(primitive_type) vertexStart:command.base_vertex vertexCount:command.vertex_count instanceCount:command.instance_count baseInstance:0u]; - +#endif // TARGET_OS_SIMULATOR } else { [encoder drawPrimitives:ToMTLPrimitiveType(primitive_type) vertexStart:command.base_vertex @@ -538,7 +538,7 @@ bool RenderPassMTL::EncodeCommands(const std::shared_ptr& allocator, #if TARGET_OS_SIMULATOR VALIDATION_LOG << "iOS Simulator does not support instanced rendering."; return false; -#endif +#else // TARGET_OS_SIMULATOR [encoder drawIndexedPrimitives:ToMTLPrimitiveType(primitive_type) indexCount:command.vertex_count indexType:ToMTLIndexType(command.index_type) @@ -547,6 +547,7 @@ bool RenderPassMTL::EncodeCommands(const std::shared_ptr& allocator, instanceCount:command.instance_count baseVertex:command.base_vertex baseInstance:0u]; +#endif // TARGET_OS_SIMULATOR } else { [encoder drawIndexedPrimitives:ToMTLPrimitiveType(primitive_type) indexCount:command.vertex_count diff --git a/engine/src/flutter/lib/ui/painting/path.cc b/engine/src/flutter/lib/ui/painting/path.cc index 4b03ae77db6..ddf2ebaac17 100644 --- a/engine/src/flutter/lib/ui/painting/path.cc +++ b/engine/src/flutter/lib/ui/painting/path.cc @@ -316,9 +316,10 @@ tonic::Float32List CanvasPath::getBounds() { } bool CanvasPath::op(CanvasPath* path1, CanvasPath* path2, int operation) { - return Op(path1->path(), path2->path(), static_cast(operation), - &tracked_path_->path); + bool result = Op(path1->path(), path2->path(), + static_cast(operation), &tracked_path_->path); resetVolatility(); + return result; } void CanvasPath::clone(Dart_Handle path_handle) { diff --git a/engine/src/flutter/shell/common/rasterizer_unittests.cc b/engine/src/flutter/shell/common/rasterizer_unittests.cc index 92d23c82dc9..5307dcacccf 100644 --- a/engine/src/flutter/shell/common/rasterizer_unittests.cc +++ b/engine/src/flutter/shell/common/rasterizer_unittests.cc @@ -1030,6 +1030,7 @@ TEST(RasterizerTest, TeardownNoSurface) { TEST(RasterizerTest, presentationTimeSetWhenVsyncTargetInFuture) { GTEST_SKIP() << "eglPresentationTime is disabled due to " "https://github.com/flutter/flutter/issues/112503"; +#if false std::string test_name = ::testing::UnitTest::GetInstance()->current_test_info()->name(); ThreadHost thread_host("io.flutter.test." + test_name + ".", @@ -1111,11 +1112,13 @@ TEST(RasterizerTest, presentationTimeSetWhenVsyncTargetInFuture) { latch.Signal(); }); latch.Wait(); +#endif // false } TEST(RasterizerTest, presentationTimeNotSetWhenVsyncTargetInPast) { GTEST_SKIP() << "eglPresentationTime is disabled due to " "https://github.com/flutter/flutter/issues/112503"; +#if false std::string test_name = ::testing::UnitTest::GetInstance()->current_test_info()->name(); ThreadHost thread_host("io.flutter.test." + test_name + ".", @@ -1188,6 +1191,7 @@ TEST(RasterizerTest, presentationTimeNotSetWhenVsyncTargetInPast) { latch.Signal(); }); latch.Wait(); +#endif // false } } // namespace flutter diff --git a/engine/src/flutter/shell/common/shell_fuchsia_unittests.cc b/engine/src/flutter/shell/common/shell_fuchsia_unittests.cc index 8a1989daf57..e8db4e9702c 100644 --- a/engine/src/flutter/shell/common/shell_fuchsia_unittests.cc +++ b/engine/src/flutter/shell/common/shell_fuchsia_unittests.cc @@ -100,6 +100,8 @@ class FuchsiaShellTest : public ShellTest { fuchsia::settings::IntlSettings save_settings_; }; +// These functions are used by tests that are currently disabled. +#if false static bool ValidateShell(Shell* shell) { if (!shell) { return false; @@ -145,6 +147,7 @@ static void RunCoroutineWithRetry(int retries, sleep(1); } } +#endif // false // Verifies that changing the Fuchsia settings timezone through the FIDL // settings interface results in a change of the reported local time in the @@ -165,7 +168,7 @@ TEST_F(FuchsiaShellTest, LocaltimesVaryOnTimezoneChanges) { #if defined(OS_FUCHSIA) GTEST_SKIP() << "This test fails after the CF V2 migration. https://fxbug.dev/110019 "; -#endif // OS_FUCHSIA +#else // See fixtures/shell_test.dart, the callback NotifyLocalTime is declared // there. @@ -258,6 +261,7 @@ TEST_F(FuchsiaShellTest, LocaltimesVaryOnTimezoneChanges) { continue_fixture = false; fixture_latch.Signal(); DestroyShell(std::move(shell)); +#endif // OS_FUCHSIA } } // namespace testing diff --git a/engine/src/flutter/shell/common/shell_unittests.cc b/engine/src/flutter/shell/common/shell_unittests.cc index 2c94ca7e7ad..cdfaf549610 100644 --- a/engine/src/flutter/shell/common/shell_unittests.cc +++ b/engine/src/flutter/shell/common/shell_unittests.cc @@ -578,7 +578,7 @@ TEST_F(ShellTest, LastEntrypointArgs) { TEST_F(ShellTest, DisallowedDartVMFlag) { #if defined(OS_FUCHSIA) GTEST_SKIP() << "This test flakes on Fuchsia. https://fxbug.dev/110006 "; -#endif // OS_FUCHSIA +#else // Run this test in a thread-safe manner, otherwise gtest will complain. ::testing::FLAGS_gtest_death_test_style = "threadsafe"; @@ -591,6 +591,7 @@ TEST_F(ShellTest, DisallowedDartVMFlag) { const char* expected = "Encountered disallowed Dart VM flag: --verify_after_gc"; ASSERT_DEATH(flutter::SettingsFromCommandLine(command_line), expected); +#endif // OS_FUCHSIA } TEST_F(ShellTest, AllowedDartVMFlag) { @@ -843,7 +844,7 @@ TEST_F(ShellTest, PushBackdropFilterToVisitedPlatformViews) { #if defined(OS_FUCHSIA) GTEST_SKIP() << "RasterThreadMerger flakes on Fuchsia. " "https://github.com/flutter/flutter/issues/59816 "; -#endif +#else auto settings = CreateSettingsForFixture(); @@ -922,6 +923,7 @@ TEST_F(ShellTest, PushBackdropFilterToVisitedPlatformViews) { SkRect::MakeLTRB(1, 1, 31, 31)); DestroyShell(std::move(shell)); +#endif // OS_FUCHSIA } // TODO(https://github.com/flutter/flutter/issues/59816): Enable on fuchsia. @@ -930,7 +932,7 @@ TEST_F(ShellTest, #if defined(OS_FUCHSIA) GTEST_SKIP() << "RasterThreadMerger flakes on Fuchsia. " "https://github.com/flutter/flutter/issues/59816 "; -#endif +#else auto settings = CreateSettingsForFixture(); fml::AutoResetWaitableEvent end_frame_latch; @@ -972,13 +974,14 @@ TEST_F(ShellTest, ASSERT_TRUE(end_frame_called); DestroyShell(std::move(shell)); +#endif // OS_FUCHSIA } TEST_F(ShellTest, OnPlatformViewDestroyDisablesThreadMerger) { #if defined(OS_FUCHSIA) GTEST_SKIP() << "RasterThreadMerger flakes on Fuchsia. " "https://github.com/flutter/flutter/issues/59816 "; -#endif +#else auto settings = CreateSettingsForFixture(); fml::RefPtr raster_thread_merger; @@ -1026,13 +1029,14 @@ TEST_F(ShellTest, OnPlatformViewDestroyDisablesThreadMerger) { ValidateShell(shell.get()); ASSERT_TRUE(raster_thread_merger->IsEnabled()); DestroyShell(std::move(shell)); +#endif // OS_FUCHSIA } TEST_F(ShellTest, OnPlatformViewDestroyAfterMergingThreads) { #if defined(OS_FUCHSIA) GTEST_SKIP() << "RasterThreadMerger flakes on Fuchsia. " "https://github.com/flutter/flutter/issues/59816 "; -#endif +#else const int ThreadMergingLease = 10; auto settings = CreateSettingsForFixture(); @@ -1099,13 +1103,14 @@ TEST_F(ShellTest, OnPlatformViewDestroyAfterMergingThreads) { ValidateShell(shell.get()); DestroyShell(std::move(shell)); +#endif // OS_FUCHSIA } TEST_F(ShellTest, OnPlatformViewDestroyWhenThreadsAreMerging) { #if defined(OS_FUCHSIA) GTEST_SKIP() << "RasterThreadMerger flakes on Fuchsia. " "https://github.com/flutter/flutter/issues/59816 "; -#endif +#else const int kThreadMergingLease = 10; auto settings = CreateSettingsForFixture(); @@ -1173,6 +1178,7 @@ TEST_F(ShellTest, OnPlatformViewDestroyWhenThreadsAreMerging) { ValidateShell(shell.get()); DestroyShell(std::move(shell)); +#endif // OS_FUCHSIA } TEST_F(ShellTest, @@ -1180,7 +1186,7 @@ TEST_F(ShellTest, #if defined(OS_FUCHSIA) GTEST_SKIP() << "RasterThreadMerger flakes on Fuchsia. " "https://github.com/flutter/flutter/issues/59816 "; -#endif +#else auto settings = CreateSettingsForFixture(); fml::AutoResetWaitableEvent end_frame_latch; @@ -1229,6 +1235,7 @@ TEST_F(ShellTest, ValidateShell(shell.get()); DestroyShell(std::move(shell)); +#endif // OS_FUCHSIA } TEST_F(ShellTest, OnPlatformViewDestroyWithoutRasterThreadMerger) { @@ -1273,7 +1280,7 @@ TEST_F(ShellTest, OnPlatformViewDestroyWithStaticThreadMerging) { #if defined(OS_FUCHSIA) GTEST_SKIP() << "RasterThreadMerger flakes on Fuchsia. " "https://github.com/flutter/flutter/issues/59816 "; -#endif +#else auto settings = CreateSettingsForFixture(); fml::AutoResetWaitableEvent end_frame_latch; @@ -1324,6 +1331,7 @@ TEST_F(ShellTest, OnPlatformViewDestroyWithStaticThreadMerging) { ValidateShell(shell.get()); DestroyShell(std::move(shell), task_runners); +#endif // OS_FUCHSIA } TEST_F(ShellTest, GetUsedThisFrameShouldBeSetBeforeEndFrame) { @@ -1376,7 +1384,7 @@ TEST_F(ShellTest, DISABLED_SkipAndSubmitFrame) { #if defined(OS_FUCHSIA) GTEST_SKIP() << "RasterThreadMerger flakes on Fuchsia. " "https://github.com/flutter/flutter/issues/59816 "; -#endif +#else auto settings = CreateSettingsForFixture(); fml::AutoResetWaitableEvent end_frame_latch; @@ -1424,6 +1432,7 @@ TEST_F(ShellTest, DISABLED_SkipAndSubmitFrame) { PlatformViewNotifyDestroyed(shell.get()); DestroyShell(std::move(shell)); +#endif // OS_FUCHSIA } TEST(SettingsTest, FrameTimingSetsAndGetsProperly) { @@ -3665,7 +3674,7 @@ TEST_F(ShellTest, CanCreateShellsWithGLBackend) { #if !SHELL_ENABLE_GL // GL emulation does not exist on Fuchsia. GTEST_SKIP(); -#endif // !SHELL_ENABLE_GL +#else auto settings = CreateSettingsForFixture(); std::unique_ptr shell = CreateShell({ .settings = settings, @@ -3682,12 +3691,13 @@ TEST_F(ShellTest, CanCreateShellsWithGLBackend) { PumpOneFrame(shell.get()); PlatformViewNotifyDestroyed(shell.get()); DestroyShell(std::move(shell)); +#endif // !SHELL_ENABLE_GL } TEST_F(ShellTest, CanCreateShellsWithVulkanBackend) { #if !SHELL_ENABLE_VULKAN GTEST_SKIP(); -#endif // !SHELL_ENABLE_VULKAN +#else auto settings = CreateSettingsForFixture(); std::unique_ptr shell = CreateShell({ .settings = settings, @@ -3705,12 +3715,13 @@ TEST_F(ShellTest, CanCreateShellsWithVulkanBackend) { PumpOneFrame(shell.get()); PlatformViewNotifyDestroyed(shell.get()); DestroyShell(std::move(shell)); +#endif // !SHELL_ENABLE_VULKAN } TEST_F(ShellTest, CanCreateShellsWithMetalBackend) { #if !SHELL_ENABLE_METAL GTEST_SKIP(); -#endif // !SHELL_ENABLE_METAL +#else auto settings = CreateSettingsForFixture(); std::unique_ptr shell = CreateShell({ .settings = settings, @@ -3728,6 +3739,7 @@ TEST_F(ShellTest, CanCreateShellsWithMetalBackend) { PumpOneFrame(shell.get()); PlatformViewNotifyDestroyed(shell.get()); DestroyShell(std::move(shell)); +#endif // !SHELL_ENABLE_METAL } TEST_F(ShellTest, UserTagSetOnStartup) { @@ -4010,7 +4022,7 @@ TEST_F(ShellTest, PictureToImageSync) { #if !SHELL_ENABLE_GL // This test uses the GL backend. GTEST_SKIP(); -#endif // !SHELL_ENABLE_GL +#else auto settings = CreateSettingsForFixture(); std::unique_ptr shell = CreateShell({ .settings = settings, @@ -4044,13 +4056,14 @@ TEST_F(ShellTest, PictureToImageSync) { PlatformViewNotifyDestroyed(shell.get()); DestroyShell(std::move(shell)); +#endif // !SHELL_ENABLE_GL } TEST_F(ShellTest, PictureToImageSyncImpellerNoSurface) { #if !SHELL_ENABLE_METAL // This test uses the Metal backend. GTEST_SKIP(); -#endif // !SHELL_ENABLE_METAL +#else auto settings = CreateSettingsForFixture(); settings.enable_impeller = true; std::unique_ptr shell = CreateShell({ @@ -4090,6 +4103,7 @@ TEST_F(ShellTest, PictureToImageSyncImpellerNoSurface) { PlatformViewNotifyDestroyed(shell.get()); DestroyShell(std::move(shell)); +#endif // !SHELL_ENABLE_METAL } #if SHELL_ENABLE_GL @@ -4297,7 +4311,7 @@ TEST_F(ShellTest, NotifyDestroyed) { TEST_F(ShellTest, PrintsErrorWhenPlatformMessageSentFromWrongThread) { #if FLUTTER_RUNTIME_MODE != FLUTTER_RUNTIME_MODE_DEBUG || OS_FUCHSIA GTEST_SKIP() << "Test is for debug mode only on non-fuchsia targets."; -#endif +#else Settings settings = CreateSettingsForFixture(); ThreadHost thread_host("io.flutter.test." + GetCurrentTestName() + ".", ThreadHost::Type::Platform); @@ -4340,12 +4354,13 @@ TEST_F(ShellTest, PrintsErrorWhenPlatformMessageSentFromWrongThread) { DestroyShell(std::move(shell), task_runners); ASSERT_FALSE(DartVMRef::IsInstanceRunning()); +#endif } TEST_F(ShellTest, DiesIfSoftwareRenderingAndImpellerAreEnabledDeathTest) { #if defined(OS_FUCHSIA) GTEST_SKIP() << "Fuchsia"; -#endif // OS_FUCHSIA +#else ::testing::FLAGS_gtest_death_test_style = "threadsafe"; Settings settings = CreateSettingsForFixture(); settings.enable_impeller = true; @@ -4358,6 +4373,7 @@ TEST_F(ShellTest, DiesIfSoftwareRenderingAndImpellerAreEnabledDeathTest) { EXPECT_DEATH_IF_SUPPORTED( CreateShell(settings, task_runners), "Software rendering is incompatible with Impeller."); +#endif // OS_FUCHSIA } } // namespace testing diff --git a/engine/src/flutter/shell/gpu/gpu_surface_metal_impeller.mm b/engine/src/flutter/shell/gpu/gpu_surface_metal_impeller.mm index 32b2f283253..2db00a25dc4 100644 --- a/engine/src/flutter/shell/gpu/gpu_surface_metal_impeller.mm +++ b/engine/src/flutter/shell/gpu/gpu_surface_metal_impeller.mm @@ -252,14 +252,18 @@ std::unique_ptr GPUSurfaceMetalImpeller::AcquireFrameFromMTLTextur display_list->Dispatch(impeller_dispatcher, sk_cull_rect); auto picture = impeller_dispatcher.EndRecordingAsPicture(); - return renderer->Render( - std::move(surface), - fml::MakeCopyable([aiks_context, picture = std::move(picture)]( - impeller::RenderTarget& render_target) -> bool { - return aiks_context->Render(picture, render_target); - })); + bool render_result = + renderer->Render(std::move(surface), + fml::MakeCopyable([aiks_context, picture = std::move(picture)]( + impeller::RenderTarget& render_target) -> bool { + return aiks_context->Render(picture, render_target); + })); + if (!render_result) { + FML_LOG(ERROR) << "Failed to render Impeller frame"; + return false; + } - delegate->PresentTexture(texture_info); + return delegate->PresentTexture(texture_info); }); SurfaceFrame::FramebufferInfo framebuffer_info; diff --git a/engine/src/flutter/shell/platform/android/surface/android_native_window.cc b/engine/src/flutter/shell/platform/android/surface/android_native_window.cc index 20e75f57aaf..34e6d438279 100644 --- a/engine/src/flutter/shell/platform/android/surface/android_native_window.cc +++ b/engine/src/flutter/shell/platform/android/surface/android_native_window.cc @@ -16,8 +16,8 @@ AndroidNativeWindow::~AndroidNativeWindow() { if (window_ != nullptr) { #if FML_OS_ANDROID ANativeWindow_release(window_); -#endif // FML_OS_ANDROID window_ = nullptr; +#endif // FML_OS_ANDROID } } diff --git a/engine/src/flutter/shell/platform/embedder/tests/embedder_a11y_unittests.cc b/engine/src/flutter/shell/platform/embedder/tests/embedder_a11y_unittests.cc index b9922c6052a..53a92dafdbd 100644 --- a/engine/src/flutter/shell/platform/embedder/tests/embedder_a11y_unittests.cc +++ b/engine/src/flutter/shell/platform/embedder/tests/embedder_a11y_unittests.cc @@ -96,7 +96,7 @@ TEST_F(EmbedderTest, CannotProvideMultipleSemanticsCallbacks) { TEST_F(EmbedderA11yTest, A11yTreeIsConsistent) { #if defined(OS_FUCHSIA) GTEST_SKIP() << "This test crashes on Fuchsia. https://fxbug.dev/87493 "; -#endif // OS_FUCHSIA +#else auto& context = GetEmbedderContext(EmbedderTestContextType::kSoftwareContext); @@ -269,12 +269,13 @@ TEST_F(EmbedderA11yTest, A11yTreeIsConsistent) { result = FlutterEngineUpdateSemanticsEnabled(engine.get(), false); ASSERT_EQ(result, FlutterEngineResult::kSuccess); notify_semantics_enabled_latch_3.Wait(); +#endif // OS_FUCHSIA } TEST_F(EmbedderA11yTest, A11yTreeIsConsistentUsingUnstableCallbacks) { #if defined(OS_FUCHSIA) GTEST_SKIP() << "This test crashes on Fuchsia. https://fxbug.dev/87493 "; -#endif // OS_FUCHSIA +#else auto& context = GetEmbedderContext(EmbedderTestContextType::kSoftwareContext); @@ -445,6 +446,7 @@ TEST_F(EmbedderA11yTest, A11yTreeIsConsistentUsingUnstableCallbacks) { result = FlutterEngineUpdateSemanticsEnabled(engine.get(), false); ASSERT_EQ(result, FlutterEngineResult::kSuccess); notify_semantics_enabled_latch_3.Wait(); +#endif // OS_FUCHSIA } TEST_F(EmbedderA11yTest, A11yTreeIsConsistentUsingLegacyCallbacks) { diff --git a/engine/src/flutter/shell/platform/embedder/tests/embedder_unittests.cc b/engine/src/flutter/shell/platform/embedder/tests/embedder_unittests.cc index 33af2a2e122..cbbf8f5bd9d 100644 --- a/engine/src/flutter/shell/platform/embedder/tests/embedder_unittests.cc +++ b/engine/src/flutter/shell/platform/embedder/tests/embedder_unittests.cc @@ -1581,7 +1581,7 @@ TEST_F(EmbedderTest, CanSuccessfullyPopulateSpecificJITSnapshotCallbacks) { // TODO(#107263): Inconsistent snapshot paths in the Linux Fuchsia FEMU test. #if defined(OS_FUCHSIA) GTEST_SKIP() << "Inconsistent paths in Fuchsia."; -#endif // OS_FUCHSIA +#else // This test is only relevant in JIT mode. if (DartVM::IsRunningPrecompiledCode()) { @@ -1624,6 +1624,7 @@ TEST_F(EmbedderTest, CanSuccessfullyPopulateSpecificJITSnapshotCallbacks) { ASSERT_NE(settings.isolate_snapshot_data(), nullptr); ASSERT_NE(settings.isolate_snapshot_instr(), nullptr); ASSERT_NE(settings.dart_library_sources_kernel(), nullptr); +#endif // OS_FUCHSIA } //------------------------------------------------------------------------------ @@ -1636,7 +1637,7 @@ TEST_F(EmbedderTest, JITSnapshotCallbacksFailWithInvalidLocation) { // TODO(#107263): Inconsistent snapshot paths in the Linux Fuchsia FEMU test. #if defined(OS_FUCHSIA) GTEST_SKIP() << "Inconsistent paths in Fuchsia."; -#endif // OS_FUCHSIA +#else // This test is only relevant in JIT mode. if (DartVM::IsRunningPrecompiledCode()) { @@ -1667,6 +1668,7 @@ TEST_F(EmbedderTest, JITSnapshotCallbacksFailWithInvalidLocation) { ASSERT_EQ(settings.vm_snapshot_instr(), nullptr); ASSERT_EQ(settings.isolate_snapshot_data(), nullptr); ASSERT_EQ(settings.isolate_snapshot_instr(), nullptr); +#endif // OS_FUCHSIA } //------------------------------------------------------------------------------ diff --git a/engine/src/flutter/shell/platform/glfw/platform_handler.cc b/engine/src/flutter/shell/platform/glfw/platform_handler.cc index 27098f98502..8659c84f585 100644 --- a/engine/src/flutter/shell/platform/glfw/platform_handler.cc +++ b/engine/src/flutter/shell/platform/glfw/platform_handler.cc @@ -87,7 +87,6 @@ void PlatformHandler::HandleMethodCall( result->Success(); } else if (method.compare(kSystemNavigatorPopMethod) == 0) { exit(EXIT_SUCCESS); - result->Success(); } else { result->NotImplemented(); } diff --git a/engine/src/flutter/testing/elf_loader.cc b/engine/src/flutter/testing/elf_loader.cc index 2c7a989dbb3..d420d841d94 100644 --- a/engine/src/flutter/testing/elf_loader.cc +++ b/engine/src/flutter/testing/elf_loader.cc @@ -30,14 +30,15 @@ ELFAOTSymbols LoadELFSymbolFromFixturesIfNeccessary(std::string elf_filename) { ELFAOTSymbols symbols; - // Must not be freed. - const char* error = nullptr; - #if OS_FUCHSIA // TODO(gw280): https://github.com/flutter/flutter/issues/50285 // Dart doesn't implement Dart_LoadELF on Fuchsia - auto loaded_elf = nullptr; + FML_LOG(ERROR) << "Dart doesn't implement Dart_LoadELF on Fuchsia"; + return {}; #else + // Must not be freed. + const char* error = nullptr; + auto loaded_elf = Dart_LoadELF(elf_path.c_str(), // file path 0, // file offset @@ -47,7 +48,6 @@ ELFAOTSymbols LoadELFSymbolFromFixturesIfNeccessary(std::string elf_filename) { &symbols.vm_isolate_data, // vm isolate data (out) &symbols.vm_isolate_instrs // vm isolate instr (out) ); -#endif if (loaded_elf == nullptr) { FML_LOG(ERROR) @@ -60,6 +60,7 @@ ELFAOTSymbols LoadELFSymbolFromFixturesIfNeccessary(std::string elf_filename) { symbols.loaded_elf.reset(loaded_elf); return symbols; +#endif // OS_FUCHSIA } ELFAOTSymbols LoadELFSplitSymbolFromFixturesIfNeccessary( @@ -79,14 +80,15 @@ ELFAOTSymbols LoadELFSplitSymbolFromFixturesIfNeccessary( ELFAOTSymbols symbols; - // Must not be freed. - const char* error = nullptr; - #if OS_FUCHSIA // TODO(gw280): https://github.com/flutter/flutter/issues/50285 // Dart doesn't implement Dart_LoadELF on Fuchsia - auto loaded_elf = nullptr; + FML_LOG(ERROR) << "Dart doesn't implement Dart_LoadELF on Fuchsia"; + return {}; #else + // Must not be freed. + const char* error = nullptr; + auto loaded_elf = Dart_LoadELF(elf_path.c_str(), // file path 0, // file offset @@ -96,7 +98,6 @@ ELFAOTSymbols LoadELFSplitSymbolFromFixturesIfNeccessary( &symbols.vm_isolate_data, // vm isolate data (out) &symbols.vm_isolate_instrs // vm isolate instr (out) ); -#endif if (loaded_elf == nullptr) { FML_LOG(ERROR) @@ -109,6 +110,7 @@ ELFAOTSymbols LoadELFSplitSymbolFromFixturesIfNeccessary( symbols.loaded_elf.reset(loaded_elf); return symbols; +#endif } bool PrepareSettingsForAOTWithSymbols(Settings& settings, diff --git a/engine/src/flutter/third_party/accessibility/ax/ax_node_position_unittest.cc b/engine/src/flutter/third_party/accessibility/ax/ax_node_position_unittest.cc index b5b993d4723..0d028c2270a 100644 --- a/engine/src/flutter/third_party/accessibility/ax/ax_node_position_unittest.cc +++ b/engine/src/flutter/third_party/accessibility/ax/ax_node_position_unittest.cc @@ -5567,8 +5567,10 @@ TEST_F(AXPositionTest, TEST_F(AXPositionTest, AsLeafTextPositionBeforeAndAfterCharacterAtInvalidGraphemeBoundary) { +#if true GTEST_SKIP() << "Skipping, current accessibility library cannot handle grapheme"; +#else std::vector text_offsets; SetTree(CreateMultilingualDocument(&text_offsets)); @@ -5608,6 +5610,7 @@ TEST_F(AXPositionTest, // should have been reset in order to provide consistent output from the // method regardless of input affinity. EXPECT_EQ(ax::mojom::TextAffinity::kDownstream, test_position->affinity()); +#endif // true } TEST_F(AXPositionTest, AsLeafTextPositionBeforeCharacterNoAdjustment) { @@ -6396,8 +6399,10 @@ TEST_F(AXPositionTest, CreatePreviousCharacterPosition) { } TEST_F(AXPositionTest, CreateNextCharacterPositionAtGraphemeBoundary) { +#if true GTEST_SKIP() << "Skipping, current accessibility library cannot handle grapheme"; +#else std::vector text_offsets; SetTree(CreateMultilingualDocument(&text_offsets)); @@ -6469,11 +6474,14 @@ TEST_F(AXPositionTest, CreateNextCharacterPositionAtGraphemeBoundary) { EXPECT_EQ(12, test_position->text_offset()); // Affinity should have been reset to downstream because there was a move. EXPECT_EQ(ax::mojom::TextAffinity::kDownstream, test_position->affinity()); +#endif // true } TEST_F(AXPositionTest, CreatePreviousCharacterPositionAtGraphemeBoundary) { +#if true GTEST_SKIP() << "Skipping, current accessibility library cannot handle grapheme"; +#else std::vector text_offsets; SetTree(CreateMultilingualDocument(&text_offsets)); @@ -6546,6 +6554,7 @@ TEST_F(AXPositionTest, CreatePreviousCharacterPositionAtGraphemeBoundary) { EXPECT_EQ(9, test_position->text_offset()); // Affinity should have been reset to downstream because there was a move. EXPECT_EQ(ax::mojom::TextAffinity::kDownstream, test_position->affinity()); +#endif // true } TEST_F(AXPositionTest, ReciprocalCreateNextAndPreviousCharacterPosition) { diff --git a/engine/src/flutter/third_party/accessibility/base/win/variant_vector.cc b/engine/src/flutter/third_party/accessibility/base/win/variant_vector.cc index 22a28e0e1c7..a60950d394a 100644 --- a/engine/src/flutter/third_party/accessibility/base/win/variant_vector.cc +++ b/engine/src/flutter/third_party/accessibility/base/win/variant_vector.cc @@ -302,8 +302,8 @@ int VariantVector::Compare(SAFEARRAY* safearray, bool ignore_case) const { // VARTYPES. For example a value within VT_TYPEMASK that's joined something // outside the typemask like VT_ARRAY or VT_BYREF. default: - BASE_UNREACHABLE(); compare_result = 1; + BASE_UNREACHABLE(); break; }