Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix issue where first build of the VM Tools status bar would cause a null pointer exception #2905

Merged
merged 3 commits into from
Apr 15, 2021

Conversation

bkonyi
Copy link
Contributor

@bkonyi bkonyi commented Apr 14, 2021

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.

@bkonyi
Copy link
Contributor Author

bkonyi commented Apr 14, 2021

I'm not 100% sure this is the right approach here, but it does fix the issue. The other approach I tried involved statically initializing the controller in _VMDeveloperToolsScreenState and not initializing the controller via Provider, which also worked.

@kenzieschmoll
Copy link
Member

discussed offline - going to try a different approach that doesn't require adding the ensureStateInitialized method

…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`.
@bkonyi bkonyi force-pushed the fix_vm_page_status_bar branch from d44af36 to d4507c4 Compare April 15, 2021 18:32
@bkonyi
Copy link
Contributor Author

bkonyi commented Apr 15, 2021

Updated with new approach (and was able to drop the static controller in _VMDeveloperToolsScreenState, hurrah!). PTAL.

Comment on lines 50 to 52
// Ensure that the current screen's state is initialized. In particular,
// make sure that the screen's controller is available before calling
// `currentScreen.showIsolateSelector`.
Copy link
Member

Choose a reason for hiding this comment

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

remove

Copy link
Member

@kenzieschmoll kenzieschmoll left a comment

Choose a reason for hiding this comment

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

approved with a few comments

@bkonyi bkonyi merged commit 018cd61 into master Apr 15, 2021
@bkonyi bkonyi deleted the fix_vm_page_status_bar branch April 15, 2021 20:31
@kenzieschmoll
Copy link
Member

Red bots caused by #2916

@bkonyi
Copy link
Contributor Author

bkonyi commented Apr 15, 2021

Landing on red because flaky bots are frustrating.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants