Clean up notDisposed leaks in Flutter Framework, phase 1.

[polina-c]

Looking for volunteers to help with this!

Subtask of: flutter/devtools#3951

Issues: all, open.
PRs: all, open.

All invocations of testWidgets under flutter/test (~ 10,000) should be converted to testWidgetsWithLeakTracking by iterating the steps:

1. Comment on this issue what subset of tests you plan to work on so that we do not duplicate work. Then delete the added comment as soon as you are done or if your plans changed. If all tests seems to be occupied, let me know, and I will redistribute them.

2. Convert set of testWidgets, create PR and, if tests pass, request review and merge. Example: Cover more tests with leak tracking. flutter/flutter#132806

3. For tests that fail, investigate them and search for the culprit class name in onen issues or PRs (check links above). If the issue for this concrete leak is already created or PR is on the way, leave this test to be 'testWidgets'. Otherwise, move the conversion to a separate PR, and do one of two in this PR:

   a. Fix the failure if it is strait-forward.

   b. Create issue that explains the leak, create allowlist, and link the issue near the allowlist. Example: Mark leak in text_form_field_test.dart. flutter/flutter#130468. Then either allow triage process to happen or assign the issue to yourself and resolve it in a separate PR.

4. Link the issue and the fix in a comment for this PR.

5. Do not:

   • Instrument new disposables, because we want to clean up leaks first for already instrumented ones
   • Convert many tests with the same issue linked to allowlists. One test is enough to repro the issue.

Focus areas:
@ksokolovskyi: test/widgets
@droidbg: test/paintings, test/rendering and test/gestures
@NobodyForNothing: paused (thank you for what is done and in process)
@polina-c: coordinating complicated cases and integration with testWidgets

Discord calls: 1, 2
Thread on discord for leak_tracker updates

[polina-c]

flutter/test:

• animation
• cupertino
• foundation
• gestures
• material
• painting
• physics
• rendering
• scheduler
• semantics
• services
• widgets

Non trivial open issues/PRs:

• Memory Leak: RenderEditable does not dispose LeaderLayer when testing with _TestRecordingCanvasMatcher flutter/flutter#134386
  • RenderEditable should dispose created layers. flutter/flutter#135942
• Memory Leak: RenderSliverEdgeInsetsPadding does not dispose OffsetLayer when testing with TestClipPaintingContext flutter/flutter#134575
  • Add disposal mechanism for created Layers to TestRecordingPaintingContext. flutter/flutter#134768
  • TestClipPaintingContext should dispose ContainerLayer flutter/flutter#135949
• Memory Leak: TestClipPaintingContext does not dispose ContainerLayer flutter/flutter#134572
  • TestClipPaintingContext should dispose ContainerLayer flutter/flutter#135949
• Instances of OpacityLayer, created by _RenderChip, should be disposed. flutter/flutter#134394
  • _RenderChip should not create OpacityLayer without disposing. flutter/flutter#134708
  • Fix _paintAvatar in chip.dart to dispose layer. flutter/flutter#135228
• Memory Leak: AnnotatedRegionLayer<SystemUiOverlayStyle> is not disposed when testing with _TestRecordingCanvasMatcher  flutter/flutter#134661
• painting layers - draft flutter/flutter#134701
• TabController should dispatch creation in constructor. flutter/flutter#133952
• Memory Leak: _MergeableMaterialState does not dispose AnimationController flutter/flutter#134838
• Fix memory leak in DropdownButton flutter/flutter#133660
• Cupertino date picker should dispose scroll controllers it created. flutter/flutter#135355
• Memory Leak: _RouterState doesn't dispose _RestorableRouteInformation flutter/flutter#134205
• NativeCodec.getNextFrame does not dispose Image working with AnimationSheetBuilder. flutter/flutter#136513

[polina-c]

progress check: 2071 tests are converted

[navaronbracke]

