[Impeller] delete Impeller sim opt out.

[jonahwilliams]

Impeller only on simulators.

[flutter-dashboard]

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group.

[zanderso]

Should this be || and not && ?

[jonahwilliams]

....yes

[jonahwilliams]

These compile errors are from the last commit...

[zanderso]

These compile errors are from the last commit...

Oof. I don't have a better suggestion than kicking it again with an empty commit.

[zanderso]

It looks like the failing jobs are just not being triggered to run. I'm going to click re-run on one of them.

[jtmcdole]

Looking at the bot for the last commit / run - its clearly checking out the head of the last commit:

https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8730815805504040081/+/u/Checkout_source_code/bot_update/stdout

[jtmcdole]

Log search:

protoPayload.line.logMessage =~ "(ca78e0147ca4bd840eb64551c9cdb3319363ebab|764e3f401b84fd97348f33808bbce1e64ccfc778|746dcb5a926560e42ace9b322bf951d2106d1c7d)"
protoPayload.line.logMessage =~ "Mac mac_clang_tidy"

Then looking later; the error is actually a different file:

FAILED: obj/flutter/shell/common/common.switches.o 
/Volumes/Work/s/w/ir/cache/builder/src/flutter/buildtools/mac-arm64/reclient/rewrapper --cfg=/Volumes/Work/s/w/ir/cache/builder/src/flutter/build/rbe/rewrapper-mac-arm64.cfg --exec_root=/Volumes/Work/s/w/ir/cache/builder/src/ --server_address=unix:///Volumes/Work/s/w/ir/x/w/rc/rbei3atvle3/reproxy.sock --exec_strategy=remote_local_fallback --remote_wrapper=../../../flutter/build/rbe/remote_wrapper.sh --local_wrapper=../../../flutter/build/rbe/local_wrapper.sh --labels=type=compile,compiler=clang,lang=cpp ../../../flutter/buildtools/mac-arm64/clang/bin/clang++ -MMD -MF obj/flutter/shell/common/common.switches.o.d -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D_FORTIFY_SOURCE=2 -D_LIBCPP_DISABLE_AVAILABILITY=1 -D_LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS -D_LIBCPP_ENABLE_THREAD_SAFETY_ANNOTATIONS -DNDEBUG -DNVALGRIND -DDYNAMIC_ANNOTATIONS_ENABLED=0 -DSK_TYPEFACE_FACTORY_CORETEXT -DSK_FONTMGR_CORETEXT_AVAILABLE -DSK_METAL -DSK_CODEC_DECODES_JPEG -DSK_CODEC_DECODES_PNG -DSK_CODEC_DECODES_ICO -DSK_CODEC_DECODES_WEBP -DSK_HAS_WUFFS_LIBRARY -DSK_CODEC_DECODES_GIF -DSK_XML -DRAPIDJSON_HAS_STDSTRING -DRAPIDJSON_HAS_CXX11_RANGE_FOR -DRAPIDJSON_HAS_CXX11_RVALUE_REFS -DRAPIDJSON_HAS_CXX11_TYPETRAITS -DRAPIDJSON_HAS_CXX11_NOEXCEPT -DFLUTTER_RUNTIME_MODE_DEBUG=1 -DFLUTTER_RUNTIME_MODE_PROFILE=2 -DFLUTTER_RUNTIME_MODE_RELEASE=3 -DFLUTTER_RUNTIME_MODE_JIT_RELEASE=4 -DDART_LEGACY_API=\[\[deprecated\]\] -DFLUTTER_RUNTIME_MODE=1 -DFLUTTER_JIT_RUNTIME=1 -DIMPELLER_DEBUG=1 -DIMPELLER_SUPPORTS_RENDERING=1 -DIMPELLER_ENABLE_METAL=1 -DSK_CODEC_DECODES_BMP -DSK_CODEC_DECODES_WBMP -DSK_ENABLE_DUMP_GPU -DSK_FORCE_AAA -DSK_LEGACY_IGNORE_DRAW_VERTICES_BLEND_WITH_NO_SHADER -DSK_DISABLE_LEGACY_METAL_BACKEND_SURFACE -DSK_DISABLE_LEGACY_PARAGRAPH_UNICODE -DSK_DISABLE_LEGACY_SHADERCONTEXT -DSK_DISABLE_LOWP_RASTER_PIPELINE -DSK_FORCE_RASTER_PIPELINE_BLITTER -DSK_METAL_WAIT_UNTIL_SCHEDULED -DSK_DISABLE_EFFECT_DESERIALIZATION -DSK_ENABLE_PRECOMPILE -DSK_ASSUME_GL_ES=1 -DSK_GANESH -DU_USING_ICU_NAMESPACE=0 -DU_ENABLE_DYLOAD=0 -DUSE_CHROMIUM_ICU=1 -DU_ENABLE_TRACING=1 -DU_ENABLE_RESOURCE_TRACING=0 -DU_STATIC_IMPLEMENTATION -DICU_UTIL_DATA_IMPL=ICU_UTIL_DATA_FILE -DSK_ENABLE_PARAGRAPH -DSK_UNICODE_AVAILABLE -DSK_UNICODE_ICU_IMPLEMENTATION -I../../.. -Igen -I../../../flutter/third_party/libcxx/include -I../../../flutter/third_party/libcxxabi/include -I../../../flutter/build/secondary/flutter/third_party/libcxx/config -I../../../flutter -I../../../flutter/third_party/rapidjson -I../../../flutter/third_party/rapidjson/include -I../../../flutter/third_party -I../../../flutter/third_party/dart/runtime -I../../../flutter/third_party/dart/runtime/include -I../../../flutter/third_party/txt/src -Igen/flutter -Igen/flutter/impeller/runtime_stage -I../../../flutter/third_party/flatbuffers/include -I../../../flutter/third_party/skia -I../../../flutter/third_party/harfbuzz/src -I../../../flutter/third_party/icu/source/common -I../../../flutter/third_party/icu/source/i18n -I../../../flutter/third_party/skia/modules/skparagraph/include -I../../../flutter/third_party/skia/modules/skparagraph/utils -I../../../flutter/third_party/skia/modules/skunicode/include -fno-strict-aliasing --target=x86_64-apple-darwin -arch x86_64 -fcolor-diagnostics -Wall -Wextra -Wendif-labels -Werror -Wno-missing-field-initializers -Wno-unused-parameter -Wno-unused-but-set-parameter -Wno-unused-but-set-variable -Wno-implicit-int-float-conversion -Wno-deprecated-copy -Wno-psabi -Wno-deprecated-literal-operator -Wno-unqualified-std-cast-call -Wno-non-c-typedef-for-linkage -Wno-range-loop-construct -Wunguarded-availability -Wno-deprecated-declarations -no-canonical-prefixes -fvisibility=hidden -Wstring-conversion -Wnewline-eof -Os -fno-ident -fdata-sections -ffunction-sections -g2 -Wunreachable-code -Wno-newline-eof -fvisibility-inlines-hidden -std=c++17 -fno-rtti -nostdinc++ -nostdinc++ -fvisibility=hidden -fno-exceptions -stdlib=libc++ -isysroot ../../../flutter/prebuilts/SDKs/iPhoneSimulator17.0.sdk -mios-simulator-version-min=12.0  -c ../../../flutter/shell/common/switches.cc -o obj/flutter/shell/common/common.switches.o
../../../flutter/shell/common/switches.cc:454:32: error: cannot assign to static data member 'enable_impeller' with const-qualified type 'const bool'
  454 |       settings.enable_impeller =

