From cd5dd1c06ff33a3ce2752dcd8b8ed7ad1592c18a Mon Sep 17 00:00:00 2001 From: kirsan31 <17767561+kirsan31@users.noreply.github.com> Date: Wed, 8 Jan 2025 21:29:17 +0300 Subject: [PATCH] Revert #4808 fix. Due to #12591 and investigations. (#12729) Fix #12591. This reverts #4808 fix commit from #11358 PR. The disposal of DisplayedItems after removal has already been implemented in DoLayoutIfHandleCreated but only if handle was created. So we have a leak only if handle is destroyed (#4808 case) and we didn't take this into account. --- .../Forms/Controls/ToolStrips/ToolStrip.cs | 9 +----- .../ToolStrips/ToolStripItemCollection.cs | 2 +- .../System/Windows/Forms/ToolStripTests.cs | 31 ------------------- 3 files changed, 2 insertions(+), 40 deletions(-) diff --git a/src/System.Windows.Forms/src/System/Windows/Forms/Controls/ToolStrips/ToolStrip.cs b/src/System.Windows.Forms/src/System/Windows/Forms/Controls/ToolStrips/ToolStrip.cs index 040b4ff8ef4..4af48720967 100644 --- a/src/System.Windows.Forms/src/System/Windows/Forms/Controls/ToolStrips/ToolStrip.cs +++ b/src/System.Windows.Forms/src/System/Windows/Forms/Controls/ToolStrips/ToolStrip.cs @@ -4680,16 +4680,9 @@ internal void OnItemAddedInternal(ToolStripItem item) } } - internal void OnItemRemovedInternal(ToolStripItem item, ToolStripItemCollection itemCollection) + internal void OnItemRemovedInternal(ToolStripItem item) { KeyboardToolTipStateMachine.Instance.Unhook(item, ToolTip); - if (itemCollection == _toolStripItemCollection) - { - // To prevent memory leaks when item removed from main collection, - // we need to remove it from _displayedItems and _overflowItems too. - _displayedItems?.Remove(item); - _overflowItems?.Remove(item); - } } internal override bool AllowsChildrenToShowToolTips() diff --git a/src/System.Windows.Forms/src/System/Windows/Forms/Controls/ToolStrips/ToolStripItemCollection.cs b/src/System.Windows.Forms/src/System/Windows/Forms/Controls/ToolStrips/ToolStripItemCollection.cs index a27deb39a39..e476bd350f5 100644 --- a/src/System.Windows.Forms/src/System/Windows/Forms/Controls/ToolStrips/ToolStripItemCollection.cs +++ b/src/System.Windows.Forms/src/System/Windows/Forms/Controls/ToolStrips/ToolStripItemCollection.cs @@ -391,7 +391,7 @@ private void OnAfterRemove(ToolStripItem item) if (_owner is not null) { - _owner.OnItemRemovedInternal(item, this); + _owner.OnItemRemovedInternal(item); if (!_owner.IsDisposingItems) { diff --git a/src/System.Windows.Forms/tests/UnitTests/System/Windows/Forms/ToolStripTests.cs b/src/System.Windows.Forms/tests/UnitTests/System/Windows/Forms/ToolStripTests.cs index c66a5c5ea3f..f92bc2e4345 100644 --- a/src/System.Windows.Forms/tests/UnitTests/System/Windows/Forms/ToolStripTests.cs +++ b/src/System.Windows.Forms/tests/UnitTests/System/Windows/Forms/ToolStripTests.cs @@ -7337,37 +7337,6 @@ void TimerStartAndItemDispose() } } - [WinFormsFact] - public void ToolStrip_displayedItems_Clear() - { - using ToolStripMenuItem toolStripMenuItem = new(nameof(toolStripMenuItem)); - using ToolStripMenuItem listToolStripMenuItem = new(nameof(listToolStripMenuItem)); - toolStripMenuItem.DropDownItems.Add(listToolStripMenuItem); - toolStripMenuItem.DropDownOpened += (sender, e) => - { - for (int i = 0; i < 4; i++) - listToolStripMenuItem.DropDownItems.Add("MenuItem" + i); - - listToolStripMenuItem.DropDown.PerformLayout(); // needed to populate DisplayedItems collection - }; - - toolStripMenuItem.DropDownClosed += (sender, e) => - { - while (listToolStripMenuItem.DropDownItems.Count > 0) - listToolStripMenuItem.DropDownItems[listToolStripMenuItem.DropDownItems.Count - 1].Dispose(); - - GC.Collect(); - GC.WaitForPendingFinalizers(); - GC.Collect(); - GC.WaitForPendingFinalizers(); - }; - - toolStripMenuItem.ShowDropDown(); - Assert.Equal(4, listToolStripMenuItem.DropDown.DisplayedItems.Count); - toolStripMenuItem.HideDropDown(); - Assert.Empty(listToolStripMenuItem.DropDown.DisplayedItems); - } - [WinFormsTheory] [InlineData(10, 10)] [InlineData(0, 0)]