Skip to content

Commit

Permalink
Fix issue where first build of the VM Tools status bar would cause a …
Browse files Browse the repository at this point in the history
…null pointer exception

`VMDeveloperToolsScreen` exposes a `ValueListenable` provided by the `VMDeveloperToolsController` in order to control the visibility of the isolate selector. Given that `VMDeveloperToolsScreen.showIsolateSelector` can be invoked before the static controller in `_VMDeveloperToolsScreenState` is initialized, a null pointer exception was being thrown when first building the VM Tools screen.

This change updates `DevToolsScreen` to take either a `createController` callback or a pre-instantiated `controller`, allowing for the same instance of `VMDeveloperToolsController` to be used by `VMDeveloperToolsScreen` while also being exposed via `Provider`.
  • Loading branch information
bkonyi committed Apr 15, 2021
1 parent 99e797b commit d4507c4
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 50 deletions.
112 changes: 68 additions & 44 deletions packages/devtools_app/lib/src/app.dart
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ class DevToolsAppState extends State<DevToolsApp> {
Widget _providedControllers({@required Widget child, bool offline = false}) {
final _providers = widget.screens
.where((s) =>
s.createController != null && (offline ? s.supportsOffline : true))
s.providesController && (offline ? s.supportsOffline : true))
.map((s) => s.controllerProvider)
.toList();

Expand Down Expand Up @@ -303,7 +303,8 @@ class DevToolsAppState extends State<DevToolsApp> {
class DevToolsScreen<C> {
const DevToolsScreen(
this.screen, {
@required this.createController,
this.createController,
this.controller,
this.supportsOffline = false,
});

Expand All @@ -315,17 +316,35 @@ class DevToolsScreen<C> {
/// widgets depending on this controller can access it by calling
/// `Provider<C>.of(context)`.
///
/// If null, [screen] will be responsible for creating and maintaining its own
/// controller.
/// If [createController] and [controller] are both null, [screen] will be
/// responsible for creating and maintaining its own controller.
final C Function() createController;

/// A provided controller for this screen, if non-null.
///
/// The controller will then be provided via [controllerProvider], and
/// widgets depending on this controller can access it by calling
/// `Provider<C>.of(context)`.
///
/// If [createController] and [controller] are both null, [screen] will be
/// responsible for creating and maintaining its own controller.
final C controller;

/// Returns true if a controller was provided for [screen]. If false,
/// [screen] is responsible for creating and maintaining its own controller.
bool get providesController => createController != null || controller != null;

/// Whether this screen has implemented offline support.
///
/// Defaults to false.
final bool supportsOffline;

Provider<C> get controllerProvider {
assert(createController != null);
assert((createController != null && controller == null) ||
(createController == null && controller != null));
if (controller != null) {
return Provider<C>.value(value: controller);
}
return Provider<C>(create: (_) => createController());
}
}
Expand Down Expand Up @@ -526,49 +545,54 @@ class SettingsDialog extends StatelessWidget {
///
/// Conditional screens can be added to this list, and they will automatically
/// be shown or hidden based on the [Screen.conditionalLibrary] provided.
List<DevToolsScreen> get defaultScreens => <DevToolsScreen>[
DevToolsScreen<InspectorSettingsController>(
const InspectorScreen(),
createController: () => InspectorSettingsController(),
),
DevToolsScreen<PerformanceController>(
const PerformanceScreen(),
createController: () => PerformanceController(),
supportsOffline: true,
),
DevToolsScreen<ProfilerScreenController>(
const ProfilerScreen(),
createController: () => ProfilerScreenController(),
supportsOffline: true,
),
DevToolsScreen<MemoryController>(
const MemoryScreen(),
createController: () => MemoryController(),
),
DevToolsScreen<DebuggerController>(
const DebuggerScreen(),
createController: () => DebuggerController(),
),
DevToolsScreen<NetworkController>(
const NetworkScreen(),
createController: () => NetworkController(),
),
DevToolsScreen<LoggingController>(
const LoggingScreen(),
createController: () => LoggingController(),
),
DevToolsScreen<AppSizeController>(
const AppSizeScreen(),
createController: () => AppSizeController(),
),
DevToolsScreen<VMDeveloperToolsController>(
const VMDeveloperToolsScreen(),
createController: () => VMDeveloperToolsController(),
List<DevToolsScreen> get defaultScreens {
final vmDeveloperToolsController = VMDeveloperToolsController();
return <DevToolsScreen>[
DevToolsScreen<InspectorSettingsController>(
const InspectorScreen(),
createController: () => InspectorSettingsController(),
),
DevToolsScreen<PerformanceController>(
const PerformanceScreen(),
createController: () => PerformanceController(),
supportsOffline: true,
),
DevToolsScreen<ProfilerScreenController>(
const ProfilerScreen(),
createController: () => ProfilerScreenController(),
supportsOffline: true,
),
DevToolsScreen<MemoryController>(
const MemoryScreen(),
createController: () => MemoryController(),
),
DevToolsScreen<DebuggerController>(
const DebuggerScreen(),
createController: () => DebuggerController(),
),
DevToolsScreen<NetworkController>(
const NetworkScreen(),
createController: () => NetworkController(),
),
DevToolsScreen<LoggingController>(
const LoggingScreen(),
createController: () => LoggingController(),
),
DevToolsScreen<AppSizeController>(
const AppSizeScreen(),
createController: () => AppSizeController(),
),
DevToolsScreen<VMDeveloperToolsController>(
VMDeveloperToolsScreen(
controller: vmDeveloperToolsController
),
controller: vmDeveloperToolsController,
),
// Uncomment to see a sample implementation of a conditional screen.
// DevToolsScreen<ExampleController>(
// const ExampleConditionalScreen(),
// createController: () => ExampleController(),
// supportsOffline: true,
// ),
];
];
}
3 changes: 3 additions & 0 deletions packages/devtools_app/lib/src/status_line.dart
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,9 @@ class StatusLine extends StatelessWidget {

// Optionally display an isolate selector.
if (currentScreen != null) {
// Ensure that the current screen's state is initialized. In particular,
// make sure that the screen's controller is available before calling
// `currentScreen.showIsolateSelector`.
children.add(
ValueListenableBuilder<bool>(
valueListenable: currentScreen.showIsolateSelector,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,9 @@ abstract class VMDeveloperView {
}

class VMDeveloperToolsScreen extends Screen {
const VMDeveloperToolsScreen()
: super.conditional(
const VMDeveloperToolsScreen({
@required this.controller,
}) : super.conditional(
id: id,
title: 'VM Tools',
icon: Icons.settings_applications,
Expand All @@ -48,9 +49,11 @@ class VMDeveloperToolsScreen extends Screen {

static const id = 'vm-tools';

final VMDeveloperToolsController controller;

@override
ValueListenable<bool> get showIsolateSelector =>
_VMDeveloperToolsScreenState.controller.showIsolateSelector;
controller.showIsolateSelector;

@override
Widget build(BuildContext context) => const VMDeveloperToolsScreenBody();
Expand All @@ -70,9 +73,8 @@ class VMDeveloperToolsScreenBody extends StatefulWidget {

class _VMDeveloperToolsScreenState extends State<VMDeveloperToolsScreenBody>
with AutoDisposeMixin {
// TODO(bkonyi): do we want this to be static? Currently necessary to provide
// access to the `showIsolateSelector` via `VMDeveloperToolsScreen`
static VMDeveloperToolsController controller;

VMDeveloperToolsController controller;

@override
void didChangeDependencies() {
Expand Down

0 comments on commit d4507c4

Please sign in to comment.