Skip to content

Commit

Permalink
Fix routing issue when refreshing DevTools (flutter#6028)
Browse files Browse the repository at this point in the history
  • Loading branch information
@kenzieschmoll
kenzieschmoll committed Jul 14, 2023
1 parent 5b08bf3 commit 3f8b720
Show file tree
Hide file tree
Showing 6 changed files with 57 additions and 63 deletions.
30 changes: 4 additions & 26 deletions packages/devtools_app/lib/src/app.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,9 +92,6 @@ class DevToolsAppState extends State<DevToolsApp> with AutoDisposeMixin {
bool get isDarkThemeEnabled => _isDarkThemeEnabled;
bool _isDarkThemeEnabled = true;

bool get vmDeveloperModeEnabled => _vmDeveloperModeEnabled;
bool _vmDeveloperModeEnabled = false;

bool get denseModeEnabled => _denseModeEnabled;
bool _denseModeEnabled = false;

Expand Down Expand Up @@ -131,13 +128,6 @@ class DevToolsAppState extends State<DevToolsApp> with AutoDisposeMixin {
});
});

_vmDeveloperModeEnabled = preferences.vmDeveloperModeEnabled.value;
addAutoDisposeListener(preferences.vmDeveloperModeEnabled, () {
setState(() {
_vmDeveloperModeEnabled = preferences.vmDeveloperModeEnabled.value;
});
});

