Skip to content

Return null from RootInfo.package for non-package: root libraries - #10033

Open
srawlins wants to merge 3 commits into
flutter:masterfrom
srawlins:fix-6734
Open

srawlins wants to merge 3 commits into
flutter:masterfrom
srawlins:fix-6734

Conversation

@srawlins

@srawlins srawlins commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #6734

Summary

RootInfo.package now returns null when the root library is not a package: URI. Before, it returned everything up to the first /, so the root library file:///app/bin/main.dart of a Dart CLI app produced the "package" file:. That is not a package, and it is what #6734 reported.

Changing RootInfo is a one-line fix, but it was not safe on its own: the Memory screen depended on the old behavior. This PR also makes the Memory screen keep behaving exactly as it did before.

Why this was more than a one-line fix

The Memory screen is the only place in this repo that reads RootInfo.package, and it does not treat the value as a package name. It uses it as a prefix: HeapClassName.classType classifies a class as a project class (ClassType.rootPackage) when its library URI starts with the prefix. For a file: root library, the old value file: made classes from file: libraries count as project classes.

Before Only the RootInfo change This PR
RootInfo.package for file:///app/bin/main.dart file: null null
Root package prefix used by the Memory screen file: null (crash) file:
  • MemoryController crashed during initialization, at rootInfoNow().package!. This is what CI caught.
  • Removing the ! would have been worse, because it would fail silently. The app's own classes would be classified as runtime classes instead of project classes. That turns off live evaluation and "drop to console" for them (LiveClassSampler.isEvalEnabled), changes their icons and what the $project and $runtime filters match, and makes exported snapshots impossible to import again (DiffPaneController.fromJson reads the root package with as String).
  • HeapClassName caches its ClassType the first time it is asked, so every call site must compute the root package the same way. Patching only the crashing line would not be enough.

So RootInfo.package stays strict, as #6734 asks, and the Memory screen gets RootInfoMemoryExtension.rootPackagePrefix, which is the old computation moved out of RootInfo. For every root library it returns exactly what RootInfo.package returned before this PR, and all five Memory screen call sites use it, so the Memory screen behaves as it does on master. It lives in devtools_app rather than on RootInfo because it is a Memory screen heuristic, not something to add to the public API of a published package.

Why only some tests noticed

The first version of this PR only changed RootInfo and its tests. The unit and widget tests passed, and only two CI jobs failed: the dart-cli integration jobs (dart2js and dart2wasm).

  • connect to app and switch tabs connects DevTools to a real app and visits every screen, including Memory, which initializes MemoryController. It runs against a Flutter app, a Flutter web app, and a Dart CLI app.
  • The Dart CLI app is empty_app.dart, started with dart run, so its root library is a file: URI. The Flutter apps have package: root libraries, so package! never failed for them.
  • The unit and widget tests use FakeServiceManager, whose rootInfoNow() always returned package:myPackage/myPackage.dart. No test below the integration level ever had a non-package: root library.

Changes

  • devtools_app_shared: RootInfo.package is null unless the root library is a package: URI. RootInfo now has doc comments, and CHANGELOG.md states the old and new values. Because RootInfo is exported from package:devtools_app_shared/service.dart, this is observable by extension authors, which is the point of Accessing the package details seems to return invalid data. #6734.
  • devtools_app: MemoryController, LiveClassSampler, the profile table, and both diff tables use rootPackagePrefix.
  • devtools_test: FakeServiceManager, FakeServiceConnectionManager, and MemoryDefaultScene.setUp accept an optional rootInfo, so a test can simulate a file: root. This is deliberately separate from rootLibrary: two InspectorPreferencesController tests build a storage key from the fake's fixed package:myPackage root, and making rootLibrary flow into rootInfoNow() broke them.

Tests

  • isolate_state_test.dart: RootInfo for package:, file:, dart:, and null libraries.
  • memory_controller_test.dart: the controller's root package for package: and file: roots. The file: case fails with the same null check failure as CI on the first version of this PR, and passes now.
  • memory_utils_test.dart: rootPackagePrefix for several root libraries, and that classes of a file: root library are classified as project classes.

The two dart-cli integration jobs that failed on the first version pass again. No release note is needed: the only user-visible change is for extension authors, and it is in the devtools_app_shared CHANGELOG (release-notes-not-required).

This is just a simple fix where we return null of a library URI is not a `package:` URI.

Fixes flutter#6734

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates RootInfo in isolate_state.dart to return null for the package property when the library URI is not a package: URI (e.g., file: or dart: URIs). It also adds comprehensive unit tests covering these scenarios and updates the CHANGELOG. No review comments were provided, and the implementation is correct and well-tested, so there is no further feedback.

@srawlins srawlins changed the title Avoid returning invalid package data from service manager. Return null from RootInfo.package for non-package: root libraries Oct 9, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Accessing the package details seems to return invalid data.

1 participant