ContextMenuController does not dispose OverlayEntry. #130467

Activity

1. polina-c

   changed the title [-]ValueNotifier<_OverlayEntryWidgetState?> is not disposed.[/-] [+]ContextMenuController does not dispose OverlayEntry.[/+] on Jul 13, 2023
2. huycozy

   added

   in triagePresently being triaged by the triage teamPresently being triaged by the triage team

   on Jul 13, 2023
3. huycozy commented on Jul 13, 2023

   

   huycozy

   on Jul 13, 2023

   Member

   More actions

   Hi @polina-c
   I can't find notDisposedAllowList from packages/flutter/test/material/text_form_field_test.dart. Could you give a simple use case of ContextMenuController that causes this issue?
4. huycozy

   added

   waiting for responseThe Flutter team cannot make further progress on this issue until the original reporter respondsThe Flutter team cannot make further progress on this issue until the original reporter responds

   on Jul 13, 2023
5. polina-c commented on Jul 13, 2023

   

   polina-c

   on Jul 13, 2023

   ContributorAuthor

   More actions

   notDisposedAllowList is added by PR that is linked in description: https://github.com/flutter/flutter/pull/130468/files
6. github-actions

   removed

   waiting for responseThe Flutter team cannot make further progress on this issue until the original reporter respondsThe Flutter team cannot make further progress on this issue until the original reporter responds

   on Jul 13, 2023
7. polina-c commented on Jul 13, 2023

   

   polina-c

   on Jul 13, 2023

   ContributorAuthor

   More actions

   Feel free to tweet me or to chat me, to setup GVC. I will be happy to explain details.
8. huycozy

   added

   a: tests"flutter test", flutter_test, or one of our tests"flutter test", flutter_test, or one of our tests

   a: text inputEntering text in a text field or keyboard related problemsEntering text in a text field or keyboard related problems

   frameworkflutter/packages/flutter repository. See also f: labels.flutter/packages/flutter repository. See also f: labels.

   p: material_uimaterial_ui package in flutter/packagesmaterial_ui package in flutter/packages

   team-designOwned by Design Languages teamOwned by Design Languages team

   and removed

   in triagePresently being triaged by the triage teamPresently being triaged by the triage team

   on Jul 14, 2023
9. polina-c

   added

   waiting for responseThe Flutter team cannot make further progress on this issue until the original reporter respondsThe Flutter team cannot make further progress on this issue until the original reporter responds

   on Jul 17, 2023
10. polina-c commented on Jul 17, 2023

    

    polina-c

    on Jul 17, 2023

    ContributorAuthor

    More actions

    This PR reproduces the issue: https://github.com/flutter/flutter/pull/130743/checks?check_run_id=15111140218
11. polina-c

    added

    in triagePresently being triaged by the triage teamPresently being triaged by the triage team

    and removed

    waiting for responseThe Flutter team cannot make further progress on this issue until the original reporter respondsThe Flutter team cannot make further progress on this issue until the original reporter responds

    on Jul 17, 2023
12. goderbauer commented on Jul 25, 2023

    

    goderbauer

    on Jul 25, 2023

    Member

    More actions

    cc @justinmc
13. justinmc commented on Jul 26, 2023

    

    justinmc

    on Jul 26, 2023

    Contributor

    More actions

    • When the test is run with testWidgetsWithLinkTracking, then the EditableTextState (and the whole widget tree in pumpWidget?) is never disposed at the end of the test. If I change it to testWidgets, then it is disposed.
    • (Using testWidgetsWithLinkTracking) Bizarrely, if I comment out this line where _handles is set to null, then EditableTextState does get disposed. This is true even if I hack out all of the accesses of _handles after it's set to null, leading me to think something deep behind the scenes is doing this.

    @polina-c Is there anything that testWidgetsWithLeakTracking does that could prevent the widget tree from being disposed?

    Here's a simpler test that still reproduces the problem:

      testWidgetsWithLeakTracking('leak test', (WidgetTester tester) async {
        final TextEditingController controller = TextEditingController(
          text: 'blah1 blah2',
        );
        await tester.pumpWidget(
          MaterialApp(
            home: Material(
              child: Center(
                child: TextField(
                  controller: controller,
                ),
              ),
            ),
          ),
        );
    
        final Offset startBlah1 = textOffsetToPosition(tester, 0);
        await tester.tapAt(startBlah1);
        await tester.pump(const Duration(milliseconds: 100));
        await tester.tapAt(startBlah1);
        await tester.pumpAndSettle();
      },
        variant: const TargetPlatformVariant(<TargetPlatform>{ TargetPlatform.macOS }),
        skip: kIsWeb, // [intended] we don't supply the cut/copy/paste buttons on the web.
      );
14. polina-c commented on Jul 27, 2023

    

    polina-c

    on Jul 27, 2023

    ContributorAuthor

    More actions

    Thanks. For some reasons with leak tracker BuildOwner.finalizeTree is invoked 6 times instead of 7, where 7th one triggers disposal.

    At the moment I am refactoring leak tracker for performance, that may side effect in this side effect disappearing.
    So, reassigning the issue to me and shelving till after refactoring.

    👍React with 👍1Reacted by Justin McCandless
15. polina-c

    self-assigned this

    on Jul 27, 2023
16. polina-c

    added

    P1High-priority issues at the top of the work listHigh-priority issues at the top of the work list

    and removed

    in triagePresently being triaged by the triage teamPresently being triaged by the triage team

    on Jul 27, 2023
17. auto-submit

    added a commit that references this issue on Aug 16, 2023

    Fix some leaks and add test to test cover remaining in a simple way. (#…

    ...

    112f429
18. polina-c commented on Aug 17, 2023

    

    polina-c

    on Aug 17, 2023

    Last edited by polina-c

    ContributorAuthor

    More actions

    @justinmc

    It seems flakiness disappeared. Returning the issue to you. Remaining leaks:

    notDisposed:
                  total: 4
                  objects:
                    RestorableBool:
                      test: can use the desktop cut/copy/paste buttons on Windows and Linux
                      identityHashCode: 1004453175
                    RestorableStringN:
                      test: can use the desktop cut/copy/paste buttons on Windows and Linux
                      identityHashCode: 715878348
                    RestorableBool:
                      test: can use the desktop cut/copy/paste buttons on Windows and Linux
                      identityHashCode: 69970577
                    RestorableStringN:
                      test: can use the desktop cut/copy/paste buttons on Windows and Linux
                      identityHashCode: 283658580
    

    To see callstack for creation, set this config:

    leakTrackingTestConfig: const LeakTrackingTestConfig.debugnotDisposed() 
    

    👍React with 👍1Reacted by Justin McCandless
19. polina-c

    assigned

    justinmc

    and unassigned

    polina-c

    on Aug 17, 2023
20. justinmc

    closed this as completedin #132987on Aug 22, 2023
21. github-actions commented on Sep 5, 2023

    

    github-actionsbot

    on Sep 5, 2023

    More actions

    This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.
22. github-actions

    locked as resolved and limited conversation to collaborators on Sep 5, 2023