Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,9 @@ class ExtensionView extends StatelessWidget {
const SizedBox(height: intermediateSpacing),
Expanded(
child: ValueListenableBuilder<ExtensionEnabledState>(
valueListenable: extensionService.enabledStateListenable(ext.name),
valueListenable: extensionService.enabledStateListenable(
ext.packageName,
),
builder: (context, activationState, _) {
if (activationState == ExtensionEnabledState.enabled) {
return KeepAliveWrapper(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ class EmbeddedExtensionHeader extends StatelessWidget {
@override
Widget build(BuildContext context) {
final theme = Theme.of(context);
final extensionName = ext.displayName;
final extensionPackage = ext.packageName;
return SizedBox(
width: double.infinity,
child: Wrap(
Expand All @@ -40,7 +40,7 @@ class EmbeddedExtensionHeader extends StatelessWidget {
padding: const EdgeInsets.only(left: borderPadding),
child: RichText(
text: TextSpan(
text: 'package:$extensionName extension',
text: 'package:$extensionPackage extension',
style: theme.regularTextStyle.copyWith(
fontWeight: FontWeight.bold,
),
Expand Down Expand Up @@ -96,7 +96,7 @@ class _ExtensionContextMenuButton extends StatelessWidget {
@override
Widget build(BuildContext context) {
return ValueListenableBuilder<ExtensionEnabledState>(
valueListenable: extensionService.enabledStateListenable(ext.displayName),
valueListenable: extensionService.enabledStateListenable(ext.packageName),
builder: (context, activationState, _) {
if (activationState != ExtensionEnabledState.enabled) {
return const SizedBox.shrink();
Expand Down Expand Up @@ -168,7 +168,10 @@ class DisableExtensionDialog extends StatelessWidget {
text: 'Are you sure you want to disable the ',
style: theme.regularTextStyle,
children: [
TextSpan(text: ext.displayName, style: theme.fixedFontStyle),
TextSpan(
text: 'package:${ext.packageName}',
style: theme.fixedFontStyle,
),
const TextSpan(text: ' extension?'),
],
),
Expand Down Expand Up @@ -233,7 +236,10 @@ class EnableExtensionPrompt extends StatelessWidget {
text: 'The ',
style: theme.regularTextStyle,
children: [
TextSpan(text: ext.name, style: theme.fixedFontStyle),
TextSpan(
text: 'package:${ext.packageName}',
style: theme.fixedFontStyle,
),
const TextSpan(
text:
' extension has not been enabled. Do you want to enable'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,12 +111,12 @@ class ExtensionService extends DisposableController
final _ignoredStaticExtensionsByHashCode = <int>{};

/// Returns the [ValueListenable] that stores the [ExtensionEnabledState] for
/// the DevTools Extension with [extensionName].
/// the DevTools Extension provided by [extensionPackageName].
ValueListenable<ExtensionEnabledState> enabledStateListenable(
String extensionName,
String extensionPackageName,
) {
return _extensionEnabledStates.putIfAbsent(
extensionName.toLowerCase(),
extensionPackageName.toLowerCase(),
() => ValueNotifier<ExtensionEnabledState>(ExtensionEnabledState.none),
);
}
Expand Down Expand Up @@ -232,7 +232,7 @@ class ExtensionService extends DisposableController
// not always be true for extensions that are not published on pub or
// extensions that do not follow best practices for naming.
final isRuntimeDuplicate = runtimeExtensions.any(
(ext) => ext.name == staticExtension.name,
(ext) => ext.packageName == staticExtension.packageName,
);
if (isRuntimeDuplicate) {
_log.fine(
Expand All @@ -256,9 +256,10 @@ class ExtensionService extends DisposableController
final stateFromOptionsFile = await server.extensionEnabledState(
devtoolsOptionsFileUri: extension.devtoolsOptionsUri,
extensionName: extension.name,
extensionPackage: extension.packageName,
);
final stateNotifier = _extensionEnabledStates.putIfAbsent(
extension.name,
extension.packageName.toLowerCase(),
() => ValueNotifier<ExtensionEnabledState>(stateFromOptionsFile),
);
stateNotifier.value = stateFromOptionsFile;
Expand Down Expand Up @@ -292,15 +293,14 @@ class ExtensionService extends DisposableController
// Set the enabled state for all matching extensions, even if some are
// marked as ignored due to being a duplicate. This ensures that
// devtools_options.yaml files are kept in sync across the project.
final allMatchingExtensions = [
...runtimeExtensions,
...staticExtensions,
].where((e) => e.name == extension.name);
final allMatchingExtensions = [...runtimeExtensions, ...staticExtensions]
.where((e) => e.packageName == extension.packageName);
await [
for (final ext in allMatchingExtensions)
server.extensionEnabledState(
devtoolsOptionsFileUri: ext.devtoolsOptionsUri,
extensionName: ext.name,
extensionPackage: ext.packageName,
enable: enable,
),
].wait;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,17 @@ void deduplicateExtensionsAndTakeLatest(
}) {
final deduped = <String>{};
for (final ext in extensions) {
if (deduped.contains(ext.name)) continue;
deduped.add(ext.name);
final dedupeKey = ext.packageName;
if (deduped.contains(dedupeKey)) continue;
deduped.add(dedupeKey);

// This includes [ext] itself.
final matchingExtensions = extensions.where((e) => e.name == ext.name);
final matchingExtensions = extensions.where(
(e) => e.packageName == ext.packageName,
);
if (matchingExtensions.length > 1) {
logger?.fine(
'detected duplicate $extensionType extensions for ${ext.name}',
'detected duplicate $extensionType extensions for package:${ext.packageName}',
);

// Ignore all matching extensions and then mark the [latest] as
Expand All @@ -45,7 +48,7 @@ void deduplicateExtensionsAndTakeLatest(
);
} else {
logger?.fine(
'no duplicates found for $extensionType extension ${ext.name}',
'no duplicates found for $extensionType extension package:${ext.packageName}',
);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -172,17 +172,17 @@ class ExtensionSetting extends StatelessWidget {
),
];
final theme = Theme.of(context);
final extensionName = extension.name.toLowerCase();
final packageName = extension.packageName.toLowerCase();
return ValueListenableBuilder(
valueListenable: extensionService.enabledStateListenable(extensionName),
valueListenable: extensionService.enabledStateListenable(packageName),
builder: (context, enabledState, _) {
return Padding(
padding: const EdgeInsets.only(bottom: denseSpacing),
child: Row(
mainAxisAlignment: MainAxisAlignment.spaceBetween,
children: [
Text(
'package:$extensionName',
'package:$packageName',
overflow: TextOverflow.ellipsis,
style: theme.fixedFontStyle,
),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

import 'package:devtools_shared/devtools_shared.dart';
import 'package:flutter/material.dart';
import 'package:meta/meta.dart';
import 'package:vm_service/vm_service.dart';

import '../../shared/primitives/utils.dart';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,11 +74,12 @@ Future<List<DevToolsExtensionConfig>> refreshAvailableExtensions(
Future<ExtensionEnabledState> extensionEnabledState({
required String devtoolsOptionsFileUri,
required String extensionName,
String? extensionPackage,
bool? enable,
}) async {
_log.fine(
'${enable != null ? 'setting' : 'getting'} extensionEnabledState for '
'$extensionName in options file ($devtoolsOptionsFileUri)',
'$extensionName (package: $extensionPackage) in options file ($devtoolsOptionsFileUri)',
);
if (debugDevToolsExtensions) {
return debugHandleExtensionEnabledState(
Expand All @@ -92,8 +93,8 @@ Future<ExtensionEnabledState> extensionEnabledState({
queryParameters: {
ExtensionsApi.devtoolsOptionsUriPropertyName: devtoolsOptionsFileUri,
ExtensionsApi.extensionNamePropertyName: extensionName,
if (enable != null)
ExtensionsApi.enabledStatePropertyName: enable.toString(),
ExtensionsApi.extensionPackagePropertyName: ?extensionPackage,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

when would we ever expect this to be null?

ExtensionsApi.enabledStatePropertyName: ?enable?.toString(),
Comment thread
johnpryan marked this conversation as resolved.
},
);
final resp = await request(uri.toString());
Expand Down
3 changes: 3 additions & 0 deletions packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,9 @@ TODO: Remove this section if there are not any updates.

* Hide the DevTools extensions menu button in single-screen embedded mode (`EmbedMode.embedOne`) on standard screens.
[#8507](https://github.com/flutter/devtools/issues/8507)
* Improved DevTools extension isolation by tracking the providing package name for
enablement, deduplication, and asset loading.
[#9965](https://github.com/flutter/devtools/pull/9965)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

incorrect pr link


## Advanced developer mode updates

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,34 +79,76 @@ void main() {
await tester.pumpWidget(wrap(Builder(builder: fooScreen.build)));
expect(find.byType(ExtensionView), findsOneWidget);
expect(find.byType(EmbeddedExtensionHeader), findsOneWidget);
expect(find.richTextContaining('package:foo extension'), findsOneWidget);
expect(
find.descendant(
of: find.byType(EmbeddedExtensionHeader),
matching: find.richTextContaining('package:foo extension'),
),
findsOneWidget,
);
expect(find.richTextContaining('(v1.0.0)'), findsOneWidget);
expect(find.richTextContaining('Report an issue'), findsOneWidget);
expect(_extensionContextMenuFinder, findsNothing);
expect(find.byType(EnableExtensionPrompt), findsOneWidget);
expect(
find.descendant(
of: find.byType(EnableExtensionPrompt),
matching: find.richTextContaining(
'The package:foo extension has not been enabled',
),
),
findsOneWidget,
);
expect(find.byType(EmbeddedExtensionView), findsNothing);

await tester.pumpWidget(wrap(Builder(builder: barScreen.build)));
expect(find.byType(ExtensionView), findsOneWidget);
expect(find.byType(EmbeddedExtensionHeader), findsOneWidget);
expect(find.richTextContaining('package:bar extension'), findsOneWidget);
expect(
find.descendant(
of: find.byType(EmbeddedExtensionHeader),
matching: find.richTextContaining('package:bar extension'),
),
findsOneWidget,
);
expect(find.richTextContaining('(v2.0.0)'), findsOneWidget);
expect(find.richTextContaining('Report an issue'), findsOneWidget);
expect(_extensionContextMenuFinder, findsNothing);
expect(find.byType(EnableExtensionPrompt), findsOneWidget);
expect(
find.descendant(
of: find.byType(EnableExtensionPrompt),
matching: find.richTextContaining(
'The package:bar extension has not been enabled',
),
),
findsOneWidget,
);
expect(find.byType(EmbeddedExtensionView), findsNothing);

await tester.pumpWidget(wrap(Builder(builder: providerScreen.build)));
expect(find.byType(ExtensionView), findsOneWidget);
expect(find.byType(EmbeddedExtensionHeader), findsOneWidget);
expect(
find.richTextContaining('package:provider extension'),
find.descendant(
of: find.byType(EmbeddedExtensionHeader),
matching: find.richTextContaining('package:provider extension'),
),
findsOneWidget,
);
expect(find.richTextContaining('(v3.0.0)'), findsOneWidget);
expect(find.richTextContaining('Report an issue'), findsOneWidget);
expect(_extensionContextMenuFinder, findsNothing);
expect(find.byType(EnableExtensionPrompt), findsOneWidget);
expect(
find.descendant(
of: find.byType(EnableExtensionPrompt),
matching: find.richTextContaining(
'The package:provider extension has not been enabled',
),
),
findsOneWidget,
);
expect(find.byType(EmbeddedExtensionView), findsNothing);
});

Expand Down Expand Up @@ -141,11 +183,26 @@ void main() {
await tester.pumpWidget(wrap(Builder(builder: fooScreen.build)));
expect(find.byType(ExtensionView), findsOneWidget);
expect(find.byType(EmbeddedExtensionHeader), findsOneWidget);
expect(find.richTextContaining('package:foo extension'), findsOneWidget);
expect(
find.descendant(
of: find.byType(EmbeddedExtensionHeader),
matching: find.richTextContaining('package:foo extension'),
),
findsOneWidget,
);
expect(find.richTextContaining('(v1.0.0)'), findsOneWidget);
expect(find.richTextContaining('Report an issue'), findsOneWidget);
expect(_extensionContextMenuFinder, findsNothing);
expect(find.byType(EnableExtensionPrompt), findsOneWidget);
expect(
find.descendant(
of: find.byType(EnableExtensionPrompt),
matching: find.richTextContaining(
'The package:foo extension has not been enabled',
),
),
findsOneWidget,
);
expect(find.byType(EmbeddedExtensionView), findsNothing);
});

Expand Down
Loading
Loading