From 5a3f245710f00ebfebce9be9a641dc20f433285a Mon Sep 17 00:00:00 2001 From: Chris Bracken Date: Wed, 6 Nov 2024 12:17:08 -0800 Subject: [PATCH] iOS,macOS: Add Obj-C cflags to all Obj-C targets (flutter/engine#56386) Ensure that all Objective-C code in the codebase is being built with the standard set of Flutter Objective-C compiler flags with ARC enabled. Also bumps the cflags config up to the top of the first block within each target in which Objective-C sources appear, so that the location is consistent. Migrates The following targets to ARC, which had been missed in previous passes since they didn't declare the standard Flutter Obj-C[++] cflags: * `//flutter/fml:fml_unittests` * `//flutter/impeller/golden_tests:metal_screenshot` * `//flutter/impeller/playground:playground` * `//flutter/impeller/backend/metal:metal` * `//flutter/impeller/backend/metal:metal_unittests` * `//shell/gpu:gpu_surface_metal_unittests` * `//flutter/shell/platform/embedder:embedder_unittests` This patch includes no semantic changes. Issue: https://github.com/flutter/flutter/issues/137801 [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style --- engine/src/flutter/fml/BUILD.gn | 18 +++++++++--------- engine/src/flutter/impeller/BUILD.gn | 5 ++++- engine/src/flutter/shell/common/BUILD.gn | 6 +++--- engine/src/flutter/shell/gpu/BUILD.gn | 9 ++++++--- .../shell/platform/darwin/common/BUILD.gn | 7 +++---- .../shell/platform/darwin/macos/BUILD.gn | 11 +++++------ .../flutter/shell/platform/embedder/BUILD.gn | 9 ++++++--- .../embedder/tests/embedder_metal_unittests.mm | 2 +- engine/src/flutter/testing/BUILD.gn | 10 +++++----- .../flutter/third_party/accessibility/BUILD.gn | 8 ++++---- .../third_party/accessibility/ax/BUILD.gn | 5 +++-- 11 files changed, 49 insertions(+), 41 deletions(-) diff --git a/engine/src/flutter/fml/BUILD.gn b/engine/src/flutter/fml/BUILD.gn index 0b1ff30be08..fb66f7ffafc 100644 --- a/engine/src/flutter/fml/BUILD.gn +++ b/engine/src/flutter/fml/BUILD.gn @@ -135,18 +135,13 @@ source_set("fml") { libs = [] if (is_ios || is_mac) { - sources += [ "platform/darwin/concurrent_message_loop_factory.mm" ] - } else { - sources += [ "concurrent_message_loop_factory.cc" ] - } - - if (is_ios || is_mac) { - cflags_objc = flutter_cflags_objc - cflags_objcc = flutter_cflags_objcc + cflags_objc = flutter_cflags_objc_arc + cflags_objcc = flutter_cflags_objcc_arc sources += [ "platform/darwin/cf_utils.cc", "platform/darwin/cf_utils.h", + "platform/darwin/concurrent_message_loop_factory.mm", "platform/darwin/message_loop_darwin.h", "platform/darwin/message_loop_darwin.mm", "platform/darwin/paths_darwin.mm", @@ -163,6 +158,8 @@ source_set("fml") { ] frameworks = [ "Foundation.framework" ] + } else { + sources += [ "concurrent_message_loop_factory.cc" ] } if (is_android) { @@ -365,7 +362,10 @@ if (enable_unittests) { "time/time_unittest.cc", ] - if (is_mac) { + if (is_mac || is_ios) { + cflags_objc = flutter_cflags_objc_arc + cflags_objcc = flutter_cflags_objcc_arc + sources += [ "platform/darwin/cf_utils_unittests.mm", "platform/darwin/string_range_sanitization_unittests.mm", diff --git a/engine/src/flutter/impeller/BUILD.gn b/engine/src/flutter/impeller/BUILD.gn index 3bd0a09c828..0f8b679abea 100644 --- a/engine/src/flutter/impeller/BUILD.gn +++ b/engine/src/flutter/impeller/BUILD.gn @@ -60,7 +60,6 @@ impeller_component("impeller_unittests") { "core:allocator_unittests", "display_list:skia_conversions_unittests", "geometry:geometry_unittests", - "renderer/backend/metal:metal_unittests", "runtime_stage:runtime_stage_unittests", "shader_archive:shader_archive_unittests", "tessellator:tessellator_unittests", @@ -80,6 +79,10 @@ impeller_component("impeller_unittests") { ] } + if (impeller_enable_metal) { + deps += [ "//flutter/impeller/renderer/backend/metal:metal_unittests" ] + } + if (impeller_enable_vulkan) { deps += [ "//flutter/impeller/renderer/backend/vulkan:vulkan_unittests" ] } diff --git a/engine/src/flutter/shell/common/BUILD.gn b/engine/src/flutter/shell/common/BUILD.gn index a2cd2be4834..96133165d20 100644 --- a/engine/src/flutter/shell/common/BUILD.gn +++ b/engine/src/flutter/shell/common/BUILD.gn @@ -296,14 +296,14 @@ if (enable_unittests) { } if (test_enable_metal) { + cflags_objc = flutter_cflags_objc_arc + cflags_objcc = flutter_cflags_objcc_arc + sources += [ "shell_test_platform_view_metal.h", "shell_test_platform_view_metal.mm", ] - cflags_objc = flutter_cflags_objc_arc - cflags_objcc = flutter_cflags_objcc_arc - public_deps += [ "//flutter/shell/platform/darwin/graphics" ] } } diff --git a/engine/src/flutter/shell/gpu/BUILD.gn b/engine/src/flutter/shell/gpu/BUILD.gn index ed3dbfbf5b9..8d216d16c79 100644 --- a/engine/src/flutter/shell/gpu/BUILD.gn +++ b/engine/src/flutter/shell/gpu/BUILD.gn @@ -71,6 +71,9 @@ source_set("gpu_surface_vulkan") { if (shell_enable_metal) { source_set("gpu_surface_metal") { + cflags_objc = flutter_cflags_objc_arc + cflags_objcc = flutter_cflags_objcc_arc + sources = [ "gpu_surface_metal_delegate.cc", "gpu_surface_metal_delegate.h", @@ -80,9 +83,6 @@ if (shell_enable_metal) { "gpu_surface_noop.mm", ] - cflags_objc = flutter_cflags_objc_arc - cflags_objcc = flutter_cflags_objcc_arc - public_deps = gpu_common_deps if (impeller_enable_metal) { @@ -99,6 +99,9 @@ if (shell_enable_metal) { if (is_mac) { impeller_component("gpu_surface_metal_unittests") { testonly = true + cflags_objc = flutter_cflags_objc_arc + cflags_objcc = flutter_cflags_objcc_arc + target_type = "executable" sources = [ "gpu_surface_metal_impeller_unittests.mm", diff --git a/engine/src/flutter/shell/platform/darwin/common/BUILD.gn b/engine/src/flutter/shell/platform/darwin/common/BUILD.gn index 58638a5bc09..459f473909d 100644 --- a/engine/src/flutter/shell/platform/darwin/common/BUILD.gn +++ b/engine/src/flutter/shell/platform/darwin/common/BUILD.gn @@ -111,6 +111,9 @@ test_fixtures("framework_common_fixtures") { # Unit tests for channels. executable("framework_common_unittests") { testonly = true + cflags_objc = flutter_cflags_objc_arc + cflags_objcc = flutter_cflags_objcc_arc + ldflags = [ "-ObjC" ] sources = [ "framework/Source/FlutterBinaryMessengerRelayTest.mm", @@ -119,10 +122,6 @@ executable("framework_common_unittests") { "framework/Source/flutter_standard_codec_unittest.mm", ] - cflags_objcc = flutter_cflags_objcc_arc - - ldflags = [ "-ObjC" ] - deps = [ ":framework_common", ":framework_common_fixtures", diff --git a/engine/src/flutter/shell/platform/darwin/macos/BUILD.gn b/engine/src/flutter/shell/platform/darwin/macos/BUILD.gn index 3ce4bfc1f0a..1274f1c4d87 100644 --- a/engine/src/flutter/shell/platform/darwin/macos/BUILD.gn +++ b/engine/src/flutter/shell/platform/darwin/macos/BUILD.gn @@ -52,6 +52,8 @@ _flutter_framework_headers_copy_dir = source_set("flutter_framework_source") { visibility = [ ":*" ] + cflags_objc = flutter_cflags_objc_arc + cflags_objcc = flutter_cflags_objcc_arc sources = [ "framework/Source/AccessibilityBridgeMac.h", @@ -141,8 +143,6 @@ source_set("flutter_framework_source") { "FLUTTER_ENGINE_NO_PROTOTYPES", ] - cflags_objcc = flutter_cflags_objcc_arc - frameworks = [ "Carbon.framework", "Cocoa.framework", @@ -173,6 +173,9 @@ test_fixtures("flutter_desktop_darwin_fixtures") { executable("flutter_desktop_darwin_unittests") { testonly = true + cflags_objc = flutter_cflags_objc_arc + cflags_objcc = flutter_cflags_objcc_arc + ldflags = [ "-ObjC" ] sources = [ "framework/Source/AccessibilityBridgeMacTest.mm", @@ -205,10 +208,6 @@ executable("flutter_desktop_darwin_unittests") { "framework/Source/TestFlutterPlatformView.mm", ] - cflags_objcc = flutter_cflags_objcc_arc - - ldflags = [ "-ObjC" ] - deps = [ ":flutter_desktop_darwin_fixtures", ":flutter_framework_source", diff --git a/engine/src/flutter/shell/platform/embedder/BUILD.gn b/engine/src/flutter/shell/platform/embedder/BUILD.gn index de450dc5ed4..a0c1b25899b 100644 --- a/engine/src/flutter/shell/platform/embedder/BUILD.gn +++ b/engine/src/flutter/shell/platform/embedder/BUILD.gn @@ -144,6 +144,9 @@ template("embedder_source_set") { } if (embedder_enable_metal) { + cflags_objc = flutter_cflags_objc_arc + cflags_objcc = flutter_cflags_objcc_arc + sources += [ "embedder_external_texture_metal.h", "embedder_external_texture_metal.mm", @@ -159,9 +162,6 @@ template("embedder_source_set") { deps += [ "//flutter/impeller/renderer/backend/metal" ] } - cflags_objc = flutter_cflags_objc_arc - cflags_objcc = flutter_cflags_objcc_arc - deps += [ "//flutter/shell/platform/darwin/graphics" ] } @@ -375,6 +375,9 @@ if (enable_unittests) { } if (test_enable_metal) { + cflags_objc = flutter_cflags_objc_arc + cflags_objcc = flutter_cflags_objcc_arc + sources += [ "tests/embedder_metal_unittests.mm" ] } diff --git a/engine/src/flutter/shell/platform/embedder/tests/embedder_metal_unittests.mm b/engine/src/flutter/shell/platform/embedder/tests/embedder_metal_unittests.mm index 3f5ec9e549d..d59c92bf62c 100644 --- a/engine/src/flutter/shell/platform/embedder/tests/embedder_metal_unittests.mm +++ b/engine/src/flutter/shell/platform/embedder/tests/embedder_metal_unittests.mm @@ -65,7 +65,7 @@ static sk_sp GetSurfaceFromTexture(const sk_sp& skia SkISize texture_size, void* texture) { GrMtlTextureInfo info; - info.fTexture.reset([(id)texture retain]); + info.fTexture.retain(texture); GrBackendTexture backend_texture = GrBackendTextures::MakeMtl( texture_size.width(), texture_size.height(), skgpu::Mipmapped::kNo, info); diff --git a/engine/src/flutter/testing/BUILD.gn b/engine/src/flutter/testing/BUILD.gn index 54bdcf40009..2da76f07d75 100644 --- a/engine/src/flutter/testing/BUILD.gn +++ b/engine/src/flutter/testing/BUILD.gn @@ -190,7 +190,12 @@ if (enable_unittests) { # On iOS, this is enabled to allow for Metal tests to run within a test app if (is_mac || is_ios) { source_set("metal") { + testonly = true + if (shell_enable_metal) { + cflags_objc = flutter_cflags_objc_arc + cflags_objcc = flutter_cflags_objcc_arc + sources = [ "test_metal_context.h", "test_metal_context.mm", @@ -210,11 +215,6 @@ if (is_mac || is_ios) { deps += [ "//flutter/vulkan" ] } } - - cflags_objc = flutter_cflags_objc_arc - cflags_objcc = flutter_cflags_objcc_arc - - testonly = true } } diff --git a/engine/src/flutter/third_party/accessibility/BUILD.gn b/engine/src/flutter/third_party/accessibility/BUILD.gn index b5eb256f4dd..9cdc8e827c1 100644 --- a/engine/src/flutter/third_party/accessibility/BUILD.gn +++ b/engine/src/flutter/third_party/accessibility/BUILD.gn @@ -76,6 +76,10 @@ if (enable_unittests) { ] if (is_mac) { + cflags_objc = flutter_cflags_objc_arc + cflags_objcc = flutter_cflags_objcc_arc + ldflags = [ "-ObjC" ] + sources += [ "ax/platform/ax_platform_node_mac_unittest.mm" ] frameworks = [ "AppKit.framework", @@ -84,10 +88,6 @@ if (enable_unittests) { "CoreText.framework", "IOSurface.framework", ] - - cflags_objc = flutter_cflags_objc_arc - cflags_objcc = flutter_cflags_objcc_arc - ldflags = [ "-ObjC" ] } if (is_win) { sources += [ diff --git a/engine/src/flutter/third_party/accessibility/ax/BUILD.gn b/engine/src/flutter/third_party/accessibility/ax/BUILD.gn index a4e366615ff..eb1a950e8e9 100644 --- a/engine/src/flutter/third_party/accessibility/ax/BUILD.gn +++ b/engine/src/flutter/third_party/accessibility/ax/BUILD.gn @@ -82,12 +82,13 @@ source_set("ax") { ] if (is_mac) { + cflags_objc = flutter_cflags_objc_arc + cflags_objcc = flutter_cflags_objcc_arc + sources += [ "platform/ax_platform_node_mac.h", "platform/ax_platform_node_mac.mm", ] - cflags_objc = flutter_cflags_objc_arc - cflags_objcc = flutter_cflags_objcc_arc } else if (is_win) { sources += [ "platform/ax_fragment_root_delegate_win.h",