@polina-c Where do we track that all required widgets/services do

    if (kFlutterMemoryAllocationsEnabled) {
      maybeDispatchObjectCreation();
    }

in their constructor?

[polina-c]

@polina-c Where do we track that all required widgets/services do

    if (kFlutterMemoryAllocationsEnabled) {
      maybeDispatchObjectCreation();
    }

in their constructor?

Yes, not all extenders of ChangeNotifier do this at the moment.
For now it is added as needed, so no tracking yet. I have two PRs for this waiting lgtm:
flutter/flutter#133716
flutter/flutter#133717

Feel free to just add it where you need.

[ksokolovskyi]

@polina-c Could you please take a look at this PR, which adds tracking to the ScrollController: flutter/flutter#133759 ?

[polina-c]

progress check: 3458 tests covered

[polina-c]

@ksokolovskyi , I updated description asking to communicate subset of tests where you plan to work.

[ksokolovskyi]

@polina-c, nice, thanks.
If you don't mind, I would like to continue with test/widgets.

[polina-c]

progress check: 5223 tests are converted

[NobodyForNothing]

working on test/semantics and test/scheduler and test/cupertino

I'm currently not actively working on this, but some PRs are still open.

[droidbg]

@polina-c I also like to contribute to this.
Can I start with test/paintings and test/gestures, if it is not taken by anyone?

[polina-c]

@polina-c I also like to contribute to this. Can I start with test/paintings and test/gestures, if it is not taken by anyone?

@droidbg, yes and thank you!
Added test/paintings and test/gestures to the focus areas in description.
And, please, take into account this issue and related PRs working with gestures: flutter/flutter#136132

[polina-c]

covered: 8826
remaining: 1036

[droidbg]

@polina-c I have raised PRs for test/gesture and test/painting, could you please review it.


Also, test/gesture is done and test/painting is almost done, so I will be picking test/rendering in parallel now.

[droidbg]

@polina-c I have raised PRs for test/rendering (all TCs are done), could you please review it.
Also, can I pick test/cupertino now if no one is working on it now?

[NobodyForNothing]

@droidbg I used to work on test/cupertino feel free to continue it. Here are some notes about the unconverted tests I still have:

A PR for cupertino/date_picker_test is still open.

The following files are not converted and have undisposed widgets:

• cupertino/text_form_field_row_test.dart (TextEditingController, LeaderLayer (23))
• cupertino/text_selection_test
• cupertino/search_field_test
• cupertino/picker_test

Once flutter/flutter#134661 is completed, you should be able to cover cupertino/nav_bar_test and cupertino/scaffold_test.

I don't know about any other unconverted tests but you should probably check for yourself in case I missed something.

[polina-c]

Will appreciate votes here: flutter/devtools#6266 (comment)

@NobodyForNothing @ksokolovskyi @droidbg @navaronbracke

[polina-c]

FYI, update on leak_tracker progress: https://discord.com/channels/608014603317936148/1106667330093723668/1162097704462196788

[droidbg]

Hello folks, these are the updates on the subsets I was working on:

• test/gesture - done.
• test/painting - 1 PR is open , rest are done.
• test/rendering - done.
• test/cupertino - 1 PR is open by NobodyForNothing , rest are done.

@polina-c
Can I pick up test/material, if no one is working on it?
or if there is anything else that I can help with, do let me know. Thanks.

[ksokolovskyi]

Hi @droidbg, thanks for your work!
Currently, I'm responsible for material and widgets tests.
All material tests will be covered after #137004 merge.
@polina-c, do you have any tasks for @droidbg?

[polina-c]

98% are covered!!!

@ksokolovskyi, @droidbg , @NobodyForNothing , I greatly appreciate your help!

Closing this issue in favor of the next phase, where help is also needed and appreciated: flutter/flutter#137311

[polina-c]

do you have any tasks for @droidbg?

Yes, pick any class or test-set in flutter/flutter#137311