[jonahwilliams]

Update: https://www.youtube.com/watch?v=11G9j9utRCs

[zanderso]

lgtm on green.

[zanderso]

It looks like some test timeouts need to be increased on mac_unopt.

[jonahwilliams]

I don't understand these failure cases. I was under the impression this change would be a no-op for our CI.

[zanderso]

I'd guess we need to delete code under this flag: https://github.com/flutter/engine/blob/main/testing/scenario_app/bin/run_ios_tests.dart#L259.

[zanderso]

Also: https://github.com/flutter/engine/blob/main/testing/ios/IosUnitTests/App/Info.plist#L50

[jonahwilliams]

@zanderso I removed the with-skia, but the plist setting shouldn't matter - we're not going to parse it anyway.

Its looking like some of these tests won't pass with impeller on iOS simulators and the slimpeller work for simulators is not actually complete.

[chinmaygarde]

Is this because of the UI thread platform thread merge and its interaction with Impeller? The hung test passes if I RunNowOrPostTask.

diff --git a/shell/platform/darwin/ios/framework/Source/FlutterEngineTest.mm b/shell/platform/darwin/ios/framework/Source/FlutterEngineTest.mm
index bb31e04866..7e0fef472d 100644
--- a/shell/platform/darwin/ios/framework/Source/FlutterEngineTest.mm
+++ b/shell/platform/darwin/ios/framework/Source/FlutterEngineTest.mm
@@ -270,12 +270,13 @@ FLUTTER_ASSERT_ARC
     fml::AutoResetWaitableEvent latch;
     [engine run];
     flutter::Shell& shell = engine.shell;
