[Impeller] Enable logging a warning when the user opts out of using Impeller. (#51849)

chinmaygarde · web-flow · commit 22be0b9fa424 · 2024-04-03T19:52:51.000Z
Part of flutter/flutter#144439 This does two things: * Logs a warning when the embedder requests a non-Impeller preference when creating a shell. * Makes the iOS embedder request a warning be logged when Impeller is not used. I decided to put the warning logs in the shell so that as we get more opinionated about Impeller on other platforms, those platforms can just flip a flag with common log origin.
diff --git a/engine/src/flutter/common/settings.h b/engine/src/flutter/common/settings.h
@@ -229,6 +229,9 @@ struct Settings {
   bool enable_impeller = false;
 #endif
 
+  // Log a warning during shell initialization if Impeller is not enabled.
+  bool warn_on_impeller_opt_out = false;
+
   // The selected Android rendering API.
   AndroidRenderingAPI android_rendering_api =
       AndroidRenderingAPI::kSkiaOpenGLES;
diff --git a/engine/src/flutter/shell/common/shell.cc b/engine/src/flutter/shell/common/shell.cc
@@ -450,6 +450,14 @@ Shell::Shell(DartVMRef vm,
       weak_factory_(this) {
   FML_CHECK(!settings.enable_software_rendering || !settings.enable_impeller)
       << "Software rendering is incompatible with Impeller.";
+  if (!settings.enable_impeller && settings.warn_on_impeller_opt_out) {
+    FML_LOG(IMPORTANT)
+        << "[Action Required] The application opted out of Impeller by either "
+           "using the --no-enable-impeller flag or FLTEnableImpeller=false "
+           "plist flag. This option is going to go away in an upcoming Flutter "
+           "release. Remove the explicit opt-out. If you need to opt-out, "
+           "report a bug describing the issue.";
+  }
   FML_CHECK(vm_) << "Must have access to VM to create a shell.";
   FML_DCHECK(task_runners_.IsValid());
   FML_DCHECK(task_runners_.GetPlatformTaskRunner()->RunsTasksOnCurrentThread());
diff --git a/engine/src/flutter/shell/common/shell_unittests.cc b/engine/src/flutter/shell/common/shell_unittests.cc
@@ -2,14 +2,14 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include <strstream>
 #define FML_USED_ON_EMBEDDER
 
 #include <algorithm>
 #include <chrono>
 #include <ctime>
 #include <future>
 #include <memory>
+#include <strstream>
 #include <thread>
 #include <utility>
 #include <vector>
@@ -4916,6 +4916,28 @@ TEST_F(ShellTest, RuntimeStageBackendWithImpeller) {
 }
 #endif  // IMPELLER_SUPPORTS_RENDERING
 
+TEST_F(ShellTest, WillLogWarningWhenImpellerIsOptedOut) {
+#if !IMPELLER_SUPPORTS_RENDERING
+  GTEST_SKIP() << "This platform doesn't support Impeller.";
+#endif
+  ASSERT_FALSE(DartVMRef::IsInstanceRunning());
+  Settings settings = CreateSettingsForFixture();
+  settings.enable_impeller = false;
+  settings.warn_on_impeller_opt_out = true;
+  // Log captures are thread specific. Just put the shell in single threaded
+  // configuration.
+  const auto& runner = fml::MessageLoop::GetCurrent().GetTaskRunner();
+  TaskRunners task_runners("test", runner, runner, runner, runner);
+  std::ostringstream stream;
+  fml::LogMessage::CaptureNextLog(&stream);
+  std::unique_ptr<Shell> shell = CreateShell(settings, task_runners);
+  ASSERT_TRUE(stream.str().find(
+                  "[Action Required] The application opted out of Impeller") !=
+              std::string::npos);
+  ASSERT_TRUE(shell);
+  DestroyShell(std::move(shell), task_runners);
+}
+
 }  // namespace testing
 }  // namespace flutter
 
diff --git a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterDartProject.mm b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterDartProject.mm
@@ -202,6 +202,8 @@ static BOOL DoesHardwareSupportWideGamut() {
     }
   }
 
+  settings.warn_on_impeller_opt_out = true;
+
   NSNumber* enableTraceSystrace = [mainBundle objectForInfoDictionaryKey:@"FLTTraceSystrace"];
   // Change the default only if the option is present.
   if (enableTraceSystrace != nil) {
diff --git a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterDartProjectTest.mm b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterDartProjectTest.mm
@@ -235,6 +235,11 @@ - (void)testDisableImpellerSettingIsCorrectlyParsed {
   [mockMainBundle stopMocking];
 }
 
+- (void)testRequestsWarningWhenImpellerOptOut {
+  auto settings = FLTDefaultSettingsForBundle();
+  XCTAssertEqual(settings.warn_on_impeller_opt_out, YES);
+}
+
 - (void)testEnableImpellerSettingIsCorrectlyParsed {
   id mockMainBundle = OCMPartialMock([NSBundle mainBundle]);
   OCMStub([mockMainBundle objectForInfoDictionaryKey:@"FLTEnableImpeller"]).andReturn(@"YES");

Original file line number	Diff line number	Diff line change
`@@ -202,6 +202,8 @@ static BOOL DoesHardwareSupportWideGamut() {`
`202`	`202`	`}`
`203`	`203`	`}`
`204`	`204`
	`205`	`+ settings.warn_on_impeller_opt_out = true;`
	`206`	`+`
`205`	`207`	`NSNumber* enableTraceSystrace = [mainBundle objectForInfoDictionaryKey:@"FLTTraceSystrace"];`
`206`	`208`	`// Change the default only if the option is present.`
`207`	`209`	`if (enableTraceSystrace != nil) {`