Skip to content

Commit

Permalink
Revert "let NOTICES be double gzip wrapped to reduce on-disk installe…
Browse files Browse the repository at this point in the history
…d space (#71899)"

This reverts commit 4ed3432.
  • Loading branch information
xster committed Dec 16, 2020
1 parent e24e763 commit 14ec1d4
Show file tree
Hide file tree
Showing 17 changed files with 20 additions and 166 deletions.
21 changes: 0 additions & 21 deletions dev/devicelab/bin/tasks/module_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,8 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

import 'dart:convert';
import 'dart:io';
import 'dart:typed_data';

import 'package:archive/archive.dart';
import 'package:flutter_devicelab/framework/apk_utils.dart';
import 'package:flutter_devicelab/framework/framework.dart';
import 'package:flutter_devicelab/framework/task_result.dart';
Expand Down Expand Up @@ -317,24 +314,6 @@ Future<void> main() async {
'lib/armeabi-v7a/libflutter.so',
], await getFilesInApk(releaseHostApk));

section('Check the NOTICE file is correct');

await inDirectory(hostApp, () async {
final File apkFile = File(releaseHostApk);
final Archive apk = ZipDecoder().decodeBytes(apkFile.readAsBytesSync());
// Shouldn't be missing since we already checked it exists above.
final ArchiveFile noticesFile = apk.findFile('assets/flutter_assets/NOTICES.Z');

final Uint8List licenseData = noticesFile.content as Uint8List;
if (licenseData == null) {
return TaskResult.failure('Invalid license file.');
}
final String licenseString = utf8.decode(gzip.decode(licenseData));
if (!licenseString.contains('skia') || !licenseString.contains('Flutter Authors')) {
return TaskResult.failure('License content missing.');
}
});

section('Check release AndroidManifest.xml');

final String androidManifestRelease = await getAndroidManifest(debugHostApk);
Expand Down
26 changes: 0 additions & 26 deletions dev/devicelab/bin/tasks/module_test_ios.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

import 'dart:convert';
import 'dart:io';
import 'dart:typed_data';

