Unverified Commit 72397fd4 authored by Jonah Williams's avatar Jonah Williams Committed by GitHub

[flutter_tools] Lazily inject logger into web devices (#55961)

Constructing the WebDevices with the global logger too early will lead to them grabbing the StdoutLogger when running in daemon mode. This prevents IDEs from seeing the correct debug message.
parent 1d0999d0
......@@ -93,7 +93,6 @@ class DeviceManager {
WebDevices(
featureFlags: featureFlags,
fileSystem: globals.fs,
logger: globals.logger,
platform: globals.platform,
processManager: globals.processManager,
),
......
......@@ -15,6 +15,7 @@ import '../base/io.dart';
import '../base/logger.dart';
import '../base/os.dart';
import '../convert.dart';
import '../globals.dart' as globals;
/// An environment variable used to override the location of Google Chrome.
const String kChromeEnvironment = 'CHROME_EXECUTABLE';
......@@ -121,7 +122,7 @@ class ChromiumLauncher {
final Platform _platform;
final ProcessManager _processManager;
final OperatingSystemUtils _operatingSystemUtils;
final Logger _logger;
Logger _logger;
final BrowserFinder _browserFinder;
final FileSystemUtils _fileSystemUtils;
......@@ -162,6 +163,7 @@ class ChromiumLauncher {
bool skipCheck = false,
Directory cacheDir,
}) async {
_logger ??= globals.logger;
if (_currentCompleter.isCompleted) {
throwToolExit('Only one instance of chrome can be started.');
}
......
......@@ -14,6 +14,7 @@ import '../base/os.dart';
import '../build_info.dart';
import '../device.dart';
import '../features.dart';
import '../globals.dart' as globals;
import '../project.dart';
import 'chrome.dart';
......@@ -35,7 +36,7 @@ abstract class ChromiumDevice extends Device {
@required String name,
@required this.chromeLauncher,
@required FileSystem fileSystem,
@required Logger logger,
Logger logger,
}) : _fileSystem = fileSystem,
_logger = logger,
super(
......@@ -48,7 +49,7 @@ abstract class ChromiumDevice extends Device {
final ChromiumLauncher chromeLauncher;
final FileSystem _fileSystem;
final Logger _logger;
Logger _logger;
/// The active chrome instance.
Chromium _chrome;
......@@ -114,6 +115,7 @@ abstract class ChromiumDevice extends Device {
bool prebuiltApplication = false,
bool ipv6 = false,
}) async {
_logger ??= globals.logger;
// See [ResidentWebRunner.run] in flutter_tools/lib/src/resident_web_runner.dart
// for the web initialization and server logic.
final String url = platformArgs['uri'] as String;
......@@ -239,7 +241,10 @@ class MicrosoftEdgeDevice extends ChromiumDevice {
class WebDevices extends PollingDeviceDiscovery {
WebDevices({
@required FileSystem fileSystem,
@required Logger logger,
// TODO(jonahwilliams): the logger is overriden by the daemon command
// to support IDE integration. Accessing the global logger too early
// will grab the old stdout logger.
Logger logger,
@required Platform platform,
@required ProcessManager processManager,
@required FeatureFlags featureFlags,
......@@ -302,7 +307,7 @@ String parseVersionForWindows(String input) {
/// A special device type to allow serving for arbitrary browsers.
class WebServerDevice extends Device {
WebServerDevice({
@required Logger logger,
Logger logger,
}) : _logger = logger,
super(
'web-server',
......@@ -311,7 +316,7 @@ class WebServerDevice extends Device {
ephemeral: false,
);
final Logger _logger;
Logger _logger;
@override
void clearLogs() { }
......@@ -367,6 +372,7 @@ class WebServerDevice extends Device {
bool prebuiltApplication = false,
bool ipv6 = false,
}) async {
_logger ??= globals.logger;
final String url = platformArgs['uri'] as String;
if (debuggingOptions.startPaused) {
_logger.printStatus('Waiting for connection from Dart debug extension at $url', emphasis: true);
......
......@@ -4,6 +4,7 @@
import 'package:file/memory.dart';
import 'package:flutter_tools/src/base/logger.dart';
import 'package:flutter_tools/src/build_info.dart';
import 'package:flutter_tools/src/device.dart';
import 'package:flutter_tools/src/web/chrome.dart';
import 'package:flutter_tools/src/web/web_device.dart';
......@@ -15,6 +16,37 @@ import '../../src/context.dart';
import '../../src/testbed.dart';
void main() {
testUsingContext('Grabs context logger if no constructor logger is provided', () async {
final WebDevices webDevices = WebDevices(
featureFlags: TestFeatureFlags(isWebEnabled: true),
fileSystem: MemoryFileSystem.test(),
platform: FakePlatform(
operatingSystem: 'linux',
environment: <String, String>{}
),
processManager: FakeProcessManager.any(),
);
final List<Device> devices = await webDevices.pollingGetDevices();
final WebServerDevice serverDevice = devices.firstWhere((Device device) => device is WebServerDevice)
as WebServerDevice;
await serverDevice.startApp(
null,
debuggingOptions: DebuggingOptions.enabled(
BuildInfo.debug,
startPaused: true,
),
platformArgs: <String, String>{
'uri': 'foo',
}
);
// Verify that the injected testLogger is used.
expect(testLogger.statusText, contains(
'Waiting for connection from Dart debug extension at foo'
));
});
testWithoutContext('No web devices listed if feature is disabled', () async {
final WebDevices webDevices = WebDevices(
featureFlags: TestFeatureFlags(isWebEnabled: false),
......
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