_denseModeEnabled = preferences.denseModeEnabled.value;
addAutoDisposeListener(preferences.denseModeEnabled, () {
setState(() {
Expand Down Expand Up @@ -220,11 +210,6 @@ class DevToolsAppState extends State<DevToolsApp> with AutoDisposeMixin {
page = params['page'];
}

final screens = _visibleScreens()
.where((p) => embed && page != null ? p.screenId == page : true)
.where((p) => !hide.contains(p.screenId))
.toList();

final connectedToVmService =
vmServiceUri != null && vmServiceUri.isNotEmpty;

Expand All @@ -234,6 +219,10 @@ class DevToolsAppState extends State<DevToolsApp> with AutoDisposeMixin {
return ValueListenableBuilder<bool>(
valueListenable: preferences.vmDeveloperModeEnabled,
builder: (_, __, child) {
final screens = _visibleScreens()
.where((p) => embed && page != null ? p.screenId == page : true)
.where((p) => !hide.contains(p.screenId))
.toList();
return MultiProvider(
providers: _providedControllers(),
child: DevToolsScaffold(
Expand Down Expand Up @@ -282,17 +271,6 @@ class DevToolsAppState extends State<DevToolsApp> with AutoDisposeMixin {
),
);
},
appSizeScreenId: (_, __, args, ____) {
final embed = isEmbedded(args);
return DevToolsScaffold.withChild(
key: const Key('appsize'),
embed: embed,
child: MultiProvider(
providers: _providedControllers(),
child: const AppSizeBody(),
),
);
},
if (FeatureFlags.memoryAnalysis)
memoryAnalysisScreenId: (_, __, args, ____) {
final embed = isEmbedded(args);
Expand Down
3 changes: 1 addition & 2 deletions packages/devtools_app/lib/src/screens/inspector/inspector_screen.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import '../../service/service_extensions.dart' as extensions;
import '../../shared/analytics/analytics.dart' as ga;
import '../../shared/analytics/constants.dart' as gac;
import '../../shared/common_widgets.dart';
import '../../shared/connected_app.dart';
import '../../shared/console/eval/inspector_tree.dart';
import '../../shared/dialogs.dart';
import '../../shared/editable_list.dart';
Expand All @@ -34,7 +33,7 @@ class InspectorScreen extends Screen {
InspectorScreen()
: super.conditional(
id: id,
requiresLibrary: flutterLibraryUri,
requiresFlutter: true,
requiresDebugBuild: true,
title: ScreenMetaData.inspector.title,
icon: ScreenMetaData.inspector.icon,
Expand Down
19 changes: 17 additions & 2 deletions packages/devtools_app/lib/src/service/service_manager.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,8 @@ class ServiceConnectionManager {
final ValueNotifier<ConnectedState> _connectedState =
ValueNotifier(const ConnectedState(false));

// TODO(kenz): try to replace all uses of this stream with a listener to the
// [connectedState] ValueListenable.
Stream<VmServiceWrapper> get onConnectionAvailable =>
_connectionAvailableController.stream;

Expand Down Expand Up @@ -305,8 +307,6 @@ class ServiceConnectionManager {
return;
}

_connectedState.value = const ConnectedState(true);

final isolates = vm?.isolatesForDevToolsMode() ?? <IsolateRef>[];
await isolateManager.init(isolates);
if (service != this.service) {
Expand All @@ -332,6 +332,7 @@ class ServiceConnectionManager {
}

_connectionAvailableController.add(service);
_connectedState.value = const ConnectedState(true);
}

void manuallyDisconnect() {
Expand Down Expand Up @@ -566,4 +567,18 @@ class ConnectedState {

/// Whether this [ConnectedState] was manually initiated by the user.
final bool userInitiatedConnectionState;

@override
bool operator ==(Object? other) {
return other is ConnectedState &&
other.connected == connected &&
other.userInitiatedConnectionState == userInitiatedConnectionState;
}

@override
int get hashCode => Object.hash(connected, userInitiatedConnectionState);

@override
String toString() =>
'ConnectedState(connected: $connected, userInitiated: $userInitiatedConnectionState)';
}
6 changes: 0 additions & 6 deletions packages/devtools_app/lib/src/shared/routing.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,6 @@ import 'globals.dart';
import 'primitives/auto_dispose.dart';
import 'primitives/utils.dart';

/// The page ID (used in routing) for the standalone app-size page.
///
/// This must be different to the AppSizeScreen ID which is also used in routing when
/// cnnected to a VM to ensure they have unique URLs.
const appSizeScreenId = 'appsize';

const memoryAnalysisScreenId = 'memoryanalysis';

const homeScreenId = '';
Expand Down
11 changes: 11 additions & 0 deletions packages/devtools_app/lib/src/shared/screen.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ abstract class Screen {
this.requiresLibrary,
this.requiresConnection = true,
this.requiresDartVm = false,
this.requiresFlutter = false,
this.requiresDebugBuild = false,
this.requiresVmDeveloperMode = false,
this.worksOffline = false,
Expand All @@ -68,6 +69,7 @@ abstract class Screen {
String? requiresLibrary,
bool requiresConnection = true,
bool requiresDartVm = false,
bool requiresFlutter = false,
bool requiresDebugBuild = false,
bool requiresVmDeveloperMode = false,
bool worksOffline = false,
Expand All @@ -82,6 +84,7 @@ abstract class Screen {
requiresLibrary: requiresLibrary,
requiresConnection: requiresConnection,
requiresDartVm: requiresDartVm,
requiresFlutter: requiresFlutter,
requiresDebugBuild: requiresDebugBuild,
requiresVmDeveloperMode: requiresVmDeveloperMode,
worksOffline: worksOffline,
Expand Down Expand Up @@ -142,6 +145,9 @@ abstract class Screen {
/// Whether this screen should only be included when the app is running on the Dart VM.
final bool requiresDartVm;

/// Whether this screen should only be included when the app is a Flutter app.
final bool requiresFlutter;

/// Whether this screen should only be included when the app is debuggable.
final bool requiresDebugBuild;

Expand Down Expand Up @@ -296,6 +302,11 @@ bool shouldShowScreen(Screen screen) {
return false;
}
}
if (screen.requiresFlutter &&
serviceManager.connectedApp!.isFlutterAppNow == false) {
_log.finest('screen requires Flutter: returning false');
return false;
}
if (screen.requiresDebugBuild) {
if (serviceManager.connectedApp!.isProfileBuildNow == true) {
_log.finest('screen requires debug build: returning false');
Expand Down
51 changes: 24 additions & 27 deletions packages/devtools_app/test/shared/initializer_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import 'package:mockito/mockito.dart';

void main() {
group('Initializer', () {
late MaterialApp app;
const Key initializedKey = Key('initialized');
setUp(() {
final serviceManager = FakeServiceManager();
Expand All @@ -22,17 +21,20 @@ void main() {
setGlobal(ServiceConnectionManager, serviceManager);
setGlobal(FrameworkController, FrameworkController());
setGlobal(OfflineModeController, OfflineModeController());
setGlobal(IdeTheme, IdeTheme());
});

app = MaterialApp(
initialRoute: '/init',
routes: {
'/init': (_) => Initializer(
url: null,
builder: (_) => const SizedBox(key: initializedKey),
),
},
Future<void> pumpInitializer(WidgetTester tester) async {
await tester.pumpWidget(
wrap(
Initializer(
url: null,
builder: (_) => const SizedBox(key: initializedKey),
),
),
);
});
await tester.pumpAndSettle();
}

testWidgets(
'shows disconnected overlay if not connected',
Expand All @@ -43,8 +45,7 @@ void main() {
hasConnection: false,
),
);

await tester.pumpFrames(app, const Duration(milliseconds: 100));
await pumpInitializer(tester);
expect(find.text('Disconnected'), findsOneWidget);
},
);
Expand All @@ -56,15 +57,16 @@ void main() {
setGlobal(ServiceConnectionManager, serviceManager);

// Expect standard connected state.
await tester.pumpFrames(app, const Duration(milliseconds: 100));
serviceManager.changeState(true);
await pumpInitializer(tester);
expect(find.byKey(initializedKey), findsOneWidget);
expect(find.text('Disconnected'), findsNothing);

// Trigger a disconnect.
serviceManager.changeState(false);
await tester.pumpAndSettle(const Duration(microseconds: 1000));

// Expect Disconnected overlay.
await tester.pumpFrames(app, const Duration(milliseconds: 100));
expect(find.text('Disconnected'), findsOneWidget);
},
);
Expand All @@ -75,28 +77,23 @@ void main() {
final serviceManager = FakeServiceManager();
setGlobal(ServiceConnectionManager, serviceManager);

// Expect standard connected state.
serviceManager.changeState(true);
await pumpInitializer(tester);
expect(find.byKey(initializedKey), findsOneWidget);
expect(find.text('Disconnected'), findsNothing);

// Trigger a disconnect and ensure the overlay appears.
await tester.pumpFrames(app, const Duration(milliseconds: 100));
serviceManager.changeState(false);
await tester.pumpFrames(app, const Duration(milliseconds: 100));
await tester.pumpAndSettle();
expect(find.text('Disconnected'), findsOneWidget);

// Trigger a reconnect
serviceManager.changeState(true);
await tester.pumpAndSettle();

// Expect no overlay.
await tester.pumpFrames(app, const Duration(milliseconds: 100));
expect(find.text('Disconnected'), findsNothing);
},
);

testWidgets(
'builds contents when initialized',
(WidgetTester tester) async {
await tester.pumpWidget(app);
await tester.pumpAndSettle();
expect(find.text('Disconnected'), findsNothing);
expect(find.byKey(initializedKey), findsOneWidget);
},
);
});
Expand Down

0 comments on commit 3f8b720

Please sign in to comment.