diff --git a/packages/devtools_app/lib/main.dart b/packages/devtools_app/lib/main.dart index b860c253911..013bd8c9d2a 100644 --- a/packages/devtools_app/lib/main.dart +++ b/packages/devtools_app/lib/main.dart @@ -9,8 +9,11 @@ import 'src/analytics/analytics_controller.dart'; import 'src/app.dart'; import 'src/config_specific/framework_initialize/framework_initialize.dart'; import 'src/config_specific/ide_theme/ide_theme.dart'; +import 'src/config_specific/url/url.dart'; +import 'src/config_specific/url_strategy/url_strategy.dart'; import 'src/extension_points/extensions_base.dart'; import 'src/extension_points/extensions_external.dart'; +import 'src/primitives/url_utils.dart'; import 'src/screens/debugger/syntax_highlighter.dart'; import 'src/screens/provider/riverpod_error_logger_observer.dart'; import 'src/shared/app_error_handling.dart'; @@ -18,6 +21,12 @@ import 'src/shared/globals.dart'; import 'src/shared/preferences.dart'; void main() async { + // Before switching to URL path strategy, check if this URL is in the legacy + // fragment format and redirect if necessary. + if (_handleLegacyUrl()) return; + + usePathUrlStrategy(); + // Initialize the framework before we do anything else, otherwise the // StorageController won't be initialized and preferences won't be loaded. await initializeFramework(); @@ -44,3 +53,21 @@ void main() async { ); }); } + +/// Checks if the request is for a legacy URL and if so, redirects to the new +/// equivalent. +/// +/// Returns `true` if a redirect was performed, in which case normal app +/// initialization should be skipped. +bool _handleLegacyUrl() { + final url = getWebUrl(); + if (url == null) return false; + + final newUrl = mapLegacyUrl(url); + if (newUrl != null) { + webRedirect(newUrl); + return true; + } + + return false; +} diff --git a/packages/devtools_app/lib/src/analytics/_analytics_web.dart b/packages/devtools_app/lib/src/analytics/_analytics_web.dart index 2029cdf364b..1a23009e3e4 100644 --- a/packages/devtools_app/lib/src/analytics/_analytics_web.dart +++ b/packages/devtools_app/lib/src/analytics/_analytics_web.dart @@ -17,6 +17,7 @@ import '../app.dart'; import '../config_specific/logger/logger.dart'; import '../config_specific/server/server.dart' as server; import '../config_specific/url/url.dart'; +import '../primitives/url_utils.dart'; import '../screens/performance/performance_screen.dart'; import '../screens/profiler/profiler_screen.dart'; import '../shared/globals.dart'; @@ -732,29 +733,8 @@ Map generateSurveyQueryParameters() { const fromKey = 'From'; const internalKey = 'Internal'; - // TODO(https://github.com/flutter/devtools/issues/2475): fix url structure - // Parsing the url via Uri.parse returns an incorrect value for fragment. - // Grab the fragment value manually. The url will be of the form - // http://127.0.0.1:9100/#/timeline?ide=IntelliJ-IDEA&uri=..., and we want the - // part equal to '/timeline'. - final url = window.location.toString(); - const fromValuePrefix = '#/'; - final startIndex = url.indexOf(fromValuePrefix); - // Use the last index because the url can be of the form - // 'http://127.0.0.1:9103/?#/?' and we want to be referencing the last '?' - // character. - final endIndex = url.lastIndexOf('?'); - var fromPage = ''; - try { - fromPage = url.substring( - startIndex + fromValuePrefix.length, - endIndex, - ); - } catch (_) { - // Fail gracefully if finding the [fromPage] value throws an exception. - } - final internalValue = (!isExternalBuild).toString(); + final fromPage = extractCurrentPageFromUrl(window.location.toString()); return { ideKey: ideLaunched, diff --git a/packages/devtools_app/lib/src/config_specific/url/_url_stub.dart b/packages/devtools_app/lib/src/config_specific/url/_url_stub.dart index b9bc9d01c06..09e684aca17 100644 --- a/packages/devtools_app/lib/src/config_specific/url/_url_stub.dart +++ b/packages/devtools_app/lib/src/config_specific/url/_url_stub.dart @@ -3,3 +3,13 @@ // in the LICENSE file. Map loadQueryParams() => {}; + +/// Gets the URL from the browser. +/// +/// Returns null for non-web platforms. +String? getWebUrl() => null; + +/// Performs a web redirect using window.location.replace(). +/// +/// No-op for non-web platforms. +void webRedirect(String url) {} diff --git a/packages/devtools_app/lib/src/config_specific/url/_url_web.dart b/packages/devtools_app/lib/src/config_specific/url/_url_web.dart index d9bad7f4a9e..ac631de2b85 100644 --- a/packages/devtools_app/lib/src/config_specific/url/_url_web.dart +++ b/packages/devtools_app/lib/src/config_specific/url/_url_web.dart @@ -9,3 +9,9 @@ import '../../primitives/utils.dart'; Map loadQueryParams() { return devToolsQueryParams(window.location.toString()); } + +String? getWebUrl() => window.location.toString(); + +void webRedirect(String url) { + window.location.replace(url); +} diff --git a/packages/devtools_app/lib/src/config_specific/url_strategy/noop.dart b/packages/devtools_app/lib/src/config_specific/url_strategy/noop.dart new file mode 100644 index 00000000000..ecf8dcf1965 --- /dev/null +++ b/packages/devtools_app/lib/src/config_specific/url_strategy/noop.dart @@ -0,0 +1,3 @@ +void usePathUrlStrategy() { + // No-op. URL Strategies are only supported for web. +} diff --git a/packages/devtools_app/lib/src/config_specific/url_strategy/url_strategy.dart b/packages/devtools_app/lib/src/config_specific/url_strategy/url_strategy.dart new file mode 100644 index 00000000000..5dde51934f5 --- /dev/null +++ b/packages/devtools_app/lib/src/config_specific/url_strategy/url_strategy.dart @@ -0,0 +1 @@ +export 'noop.dart' if (dart.library.html) 'web.dart'; diff --git a/packages/devtools_app/lib/src/config_specific/url_strategy/web.dart b/packages/devtools_app/lib/src/config_specific/url_strategy/web.dart new file mode 100644 index 00000000000..386533b1850 --- /dev/null +++ b/packages/devtools_app/lib/src/config_specific/url_strategy/web.dart @@ -0,0 +1,5 @@ +import 'package:flutter_web_plugins/flutter_web_plugins.dart'; + +void usePathUrlStrategy() { + setUrlStrategy(PathUrlStrategy()); +} diff --git a/packages/devtools_app/lib/src/primitives/url_utils.dart b/packages/devtools_app/lib/src/primitives/url_utils.dart new file mode 100644 index 00000000000..7c473566b76 --- /dev/null +++ b/packages/devtools_app/lib/src/primitives/url_utils.dart @@ -0,0 +1,50 @@ +// Copyright 2019 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +/// Extracts the current DevTools page from the given [url]. +String extractCurrentPageFromUrl(String url) { + // The url can be in one of two forms: + // - /page?uri=xxx + // - /?page=xxx&uri=yyy (original formats IDEs may use) + // Use the path in preference to &page= as it's the one DevTools is updating + final uri = Uri.parse(url); + return uri.path == '/' + ? uri.queryParameters['page'] ?? '' + : uri.path.substring(1); +} + +/// Maps DevTools URLs in the original fragment format onto the equivalent URLs +/// in the new URL format. +/// +/// Returns `null` if [url] is not a legacy URL. +String? mapLegacyUrl(String url) { + final uri = Uri.parse(url); + // Old formats include: + // http://localhost:123/#/inspector?uri=ws://... + // http://localhost:123/#/?page=inspector&uri=ws://... + final isRootRequest = uri.path == '/' || uri.path.endsWith('/devtools/'); + if (isRootRequest && uri.fragment.isNotEmpty) { + final basePath = uri.path; + // Convert the URL by removing the fragment separator. + final newUrl = url + // Handle localhost:123/#/inspector?uri=xxx + .replaceFirst('/#/', '/') + // Handle localhost:123/#?page=inspector&uri=xxx + .replaceFirst('/#', ''); + + // Move page names from the querystring into the path. + var newUri = Uri.parse(newUrl); + final page = newUri.queryParameters['page']; + if (newUri.path == basePath && page != null) { + final newParams = {...newUri.queryParameters}..remove('page'); + newUri = newUri.replace( + path: '$basePath$page', + queryParameters: newParams, + ); + } + return newUri.toString(); + } + + return null; +} diff --git a/packages/devtools_app/lib/src/shared/routing.dart b/packages/devtools_app/lib/src/shared/routing.dart index fc5edfb1eea..75928d75bc0 100644 --- a/packages/devtools_app/lib/src/shared/routing.dart +++ b/packages/devtools_app/lib/src/shared/routing.dart @@ -139,7 +139,7 @@ class DevToolsRouterDelegate extends RouterDelegate /// If page and args would be the same, does nothing. /// Existing arguments (for example &uri=) will be preserved unless /// overwritten by [argUpdates]. - void navigateIfNotCurrent(String page, [Map? argUpdates]) { + void navigateIfNotCurrent(String page, [Map? argUpdates]) { final pageChanged = page != currentConfiguration!.page; final argsChanged = _changesArgs(argUpdates); if (!pageChanged && !argsChanged) { @@ -211,7 +211,7 @@ class DevToolsRouterDelegate extends RouterDelegate /// Checks whether applying [changes] over the current routes args will result /// in any changes. - bool _changesArgs(Map? changes) => !mapEquals( + bool _changesArgs(Map? changes) => !mapEquals( {...currentConfiguration!.args, ...?changes}, {...currentConfiguration!.args}, ); diff --git a/packages/devtools_app/lib/src/shared/scaffold.dart b/packages/devtools_app/lib/src/shared/scaffold.dart index 2faf314239f..f5a42166c03 100644 --- a/packages/devtools_app/lib/src/shared/scaffold.dart +++ b/packages/devtools_app/lib/src/shared/scaffold.dart @@ -215,21 +215,27 @@ class DevToolsScaffoldState extends State // Clear error count when navigating to a screen. serviceManager.errorBadgeManager.clearErrors(screen.screenId); - // If the tab index is 0 and the current route has no page ID (eg. we're - // at the URL /?uri= with no page ID), those are equivalent pages but - // navigateIfNotCurrent does not know that and will try to navigate, so - // skip that here. - final routerDelegate = DevToolsRouterDelegate.of(context); - if (_tabController!.index == 0 && - (routerDelegate.currentConfiguration!.page.isEmpty)) { - return; - } - // Update routing with the change. + final routerDelegate = DevToolsRouterDelegate.of(context); routerDelegate.navigateIfNotCurrent(screen.screenId); } }); + // If we had no explicit page, we want to write one into the URL but + // without triggering a navigation. Since we can't nagivate during a build + // we have to wrap this in `Future.microtask`. + if (widget.page == null && _currentScreen is! SimpleScreen) { + WidgetsBinding.instance.addPostFrameCallback((_) { + final routerDelegate = DevToolsRouterDelegate.of(context); + Router.neglect(context, () { + routerDelegate.navigateIfNotCurrent( + _currentScreen.screenId, + routerDelegate.currentConfiguration?.args, + ); + }); + }); + } + // Broadcast the initial page. frameworkController.notifyPageChange( PageChangeEvent(_currentScreen.screenId, widget.embed), diff --git a/packages/devtools_app/test/url_utils_test.dart b/packages/devtools_app/test/url_utils_test.dart new file mode 100644 index 00000000000..ad13418ea77 --- /dev/null +++ b/packages/devtools_app/test/url_utils_test.dart @@ -0,0 +1,80 @@ +// Copyright 2019 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'package:devtools_app/src/primitives/url_utils.dart'; +import 'package:flutter_test/flutter_test.dart'; + +void main() { + group('url utils', () { + group('extractCurrentPageFromUrl', () { + test('parses the current page from the path', () { + final page = + extractCurrentPageFromUrl('http://localhost:9000/inspector?uri=x'); + expect(page, 'inspector'); + }); + + test('parses the current page from the query string', () { + final page = extractCurrentPageFromUrl( + 'http://localhost:9000/?uri=x&page=inspector&theme=dark', + ); + expect(page, 'inspector'); + }); + + test( + 'parses the current page from the path even if query string is populated', + () { + final page = extractCurrentPageFromUrl( + 'http://localhost:9000/memory?uri=x&page=inspector&theme=dark', + ); + expect(page, 'memory'); + }); + }); + + group('mapLegacyUrl', () { + for (final prefix in [ + 'http://localhost:123', + 'http://localhost:123/authToken=/devtools' + ]) { + group(' with $prefix prefix', () { + test('does not map new-style URLs', () { + expect(mapLegacyUrl('$prefix'), isNull); + expect(mapLegacyUrl('$prefix/'), isNull); + expect(mapLegacyUrl('$prefix/foo?uri=ws://foo'), isNull); + expect(mapLegacyUrl('$prefix?uri=ws://foo'), isNull); + expect(mapLegacyUrl('$prefix/?uri=ws://foo'), isNull); + expect(mapLegacyUrl('$prefix/?uri=ws://foo#'), isNull); + }); + + test('maps legacy URIs with page names in path', () { + expect( + mapLegacyUrl('$prefix/#/inspector?foo=bar'), + '$prefix/inspector?foo=bar', + ); + }); + + test('maps legacy URIs with page names in querystring', () { + expect( + mapLegacyUrl('$prefix/#/?page=inspector&foo=bar'), + '$prefix/inspector?foo=bar', + ); + }); + + test('maps legacy URIs with no page names', () { + expect( + mapLegacyUrl('$prefix/#/?foo=bar'), + '$prefix/?foo=bar', + ); + }); + }); + } + }); + }); +} + +const dartSdkUrl = + 'org-dartlang-sdk:///third_party/dart/sdk/lib/async/zone.dart'; +const flutterUrl = + 'file:///path/to/flutter/packages/flutter/lib/src/widgets/binding.dart'; +const flutterUrlFromNonFlutterDir = + 'file:///path/to/non-flutter/packages/flutter/lib/src/widgets/binding.dart'; diff --git a/packages/devtools_app/web/index.html b/packages/devtools_app/web/index.html index 13601bbd5c3..8f71eb7f569 100644 --- a/packages/devtools_app/web/index.html +++ b/packages/devtools_app/web/index.html @@ -10,6 +10,8 @@ + + @@ -41,18 +43,6 @@ if (!supportsES6Classes()) { window.location.href = '/unsupported-browser.html'; } - - // Handle URLs that pass all variables directly on a querystring without - // the fragment (for ex. VS Code while it has some encoding bugs preventing - // building the correct URLs using fragments - // https://github.com/microsoft/vscode/issues/85930). - if (window.location.search && window.location.search.length > 1) { - // Ensure each component is encoded, because if the URI contains / slashes - // Flutter will split on them and try to push multiple routes. - const params = new URLSearchParams(unescape(window.location.search)); - params.forEach(function(v, k) { params.set(k, encodeURIComponent(v)) }); - window.location.replace(window.location.origin + '/#/?' + params.toString()); - }