It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group.

@Piinks I have made required changes.

Detaching the controller from the state when it's deactivated makes sense to me.

However this would create a new corner case: when the state is deactivated, we don't want the developer to call show or hide on the associated controller because the state the controller attaches to may not rebuild. I think that can happen if you cache the OverlayPortal and gives it a global key.

I will write a test for this and check.

Detaching the controller from the state when it's deactivated makes sense to me.
However this would create a new corner case: when the state is deactivated, we don't want the developer to call show or hide on the associated controller because the state the controller attaches to may not rebuild. I think that can happen if you cache the OverlayPortal and gives it a global key.

I will write a test for this and check.

I have added the test, we don't need to call show method again after reactivation.

It seems there's a leak in one of the new tests. Maybe one of the OverlayEntries were not disposed at the end of the test?

I think setting _attatchTarget to null when the Element is deactivated could change the behavior in some corner cases, and what we want to fix is just the assert in _setupController. Can fix the asserts by:

• In the _setupController assert, if the attached state is not Element.debugIsActive, then we could allow the re-attach,
• assert in State.activate that _attachTarget == this

I think setting _attatchTarget to null when the Element is deactivated could change the behavior in some corner cases, and what we want to fix is just the assert in _setupController. Can fix the asserts by:

• In the _setupController assert, if the attached state is not Element.debugIsActive, then we could allow the re-attach,
• assert in State.activate that _attachTarget == this

Element.debugIsActive
Returns true if the Element is active.
This getter always returns false in profile and release builds.

I think this will not solve the issue because in release it will always reattach to newer element.

It seems there's a leak in one of the new tests. Maybe one of the OverlayEntries were not disposed at the end of the test?

Even after disposing overlay entries, its showing leak.

in release

The issue won't happen in release builds. The crash is caused by an assert.

It seems there's a leak in one of the new tests. Maybe one of the OverlayEntries were not disposed at the end of the test?

Even after disposing overlay entries, its showing leak.

The relevant error messages were:

     Actual: <Instance of 'Leaks'>
     Which: contains leaks:
            # The text is generated by leak_tracker.
            # For leak troubleshooting tips open:
            # https://github.com/flutter/flutter/blob/main/docs/contributing/testing/Leak-tracking.md
            notDisposed:
              total: 4
              objects:
                ValueNotifier<_OverlayEntryWidgetState?>:
                  test: attachTarget is restored after reparenting
                  identityHashCode: 933371839
                OverlayEntry:
                  test: attachTarget is restored after reparenting
                  identityHashCode: 947575411
                ValueNotifier<_OverlayEntryWidgetState?>:
                  test: attachTarget is restored after reparenting
                  identityHashCode: 51179505
                OverlayEntry:
                  test: attachTarget is restored after reparenting
                  identityHashCode: 662836894

in release

The issue won't happen in release builds. The crash is caused by an assert.

Okay, then I will do the change in _setupController.