-    engine.shell.GetTaskRunners().GetUITaskRunner()->PostTask([&latch, &shell] {
-      flutter::Engine::Delegate& delegate = shell;
-      auto message = std::make_unique<flutter::PlatformMessage>("foo", nullptr);
-      delegate.OnEngineHandlePlatformMessage(std::move(message));
-      latch.Signal();
-    });
+    fml::TaskRunner::RunNowOrPostTask(
+        engine.shell.GetTaskRunners().GetUITaskRunner(), [&latch, &shell] {
+          flutter::Engine::Delegate& delegate = shell;
+          auto message = std::make_unique<flutter::PlatformMessage>("foo", nullptr);
+          delegate.OnEngineHandlePlatformMessage(std::move(message));
+          latch.Signal();
+        });
     latch.Wait();
     [registrar.messenger setMessageHandlerOnChannel:@"foo"
                                binaryMessageHandler:^(NSData* message, FlutterBinaryReply reply) {
diff --git a/testing/ios/IosUnitTests/App/Info.plist b/testing/ios/IosUnitTests/App/Info.plist
index 885a3b1804..e87b8d5186 100644
--- a/testing/ios/IosUnitTests/App/Info.plist
+++ b/testing/ios/IosUnitTests/App/Info.plist
@@ -46,8 +46,6 @@
 	</dict>
 	<key>FLTLeakDartVM</key>
 	<false/>
-	<key>FLTEnableImpeller</key>
-	<false/>
 	<key>FLTTraceSystrace</key>
 	<false/>
 	<key>FLTEnableDartProfiling</key>

[chinmaygarde]

Uhh, another test further along called testThreadPrioritySetCorrectly is now failing. I suspect for similar reasons.

[chinmaygarde]

Yup, its because the erstwhile UI thread no longer gets is priority set. [thread.name hasSuffix:@".ui"] and the test hangs.

We could split this patch into two with the first step being just to get rid of the Info.plist setting opting out of Impeller and fixing up the tests. Then this patch should land without issue.

[chinmaygarde]

Rebasing this on top of #56740 after it lands should unblock this.

[zanderso]

I believe the failing unit tests of flag parsing can be deleted?

[auto-submit]

auto label is removed for flutter/engine/56706, due to - The status or check suite Mac mac_unopt has failed. Please fix the issues identified (or deflake) before re-applying this label.

[auto-submit]

auto label is removed for flutter/engine/56706, due to - The status or check suite Linux mac_android_aot_engine has failed. Please fix the issues identified (or deflake) before re-applying this label.