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.
dotnet · Jan 8, 2025 · cd5dd1c · cd5dd1c
1 parent 696f082
commit cd5dd1c
Show file tree

Hide file tree

Showing 3 changed files with 2 additions and 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
@@ -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/...tem.Windows.Forms/src/System/Windows/Forms/Controls/ToolStrips/ToolStripItemCollection.cs b/...tem.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
@@ -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)]