diff --git a/packages/flutter_tools/lib/src/web/chrome.dart b/packages/flutter_tools/lib/src/web/chrome.dart index f35a6634a2d..90af6e5cf8c 100644 --- a/packages/flutter_tools/lib/src/web/chrome.dart +++ b/packages/flutter_tools/lib/src/web/chrome.dart @@ -415,7 +415,7 @@ class ChromiumLauncher { // connection is valid. if (!skipCheck) { try { - await _getFirstTab(chrome); + await chrome._validateChromeConnection(); } on Exception catch (error, stackTrace) { _logger.printError('$error', stackTrace: stackTrace); await chrome.close(); @@ -427,40 +427,6 @@ class ChromiumLauncher { return chrome; } - /// Gets the first [chrome] tab. - /// - /// Retries getting tabs from Chrome for a few seconds and retries finding - /// the tab a few times. This reduces flakes caused by Chrome not returning - /// correct output if the call was too close to the start. - // - // TODO(ianh): remove the timeouts here, they violate our style guide. - // (We should just keep waiting forever, and print a warning when it's - // taking too long.) - Future _getFirstTab(Chromium chrome) async { - const Duration retryFor = Duration(seconds: 2); - const int attempts = 5; - - for (int i = 1; i <= attempts; i++) { - try { - final List tabs = - await chrome.chromeConnection.getTabs(retryFor: retryFor); - - if (tabs.isNotEmpty) { - return tabs.first; - } - if (i == attempts) { - return null; - } - } on ConnectionException catch (_) { - if (i == attempts) { - rethrow; - } - } - await Future.delayed(const Duration(milliseconds: 25)); - } - return null; - } - Future get connectedInstance => currentCompleter.future; } @@ -483,6 +449,7 @@ class Chromium { final ChromeConnection chromeConnection; final ChromiumLauncher _chromiumLauncher; final Logger _logger; + bool _hasValidChromeConnection = false; /// Resolves to browser's main process' exit code, when the browser exits. Future get onExit async => _process.exitCode; @@ -493,6 +460,41 @@ class Chromium { @visibleForTesting Process get process => _process; + /// Gets the first [chrome] tab in order to verify that the connection to + /// the Chrome debug protocol is working properly. + /// + /// Retries getting tabs from Chrome for a few seconds and retries finding + /// the tab a few times. This reduces flakes caused by Chrome not returning + /// correct output if the call was too close to the start. + // + // TODO(ianh): remove the timeouts here, they violate our style guide. + // (We should just keep waiting forever, and print a warning when it's + // taking too long.) + Future _validateChromeConnection() async { + const Duration retryFor = Duration(seconds: 2); + const int attempts = 5; + + for (int i = 1; i <= attempts; i++) { + try { + final List tabs = + await chromeConnection.getTabs(retryFor: retryFor); + + if (tabs.isNotEmpty) { + _hasValidChromeConnection = true; + return; + } + if (i == attempts) { + return; + } + } on ConnectionException catch (_) { + if (i == attempts) { + rethrow; + } + } + await Future.delayed(const Duration(milliseconds: 25)); + } + } + /// Closes all connections to the browser and asks the browser to exit. Future close() async { if (_logger.isVerbose) { @@ -501,12 +503,31 @@ class Chromium { if (_chromiumLauncher.hasChromeInstance) { _chromiumLauncher.currentCompleter = Completer(); } - chromeConnection.close(); - // Try to exit Chromium nicely using SIGTERM, before exiting it rudely using - // SIGKILL. Wait no longer than 5 seconds for Chromium to exit before - // falling back to SIGKILL, and then to a warning message. - ProcessSignal.sigterm.kill(_process); + // Send a command to shut down the browser cleanly. + Duration sigtermDelay = Duration.zero; + if (_hasValidChromeConnection) { + final ChromeTab? tab = await chromeConnection.getTab( + (_) => true, retryFor: const Duration(seconds: 1)); + if (tab != null) { + final WipConnection wipConnection = await tab.connect(); + await wipConnection.sendCommand('Browser.close'); + await wipConnection.close(); + sigtermDelay = const Duration(seconds: 1); + } + } + chromeConnection.close(); + _hasValidChromeConnection = false; + + // If the browser close command did not shut down the process, then try to + // exit Chromium using SIGTERM. + await _process.exitCode.timeout(sigtermDelay, onTimeout: () { + ProcessSignal.sigterm.kill(_process); + return 0; + }); + // If the process still has not ended, then use SIGKILL. Wait up to 5 + // seconds for Chromium to exit before falling back to SIGKILL and then to + // a warning message. await _process.exitCode.timeout(const Duration(seconds: 5), onTimeout: () { _logger.printWarning( 'Failed to exit Chromium (pid: ${_process.pid}) using SIGTERM. Will try ' diff --git a/packages/flutter_tools/test/web.shard/chrome_test.dart b/packages/flutter_tools/test/web.shard/chrome_test.dart index f9611f4e7ee..08bc8a0502f 100644 --- a/packages/flutter_tools/test/web.shard/chrome_test.dart +++ b/packages/flutter_tools/test/web.shard/chrome_test.dart @@ -800,6 +800,26 @@ void main() { contains(' ...'), )); }); + + test('Chromium close sends browser close command', () async { + final BufferLogger logger = BufferLogger.test(); + final List commands = []; + void onSendCommand(String cmd) { commands.add(cmd); } + final FakeChromeConnectionWithTab chromeConnection = FakeChromeConnectionWithTab(onSendCommand: onSendCommand); + final ChromiumLauncher chromiumLauncher = ChromiumLauncher( + fileSystem: fileSystem, + platform: platform, + processManager: processManager, + operatingSystemUtils: operatingSystemUtils, + browserFinder: findChromeExecutable, + logger: logger, + ); + final FakeProcess process = FakeProcess(); + final Chromium chrome = Chromium(0, chromeConnection, chromiumLauncher: chromiumLauncher, process: process, logger: logger); + expect(await chromiumLauncher.connect(chrome, false), equals(chrome)); + await chrome.close(); + expect(commands, contains('Browser.close')); + }); } /// Fake chrome connection that fails to get tabs a few times. @@ -834,3 +854,52 @@ class FakeChromeConnection extends Fake implements ChromeConnection { @override void close() {} } + +typedef OnSendCommand = void Function(String); + +/// Fake chrome connection that returns a tab. +class FakeChromeConnectionWithTab extends Fake implements ChromeConnection { + FakeChromeConnectionWithTab({OnSendCommand? onSendCommand}) + : _tab = FakeChromeTab(onSendCommand); + + final FakeChromeTab _tab; + + @override + Future getTab(bool Function(ChromeTab tab) accept, {Duration? retryFor}) async { + return _tab; + } + + @override + Future> getTabs({Duration? retryFor}) async { + return [_tab]; + } + + @override + void close() {} +} + +class FakeChromeTab extends Fake implements ChromeTab { + FakeChromeTab(this.onSendCommand); + + OnSendCommand? onSendCommand; + + @override + Future connect({Function? onError}) async { + return FakeWipConnection(onSendCommand); + } +} + +class FakeWipConnection extends Fake implements WipConnection { + FakeWipConnection(this.onSendCommand); + + OnSendCommand? onSendCommand; + + @override + Future sendCommand(String method, [Map? params]) async { + onSendCommand?.call(method); + return WipResponse({'id': 0, 'result': {}}); + } + + @override + Future close() async {} +}