commit 4845df906ab740243664290ef918a248e10f84da
author Jonah Williams <jonahwilliams@google.com> Mon Jun 01 16:30:43 2020 -0700
committer GitHub <noreply@github.com> Mon Jun 01 16:30:43 2020 -0700
tree fe7877d0f608b1a1c76b0bff0a2830297093718a
parent eb236bd6149ace8a3703cb5f46b7233f706610b8

[flutter_tools] do not include material icon incorrectly (#58335)

If a dependency specified uses-material-design: true and the main pubspec specifies uses-material-design: false, then the MaterialIcons font would be included in the font manifest, but not in the AssetManifest or final bundle. Remove it from the FontManifest if this occurs

packages/flutter_tools/lib/src/asset.dart

diff --git a/packages/flutter_tools/lib/src/asset.dart b/packages/flutter_tools/lib/src/asset.dart
index ce49e34..c680de5 100644
--- a/packages/flutter_tools/lib/src/asset.dart
+++ b/packages/flutter_tools/lib/src/asset.dart
@@ -176,10 +176,12 @@
       return 1;
     }
 
+    final bool includesMaterialFonts = flutterManifest.usesMaterialDesign;
     final List<Map<String, dynamic>> fonts = _parseFonts(
       flutterManifest,
       includeDefaultFonts,
       packageConfig,
+      primary: true,
     );
 
     // Add fonts and assets from packages.
@@ -215,12 +217,20 @@
           return 1;
         }
         assetVariants.addAll(packageAssets);
-
+        if (!includesMaterialFonts && packageFlutterManifest.usesMaterialDesign) {
+          globals.printError(
+            'package:${package.name} has `uses-material-design: true` set but '
+            'the primary pubspec contains `uses-material-design: false`. '
+            'If the application needs material icons, then `uses-material-design` '
+            ' must be set to true.'
+          );
+        }
         fonts.addAll(_parseFonts(
           packageFlutterManifest,
           includeDefaultFonts,
           packageConfig,
           packageName: package.name,
+          primary: false,
         ));
       }
     }
@@ -251,7 +261,6 @@
         entries[variant.entryUri.path] ??= DevFSFileContent(variant.assetFile);
       }
     }
-
     final List<_Asset> materialAssets = <_Asset>[
       if (flutterManifest.usesMaterialDesign && includeDefaultFonts)
         ..._getMaterialAssets(_kFontSetMaterial),
@@ -523,9 +532,10 @@
   bool includeDefaultFonts,
   PackageConfig packageConfig, {
   String packageName,
+  @required bool primary,
 }) {
   return <Map<String, dynamic>>[
-    if (manifest.usesMaterialDesign && includeDefaultFonts)
+    if (primary && manifest.usesMaterialDesign && includeDefaultFonts)
       ..._getMaterialFonts(ManifestAssetBundle._kFontSetMaterial),
     if (packageName == null)
       ...manifest.fontsDescriptor

packages/flutter_tools/test/general.shard/asset_bundle_test.dart

diff --git a/packages/flutter_tools/test/general.shard/asset_bundle_test.dart b/packages/flutter_tools/test/general.shard/asset_bundle_test.dart
index 20f7bc0..25b0600 100644
--- a/packages/flutter_tools/test/general.shard/asset_bundle_test.dart
+++ b/packages/flutter_tools/test/general.shard/asset_bundle_test.dart
@@ -399,6 +399,44 @@
     ProcessManager: () => FakeProcessManager.any(),
     Platform: () => FakePlatform(operatingSystem: 'linux'),
   });
+
+  testUsingContext('does not include material design assets if uses-material-design: true is '
+    'specified only by a dependency', () async {
+    globals.fs.file('.packages').writeAsStringSync(r'''
+example:lib/
+foo:foo/lib/
+''');
+    globals.fs.file('pubspec.yaml')
+      ..createSync()
+      ..writeAsStringSync(r'''
+name: example
+dependencies:
+  foo: any
+
+flutter:
+  uses-material-design: false
+''');
+    globals.fs.file('foo/pubspec.yaml')
+      ..createSync(recursive: true)
+      ..writeAsStringSync(r'''
+name: foo
+
+flutter:
+  uses-material-design: true
+''');
+    final AssetBundle bundle = AssetBundleFactory.instance.createBundle();
+
+    expect(await bundle.build(manifestPath: 'pubspec.yaml'), 0);
+    expect((bundle.entries['FontManifest.json'] as DevFSStringContent).string, '[]');
+    expect((bundle.entries['AssetManifest.json'] as DevFSStringContent).string, '{}');
+    expect(testLogger.errorText, contains(
+      'package:foo has `uses-material-design: true` set'
+    ));
+  }, overrides: <Type, Generator>{
+    FileSystem: () => MemoryFileSystem.test(),
+    ProcessManager: () => FakeProcessManager.any(),
+    Platform: () => FakePlatform(operatingSystem: 'linux'),
+  });
 }
 
 class MockDirectory extends Mock implements Directory {}