[macOS] Remigrate principal class to NSApplication (#124173)

cbracken · web-flow · commit e5fc6f65b90a · 2023-04-05T09:05:37.000-07:00
In #122336 we migrated the principal class from NSApplication to FlutterApplication in the app Info.plist. We removed the need for FlutterApplication in flutter/engine#40929. This reverses the migration for anyone who previously upgraded on the Flutter master branch. Issue: flutter/flutter#30735 ## Pre-launch Checklist - [X] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [X] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [X] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [X] I signed the [CLA]. - [X] I listed at least one issue that this PR fixes in the description above. - [X] I updated/added relevant documentation (doc comments with `///`). - [X] I added new tests to check the change I am making, or this PR is [test-exempt]. - [X] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord].  [Contributor Guide]: https://github.com/flutter/flutter/wiki/Tree-hygiene#overview [Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene [test-exempt]: https://github.com/flutter/flutter/wiki/Tree-hygiene#tests [Flutter Style Guide]: https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo [Features we expect every widget to implement]: https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/wiki/Chat
diff --git a/dev/benchmarks/complex_layout/macos/Runner/Info.plist b/dev/benchmarks/complex_layout/macos/Runner/Info.plist
@@ -27,6 +27,6 @@
 	<key>NSMainNibFile</key>
 	<string>MainMenu</string>
 	<key>NSPrincipalClass</key>
-	<string>FlutterApplication</string>
+	<string>NSApplication</string>
 </dict>
 </plist>
diff --git a/dev/benchmarks/macrobenchmarks/macos/Runner/Info.plist b/dev/benchmarks/macrobenchmarks/macos/Runner/Info.plist
@@ -27,6 +27,6 @@
 	<key>NSMainNibFile</key>
 	<string>MainMenu</string>
 	<key>NSPrincipalClass</key>
-	<string>FlutterApplication</string>
+	<string>NSApplication</string>
 </dict>
 </plist>
diff --git a/dev/integration_tests/channels/macos/Runner/Info.plist b/dev/integration_tests/channels/macos/Runner/Info.plist
@@ -27,6 +27,6 @@
 	<key>NSMainNibFile</key>
 	<string>MainMenu</string>
 	<key>NSPrincipalClass</key>
-	<string>FlutterApplication</string>
+	<string>NSApplication</string>
 </dict>
 </plist>
diff --git a/dev/integration_tests/flavors/macos/Runner/Info.plist b/dev/integration_tests/flavors/macos/Runner/Info.plist
@@ -29,6 +29,6 @@
 	<key>NSMainNibFile</key>
 	<string>MainMenu</string>
 	<key>NSPrincipalClass</key>
-	<string>FlutterApplication</string>
+	<string>NSApplication</string>
 </dict>
 </plist>
diff --git a/dev/integration_tests/flutter_gallery/macos/Runner/Info.plist b/dev/integration_tests/flutter_gallery/macos/Runner/Info.plist
@@ -27,6 +27,6 @@
 	<key>NSMainNibFile</key>
 	<string>MainMenu</string>
 	<key>NSPrincipalClass</key>
-	<string>FlutterApplication</string>
+	<string>NSApplication</string>
 </dict>
 </plist>
diff --git a/dev/integration_tests/ui/macos/Runner/Info.plist b/dev/integration_tests/ui/macos/Runner/Info.plist
@@ -27,6 +27,6 @@
 	<key>NSMainNibFile</key>
 	<string>MainMenu</string>
 	<key>NSPrincipalClass</key>
-	<string>FlutterApplication</string>
+	<string>NSApplication</string>
 </dict>
 </plist>
diff --git a/dev/manual_tests/macos/Runner/Info.plist b/dev/manual_tests/macos/Runner/Info.plist
@@ -27,6 +27,6 @@
 	<key>NSMainNibFile</key>
 	<string>MainMenu</string>
 	<key>NSPrincipalClass</key>
-	<string>FlutterApplication</string>
+	<string>NSApplication</string>
 </dict>
 </plist>
diff --git a/examples/api/macos/Runner/Info.plist b/examples/api/macos/Runner/Info.plist
@@ -27,6 +27,6 @@
 	<key>NSMainNibFile</key>
 	<string>MainMenu</string>
 	<key>NSPrincipalClass</key>
-	<string>FlutterApplication</string>
+	<string>NSApplication</string>
 </dict>
 </plist>
diff --git a/examples/flutter_view/macos/Runner/Info.plist b/examples/flutter_view/macos/Runner/Info.plist
@@ -27,6 +27,6 @@
 	<key>NSMainNibFile</key>
 	<string>MainMenu</string>
 	<key>NSPrincipalClass</key>
-	<string>FlutterApplication</string>
+	<string>NSApplication</string>
 </dict>
 </plist>
diff --git a/examples/hello_world/macos/Runner/Info.plist b/examples/hello_world/macos/Runner/Info.plist
@@ -27,6 +27,6 @@
 	<key>NSMainNibFile</key>
 	<string>MainMenu</string>
 	<key>NSPrincipalClass</key>
-	<string>FlutterApplication</string>
+	<string>NSApplication</string>
 </dict>
 </plist>
diff --git a/examples/image_list/macos/Runner/Info.plist b/examples/image_list/macos/Runner/Info.plist
@@ -27,6 +27,6 @@
 	<key>NSMainNibFile</key>
 	<string>MainMenu</string>
 	<key>NSPrincipalClass</key>
-	<string>FlutterApplication</string>
+	<string>NSApplication</string>
 </dict>
 </plist>
diff --git a/examples/layers/macos/Runner/Info.plist b/examples/layers/macos/Runner/Info.plist
@@ -27,6 +27,6 @@
 	<key>NSMainNibFile</key>
 	<string>MainMenu</string>
 	<key>NSPrincipalClass</key>
-	<string>FlutterApplication</string>
+	<string>NSApplication</string>
 </dict>
 </plist>
diff --git a/examples/platform_channel/macos/Runner/Info.plist b/examples/platform_channel/macos/Runner/Info.plist
@@ -27,6 +27,6 @@
 	<key>NSMainNibFile</key>
 	<string>MainMenu</string>
 	<key>NSPrincipalClass</key>
-	<string>FlutterApplication</string>
+	<string>NSApplication</string>
 </dict>
 </plist>
diff --git a/examples/platform_view/macos/Runner/Info.plist b/examples/platform_view/macos/Runner/Info.plist
@@ -27,6 +27,6 @@
 	<key>NSMainNibFile</key>
 	<string>MainMenu</string>
 	<key>NSPrincipalClass</key>
-	<string>FlutterApplication</string>
+	<string>NSApplication</string>
 </dict>
 </plist>
diff --git a/packages/flutter_tools/lib/src/macos/migrations/flutter_application_migration.dart b/packages/flutter_tools/lib/src/macos/migrations/flutter_application_migration.dart
@@ -8,7 +8,13 @@ import '../../globals.dart' as globals;
 import '../../ios/plist_parser.dart';
 import '../../xcode_project.dart';
 
-/// Update the minimum macOS deployment version to the minimum allowed by Xcode without causing a warning.
+/// Migrate principle class from FlutterApplication to NSApplication.
+///
+/// For several weeks, we required macOS apps to use FlutterApplication as the
+/// app's NSPrincipalClass rather than NSApplication. During that time an
+/// automated migration migrated the NSPrincipalClass in the Info.plist from
+/// NSApplication to FlutterApplication. Now that this is no longer necessary,
+/// we apply the reverse migration for anyone who was previously migrated.
 class FlutterApplicationMigration extends ProjectMigrator {
   FlutterApplicationMigration(
     MacOSProject project,
@@ -20,29 +26,23 @@ class FlutterApplicationMigration extends ProjectMigrator {
   @override
   void migrate() {
     if (_infoPlistFile.existsSync()) {
-      final String? principleClass =
+      final String? principalClass =
           globals.plistParser.getStringValueFromFile(_infoPlistFile.path, PlistParser.kNSPrincipalClassKey);
-      if (principleClass == null || principleClass == 'FlutterApplication') {
-        // No NSPrincipalClass defined, or already converted, so no migration
+      if (principalClass == null || principalClass == 'NSApplication') {
+        // No NSPrincipalClass defined, or already converted. No migration
         // needed.
         return;
       }
-      if (principleClass != 'NSApplication') {
-        // Only replace NSApplication values, since we don't know why they might
-        // have substituted something else.
-        logger.printTrace('${_infoPlistFile.basename} has an '
-          '${PlistParser.kNSPrincipalClassKey} of $principleClass, not '
-          'NSApplication, skipping FlutterApplication migration.\nYou will need '
-          'to modify your application class to derive from FlutterApplication.');
+      if (principalClass != 'FlutterApplication') {
+        // If the principal class wasn't already migrated to
+        // FlutterApplication, there's no need to revert the migration.
         return;
       }
-      logger.printStatus('Updating ${_infoPlistFile.basename} to use FlutterApplication instead of NSApplication.');
-      final bool success = globals.plistParser.replaceKey(_infoPlistFile.path, key: PlistParser.kNSPrincipalClassKey, value: 'FlutterApplication');
+      logger.printStatus('Updating ${_infoPlistFile.basename} to use NSApplication instead of FlutterApplication.');
+      final bool success = globals.plistParser.replaceKey(_infoPlistFile.path, key: PlistParser.kNSPrincipalClassKey, value: 'NSApplication');
       if (!success) {
         logger.printError('Updating ${_infoPlistFile.basename} failed.');
       }
-    } else {
-      logger.printTrace('${_infoPlistFile.basename} not found, skipping FlutterApplication migration.');
     }
   }
 }
diff --git a/packages/flutter_tools/templates/app_shared/macos.tmpl/Runner/Info.plist b/packages/flutter_tools/templates/app_shared/macos.tmpl/Runner/Info.plist
@@ -27,6 +27,6 @@
 	<key>NSMainNibFile</key>
 	<string>MainMenu</string>
 	<key>NSPrincipalClass</key>
-	<string>FlutterApplication</string>
+	<string>NSApplication</string>
 </dict>
 </plist>
diff --git a/packages/flutter_tools/test/general.shard/macos/macos_project_migration_test.dart b/packages/flutter_tools/test/general.shard/macos/macos_project_migration_test.dart
@@ -280,7 +280,7 @@ platform :osx, '10.14'
     });
   });
 
-  group('update NSPrincipalClass to FlutterApplication', () {
+  group('update NSPrincipalClass from FlutterApplication to NSApplication', () {
     late MemoryFileSystem memoryFileSystem;
     late BufferLogger testLogger;
     late FakeMacOSProject project;
@@ -318,7 +318,7 @@ platform :osx, '10.14'
       macOSProjectMigration.migrate();
       expect(infoPlistFile.existsSync(), isFalse);
 
-      expect(testLogger.traceText, contains('${infoPlistFile.basename} not found, skipping FlutterApplication migration.'));
+      expect(testLogger.traceText, isEmpty);
       expect(testLogger.statusText, isEmpty);
     });
 
@@ -333,29 +333,29 @@ platform :osx, '10.14'
       expect(testLogger.statusText, isEmpty);
     });
 
-    testWithMocks('skipped if already upgraded', () async {
-      fakePlistParser.setProperty(PlistParser.kNSPrincipalClassKey, 'FlutterApplication');
+    testWithMocks('skipped if already de-upgraded (or never migrated)', () async {
+      fakePlistParser.setProperty(PlistParser.kNSPrincipalClassKey, 'NSApplication');
       final FlutterApplicationMigration macOSProjectMigration = FlutterApplicationMigration(
         project,
         testLogger,
       );
       infoPlistFile.writeAsStringSync('contents'); // Just so it exists: parser is a fake.
       macOSProjectMigration.migrate();
-      expect(fakePlistParser.getStringValueFromFile(infoPlistFile.path, PlistParser.kNSPrincipalClassKey), 'FlutterApplication');
+      expect(fakePlistParser.getStringValueFromFile(infoPlistFile.path, PlistParser.kNSPrincipalClassKey), 'NSApplication');
       expect(testLogger.statusText, isEmpty);
     });
 
-    testWithMocks('Info.plist migrated to use FlutterApplication', () async {
-      fakePlistParser.setProperty(PlistParser.kNSPrincipalClassKey, 'NSApplication');
+    testWithMocks('Info.plist migrated to use NSApplication', () async {
+      fakePlistParser.setProperty(PlistParser.kNSPrincipalClassKey, 'FlutterApplication');
       final FlutterApplicationMigration macOSProjectMigration = FlutterApplicationMigration(
         project,
         testLogger,
       );
       infoPlistFile.writeAsStringSync('contents'); // Just so it exists: parser is a fake.
       macOSProjectMigration.migrate();
-      expect(fakePlistParser.getStringValueFromFile(infoPlistFile.path, PlistParser.kNSPrincipalClassKey), 'FlutterApplication');
+      expect(fakePlistParser.getStringValueFromFile(infoPlistFile.path, PlistParser.kNSPrincipalClassKey), 'NSApplication');
       // Only print once.
-      expect('Updating ${infoPlistFile.basename} to use FlutterApplication instead of NSApplication.'.allMatches(testLogger.statusText).length, 1);
+      expect('Updating ${infoPlistFile.basename} to use NSApplication instead of FlutterApplication.'.allMatches(testLogger.statusText).length, 1);
     });
 
     testWithMocks('Skip if NSPrincipalClass is not NSApplication', () async {
@@ -368,7 +368,7 @@ platform :osx, '10.14'
       infoPlistFile.writeAsStringSync('contents'); // Just so it exists: parser is a fake.
       macOSProjectMigration.migrate();
       expect(fakePlistParser.getStringValueFromFile(infoPlistFile.path, PlistParser.kNSPrincipalClassKey), differentApp);
-      expect(testLogger.traceText, contains('${infoPlistFile.basename} has an ${PlistParser.kNSPrincipalClassKey} of $differentApp, not NSApplication, skipping FlutterApplication migration'));
+      expect(testLogger.traceText, isEmpty);
     });
   });
 }
