CrashReportSender dependency injection (#54924)

packages/flutter_tools/lib/runner.dart

@@ -7,7 +7,7 @@ import 'dart:async';
 import 'package:args/command_runner.dart';
 import 'package:intl/intl.dart' as intl;
 import 'package:intl/intl_standalone.dart' as intl_standalone;
-import 'package:meta/meta.dart';
+import 'package:http/http.dart' as http;
 
 import 'src/base/common.dart';
 import 'src/base/context.dart';
@@ -27,13 +27,13 @@ import 'src/runner/flutter_command_runner.dart';
 Future<int> run(
   List<String> args,
   List<FlutterCommand> commands, {
-  bool muteCommandLogging = false,
-  bool verbose = false,
-  bool verboseHelp = false,
-  bool reportCrashes,
-  String flutterVersion,
-  Map<Type, Generator> overrides,
-}) async {
+    bool muteCommandLogging = false,
+    bool verbose = false,
+    bool verboseHelp = false,
+    bool reportCrashes,
+    String flutterVersion,
+    Map<Type, Generator> overrides,
+  }) async {
   if (muteCommandLogging) {
     // Remove the verbose option; for help and doctor, users don't need to see
     // verbose logs.
@@ -121,7 +121,14 @@ Future<int> _handleToolError(
 
     // Report to both [Usage] and [CrashReportSender].
     globals.flutterUsage.sendException(error);
-    await CrashReportSender.instance.sendReport(
+    final CrashReportSender crashReportSender = CrashReportSender(
+      client: http.Client(),
+      usage: globals.flutterUsage,
+      platform: globals.platform,
+      logger: globals.logger,
+      operatingSystemUtils: globals.os,
+    );
+    await crashReportSender.sendReport(
       error: error,
       stackTrace: stackTrace,
       getFlutterVersion: getFlutterVersion,
@@ -184,18 +191,10 @@ String _crashCommand(List<String> args) => 'flutter ${args.join(' ')}';
 
 String _crashException(dynamic error) => '${error.runtimeType}: $error';
 
-/// File system used by the crash reporting logic.
-///
-/// We do not want to use the file system stored in the context because it may
-/// be recording. Additionally, in the case of a crash we do not trust the
-/// integrity of the [AppContext].
-@visibleForTesting
-FileSystem crashFileSystem = const LocalFileSystem();
-
 /// Saves the crash report to a local file.
 Future<File> _createLocalCrashReport(List<String> args, dynamic error, StackTrace stackTrace, String doctorText) async {
   File crashFile = globals.fsUtils.getUniqueFile(
-    crashFileSystem.currentDirectory,
+    globals.fs.currentDirectory,
     'flutter',
     'log',
   );
@@ -219,7 +218,7 @@ Future<File> _createLocalCrashReport(List<String> args, dynamic error, StackTrac
   } on FileSystemException catch (_) {
     // Fallback to the system temporary directory.
     crashFile = globals.fsUtils.getUniqueFile(
-      crashFileSystem.systemTempDirectory,
+      globals.fs.systemTempDirectory,
       'flutter',
       'log',
     );

packages/flutter_tools/lib/src/reporting/crash_reporting.dart

@@ -35,28 +35,29 @@ const String _kStackTraceFilename = 'stacktrace_file';
 ///   environment is behind a firewall and unable to send crash reports to
 ///   Google, or when you wish to use your own server for collecting crash
 ///   reports from Flutter Tools.
-/// * In tests call [initializeWith] and provide a mock implementation of
-///   [http.Client].
 class CrashReportSender {
-  CrashReportSender._(this._client);
+  CrashReportSender({
+    @required http.Client client,
+    @required Usage usage,
+    @required Platform platform,
+    @required Logger logger,
+    @required OperatingSystemUtils operatingSystemUtils,
+  }) : _client = client,
+      _usage = usage,
+      _platform = platform,
+      _logger = logger,
+      _operatingSystemUtils = operatingSystemUtils;
 
-  static CrashReportSender _instance;
-
-  static CrashReportSender get instance => _instance ?? CrashReportSender._(http.Client());
+  final http.Client _client;
+  final Usage _usage;
+  final Platform _platform;
+  final Logger _logger;
+  final OperatingSystemUtils _operatingSystemUtils;
 
   bool _crashReportSent = false;
 
-  /// Overrides the default [http.Client] with [client] for testing purposes.
-  @visibleForTesting
-  static void initializeWith(http.Client client) {
-    _instance = CrashReportSender._(client);
-  }
-
-  final http.Client _client;
-  final Usage _usage = globals.flutterUsage;
-
   Uri get _baseUrl {
-    final String overrideUrl = globals.platform.environment['FLUTTER_CRASH_SERVER_BASE_URL'];
+    final String overrideUrl = _platform.environment['FLUTTER_CRASH_SERVER_BASE_URL'];
 
     if (overrideUrl != null) {
       return Uri.parse(overrideUrl);
@@ -90,7 +91,7 @@ class CrashReportSender {
         return;
       }
 
-      globals.printTrace('Sending crash report to Google.');
+      _logger.printTrace('Sending crash report to Google.');
 
       final Uri uri = _baseUrl.replace(
         queryParameters: <String, String>{
@@ -103,8 +104,8 @@ class CrashReportSender {
       req.fields['uuid'] = _usage.clientId;
       req.fields['product'] = _kProductId;
       req.fields['version'] = flutterVersion;
-      req.fields['osName'] = globals.platform.operatingSystem;
-      req.fields['osVersion'] = globals.os.name; // this actually includes version
+      req.fields['osName'] = _platform.operatingSystem;
+      req.fields['osVersion'] = _operatingSystemUtils.name; // this actually includes version
       req.fields['type'] = _kDartTypeId;
       req.fields['error_runtime_type'] = '${error.runtimeType}';
       req.fields['error_message'] = '$error';
@@ -120,20 +121,20 @@ class CrashReportSender {
 
       if (resp.statusCode == 200) {
         final String reportId = await http.ByteStream(resp.stream)
-            .bytesToString();
-        globals.printTrace('Crash report sent (report ID: $reportId)');
+          .bytesToString();
+        _logger.printTrace('Crash report sent (report ID: $reportId)');
         _crashReportSent = true;
       } else {
-        globals.printError('Failed to send crash report. Server responded with HTTP status code ${resp.statusCode}');
+        _logger.printError('Failed to send crash report. Server responded with HTTP status code ${resp.statusCode}');
       }
     // Catch all exceptions to print the message that makes clear that the
     // crash logger crashed.
     } catch (sendError, sendStackTrace) { // ignore: avoid_catches_without_on_clauses
       if (sendError is SocketException || sendError is HttpException) {
-        globals.printError('Failed to send crash report due to a network error: $sendError');
+        _logger.printError('Failed to send crash report due to a network error: $sendError');
       } else {
         // If the sender itself crashes, just print. We did our best.
-        globals.printError('Crash report sender itself crashed: $sendError\n$sendStackTrace');
+        _logger.printError('Crash report sender itself crashed: $sendError\n$sendStackTrace');
       }
     }
   }

packages/flutter_tools/lib/src/reporting/reporting.dart

@@ -10,11 +10,13 @@ import 'package:file/file.dart';
 import 'package:http/http.dart' as http;
 import 'package:intl/intl.dart';
 import 'package:meta/meta.dart';
+import 'package:platform/platform.dart';
 import 'package:usage/usage_io.dart';
 
 import '../base/file_system.dart';
 import '../base/io.dart';
 import '../base/logger.dart';
+import '../base/os.dart';
 import '../base/process.dart';
 import '../base/time.dart';
 import '../build_system/exceptions.dart';

packages/flutter_tools/test/general.shard/runner/runner_test.dart

@@ -24,7 +24,6 @@ void main() {
     MockGitHubTemplateCreator mockGitHubTemplateCreator;
     setUp(() {
       mockGitHubTemplateCreator = MockGitHubTemplateCreator();
-      runner.crashFileSystem = MemoryFileSystem();
       // Instead of exiting with dart:io exit(), this causes an exception to
       // be thrown, which we catch with the onError callback in the zone below.
       io.setExitFunctionForTests((int _) { throw 'test exit';});
@@ -32,7 +31,6 @@ void main() {
     });
 
     tearDown(() {
-      runner.crashFileSystem = const LocalFileSystem();
       io.restoreExitFunction();
       Cache.enableLocking();
     });