import 'package:flutter_devicelab/framework/framework.dart';
import 'package:flutter_devicelab/framework/ios.dart';
Expand Down Expand Up @@ -214,8 +212,6 @@ Future<void> main() async {

final File objectiveCAnalyticsOutputFile = File(path.join(tempDir.path, 'analytics-objc.log'));
final Directory objectiveCBuildDirectory = Directory(path.join(tempDir.path, 'build-objc'));

section('Build iOS Objective-C host app');
await inDirectory(objectiveCHostApp, () async {
await exec(
'pod',
Expand Down Expand Up @@ -272,28 +268,6 @@ Future<void> main() async {
'isolate_snapshot_data',
));

section('Check the NOTICE file is correct');

final String licenseFilePath = path.join(
objectiveCBuildDirectory.path,
'Host.app',
'Frameworks',
'App.framework',
'flutter_assets',
'NOTICES.Z',
);
checkFileExists(licenseFilePath);

await inDirectory(objectiveCBuildDirectory, () async {
final Uint8List licenseData = File(licenseFilePath).readAsBytesSync();
final String licenseString = utf8.decode(gzip.decode(licenseData));
if (!licenseString.contains('skia') || !licenseString.contains('Flutter Authors')) {
return TaskResult.failure('License content missing');
}
});

section('Check that the host build sends the correct analytics');

final String objectiveCAnalyticsOutput = objectiveCAnalyticsOutputFile.readAsStringSync();
if (!objectiveCAnalyticsOutput.contains('cd24: ios')
|| !objectiveCAnalyticsOutput.contains('cd25: true')
Expand Down
2 changes: 1 addition & 1 deletion dev/devicelab/lib/framework/apk_utils.dart
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ final String platformLineSep = Platform.isWindows ? '\r\n' : '\n';

final List<String> flutterAssets = <String>[
'assets/flutter_assets/AssetManifest.json',
'assets/flutter_assets/NOTICES.Z',
'assets/flutter_assets/NOTICES',
'assets/flutter_assets/fonts/MaterialIcons-Regular.otf',
'assets/flutter_assets/packages/cupertino_icons/assets/CupertinoIcons.ttf',
];
Expand Down
2 changes: 1 addition & 1 deletion dev/devicelab/lib/tasks/perf_tests.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1070,7 +1070,7 @@ class CompileTest {

final _UnzipListEntry libflutter = fileToMetadata['lib/armeabi-v7a/libflutter.so'];
final _UnzipListEntry libapp = fileToMetadata['lib/armeabi-v7a/libapp.so'];
final _UnzipListEntry license = fileToMetadata['assets/flutter_assets/NOTICES.Z'];
final _UnzipListEntry license = fileToMetadata['assets/flutter_assets/NOTICES'];

return <String, dynamic>{
'libflutter_uncompressed_bytes': libflutter.uncompressedSize,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ class TestAssetBundle extends AssetBundle {
}

@override
Future<String> loadString(String key, { bool cache = true, bool unzip = false }) async {
Future<String> loadString(String key, { bool cache = true }) async {
if (key == 'lib/gallery/example_code.dart')
return testCodeFile;
return '';
Expand Down
36 changes: 8 additions & 28 deletions packages/flutter/lib/src/services/asset_bundle.dart
Original file line number Diff line number Diff line change
Expand Up @@ -64,16 +64,7 @@ abstract class AssetBundle {
/// caller is going to be doing its own caching. (It might not be cached if
/// it's set to true either, that depends on the asset bundle
/// implementation.)
///
/// If the `unzip` argument is set to true, it would first unzip file at the
/// specified location before retrieving the string content.
Future<String> loadString(
String key,
{
bool cache = true,
bool unzip = false,
}
) async {
Future<String> loadString(String key, { bool cache = true }) async {
final ByteData data = await load(key);
// Note: data has a non-nullable type, but might be null when running with
// weak checking, so we need to null check it anyway (and ignore the warning
Expand All @@ -82,26 +73,15 @@ abstract class AssetBundle {
throw FlutterError('Unable to load asset: $key'); // ignore: dead_code
// 50 KB of data should take 2-3 ms to parse on a Moto G4, and about 400 μs
// on a Pixel 4.
if (data.lengthInBytes < 50 * 1024 && !unzip) {
return _utf8Decode(data);
if (data.lengthInBytes < 50 * 1024) {
return utf8.decode(data.buffer.asUint8List());
}

// For strings larger than 50 KB, run the computation in an isolate to
// avoid causing main thread jank.
return compute(
unzip ? _utf8ZipDecode : _utf8Decode,
data,
debugLabel: '${unzip ? "Unzip and ": ""}UTF8 decode for "$key"',
);
}

static String _utf8ZipDecode(ByteData data) {
List<int> bytes = data.buffer.asUint8List();
bytes = gzip.decode(bytes);
return utf8.decode(bytes);
return compute(_utf8decode, data, debugLabel: 'UTF8 decode for "$key"');
}

static String _utf8Decode(ByteData data) {
static String _utf8decode(ByteData data) {
return utf8.decode(data.buffer.asUint8List());
}

Expand Down Expand Up @@ -183,10 +163,10 @@ abstract class CachingAssetBundle extends AssetBundle {
final Map<String, Future<dynamic>> _structuredDataCache = <String, Future<dynamic>>{};

@override
Future<String> loadString(String key, { bool cache = true, bool unzip = false }) {
Future<String> loadString(String key, { bool cache = true }) {
if (cache)
return _stringCache.putIfAbsent(key, () => super.loadString(key, unzip: unzip));
return super.loadString(key, unzip: unzip);
return _stringCache.putIfAbsent(key, () => super.loadString(key));
return super.loadString(key);
}

/// Retrieve a string from the asset bundle, parse it with the given function,
Expand Down
14 changes: 1 addition & 13 deletions packages/flutter/lib/src/services/binding.dart
Original file line number Diff line number Diff line change
Expand Up @@ -104,19 +104,7 @@ mixin ServicesBinding on BindingBase, SchedulerBinding {
// TODO(ianh): Remove this complexity once these bugs are fixed.
final Completer<String> rawLicenses = Completer<String>();
scheduleTask(() async {
rawLicenses.complete(
await rootBundle.loadString(
// NOTICES for web isn't compressed since we don't have access to
// dart:io on the client side and it's already compressed between
// the server and client.
//
// The compressed version doesn't have a more common .gz extension
// because gradle for Android non-transparently manipulates .gz files.
kIsWeb ? 'NOTICES' : 'NOTICES.Z',
cache: false,
unzip: !kIsWeb,
)
);
rawLicenses.complete(await rootBundle.loadString('NOTICES', cache: false));
}, Priority.animation);
await rawLicenses.future;
final Completer<List<LicenseEntry>> parsedLicenses = Completer<List<LicenseEntry>>();
Expand Down
26 changes: 1 addition & 25 deletions packages/flutter/test/services/asset_bundle_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
// found in the LICENSE file.

import 'dart:convert';
import 'dart:io';
import 'dart:typed_data';

import 'package:flutter/foundation.dart';
Expand All @@ -16,13 +15,10 @@ class TestAssetBundle extends CachingAssetBundle {

@override
Future<ByteData> load(String key) async {
loadCallCount[key] = loadCallCount[key] ?? 0 + 1;
if (key == 'AssetManifest.json')
return ByteData.view(Uint8List.fromList(const Utf8Encoder().convert('{"one": ["one"]}')).buffer);

if (key == 'NOTICES.Z')
return ByteData.view(Uint8List.fromList(gzip.encode(utf8.encode('All your base are belong to us'))).buffer);

loadCallCount[key] = loadCallCount[key] ?? 0 + 1;
if (key == 'one')
return ByteData(1)..setInt8(0, 49);
throw FlutterError('key not found');
Expand Down Expand Up @@ -52,26 +48,6 @@ void main() {
expect(loadException, isFlutterError);
});

test('Test loading zipped strings', () async {
final TestAssetBundle bundle = TestAssetBundle();

String assetString = await bundle.loadString('NOTICES.Z', unzip: true);
expect(assetString, equals('All your base are belong to us'));

expect(bundle.loadCallCount['NOTICES.Z'], 1);

assetString = await bundle.loadString('NOTICES.Z', unzip: true);
expect(assetString, equals('All your base are belong to us'));

// Should have been cached and shouldn't retrieve and decode another time.
expect(bundle.loadCallCount['NOTICES.Z'], 1);
}, onPlatform: <String, dynamic>{
'browser': const Skip(
'Skip the NOTICES unzipping test because NOTICES are'
'not zipped for the web'
),
});

test('AssetImage.obtainKey succeeds with ImageConfiguration.empty', () async {
// This is a regression test for https://github.com/flutter/flutter/issues/12392
final AssetImage assetImage = AssetImage('one', bundle: TestAssetBundle());
Expand Down
7 changes: 1 addition & 6 deletions packages/flutter/test/services/binding_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

import 'dart:convert';
import 'dart:io';
import 'dart:typed_data';

import 'package:flutter/foundation.dart';
Expand Down Expand Up @@ -46,10 +44,7 @@ class TestBinding extends BindingBase with SchedulerBinding, ServicesBinding {
BinaryMessenger createBinaryMessenger() {
return super.createBinaryMessenger()
..setMockMessageHandler('flutter/assets', (ByteData? message) async {
if (const StringCodec().decodeMessage(message) == 'NOTICES.Z' && !kIsWeb) {
return Uint8List.fromList(gzip.encode(utf8.encode(licenses))).buffer.asByteData();
}
if (const StringCodec().decodeMessage(message) == 'NOTICES' && kIsWeb) {
if (const StringCodec().decodeMessage(message) == 'NOTICES') {
return const StringCodec().encodeMessage(licenses);
}
return null;
Expand Down
2 changes: 1 addition & 1 deletion packages/flutter/test/widgets/image_resolution_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ class TestAssetBundle extends CachingAssetBundle {
}

@override
Future<String> loadString(String key, { bool cache = true, bool unzip = false }) {
Future<String> loadString(String key, { bool cache = true }) {
if (key == 'AssetManifest.json')
return SynchronousFuture<String>(manifest);
return SynchronousFuture<String>('');
Expand Down
2 changes: 0 additions & 2 deletions packages/flutter_tools/bin/fuchsia_asset_builder.dart
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import 'package:flutter_tools/src/asset.dart' hide defaultManifestPath;
import 'package:flutter_tools/src/base/context.dart';
import 'package:flutter_tools/src/base/file_system.dart' as libfs;
import 'package:flutter_tools/src/base/io.dart';
import 'package:flutter_tools/src/build_info.dart';
import 'package:flutter_tools/src/cache.dart';
import 'package:flutter_tools/src/context_runner.dart';
import 'package:flutter_tools/src/devfs.dart';
Expand Down Expand Up @@ -60,7 +59,6 @@ Future<void> run(List<String> args) async {
manifestPath: argResults[_kOptionManifest] as String ?? defaultManifestPath,
assetDirPath: assetDir,
packagesPath: argResults[_kOptionPackages] as String,
targetPlatform: TargetPlatform.fuchsia_arm64 // This is not arch specific.
);

if (assets == null) {
Expand Down
33 changes: 2 additions & 31 deletions packages/flutter_tools/lib/src/asset.dart
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import 'package:package_config/package_config.dart';

import 'base/context.dart';
import 'base/file_system.dart';
import 'base/io.dart';
import 'base/logger.dart';
import 'base/platform.dart';
import 'build_info.dart';
Expand Down Expand Up @@ -73,7 +72,6 @@ abstract class AssetBundle {
String manifestPath = defaultManifestPath,
String assetDirPath,
@required String packagesPath,
TargetPlatform targetPlatform,
});
}

Expand Down Expand Up @@ -124,13 +122,6 @@ class ManifestAssetBundle implements AssetBundle {

static const String _kAssetManifestJson = 'AssetManifest.json';
static const String _kNoticeFile = 'NOTICES';
// Comically, this can't be name with the more common .gz file extension
// because when it's part of an AAR and brought into another APK via gradle,
// gradle individually traverses all the files of the AAR and unzips .gz
// files (b/37117906). A less common .Z extension still describes how the
// file is formatted if users want to manually inspect the application
// bundle and is recognized by default file handlers on OS such as macOS.˚
static const String _kNoticeZippedFile = 'NOTICES.Z';

@override
bool wasBuiltOnce() => _lastBuildTimestamp != null;
Expand Down Expand Up @@ -169,7 +160,6 @@ class ManifestAssetBundle implements AssetBundle {
String manifestPath = defaultManifestPath,
String assetDirPath,
@required String packagesPath,
TargetPlatform targetPlatform,
}) async {
assetDirPath ??= getAssetBuildDirectory();
FlutterProject flutterProject;
Expand Down Expand Up @@ -330,6 +320,7 @@ class ManifestAssetBundle implements AssetBundle {
final DevFSStringContent assetManifest = _createAssetManifest(assetVariants);
final DevFSStringContent fontManifest = DevFSStringContent(json.encode(fonts));
final LicenseResult licenseResult = _licenseCollector.obtainLicenses(packageConfig);
final DevFSStringContent licenses = DevFSStringContent(licenseResult.combinedLicenses);
additionalDependencies = licenseResult.dependencies;

if (wildcardDirectories.isNotEmpty) {
Expand All @@ -347,27 +338,7 @@ class ManifestAssetBundle implements AssetBundle {

_setIfChanged(_kAssetManifestJson, assetManifest);
_setIfChanged(kFontManifestJson, fontManifest);
if (targetPlatform == TargetPlatform.web_javascript) {
// Don't compress the NOTICES file on web since the client doesn't have
// dart:io to decompress it.
_setIfChanged(_kNoticeFile, DevFSStringContent(licenseResult.combinedLicenses));
} else {
final List<int> licenseBytes = utf8.encode(licenseResult.combinedLicenses);
if (entries[_kNoticeZippedFile] == null ||
gzip.decode((entries[_kNoticeZippedFile] as DevFSByteContent).bytes)
!= licenseBytes) {
entries[_kNoticeZippedFile] = DevFSByteContent(
ZLibEncoder(
// A zlib dictionary is a hinting string sequence with the most
// likely string occurrences at the end. This ends up just being
// common English words with domain specific words like copyright.
dictionary: utf8.encode('copyrightsoftwaretothisinandorofthe'),
gzip: true,
level: 9,
).convert(licenseBytes)
);
}
}
_setIfChanged(_kNoticeFile, licenses);
return 0;
}

Expand Down
3 changes: 1 addition & 2 deletions packages/flutter_tools/lib/src/base/io.dart
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,7 @@ export 'dart:io'
systemEncoding,
WebSocket,
WebSocketException,
WebSocketTransformer,
ZLibEncoder;
WebSocketTransformer;

/// Exits the process with the given [exitCode].
typedef ExitFunction = void Function(int exitCode);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ Future<Depfile> copyAssets(Environment environment, Directory outputDirectory, {
manifestPath: pubspecFile.path,
packagesPath: environment.projectDir.childFile('.packages').path,
assetDirPath: null,
targetPlatform: targetPlatform,
);
if (resultCode != 0) {
throw Exception('Failed to bundle asset files.');
Expand Down

0 comments on commit 14ec1d4

Please sign in to comment.