Closed Bug 346878 Opened 14 years ago Closed 14 years ago

Mac 1.8.1: leaking menu items and toplevel windows due to problems in nsMenuX and nsMacWindow

Categories

(Core Graveyard :: Widget: Mac, defect)

PowerPC
macOS
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla1.8.1beta2

People

(Reporter: mark, Assigned: mark)

References

Details

(Keywords: fixed1.8.1)

Attachments

(1 file)

This leak was fixed on the trunk in the checkin for bug 46177.  It needs to be fixed on the 1.8 branch even if bug 46177 doesn't land there.
1.8 branch seems to be leaking for other reasons as well...
Note that this bug was spun off from bug 46177 comment 95.
Summary: Leaking menu items due to a problem in nsMenuX → Mac 1.8.1: leaking menu items and toplevel windows due to a problems in nsMenuX and nsMacWindow
Target Milestone: --- → mozilla1.8.1beta2
Attached patch PatchSplinter Review
There were two leaks, both of them only present on the 1.8 branch:

 - The patch to bug 344238 created a leak in nsMacWindow because
   nsIWidget::GetParent returns an addrefed object on the 1.8 branch.  That
   patch was initially developed on the trunk, where bug 227489 made
   the same method return a weak pointer, therefore there is no leak on the
   trunk.

 - Bug 50590 introduced a leak in
   nsMenuX::ChangeNativeEnabledStatusForMenuItem.  This has already been
   corrected on the trunk as part of bug 46177.

Testing with this patch and global object counters, I have verified that all windows and menu items are properly freed by the time the application quits.
Attachment #231776 - Flags: superreview?(darin)
Attachment #231776 - Flags: review?(joshmoz)
Blocks: 50590, 344238
Attachment #231776 - Flags: review?(joshmoz) → review+
Attachment #231776 - Flags: superreview?(darin) → superreview+
Attachment #231776 - Flags: approval1.8.1?
Summary: Mac 1.8.1: leaking menu items and toplevel windows due to a problems in nsMenuX and nsMacWindow → Mac 1.8.1: leaking menu items and toplevel windows due to problems in nsMenuX and nsMacWindow
Mark - since we approved 46177 is this still needed?
Yes, 46177 included the second fix from comment 3, but the first fix is still needed on the 1.8 branch.
Comment on attachment 231776 [details] [diff] [review]
Patch

a=schrep
Attachment #231776 - Flags: approval1.8.1? → approval1.8.1+
Checked in on MOZILLA_1_8_BRANCH before 1.8.1b2.
Status: NEW → RESOLVED
Closed: 14 years ago
Keywords: fixed1.8.1
Resolution: --- → FIXED
Product: Core → Core Graveyard
Version: 1.8 Branch → Trunk
You need to log in before you can comment on or make changes to this bug.