Don't crash flutter tool if Chrome is not available (#154941)
Instead of unawaiting the future, let's ignore it. Fixes issue #154940 I am not sure if tests would be required for this change or not. ## Pre-launch Checklist - [ ] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [ ] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [ ] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [ ] I signed the [CLA]. - [ ] I listed at least one issue that this PR fixes in the description above. - [ ] I updated/added relevant documentation (doc comments with `///`). - [ ] I added new tests to check the change I am making, or this PR is [test-exempt]. - [ ] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [ ] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#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/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md --------- Co-authored-by: Christopher Fujino <christopherfujino@gmail.com> Co-authored-by: Andrew Kolos <andrewrkolos@gmail.com> Co-authored-by: Ben Konyi <bkonyi@google.com>
This commit is contained in:
saltedpotatos
GitHub
parent
539727fb9d
commit
cc44dba520
@ -8,23 +8,25 @@
|
||||
|
||||
import 'dart:async';
|
||||
|
||||
import 'package:browser_launcher/browser_launcher.dart';
|
||||
import 'package:meta/meta.dart';
|
||||
|
||||
import 'base/io.dart';
|
||||
import 'base/logger.dart';
|
||||
import 'build_info.dart';
|
||||
import 'resident_runner.dart';
|
||||
import 'vmservice.dart';
|
||||
import 'web/chrome.dart';
|
||||
|
||||
typedef ResidentDevtoolsHandlerFactory =
|
||||
ResidentDevtoolsHandler Function(DevtoolsLauncher?, ResidentRunner, Logger);
|
||||
ResidentDevtoolsHandler Function(DevtoolsLauncher?, ResidentRunner, Logger, ChromiumLauncher);
|
||||
|
||||
ResidentDevtoolsHandler createDefaultHandler(
|
||||
DevtoolsLauncher? launcher,
|
||||
ResidentRunner runner,
|
||||
Logger logger,
|
||||
ChromiumLauncher chromiumLauncher,
|
||||
) {
|
||||
return FlutterResidentDevtoolsHandler(launcher, runner, logger);
|
||||
return FlutterResidentDevtoolsHandler(launcher, runner, logger, chromiumLauncher);
|
||||
}
|
||||
|
||||
/// Helper class to manage the life-cycle of devtools and its interaction with
|
||||
@ -66,12 +68,18 @@ abstract class ResidentDevtoolsHandler {
|
||||
}
|
||||
|
||||
class FlutterResidentDevtoolsHandler implements ResidentDevtoolsHandler {
|
||||
FlutterResidentDevtoolsHandler(this._devToolsLauncher, this._residentRunner, this._logger);
|
||||
FlutterResidentDevtoolsHandler(
|
||||
this._devToolsLauncher,
|
||||
this._residentRunner,
|
||||
this._logger,
|
||||
this._chromiumLauncher,
|
||||
);
|
||||
|
||||
static const Duration launchInBrowserTimeout = Duration(seconds: 15);
|
||||
|
||||
final DevtoolsLauncher? _devToolsLauncher;
|
||||
final ResidentRunner _residentRunner;
|
||||
final ChromiumLauncher _chromiumLauncher;
|
||||
final Logger _logger;
|
||||
bool _shutdown = false;
|
||||
|
||||
@ -184,11 +192,14 @@ class FlutterResidentDevtoolsHandler implements ResidentDevtoolsHandler {
|
||||
queryParameters: <String, dynamic>{'uri': '${device!.vmService!.httpAddress}'},
|
||||
)
|
||||
.toString();
|
||||
_logger.printStatus(
|
||||
'Launching Flutter DevTools for '
|
||||
'${device.device!.displayName} at $devToolsUrl',
|
||||
);
|
||||
unawaited(Chrome.start(<String>[devToolsUrl]));
|
||||
_logger.printStatus('Launching Flutter DevTools for ${device.device!.name} at $devToolsUrl');
|
||||
|
||||
_chromiumLauncher.launch(devToolsUrl).catchError((Object e) {
|
||||
_logger.printError('Failed to launch web browser: $e');
|
||||
throw ProcessException('Chrome', <String>[
|
||||
devToolsUrl,
|
||||
], 'Failed to launch browser for dev tools');
|
||||
}).ignore();
|
||||
}
|
||||
launchedInBrowser = true;
|
||||
}
|
||||
@ -316,6 +327,7 @@ NoOpDevtoolsHandler createNoOpHandler(
|
||||
DevtoolsLauncher? launcher,
|
||||
ResidentRunner runner,
|
||||
Logger logger,
|
||||
ChromiumLauncher? chromiumLauncher,
|
||||
) {
|
||||
return NoOpDevtoolsHandler();
|
||||
}
|
||||
|
||||
@ -41,6 +41,7 @@ import 'run_cold.dart';
|
||||
import 'run_hot.dart';
|
||||
import 'sksl_writer.dart';
|
||||
import 'vmservice.dart';
|
||||
import 'web/chrome.dart';
|
||||
|
||||
class FlutterDevice {
|
||||
FlutterDevice(
|
||||
@ -1048,7 +1049,19 @@ abstract class ResidentRunner extends ResidentHandlers {
|
||||
artifactDirectory.createSync(recursive: true);
|
||||
}
|
||||
// TODO(bkonyi): remove when ready to serve DevTools from DDS.
|
||||
_residentDevtoolsHandler = devtoolsHandler(DevtoolsLauncher.instance, this, globals.logger);
|
||||
_residentDevtoolsHandler = devtoolsHandler(
|
||||
DevtoolsLauncher.instance,
|
||||
this,
|
||||
globals.logger,
|
||||
ChromiumLauncher(
|
||||
fileSystem: globals.fs,
|
||||
platform: globals.platform,
|
||||
processManager: globals.processManager,
|
||||
operatingSystemUtils: globals.os,
|
||||
browserFinder: findChromeExecutable,
|
||||
logger: globals.logger,
|
||||
),
|
||||
);
|
||||
}
|
||||
|
||||
@override
|
||||
|
||||
@ -12,7 +12,6 @@ dependencies:
|
||||
# https://github.com/flutter/flutter/blob/main/docs/infra/Updating-dependencies-in-Flutter.md
|
||||
archive: 3.6.1
|
||||
args: 2.6.0
|
||||
browser_launcher: 1.1.3
|
||||
dds: 5.0.0
|
||||
dwds: 24.3.2
|
||||
completion: 1.0.1
|
||||
@ -71,6 +70,7 @@ dependencies:
|
||||
_fe_analyzer_shared: 76.0.0 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
|
||||
analyzer: 6.11.0 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
|
||||
boolean_selector: 2.1.2 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
|
||||
browser_launcher: 1.1.3 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
|
||||
built_collection: 5.1.1 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
|
||||
built_value: 8.9.3 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
|
||||
clock: 1.1.2 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
|
||||
|
||||
@ -6,6 +6,8 @@ import 'dart:async';
|
||||
|
||||
import 'package:flutter_tools/src/artifacts.dart';
|
||||
import 'package:flutter_tools/src/base/dds.dart';
|
||||
import 'package:flutter_tools/src/base/file_system.dart';
|
||||
import 'package:flutter_tools/src/base/io.dart';
|
||||
import 'package:flutter_tools/src/base/logger.dart';
|
||||
import 'package:flutter_tools/src/build_info.dart';
|
||||
import 'package:flutter_tools/src/cache.dart';
|
||||
@ -14,6 +16,7 @@ import 'package:flutter_tools/src/devtools_launcher.dart';
|
||||
import 'package:flutter_tools/src/resident_devtools_handler.dart';
|
||||
import 'package:flutter_tools/src/resident_runner.dart';
|
||||
import 'package:flutter_tools/src/vmservice.dart';
|
||||
import 'package:flutter_tools/src/web/chrome.dart';
|
||||
import 'package:test/fake.dart';
|
||||
import 'package:vm_service/vm_service.dart' as vm_service;
|
||||
import 'package:vm_service/vm_service.dart';
|
||||
@ -58,6 +61,7 @@ void main() {
|
||||
null,
|
||||
FakeResidentRunner(),
|
||||
BufferLogger.test(),
|
||||
_FakeChromiumLauncher(),
|
||||
);
|
||||
|
||||
await handler.serveAndAnnounceDevTools(flutterDevices: <FlutterDevice>[]);
|
||||
@ -72,8 +76,8 @@ void main() {
|
||||
FakeDevtoolsLauncher(),
|
||||
FakeResidentRunner()..supportsServiceProtocol = false,
|
||||
BufferLogger.test(),
|
||||
_FakeChromiumLauncher(),
|
||||
);
|
||||
|
||||
await handler.serveAndAnnounceDevTools(flutterDevices: <FlutterDevice>[]);
|
||||
|
||||
expect(handler.activeDevToolsServer, null);
|
||||
@ -94,6 +98,7 @@ void main() {
|
||||
launcher,
|
||||
FakeResidentRunner(),
|
||||
BufferLogger.test(),
|
||||
_FakeChromiumLauncher(),
|
||||
);
|
||||
|
||||
await handler.serveAndAnnounceDevTools(
|
||||
@ -114,6 +119,7 @@ void main() {
|
||||
..devToolsUrl = Uri.parse('http://localhost:8080'),
|
||||
FakeResidentRunner(),
|
||||
BufferLogger.test(),
|
||||
_FakeChromiumLauncher(),
|
||||
);
|
||||
|
||||
// VM Service is intentionally null
|
||||
@ -132,6 +138,7 @@ void main() {
|
||||
..devToolsUrl = Uri.parse('http://localhost:8080'),
|
||||
FakeResidentRunner(),
|
||||
BufferLogger.test(),
|
||||
_FakeChromiumLauncher(),
|
||||
);
|
||||
final FakeVmServiceHost fakeVmServiceHost = FakeVmServiceHost(
|
||||
requests: <VmServiceExpectation>[
|
||||
@ -170,6 +177,7 @@ void main() {
|
||||
FakeDevtoolsLauncher()..activeDevToolsServer = null,
|
||||
FakeResidentRunner(),
|
||||
BufferLogger.test(),
|
||||
_FakeChromiumLauncher(),
|
||||
);
|
||||
final FakeVmServiceHost fakeVmServiceHost = FakeVmServiceHost(
|
||||
requests: <VmServiceExpectation>[],
|
||||
@ -188,6 +196,7 @@ void main() {
|
||||
..devToolsUrl = Uri.parse('http://localhost:8080'),
|
||||
FakeResidentRunner(),
|
||||
BufferLogger.test(),
|
||||
_FakeChromiumLauncher(),
|
||||
);
|
||||
final FakeVmServiceHost fakeVmServiceHost = FakeVmServiceHost(
|
||||
requests: <VmServiceExpectation>[
|
||||
@ -228,6 +237,7 @@ void main() {
|
||||
FakeDevtoolsLauncher()..activeDevToolsServer = DevToolsServerAddress('localhost', 8080),
|
||||
FakeResidentRunner(),
|
||||
BufferLogger.test(),
|
||||
_FakeChromiumLauncher(),
|
||||
);
|
||||
final FakeVmServiceHost fakeVmServiceHost = FakeVmServiceHost(
|
||||
requests: <VmServiceExpectation>[
|
||||
@ -258,6 +268,7 @@ void main() {
|
||||
..devToolsUrl = Uri.parse('http://localhost:8080'),
|
||||
FakeResidentRunner(),
|
||||
BufferLogger.test(),
|
||||
_FakeChromiumLauncher(),
|
||||
);
|
||||
|
||||
final FakeVmServiceHost vmServiceHost = FakeVmServiceHost(
|
||||
@ -314,6 +325,7 @@ void main() {
|
||||
null,
|
||||
FakeResidentRunner(),
|
||||
BufferLogger.test(),
|
||||
_FakeChromiumLauncher(),
|
||||
);
|
||||
|
||||
handler.launchDevToolsInBrowser(flutterDevices: <FlutterDevice>[]);
|
||||
@ -328,6 +340,7 @@ void main() {
|
||||
FakeDevtoolsLauncher(),
|
||||
FakeResidentRunner()..supportsServiceProtocol = false,
|
||||
BufferLogger.test(),
|
||||
_FakeChromiumLauncher(),
|
||||
);
|
||||
|
||||
handler.launchDevToolsInBrowser(flutterDevices: <FlutterDevice>[]);
|
||||
@ -349,6 +362,7 @@ void main() {
|
||||
..readyCompleter = completer,
|
||||
FakeResidentRunner(),
|
||||
BufferLogger.test(),
|
||||
_FakeChromiumLauncher(),
|
||||
);
|
||||
|
||||
expect(handler.launchDevToolsInBrowser(flutterDevices: <FlutterDevice>[]), isTrue);
|
||||
@ -369,12 +383,27 @@ void main() {
|
||||
..activeDevToolsServer = DevToolsServerAddress('localhost', 8080),
|
||||
FakeResidentRunner(),
|
||||
BufferLogger.test(),
|
||||
_FakeChromiumLauncher(),
|
||||
);
|
||||
|
||||
expect(handler.launchDevToolsInBrowser(flutterDevices: <FlutterDevice>[]), isTrue);
|
||||
expect(handler.launchedInBrowser, isTrue);
|
||||
});
|
||||
|
||||
testWithoutContext('launchDevToolsInBrowser fails without Chrome installed', () async {
|
||||
final FlutterResidentDevtoolsHandler handler = FlutterResidentDevtoolsHandler(
|
||||
FakeDevtoolsLauncher()
|
||||
..devToolsUrl = Uri(host: 'localhost', port: 8080)
|
||||
..activeDevToolsServer = DevToolsServerAddress('localhost', 8080),
|
||||
FakeResidentRunner(),
|
||||
BufferLogger.test(),
|
||||
_ThrowingChromiumLauncher(),
|
||||
);
|
||||
|
||||
expect(handler.launchedInBrowser, isFalse);
|
||||
expect(handler.launchDevToolsInBrowser(flutterDevices: <FlutterDevice>[]), isTrue);
|
||||
});
|
||||
|
||||
testWithoutContext(
|
||||
'Converts a VM Service URI with a query parameter to a pretty display string',
|
||||
() {
|
||||
@ -443,3 +472,19 @@ class FakeDartDevelopmentService extends Fake implements DartDevelopmentService
|
||||
disposed = true;
|
||||
}
|
||||
}
|
||||
|
||||
class _ThrowingChromiumLauncher extends Fake implements ChromiumLauncher {
|
||||
@override
|
||||
Future<Chromium> launch(
|
||||
String url, {
|
||||
bool headless = false,
|
||||
int? debugPort,
|
||||
bool skipCheck = false,
|
||||
Directory? cacheDir,
|
||||
List<String> webBrowserFlags = const <String>[],
|
||||
}) async {
|
||||
throw ProcessException('ChromiumLauncher', <String>[url]);
|
||||
}
|
||||
}
|
||||
|
||||
class _FakeChromiumLauncher extends Fake implements ChromiumLauncher {}
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user