Inspector robustness fixes. #14154
Merged
Inspector robustness fixes. #14154
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Avoid cases where setState was called on disposed object. Avoid cases where the inspector wouldn't render as it was called after the first frame rendered and no other frames were rendered.
Hixie
reviewed
Jan 18, 2018
| bool get _reportFirstFrame => _deferFirstFrameReportCount == 0; | ||
|
|
||
| /// Whether the first frame has finished rendering. | ||
| bool get debugDidSendFirstFrameEvent => !_needToReportFirstFrame; |
Contributor
There was a problem hiding this comment.
These docs should mention:
- that this is only valid in debug and profile builds, it can't be used in release builds
- that it can be deferred using [deferFirstFrameReport] and [allowFirstFrameReport]
- that it is set at the end of the call to drawFrame
Hixie
reviewed
Jan 18, 2018
| /// appropriate to display the Widget tree in the inspector. | ||
| bool isWidgetTreeReady([String groupName]) { | ||
| return WidgetsBinding.instance != null && | ||
| WidgetsBinding.instance.debugDidSendFirstFrameEvent; |
Hixie
reviewed
Jan 18, 2018
| } else { | ||
| // It isn't safe to trigger the selection change callback if we are in | ||
| // the middle of rendering the frame. | ||
| WidgetsBinding.instance.scheduleTask( |
Contributor
There was a problem hiding this comment.
probably better to use SchedulerBinding here (the result is the same but it's more idiomatic)
Hixie
reviewed
Jan 18, 2018
| /// as selecting the edge of the bounding box. | ||
| static const double _kEdgeHitMargin = 2.0; | ||
|
|
||
| InspectorSelectionChangedCallback selectionChangedCallback; |
Contributor
Author
There was a problem hiding this comment.
should be private. done.
Contributor
Contributor
|
Bonus points for a test, but I'm not sure how you'd test this really. |
Contributor
Author
|
Its on my list to create some good inspector integration tests that can verify these sorts of cases as well as verifying that widget creation locations are right once that CL lands. |
DaveShuckerow
pushed a commit
to DaveShuckerow/flutter
that referenced
this pull request
May 14, 2018
* Inspector robustness fixes. Avoid cases where setState was called on disposed object. Avoid cases where the inspector wouldn't render as it was called after the first frame rendered and no other frames were rendered.
engine-flutter-autoroll
added a commit
that referenced
this pull request
Dec 6, 2019
…utter/engine#14154) (#46220) git@github.com:flutter/engine.git/compare/fd240d0d532a...ed2d00b git log fd240d0..ed2d00b --first-parent --oneline 2019-12-06 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/mac-amd64 from qQlb5... to VKso5... (#14154) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC chinmaygarde@google.com on the revert to ensure that a human is aware of the problem. To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Avoid cases where setState was called on disposed object.
Avoid cases where the inspector wouldn't render as it was called
after the first frame rendered and no other frames were rendered.