Skip to content

Commit

Permalink
Fix project directory inference for inspector panel (#6857)
Browse files Browse the repository at this point in the history
Fixes #6841, #6853

Instead of using the top level widget in the widget tree to determine the project directory, we instead use the main isolate's root library.

For google3, we also have extra logic to strip out everything before `/google3`, and to only use top-level directory after `google3` directory (or the top-level directory after `third_party`)
  • Loading branch information
elliette committed Nov 30, 2023
1 parent 7a26142 commit bdfdeab
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 66 deletions.
Expand Up @@ -27,7 +27,10 @@ import 'object_group_api.dart';
import 'primitives/instance_ref.dart';
import 'primitives/source_location.dart';

const inspectorLibraryUri = 'package:flutter/src/widgets/widget_inspector.dart';
const _inspectorLibraryUri =
'package:flutter/src/widgets/widget_inspector.dart';
const _google3PathSegment = 'google3';
const _thirdPartyPathSegment = 'third_party';

abstract class InspectorServiceBase extends DisposableController
with AutoDisposeControllerMixin {
Expand Down Expand Up @@ -195,7 +198,7 @@ class InspectorService extends InspectorServiceBase {
: super(
clientInspectorName: 'WidgetInspectorService',
serviceExtensionPrefix: inspectorExtensionPrefix,
inspectorLibraryUri: inspectorLibraryUri,
inspectorLibraryUri: _inspectorLibraryUri,
evalIsolate:
serviceConnection.serviceManager.isolateManager.mainIsolate,
) {
Expand Down Expand Up @@ -352,9 +355,8 @@ class InspectorService extends InspectorServiceBase {
final libIndex = parts.lastIndexOf('lib');
final path = libIndex > 0 ? parts.sublist(0, libIndex) : parts;
// Special case handling of bazel packages.
final google3Index = path.lastIndexOf('google3');
if (google3Index != -1 && google3Index + 1 < path.length) {
var packageParts = path.sublist(google3Index + 1);
if (_isGoogle3Path(path)) {
var packageParts = _stripGoogle3(path);
// A well formed third_party dart package should be in a directory of
// the form
// third_party/dart/packageName (package:packageName)
Expand Down Expand Up @@ -496,60 +498,69 @@ class InspectorService extends InspectorServiceBase {
}

Future<String?> inferPubRootDirectoryIfNeededHelper() async {
final group = createObjectGroup('temp');
final root = await group.getRoot(FlutterTreeType.widget);

if (root == null) {
// No need to do anything as there isn't a valid tree (yet?).
await group.dispose();
return null;
}
List<RemoteDiagnosticsNode?> children = await root.children ?? [];

if (children.isEmpty) {
children = await group.getChildren(root.valueRef, false, null);
}

if (children.isEmpty) {
await group.dispose();
return null;
}
final path = children.first!.creationLocation?.path;
final path = await serviceConnection.rootLibraryForMainIsolate();
if (path == null) {
await group.dispose();
return null;
}
// TODO(jacobr): it would be nice to use Isolate.rootLib similar to how
// debugger.dart does but we are currently blocked by the
// --track-widget-creation transformer generating absolute paths instead of
// package:paths.
// Once https://github.com/flutter/flutter/issues/26615 is fixed we will be
// able to use package: paths. Temporarily all tools tracking widget
// locations will need to support both path formats.
// TODO(jacobr): Once https://github.com/flutter/flutter/issues/26615 is
// fixed we will be able to use package: paths. Temporarily all tools
// tracking widget locations will need to support both path formats.
// TODO(jacobr): use the list of loaded scripts to determine the appropriate
// package root directory given that the root script of this project is in
// this directory rather than guessing based on url structure.
final parts = path.split('/');
String? pubRootDirectory;
for (int i = parts.length - 1; i >= 0; i--) {
final part = parts[i];
if (part == 'lib' || part == 'web') {
pubRootDirectory = parts.sublist(0, i).join('/');
break;
}
// For google3, we grab the top-level directory in the google3 directory
// (e.g. /education), or the top-level directory in third_party (e.g.
// /third_party/dart):
if (_isGoogle3Path(parts)) {
pubRootDirectory = _pubRootDirectoryForGoogle3(parts);
} else {
final parts = path.split('/');

if (part == 'packages') {
pubRootDirectory = parts.sublist(0, i + 1).join('/');
break;
for (int i = parts.length - 1; i >= 0; i--) {
final part = parts[i];
if (part == 'lib' || part == 'web') {
pubRootDirectory = parts.sublist(0, i).join('/');
break;
}

if (part == 'packages') {
pubRootDirectory = parts.sublist(0, i + 1).join('/');
break;
}
}
}
pubRootDirectory ??= (parts..removeLast()).join('/');

await _addPubRootDirectories([pubRootDirectory]);
await group.dispose();
return pubRootDirectory;
}

bool _isGoogle3Path(List<String> pathParts) =>
pathParts.contains(_google3PathSegment);

List<String> _stripGoogle3(List<String> pathParts) {
final google3Index = pathParts.lastIndexOf(_google3PathSegment);
if (google3Index != -1 && google3Index + 1 < pathParts.length) {
return pathParts.sublist(google3Index + 1);
}
return pathParts;
}

String? _pubRootDirectoryForGoogle3(List<String> pathParts) {
final strippedParts = _stripGoogle3(pathParts);
if (strippedParts.isEmpty) return null;

final topLevelDirectory = strippedParts.first;
if (topLevelDirectory == _thirdPartyPathSegment &&
strippedParts.length >= 2) {
return '/${strippedParts.sublist(0, 2).join('/')}';
} else {
return '/${strippedParts.first}';
}
}

RemoteDiagnosticsNode? _currentSelection;

InspectorObjectGroupManager get _selectionGroups {
Expand Down
31 changes: 7 additions & 24 deletions packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md
@@ -1,36 +1,25 @@
This is draft for future release notes, that are going to land on
[the Flutter website](https://docs.flutter.dev/tools/devtools/release-notes).

# DevTools 2.28.3 release notes
# DevTools 2.28.4 release notes

The 2.28.3 release of the Dart and Flutter DevTools
The 2.28.4 release of the Dart and Flutter DevTools
includes the following changes among other general improvements.
To learn more about DevTools, check out the
[DevTools overview](https://docs.flutter.dev/tools/devtools/overview).

## General updates

* Added a link to the new "Dive in to DevTools" YouTube
[video](https://www.youtube.com/watch?v=_EYk-E29edo) in the bottom status bar. This
video provides a brief tutorial for each DevTools screen.
[#6554](https://github.com/flutter/devtools/pull/6554)

![Link to watch a DevTools tutorial video](images/watch_tutorial_link.png "Link to watch a DevTools tutorial video")

* Added a work around to fix copy button functionality in VSCode. [#6598](https://github.com/flutter/devtools/pull/6598)

* Enabled DevTools extensions when debugging a Dart entry point that is not
under `lib` (e.g. a unit test or integration test). Thanks to
[@bartekpacia](https://github.com/bartekpacia) for this change! -
[#6644](https://github.com/flutter/devtools/pull/6644)
TODO: Remove this section if there are not any general updates.

## Inspector updates

TODO: Remove this section if there are not any general updates.
* Fix bug where widgets owned by the Flutter framework were showing up in the widget tree view -
[6857](https://github.com/flutter/devtools/pull/6857)

## Performance updates

* Disable the Raster Stats tool for the Impeller backend since it is not supported. [#6616](https://github.com/flutter/devtools/pull/6616)
TODO: Remove this section if there are not any general updates.

## CPU profiler updates

Expand Down Expand Up @@ -58,10 +47,4 @@ TODO: Remove this section if there are not any general updates.

## VS Code Sidebar updates

* When using VS Code with a light theme, the embedded sidebar provided by DevTools will now also show in the light
theme [#6581](https://github.com/flutter/devtools/pull/6581)

## Full commit history

To find a complete list of changes in this release, check out the
[DevTools git log](https://github.com/flutter/devtools/tree/v2.29.0).
TODO: Remove this section if there are not any general updates.
Expand Up @@ -17,6 +17,10 @@ import '../test_infra/flutter_test_driver.dart' show FlutterRunConfiguration;
import '../test_infra/flutter_test_environment.dart';
import '../test_infra/matchers/matchers.dart';

// TODO(elliette): Add testing that project directories can be inferred from
// google3-paths. This will require mocking the main isolate so that we can
// change the root library during testing instead of using the
// LiveTestWidgetsFlutterBinding.
void main() {
initializeLiveTestWidgetsFlutterBindingWithAssets();

Expand Down Expand Up @@ -82,6 +86,9 @@ void main() {
await env.tearDownEnvironment(force: true);
});

// TODO(elliette): Figure out why this didn't catch
// https://github.com/flutter/devtools/issues/6841 and fix so that it
// catches future regressions.
test('can be inferred', () async {
await env.setupEnvironment();
final inspectorServiceLocal = inspectorService!;
Expand Down

0 comments on commit bdfdeab

Please sign in to comment.