Skip to content

F
Front-End
Unverified Commit 6180a4c1 authored by Jonah Williams's avatar Jonah Williams Committed by GitHub
Browse files
Options

[flutter_tools] fix documentation, globals, and todos in the android codebase (#66980) 

Cleans up some undocumented classes and re-organizes the AndroidDevices class to avoid the need for the static testing only member. Adds a script for tracking globals.
parent 5339cba6
Expand all Hide whitespace changes
Inline Side-by-side
Showing with 259 additions and 228 deletions
packages/flutter_tools/lib/src/android/adb.dart
View file @ 6180a4c1
......@@ -2,8 +2,9 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
final RegExp _whitespaceRegex = RegExp(r'\s+');
final RegExp _whitespace = RegExp(r'\s+');
/// Convert adb device names into more human readable descriptions.
String cleanAdbDeviceName(String name) {
// Some emulators use `___` in the name as separators.
name = name.replaceAll('___', ', ');
......@@ -11,7 +12,7 @@ String cleanAdbDeviceName(String name) {
// Convert `Nexus_7` / `Nexus_5X` style names to `Nexus 7` ones.
name = name.replaceAll('_', ' ');
name = name.replaceAll(_whitespaceRegex, ' ').trim();
name = name.replaceAll(_whitespace, ' ').trim();
return name;
}
packages/flutter_tools/lib/src/android/android_device.dart
View file @ 6180a4c1
......@@ -26,33 +26,32 @@ import 'android.dart';
import 'android_console.dart';
import 'android_sdk.dart';
// TODO(jonahwilliams): update google3 client after roll to remove export.
export 'android_device_discovery.dart';
enum _HardwareType { emulator, physical }
/// Whether the [AndroidDevice] is believed to be a physical device or an emulator.
enum HardwareType { emulator, physical }
/// Map to help our `isLocalEmulator` detection.
const Map<String, _HardwareType> _kKnownHardware = <String, _HardwareType>{
'goldfish': _HardwareType.emulator,
'qcom': _HardwareType.physical,
'ranchu': _HardwareType.emulator,
'samsungexynos7420': _HardwareType.physical,
'samsungexynos7580': _HardwareType.physical,
'samsungexynos7870': _HardwareType.physical,
'samsungexynos7880': _HardwareType.physical,
'samsungexynos8890': _HardwareType.physical,
'samsungexynos8895': _HardwareType.physical,
'samsungexynos9810': _HardwareType.physical,
'samsungexynos7570': _HardwareType.physical,
///
/// See [AndroidDevice] for more explanation of why this is needed.
const Map<String, HardwareType> kKnownHardware = <String, HardwareType>{
'goldfish': HardwareType.emulator,
'qcom': HardwareType.physical,
'ranchu': HardwareType.emulator,
'samsungexynos7420': HardwareType.physical,
'samsungexynos7580': HardwareType.physical,
'samsungexynos7870': HardwareType.physical,
'samsungexynos7880': HardwareType.physical,
'samsungexynos8890': HardwareType.physical,
'samsungexynos8895': HardwareType.physical,
'samsungexynos9810': HardwareType.physical,
'samsungexynos7570': HardwareType.physical,
};
bool allowHeapCorruptionOnWindows(int exitCode, Platform platform) {
// In platform tools 29.0.0 adb.exe seems to be ending with this heap
// corruption error code on seemingly successful termination.
// So we ignore this error on Windows.
return exitCode == -1073740940 && platform.isWindows;
}
/// A physical Android device or emulator.
///
/// While [isEmulator] attempts to distinguish between the device categories,
/// this is a best effort process and not a guarantee; certain phyiscal devices
/// identify as emulators. These device identifiers may be added to the [kKnownHardware]
/// map to specify that they are actually physical devices.
class AndroidDevice extends Device {
AndroidDevice(
String id, {
......@@ -113,7 +112,7 @@ class AndroidDevice extends Device {
stdoutEncoding: latin1,
stderrEncoding: latin1,
);
if (result.exitCode == 0 || allowHeapCorruptionOnWindows(result.exitCode, _platform)) {
if (result.exitCode == 0 || _allowHeapCorruptionOnWindows(result.exitCode, _platform)) {
_properties = parseAdbDeviceProperties(result.stdout as String);
} else {
_logger.printError('Error ${result.exitCode} retrieving device properties for $name:');
......@@ -132,9 +131,9 @@ class AndroidDevice extends Device {
if (_isLocalEmulator == null) {
final String hardware = await _getProperty('ro.hardware');
_logger.printTrace('ro.hardware = $hardware');
if (_kKnownHardware.containsKey(hardware)) {
if (kKnownHardware.containsKey(hardware)) {
// Look for known hardware models.
_isLocalEmulator = _kKnownHardware[hardware] == _HardwareType.emulator;
_isLocalEmulator = kKnownHardware[hardware] == HardwareType.emulator;
} else {
// Fall back to a best-effort heuristic-based approach.
final String characteristics = await _getProperty('ro.build.characteristics');
......@@ -242,8 +241,7 @@ class AndroidDevice extends Device {
}
@override
Future<String> get sdkNameAndVersion async =>
'Android ${await _sdkVersion} (API ${await apiVersion})';
Future<String> get sdkNameAndVersion async => 'Android ${await _sdkVersion} (API ${await apiVersion})';
Future<String> get _sdkVersion => _getProperty('ro.build.version.release');
......@@ -258,22 +256,6 @@ class AndroidDevice extends Device {
return <String>[_androidSdk.adbPath, '-s', id, ...args];
}
String runAdbCheckedSync(
List<String> params, {
String workingDirectory,
bool allowReentrantFlutter = false,
Map<String, String> environment,
}) {
return _processUtils.runSync(
adbCommandForDevice(params),
throwOnError: true,
workingDirectory: workingDirectory,
allowReentrantFlutter: allowReentrantFlutter,
environment: environment,
allowedFailures: (int value) => allowHeapCorruptionOnWindows(value, _platform),
).stdout.trim();
}
Future<RunResult> runAdbCheckedAsync(
List<String> params, {
String workingDirectory,
......@@ -284,7 +266,7 @@ class AndroidDevice extends Device {
throwOnError: true,
workingDirectory: workingDirectory,
allowReentrantFlutter: allowReentrantFlutter,
allowedFailures: (int value) => allowHeapCorruptionOnWindows(value, _platform),
allowedFailures: (int value) => _allowHeapCorruptionOnWindows(value, _platform),
);
}
......@@ -738,7 +720,7 @@ class AndroidDevice extends Device {
app.id,
]);
return _processUtils.stream(command).then<bool>(
(int exitCode) => exitCode == 0 || allowHeapCorruptionOnWindows(exitCode, _platform));
(int exitCode) => exitCode == 0 || _allowHeapCorruptionOnWindows(exitCode, _platform));
}
@override
......@@ -794,17 +776,18 @@ class AndroidDevice extends Device {
/// Return the most recent timestamp in the Android log or [null] if there is
/// no available timestamp. The format can be passed to logcat's -T option.
String get lastLogcatTimestamp {
String output;
@visibleForTesting
Future<String> lastLogcatTimestamp() async {
RunResult output;
try {
output = runAdbCheckedSync(<String>[
output = await runAdbCheckedAsync(<String>[
'shell', '-x', 'logcat', '-v', 'time', '-t', '1'
]);
} on Exception catch (error) {
_logger.printError('Failed to extract the most recent timestamp from the Android log: $error.');
return null;
}
final Match timeMatch = _timeRegExp.firstMatch(output);
final Match timeMatch = _timeRegExp.firstMatch(output.stdout);
return timeMatch?.group(0);
}
......@@ -1012,11 +995,9 @@ class AdbLogReader extends DeviceLogReader {
/// Create a new [AdbLogReader] from an [AndroidDevice] instance.
static Future<AdbLogReader> createLogReader(
AndroidDevice device,
ProcessManager processManager,
{
bool includePastLogs = false,
}
) async {
ProcessManager processManager, {
bool includePastLogs = false,
}) async {
// logcat -T is not supported on Android releases before Lollipop.
const int kLollipopVersionCode = 21;
final int apiVersion = (String v) {
......@@ -1034,17 +1015,20 @@ class AdbLogReader extends DeviceLogReader {
'logcat',
'-v',
'time',
// If we include logs from the past, filter for 'flutter' logs only.
if (includePastLogs) ...<String>[
'-s',
'flutter',
] else if (apiVersion != null && apiVersion >= kLollipopVersionCode) ...<String>[
// Otherwise, filter for logs appearing past the present.
// '-T 0` means the timestamp of the logcat command invocation.
'-T',
if (device.lastLogcatTimestamp != null) '\'${device.lastLogcatTimestamp}\'' else '0',
],
];
// If past logs are included then filter for 'flutter' logs only.
if (includePastLogs) {
args.addAll(<String>['-s', 'flutter']);
} else if (apiVersion != null && apiVersion >= kLollipopVersionCode) {
// Otherwise, filter for logs appearing past the present.
// '-T 0` means the timestamp of the logcat command invocation.
final String lastLogcatTimestamp = await device.lastLogcatTimestamp();
args.addAll(<String>[
'-T',
if (lastLogcatTimestamp != null) '\'$lastLogcatTimestamp\'' else '0',
]);
}
final Process process = await processManager.start(device.adbCommandForDevice(args));
return AdbLogReader._(process, device.name);
}
......@@ -1340,3 +1324,10 @@ class AndroidDevicePortForwarder extends DevicePortForwarder {
}
}
}
// In platform tools 29.0.0 adb.exe seems to be ending with this heap
// corruption error code on seemingly successful termination. Ignore
// this error on windows.
bool _allowHeapCorruptionOnWindows(int exitCode, Platform platform) {
return exitCode == -1073740940 && platform.isWindows;
}
packages/flutter_tools/lib/src/android/android_device_discovery.dart
View file @ 6180a4c1
......@@ -11,22 +11,29 @@ import '../base/io.dart';
import '../base/logger.dart';
import '../base/platform.dart';
import '../base/process.dart';
import '../base/user_messages.dart';
import '../device.dart';
import '../globals.dart' as globals;
import 'adb.dart';
import 'android_device.dart';
import 'android_sdk.dart';
import 'android_workflow.dart' hide androidWorkflow;
/// Device discovery for Android physical devices and emulators.
///
/// This class primarily delegates to the `adb` command line tool provided by
/// the Android SDK to discover instances of connected android devices.
///
/// See also:
/// * [AndroidDevice], the type of discovered device.
class AndroidDevices extends PollingDeviceDiscovery {
AndroidDevices({
@required AndroidWorkflow androidWorkflow,
@required ProcessManager processManager,
@required Logger logger,
@required AndroidSdk androidSdk,
FileSystem fileSystem, // TODO(jonahwilliams): remove after rolling into google3
Platform platform,
@required FileSystem fileSystem,
@required Platform platform,
@required UserMessages userMessages,
}) : _androidWorkflow = androidWorkflow,
_androidSdk = androidSdk,
_processUtils = ProcessUtils(
......@@ -35,9 +42,10 @@ class AndroidDevices extends PollingDeviceDiscovery {
),
_processManager = processManager,
_logger = logger,
_fileSystem = fileSystem ?? globals.fs,
_platform = platform ?? globals.platform,
super('Android devices');
_fileSystem = fileSystem,
_platform = platform,
_userMessages = userMessages,
super('Android devices');
final AndroidWorkflow _androidWorkflow;
final ProcessUtils _processUtils;
......@@ -46,6 +54,7 @@ class AndroidDevices extends PollingDeviceDiscovery {
final Logger _logger;
final FileSystem _fileSystem;
final Platform _platform;
final UserMessages _userMessages;
@override
bool get supportsPlatform => _androidWorkflow.appliesToHostPlatform;
......@@ -60,27 +69,19 @@ class AndroidDevices extends PollingDeviceDiscovery {
}
String text;
try {
text = (await _processUtils.run(
<String>[_androidSdk.adbPath, 'devices', '-l'],
text = (await _processUtils.run(<String>[_androidSdk.adbPath, 'devices', '-l'],
throwOnError: true,
)).stdout.trim();
} on ArgumentError catch (exception) {
throwToolExit('Unable to find "adb", check your Android SDK installation and '
'$kAndroidSdkRoot environment variable: ${exception.message}');
} on ProcessException catch (exception) {
throwToolExit('Unable to run "adb", check your Android SDK installation and '
'$kAndroidSdkRoot environment variable: ${exception.executable}');
throwToolExit(
'Unable to run "adb", check your Android SDK installation and '
'$kAndroidSdkRoot environment variable: ${exception.executable}',
);
}
final List<AndroidDevice> devices = <AndroidDevice>[];
parseADBDeviceOutput(
_parseADBDeviceOutput(
text,
devices: devices,
timeoutConfiguration: timeoutConfiguration,
processManager: _processManager,
logger: _logger,
fileSystem: _fileSystem,
androidSdk: _androidSdk,
platform: _platform,
);
return devices;
}
......@@ -94,21 +95,13 @@ class AndroidDevices extends PollingDeviceDiscovery {
final RunResult result = await _processUtils.run(<String>[_androidSdk.adbPath, 'devices', '-l']);
if (result.exitCode != 0) {
return <String>[];
} else {
final String text = result.stdout;
final List<String> diagnostics = <String>[];
parseADBDeviceOutput(
text,
diagnostics: diagnostics,
timeoutConfiguration: timeoutConfiguration,
processManager: _processManager,
logger: _logger,
fileSystem: _fileSystem,
androidSdk: _androidSdk,
platform: _platform,
);
return diagnostics;
}
final List<String> diagnostics = <String>[];
_parseADBDeviceOutput(
result.stdout,
diagnostics: diagnostics,
);
return diagnostics;
}
// 015d172c98400a03 device usb:340787200X product:nakasi model:Nexus_7 device:grouper
......@@ -117,17 +110,10 @@ class AndroidDevices extends PollingDeviceDiscovery {
/// Parse the given `adb devices` output in [text], and fill out the given list
/// of devices and possible device issue diagnostics. Either argument can be null,
/// in which case information for that parameter won't be populated.
@visibleForTesting
static void parseADBDeviceOutput(
void _parseADBDeviceOutput(
String text, {
List<AndroidDevice> devices,
List<String> diagnostics,
@required AndroidSdk androidSdk,
@required FileSystem fileSystem,
@required Logger logger,
@required Platform platform,
@required ProcessManager processManager,
@required TimeoutConfiguration timeoutConfiguration,
}) {
// Check for error messages from adb
if (!text.contains('List of devices')) {
......@@ -186,19 +172,19 @@ class AndroidDevices extends PollingDeviceDiscovery {
productID: info['product'],
modelID: info['model'] ?? deviceID,
deviceCodeName: info['device'],
androidSdk: androidSdk,
fileSystem: fileSystem,
logger: logger,
platform: platform,
processManager: processManager,
timeoutConfiguration: timeoutConfiguration,
androidSdk: _androidSdk,
fileSystem: _fileSystem,
logger: _logger,
platform: _platform,
processManager: _processManager,
timeoutConfiguration: const TimeoutConfiguration(),
));
}
} else {
diagnostics?.add(
'Unexpected failure parsing device information from adb output:\n'
'$line\n'
'${globals.userMessages.flutterToolBugInstructions}');
'${_userMessages.flutterToolBugInstructions}');
}
}
}
......
packages/flutter_tools/lib/src/android/android_workflow.dart
View file @ 6180a4c1
......@@ -70,6 +70,13 @@ class AndroidWorkflow implements Workflow {
&& _androidSdk.emulatorPath != null;
}
/// A validator that checks if the Android SDK and Java SDK are available and
/// installed correctly.
///
/// Android development requires the Android SDK, and at least one Java SDK. While
/// newer Java compilers can be used to compile the Java application code, the SDK
/// tools themselves required JDK 1.8. This older JDK is normally bundled with
/// Android Studio.
class AndroidValidator extends DoctorValidator {
AndroidValidator({
@required AndroidSdk androidSdk,
......@@ -244,6 +251,8 @@ class AndroidValidator extends DoctorValidator {
}
}
/// A subvalidator that checks if the licenses within the detected Android
/// SDK have been accepted.
class AndroidLicenseValidator extends DoctorValidator {
AndroidLicenseValidator() : super('Android license subvalidator',);
......
packages/flutter_tools/lib/src/context_runner.dart
View file @ 6180a4c1
......@@ -144,6 +144,7 @@ Future<T> runInContext<T>(
config: globals.config,
fuchsiaWorkflow: fuchsiaWorkflow,
xcDevice: globals.xcdevice,
userMessages: globals.userMessages,
windowsWorkflow: windowsWorkflow,
macOSWorkflow: MacOSWorkflow(
platform: globals.platform,
......
packages/flutter_tools/lib/src/device.dart
View file @ 6180a4c1
......@@ -342,6 +342,7 @@ class FlutterDeviceManager extends DeviceManager {
@required Config config,
@required Artifacts artifacts,
@required MacOSWorkflow macOSWorkflow,
@required UserMessages userMessages,
@required OperatingSystemUtils operatingSystemUtils,
@required WindowsWorkflow windowsWorkflow,
}) : deviceDiscoverers = <DeviceDiscovery>[
......@@ -350,6 +351,9 @@ class FlutterDeviceManager extends DeviceManager {
androidSdk: androidSdk,
androidWorkflow: androidWorkflow,
processManager: processManager,
fileSystem: fileSystem,
platform: platform,
userMessages: userMessages,
),
IOSDevices(
platform: platform,
......
packages/flutter_tools/test/general.shard/android/adb_log_reader_test.dart
View file @ 6180a4c1
......@@ -185,7 +185,7 @@ MockAndroidDevice createMockDevice(int sdkLevel) {
final MockAndroidDevice mockAndroidDevice = MockAndroidDevice();
when(mockAndroidDevice.apiVersion)
.thenAnswer((Invocation invocation) async => sdkLevel.toString());
when(mockAndroidDevice.lastLogcatTimestamp).thenReturn(kLastLogcatTimestamp);
when(mockAndroidDevice.lastLogcatTimestamp()).thenAnswer((Invocation _) async => kLastLogcatTimestamp);
when(mockAndroidDevice.adbCommandForDevice(any))
.thenAnswer((Invocation invocation) => <String>[
'adb', '-s', '1234', ...invocation.positionalArguments.first as List<String>
......
packages/flutter_tools/test/general.shard/android/android_device_test.dart
View file @ 6180a4c1
......@@ -131,23 +131,7 @@ void main() {
});
testWithoutContext('AndroidDevice can detect local emulator for known types', () async {
final Set<String> knownPhyiscal = <String>{
'qcom',
'samsungexynos7420',
'samsungexynos7580',
'samsungexynos7870',
'samsungexynos7880',
'samsungexynos8890',
'samsungexynos8895',
'samsungexynos9810',
'samsungexynos7570',
};
final Set<String> knownEmulator = <String>{
'goldfish',
'ranchu',
};
for (final String hardware in knownPhyiscal.followedBy(knownEmulator)) {
for (final String hardware in kKnownHardware.keys) {
final AndroidDevice device = setUpAndroidDevice(
processManager: FakeProcessManager.list(<FakeCommand>[
FakeCommand(
......@@ -160,7 +144,7 @@ void main() {
])
);
expect(await device.isLocalEmulator, knownEmulator.contains(hardware));
expect(await device.isLocalEmulator, kKnownHardware[hardware] == HardwareType.emulator);
}
});
......@@ -358,7 +342,7 @@ flutter:
])
);
expect(device.lastLogcatTimestamp, isNull);
expect(await device.lastLogcatTimestamp(), isNull);
});
testWithoutContext('AndroidDevice AdbLogReaders for past+future and future logs are not the same', () async {
......@@ -657,7 +641,7 @@ const String kAdbShellGetprop = '''
/// A mock Android Console that presents a connection banner and responds to
/// "avd name" requests with the supplied name.
class MockWorkingAndroidConsoleSocket extends Mock implements Socket {
class MockWorkingAndroidConsoleSocket extends Fake implements Socket {
MockWorkingAndroidConsoleSocket(this.avdName) {
_controller.add('Android Console: Welcome!\n');
// Include OK in the same packet here. In the response to "avd name"
......@@ -683,10 +667,13 @@ class MockWorkingAndroidConsoleSocket extends Mock implements Socket {
throw 'Unexpected command $text';
}
}
@override
void destroy() { }
}
/// An Android console socket that drops all input and returns no output.
class MockUnresponsiveAndroidConsoleSocket extends Mock implements Socket {
class MockUnresponsiveAndroidConsoleSocket extends Fake implements Socket {
final StreamController<String> _controller = StreamController<String>();
@override
......@@ -694,10 +681,13 @@ class MockUnresponsiveAndroidConsoleSocket extends Mock implements Socket {
@override
void add(List<int> data) {}
@override
void destroy() { }
}
/// An Android console socket that drops all input and returns no output.
class MockDisconnectingAndroidConsoleSocket extends Mock implements Socket {
class MockDisconnectingAndroidConsoleSocket extends Fake implements Socket {
MockDisconnectingAndroidConsoleSocket() {
_controller.add('Android Console: Welcome!\n');
// Include OK in the same packet here. In the response to "avd name"
......@@ -714,4 +704,7 @@ class MockDisconnectingAndroidConsoleSocket extends Mock implements Socket {
void add(List<int> data) {
_controller.close();
}
@override
void destroy() { }
}
packages/flutter_tools/test/integration.shard/test_driver.dart
View file @ 6180a4c1
......@@ -814,11 +814,11 @@ Stream<String> transformToLines(Stream<List<int>> byteStream) {
}
Map<String, dynamic> parseFlutterResponse(String line) {
if (line.startsWith('[') && line.endsWith(']')) {
if (line.startsWith('[') && line.endsWith(']') && line.length > 2) {
try {
final Map<String, dynamic> response = castStringKeyedMap(json.decode(line)[0]);
return response;
} on Exception {
} on FormatException {
// Not valid JSON, so likely some other output that was surrounded by [brackets]
return null;
}
......
packages/flutter_tools/tool/global_count.dart 0 → 100644
View file @ 6180a4c1
// Copyright 2014 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
import 'dart:io';
import 'package:path/path.dart' as path;
/// Count the number of libraries that import globals.dart in lib and test.
///
/// This must be run from the flutter_tools project root directory.
void main() {
final Directory sources = Directory(path.join(Directory.current.path, 'lib'));
final Directory tests = Directory(path.join(Directory.current.path, 'test'));
final int sourceGlobals = countGlobalImports(sources);
final int testGlobals = countGlobalImports(tests);
print('lib/ contains $sourceGlobals libraries with global usage');
print('test/ contains $testGlobals libraries with global usage');
}
final RegExp globalImport = RegExp('import.*globals.dart\' as globals;');
int countGlobalImports(Directory directory) {
int count = 0;
for (final FileSystemEntity file in directory.listSync(recursive: true)) {
if (!file.path.endsWith('.dart') || file is! File) {
continue;
}
final bool hasImport = (file as File).readAsLinesSync().any((String line) {
return globalImport.hasMatch(line);
});
if (hasImport) {
count += 1;
}
}
return count;
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment

HIAST - GitLab