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

Next attempt to make inspector weakly referencing the inspected objects. #129962

Merged
merged 28 commits into from Jul 6, 2023

Conversation

polina-c
Copy link
Contributor

@polina-c polina-c commented Jul 4, 2023

Bugger issue: flutter/devtools#3951
Reverts #128506

Previous failure: https://ci.chromium.org/ui/p/flutter/builders/prod/Mac%20framework_tests_widgets/11385/overview

Failed line:

final DiagnosticsNode diagnosticsNode = service.toObject(childJson['objectId']! as String)! as DiagnosticsNode;

Changes since previous attempts:

  1. Replaced ...)! as DiagnosticsNode; with ...) as DiagnosticsNode?;
  2. Removed _forceGC
  3. Cleaned up inspector API:

Reproduced issue and tested locally with commands:

flutter test test/widgets/widget_inspector_test.dart --update-goldens --fatal-warnings

flutter test --test-randomize-ordering-seed=20230608 --fatal-warnings --track-widget-creation test/widgets/widget_inspector_test.dart --update-goldens

flutter test --test-randomize-ordering-seed=20230609 --fatal-warnings --track-widget-creation test/widgets/widget_inspector_test.dart --update-goldens

flutter test test/widgets/widget_inspector_test.dart --update-goldens --track-widget-creation --fatal-warnings --plain-name ext.flutter.inspector.getRootWidgetSummaryTree

@github-actions github-actions bot added the framework flutter/packages/flutter repository. See also f: labels. label Jul 4, 2023
@polina-c polina-c changed the title Redo4 Next attempt to make inspector weakly referencing the inspected objects. Jul 4, 2023
@polina-c polina-c marked this pull request as ready for review July 5, 2023 21:18
@zanderso zanderso self-requested a review July 5, 2023 21:27
Copy link
Member

@zanderso zanderso left a comment

Choose a reason for hiding this comment

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

lgtm w/ nit

if (id == null) {
// TODO(polina-c): comment here why we increase memory footprint by the prefix 'inspector-'.
Copy link
Member

Choose a reason for hiding this comment

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

nit: TODOs require an issue link: https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo#avoid-checking-in-comments-that-ask-questions.

But if this is a TODO just to add a comment, could it just be added in this PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added issue. I do not know the answer and it will require research to get it.

@polina-c polina-c added the autosubmit Merge PR when tree becomes green via auto submit App label Jul 6, 2023
@polina-c polina-c merged commit bff6b93 into flutter:master Jul 6, 2023
71 of 72 checks passed
@polina-c polina-c deleted the redo4 branch July 6, 2023 19:19
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 7, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 7, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 7, 2023
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Jul 7, 2023
flutter/flutter@bc49cd1...d55a7d8

2023-07-07 hans.muller@gmail.com Revert "fix a bug when android uses CupertinoPageTransitionsBuilder..." (flutter/flutter#130144)
2023-07-07 engine-flutter-autoroll@skia.org Roll Packages from 771ec9b to 9bcf4bf (12 revisions) (flutter/flutter#130143)
2023-07-07 engine-flutter-autoroll@skia.org Roll Flutter Engine from 650ff096488a to 8aa2e6516af1 (1 revision) (flutter/flutter#130116)
2023-07-07 engine-flutter-autoroll@skia.org Roll Flutter Engine from 1fa222fae283 to 650ff096488a (3 revisions) (flutter/flutter#130114)
2023-07-07 engine-flutter-autoroll@skia.org Roll Flutter Engine from 491f317978f4 to 1fa222fae283 (1 revision) (flutter/flutter#130110)
2023-07-07 engine-flutter-autoroll@skia.org Roll Flutter Engine from 48bf7ac59254 to 491f317978f4 (2 revisions) (flutter/flutter#130107)
2023-07-06 engine-flutter-autoroll@skia.org Roll Flutter Engine from 42df55a9e63e to 48bf7ac59254 (1 revision) (flutter/flutter#130106)
2023-07-06 5236035+fzyzcjy@users.noreply.github.com Super tiny code optimization: No need to redundantly check whether value has changed (flutter/flutter#130050)
2023-07-06 engine-flutter-autoroll@skia.org Roll Flutter Engine from 06c936205d96 to 42df55a9e63e (3 revisions) (flutter/flutter#130100)
2023-07-06 engine-flutter-autoroll@skia.org Manual roll Flutter Engine from bd2e42b203e1 to 06c936205d96 (27 revisions) (flutter/flutter#130097)
2023-07-06 fluttergithubbot@gmail.com Marks Linux firebase_oriol33_abstract_method_smoke_test to be unflaky (flutter/flutter#128398)
2023-07-06 polinach@google.com Next attempt to make inspector weakly referencing the inspected objects. (flutter/flutter#129962)
2023-07-06 82336674+gilnobrega@users.noreply.github.com [a11y] CupertinoSwitch On/Off labels (flutter/flutter#127776)
2023-07-06 henriquenas.dev@gmail.com Add option for flexible space on material SearchDelegate (flutter/flutter#128132)
2023-07-06 engine-flutter-autoroll@skia.org Roll Packages from 7042079 to 771ec9b (12 revisions) (flutter/flutter#130077)
2023-07-06 2539699336@qq.com [framework] Add textField OCR support for framework side (flutter/flutter#96637)
2023-07-06 37002358+fabiancrx@users.noreply.github.com [flutter_tools] modify Skeleton template to use ListenableBuilder instead of AnimatedBuilder (flutter/flutter#128810)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages
Please CC bmparr@google.com,rmistry@google.com,stuartmorgan@google.com on the revert to ensure that a human
is aware of the problem.

To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose

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/+doc/main/autoroll/README.md
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 16, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 17, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 17, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 17, 2023
@polina-c polina-c added the a: leak tracking Issues and PRs related to memory leaks detected by leak_tracker label Sep 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a: leak tracking Issues and PRs related to memory leaks detected by leak_tracker autosubmit Merge PR when tree becomes green via auto submit App framework flutter/packages/flutter repository. See also f: labels.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants