diff --git a/packages/devtools_app/lib/src/extensions/embedded/_controller_web.dart b/packages/devtools_app/lib/src/extensions/embedded/_controller_web.dart index ec923e573f4..e510f23d739 100644 --- a/packages/devtools_app/lib/src/extensions/embedded/_controller_web.dart +++ b/packages/devtools_app/lib/src/extensions/embedded/_controller_web.dart @@ -46,7 +46,11 @@ class EmbeddedExtensionControllerImpl extends EmbeddedExtensionController } final basePath = devtoolsAssetsBasePath( - origin: window.location.origin, + // This needs to use the DevTools server URI as the origin because the + // extension assets are served through a DevTools server handler. The + // DevTools server handler loads them directly from their location in the + // user's pub-cache. + origin: devToolsServerUriAsString, path: window.location.pathname, ); final baseUri = path.join( diff --git a/packages/devtools_app/lib/src/shared/analytics/_analytics_web.dart b/packages/devtools_app/lib/src/shared/analytics/_analytics_web.dart index ec56c46797c..71383d999dc 100644 --- a/packages/devtools_app/lib/src/shared/analytics/_analytics_web.dart +++ b/packages/devtools_app/lib/src/shared/analytics/_analytics_web.dart @@ -522,7 +522,8 @@ void cancelTimingOperation(String screenName, String timedOperation) { final operation = _timedOperationsInProgress.remove(operationKey); assert( operation != null, - 'The operation cannot be cancelled because it does not exist.', + 'The operation $screenName.$timedOperation cannot be cancelled because it ' + 'does not exist.', ); } diff --git a/packages/devtools_app/lib/src/shared/config_specific/framework_initialize/_framework_initialize_web.dart b/packages/devtools_app/lib/src/shared/config_specific/framework_initialize/_framework_initialize_web.dart index f8fceb02bdd..4159bd58a46 100644 --- a/packages/devtools_app/lib/src/shared/config_specific/framework_initialize/_framework_initialize_web.dart +++ b/packages/devtools_app/lib/src/shared/config_specific/framework_initialize/_framework_initialize_web.dart @@ -32,9 +32,11 @@ Future initializePlatform() async { // Here, we try and initialize the connection between the DevTools web app and // its local server. DevTools can be launched without the server however, so // establishing this connection is a best-effort. + // TODO(kenz): investigate it we can remove the DevToolsServerConnection + // code in general. We do not appear to be using the SSE connection. final connection = await DevToolsServerConnection.connect(); if (connection != null) { - setGlobal(Storage, ServerConnectionStorage()); + setGlobal(Storage, server.ServerConnectionStorage()); } else { setGlobal(Storage, BrowserStorage()); } @@ -93,19 +95,6 @@ void _sendKeyPressToParent(KeyboardEvent event) { ); } -class ServerConnectionStorage implements Storage { - @override - Future getValue(String key) async { - final value = await server.getPreferenceValue(key); - return value == null ? null : '$value'; - } - - @override - Future setValue(String key, String value) async { - await server.setPreferenceValue(key, value); - } -} - class BrowserStorage implements Storage { @override Future getValue(String key) async { diff --git a/packages/devtools_app/lib/src/shared/preferences/preferences.dart b/packages/devtools_app/lib/src/shared/preferences/preferences.dart index c1379553d6a..9818fee8531 100644 --- a/packages/devtools_app/lib/src/shared/preferences/preferences.dart +++ b/packages/devtools_app/lib/src/shared/preferences/preferences.dart @@ -25,8 +25,8 @@ import '../utils/utils.dart'; part '_cpu_profiler_preferences.dart'; part '_extension_preferences.dart'; part '_inspector_preferences.dart'; -part '_memory_preferences.dart'; part '_logging_preferences.dart'; +part '_memory_preferences.dart'; part '_network_preferences.dart'; part '_performance_preferences.dart'; @@ -211,8 +211,17 @@ class PreferencesController extends DisposableController return; } + // Whether DevTools was run using the `dt serve --run-app` command, which + // runs DevTools in debug mode using `flutter run` and connects it to an + // instance of the DevTools server. + final usingDebugDevToolsServer = + (const String.fromEnvironment('debug_devtools_server')).isNotEmpty && + !kReleaseMode; final shouldEnableWasm = - (enabledFromStorage || enabledFromQueryParams) && kIsWeb; + (enabledFromStorage || enabledFromQueryParams) && + kIsWeb && + // Wasm cannot be enabled if DevTools was built using `flutter run`. + !usingDebugDevToolsServer; assert(kIsWasm == shouldEnableWasm); // This should be a no-op if the flutter_bootstrap.js logic set the // renderer properly, but we call this to be safe in case something went diff --git a/packages/devtools_app/lib/src/shared/server/server.dart b/packages/devtools_app/lib/src/shared/server/server.dart index e8e7d12d4b5..bffbfe63ed0 100644 --- a/packages/devtools_app/lib/src/shared/server/server.dart +++ b/packages/devtools_app/lib/src/shared/server/server.dart @@ -11,25 +11,49 @@ import 'package:devtools_shared/devtools_shared.dart'; import 'package:flutter/foundation.dart'; import 'package:http/http.dart'; import 'package:logging/logging.dart'; +import 'package:path/path.dart' as path; import '../development_helpers.dart'; +import '../globals.dart'; +import '../primitives/storage.dart'; import '../primitives/utils.dart'; part '_analytics_api.dart'; part '_app_size_api.dart'; part '_deep_links_api.dart'; +part '_dtd_api.dart'; part '_extensions_api.dart'; part '_preferences_api.dart'; part '_release_notes_api.dart'; part '_survey_api.dart'; -part '_dtd_api.dart'; final _log = Logger('devtools_server_client'); -// The DevTools server is only available in release mode right now. -// TODO(kenz): design a way to run the DevTools server and DevTools app together -// in debug mode. -bool get isDevToolsServerAvailable => kReleaseMode; +/// Whether the DevTools server is available. +/// +/// Since the DevTools server is a web server, it is only available for the +/// web platform. +/// +/// In `framework_initialize_web.dart`, we test the DevTools server connection +/// by pinging the server and checking the response. If this is successful, we +/// set the [storage] global to an instance of [ServerConnectionStorage]. +bool get isDevToolsServerAvailable => + kIsWeb && storage is ServerConnectionStorage; + +const _debugDevToolsServerFlag = 'debug_devtools_server'; + +String get devToolsServerUriAsString { + const debugDevToolsServerUriAsString = String.fromEnvironment( + _debugDevToolsServerFlag, + ); + // Ensure we only use the debug DevTools server URI in non-release + // builds. By running `dt serve --run-app`, an instance of DevTools ran + // with `flutter run` can be connected to the DevTools server on a + // different port. + return debugDevToolsServerUriAsString.isNotEmpty && !kReleaseMode + ? debugDevToolsServerUriAsString + : Uri.base.toString(); +} /// Helper to catch any server request which could fail. /// @@ -38,8 +62,10 @@ Future request(String url) async { Response? response; try { - _log.fine('requesting $url'); - response = await post(Uri.parse(url)); + // This will be the empty string if this environment declaration was not + // set using `--dart-define`. + const baseUri = String.fromEnvironment(_debugDevToolsServerFlag); + response = await post(Uri.parse(path.join(baseUri, url))); } catch (_) {} return response; @@ -107,3 +133,16 @@ extension ResponseExtension on Response { bool get statusForbidden => statusCode == 403; bool get statusError => statusCode == 500; } + +class ServerConnectionStorage implements Storage { + @override + Future getValue(String key) async { + final value = await getPreferenceValue(key); + return value == null ? null : '$value'; + } + + @override + Future setValue(String key, String value) async { + await setPreferenceValue(key, value); + } +} diff --git a/packages/devtools_app/lib/src/shared/server/server_api_client.dart b/packages/devtools_app/lib/src/shared/server/server_api_client.dart index 547143d7a01..a8a55ec3198 100644 --- a/packages/devtools_app/lib/src/shared/server/server_api_client.dart +++ b/packages/devtools_app/lib/src/shared/server/server_api_client.dart @@ -13,6 +13,7 @@ import 'package:logging/logging.dart'; import '../config_specific/notifications/notifications.dart'; import '../framework/framework_controller.dart'; import '../globals.dart'; +import 'server.dart'; final _log = Logger('lib/src/shared/server_api_client'); @@ -46,7 +47,8 @@ class DevToolsServerConnection { : baseUri.resolve('api/'); static Future connect() async { - final apiUri = apiUriFor(Uri.base); + final serverUri = Uri.parse(devToolsServerUriAsString); + final apiUri = apiUriFor(serverUri); final pingUri = apiUri.resolve('ping'); try { diff --git a/packages/devtools_app/web/flutter_bootstrap.js b/packages/devtools_app/web/flutter_bootstrap.js index b9a06485ea3..2836e262e52 100644 --- a/packages/devtools_app/web/flutter_bootstrap.js +++ b/packages/devtools_app/web/flutter_bootstrap.js @@ -28,6 +28,10 @@ const wasmQueryParameterKey = 'wasm'; // Calls the DevTools server API to read the user's wasm preference. async function getDevToolsWasmPreference() { + // Note: when the DevTools server is running on a different port than the + // DevTools web app, this request path will be incorrect and the request + // will fail. This is okay because DevTools cannot be built with WASM when + // running from `flutter run` anyway. const request = 'api/getPreferenceValue?key=experiment.wasm'; try { const response = await fetch(request); diff --git a/tool/lib/commands/serve.dart b/tool/lib/commands/serve.dart index b202b3ce401..02bdde57f90 100644 --- a/tool/lib/commands/serve.dart +++ b/tool/lib/commands/serve.dart @@ -2,6 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +import 'dart:async'; import 'dart:io'; import 'package:args/command_runner.dart'; @@ -13,6 +14,9 @@ import 'package:path/path.dart' as path; import 'shared.dart'; const _buildAppFlag = 'build-app'; +// TODO(kenz): consider adding a `dt run` command to simplify the dev workflow +// and reduce the cognitive load of remembering the args to pass to `dt serve`. +const _runAppFlag = 'run-app'; const _debugServerFlag = 'debug-server'; // TODO(https://github.com/flutter/devtools/issues/7232): Consider using @@ -32,6 +36,11 @@ const _serveWithDartSdkFlag = 'serve-with-dart-sdk'; /// to this command. All of the following commands are passed along to the /// `dt build` command. /// +/// If the [_runAppFlag] argument is passed (e.g. --run-app), then DevTools will +/// be run with `flutter run `instead of being built with `flutter build web`. +/// The debug instance of DevTools app running from Flutter Tool will be +/// connected to a locally running instance of the DevTools server. +/// /// If the [_debugServerFlag] argument is present, the DevTools server will be /// started with the `--observe` flag. This will allow you to debug and profile /// the server with a local VM service connection. By default, this will set @@ -67,7 +76,17 @@ class ServeCommand extends Command { defaultsTo: true, help: 'Whether to build the DevTools web app before starting the DevTools' - ' server.', + ' server. If --no-build-app is passed, the existing assets from' + ' devtools_app/build/web will be used.', + ) + ..addFlag( + _runAppFlag, + negatable: false, + defaultsTo: false, + help: + 'Whether to run the DevTools web app using `flutter run` instead' + ' of building it using `flutter build web` and serving the assets' + ' directly from the DevTools server.', ) ..addFlag( _debugServerFlag, @@ -124,6 +143,7 @@ class ServeCommand extends Command { final results = argResults!; final buildApp = results[_buildAppFlag] as bool; + final runApp = results[_runAppFlag] as bool; final debugServer = results[_debugServerFlag] as bool; final updateFlutter = results[BuildCommandArgs.updateFlutter.flagName] as bool; @@ -146,6 +166,7 @@ class ServeCommand extends Command { ..remove(BuildCommandArgs.noStripWasm.asArg()) ..remove(valueAsArg(_buildAppFlag)) ..remove(valueAsArg(_buildAppFlag, negated: true)) + ..remove(valueAsArg(_runAppFlag)) ..remove(valueAsArg(_debugServerFlag)) ..remove(BuildCommandArgs.pubGet.asArg()) ..remove(BuildCommandArgs.pubGet.asArg(negated: true)) @@ -181,7 +202,7 @@ class ServeCommand extends Command { 'web', ); - if (buildApp) { + if (buildApp && !runApp) { final process = await processManager.runProcess( CliCommand.tool([ 'build', @@ -206,7 +227,7 @@ class ServeCommand extends Command { ); logStatus('serving DevTools with a local devtools server...'); - final serveLocalScriptPath = path.join( + final ddsServeLocalScriptPath = path.join( 'pkg', 'dds', 'tool', @@ -214,18 +235,125 @@ class ServeCommand extends Command { 'serve_local.dart', ); + String? devToolsServerAddress; + // This call will not exit until explicitly terminated by the user. - await processManager.runProcess( + final serveLocalProcess = await startIndependentProcess( CliCommand.dart([ if (debugServer) ...['run', '--observe=0'], - serveLocalScriptPath, - '--devtools-build=$devToolsBuildLocation', + ddsServeLocalScriptPath, + if (runApp) + // When running DevTools via `flutter run`, the [flutterRunProcess] + // below will launch DevTools in the browser. + '--no-launch-browser' + else + // Only pass a build location if the server is serving the web assets + // (i.e. not when DevTools app is ran via `flutter run`). + '--devtools-build=$devToolsBuildLocation', // Pass any args that were provided to our script along. This allows IDEs // to pass `--machine` (etc.) so that this script can behave the same as // the "dart devtools" command for testing local DevTools/server changes. ...remainingArguments, ], sdkOverride: serveWithDartSdk), workingDirectory: localDartSdkLocation, + waitForOutput: 'Serving DevTools at ', + onWaitForOutputReceived: (line) { + // This will pull the server address from a String like: + // "Serving DevTools at http://127.0.0.1:9104.". + final regexp = RegExp( + r'http:\/\/\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}:\d+', + ); + final match = regexp.firstMatch(line); + if (match != null) { + devToolsServerAddress = match.group(0); + } + }, + ); + + Process? flutterRunProcess; + if (runApp) { + if (devToolsServerAddress == null) { + await _killProcess(serveLocalProcess); + throw Exception( + 'Cannot run DevTools and connect to the DevTools server because ' + 'devToolsServerAddress is null.', + ); + } + + logStatus('running DevTools'); + flutterRunProcess = await startIndependentProcess( + CliCommand.flutter([ + 'run', + '-d', + 'chrome', + // TODO(https://github.com/flutter/flutter/issues/160130): + // [flutterRunProcess] exits without the --verbose flag. + '--verbose', + // Add the trailing slash because this is what DevTools app expects. + '--dart-define=debug_devtools_server=$devToolsServerAddress/', + ]), + workingDirectory: repo.devtoolsAppDirectoryPath, + waitForOutput: + 'The Flutter DevTools debugger and profiler on Chrome is available at:', + ); + } + + await _waitForAndHandleExit( + serveLocalProcess: serveLocalProcess, + flutterRunProcess: flutterRunProcess, ); } + + Future _waitForAndHandleExit({ + required Process serveLocalProcess, + required Process? flutterRunProcess, + }) async { + final serveLocalProcessExited = Completer(); + final flutterRunProcessExited = Completer(); + unawaited( + serveLocalProcess.exitCode.then((code) { + serveLocalProcessExited.complete(code); + }), + ); + if (flutterRunProcess != null) { + unawaited( + flutterRunProcess.exitCode.then((code) { + flutterRunProcessExited.complete(code); + }), + ); + } + + await Future.any([ + serveLocalProcessExited.future, + flutterRunProcessExited.future, + ]); + + if (serveLocalProcessExited.isCompleted && + !flutterRunProcessExited.isCompleted && + flutterRunProcess != null) { + final exitCode = await serveLocalProcessExited.future; + logStatus( + 'Killing the flutterRunProcess because the serveLocalProcess has ' + 'exited with code $exitCode.', + ); + await _killProcess(flutterRunProcess); + } + + if (flutterRunProcessExited.isCompleted && + !serveLocalProcessExited.isCompleted) { + final exitCode = await flutterRunProcessExited.future; + logStatus( + 'Killing the serveLocalProcess because the flutterRunProcess has ' + 'exited with code $exitCode.', + ); + await _killProcess(serveLocalProcess); + } + } + + Future _killProcess(Process? process) async { + if (process != null) { + Process.killPid(process.pid, ProcessSignal.sigint); + await process.exitCode; + } + } } diff --git a/tool/lib/utils.dart b/tool/lib/utils.dart index f4f5812f1c3..766b3c1ab53 100644 --- a/tool/lib/utils.dart +++ b/tool/lib/utils.dart @@ -170,6 +170,52 @@ extension DevToolsProcessManagerExtension on ProcessManager { } } +Future startIndependentProcess( + CliCommand command, { + String? workingDirectory, + String? waitForOutput, + Duration waitForOutputTimeout = const Duration(minutes: 2), + void Function(String line)? onWaitForOutputReceived, +}) async { + final commandDisplay = '${workingDirectory ?? ''} > $command'; + print(commandDisplay); + final process = await Process.start( + command.exe, + command.args, + workingDirectory: workingDirectory, + ); + + if (waitForOutput != null) { + final completer = Completer(); + final stdoutSub = process.stdout.transform(utf8.decoder).listen((line) { + print('> [stdout] $line'); + if (line.contains(waitForOutput)) { + onWaitForOutputReceived?.call(line); + completer.complete(); + } + }); + final stderrSub = process.stderr.transform(utf8.decoder).listen((line) { + print('> [stderr] $line'); + if (line.contains(waitForOutput)) { + onWaitForOutputReceived?.call(line); + completer.complete(); + } + }); + await completer.future.timeout( + waitForOutputTimeout, + onTimeout: () { + throw Exception( + 'Expected output "$waitForOutput" not received before timeout.', + ); + }, + ); + await stdoutSub.cancel(); + await stderrSub.cancel(); + } + + return process; +} + String pathFromRepoRoot(String pathFromRoot) { return path.join(DevToolsRepo.getInstance().repoPath, pathFromRoot); }