Closed
Bug 460532
Opened 16 years ago
Closed 16 years ago
Firefox leaks SelectionDetails after running for a while
Categories
(Core :: General, defect, P2)
Tracking
()
RESOLVED
FIXED
mozilla1.9.1b2
People
(Reporter: cbook, Assigned: uriber)
References
Details
(Keywords: fixed1.9.1, memory-leak)
Attachments
(2 files)
2.78 KB,
patch
|
roc
:
review+
roc
:
superreview+
|
Details | Diff | Splinter Review |
2.65 KB,
patch
|
Details | Diff | Splinter Review |
Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b2pre) Gecko/20081017 Firefox/3.1b2pre Steps to reproduce: -> 3 Tabs open with various sites + running firefox for a while and and was filing a bug :) --> Leak == BloatView: ALL (cumulative) LEAK STATISTICS |<----------------Class--------------->|<-----Bytes------>|<----------------Objects---------------->|<--------------References-------------->| Per-Inst Leaked Total Rem Mean StdDev Total Rem Mean StdDev 0 TOTAL 23 32 18138505 2 ( 4630.88 +/- 3494.33) 32592776 0 ( 4443.49 +/- 6241.90) 74 SelectionDetails 16 32 4646 2 ( 2.13 +/- 0.98) 0 0 ( 0.00 +/- 0.00) nsTraceRefcntImpl::DumpStatistics: 939 entries nsStringStats => mAllocCount: 184172 => mReallocCount: 22570 => mFreeCount: 184172 => mShareCount: 172248 => mAdoptCount: 18508 => mAdoptFreeCount: 18508
Assignee | ||
Comment 1•16 years ago
|
||
Funny coincidence - I just stumbled upon this code a couple of days ago and noticed it's leaky. This patch should fix the leak, and also gets rid of an unnecessary "if".
Not sure if this is a regression. If it is it might be a blocker.
Flags: blocking1.9.1?
Comment on attachment 343736 [details] [diff] [review] patch awesome, thanks! So to reproduce this, you just have to have a multiple selection and click in the first part of the selection, right? Can we write a mochitest for that that leaks without this fix? I guess what we really want is for SelectionDetails to not be such an error-prone mess...
Attachment #343736 -
Flags: superreview+
Attachment #343736 -
Flags: review?(roc)
Attachment #343736 -
Flags: review+
Assignee | ||
Comment 4•16 years ago
|
||
(In reply to comment #3) > So to reproduce this, you just have to have a multiple selection and click in > the first part of the selection, right? Not exactly - you need more than one selection range (either of the same type or of different types) to overlap the clicked point (in practice, this probably occurs when you click in a selection that is also marked as misspelled or highlighted by "Highlight all"). I'll try to make a mochitest with two overlapping normal selection ranges.
Assignee | ||
Comment 5•16 years ago
|
||
This leaks one SelectionDetails object without the fix.
Assignee | ||
Comment 6•16 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/6c2f8bd79cbc
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.1b2
That's great, thanks!
Comment 8•16 years ago
|
||
Uri left a message on the tinderbox waterfall at 16:32 that he'd pushed a fix for the test bustage (on linux, mochitest is orange with focus errors after starting test_word_movement.html; on win32, redness from hanging test_character_movement.html). That doesn't appear in the pushlog. #developers suggested disabling the new test for this bug, to try to isolate if it leaves mochitest in a bad state for subsequent tests, or if there is a problem with the code change. That's http://hg.mozilla.org/mozilla-central/rev/fba837242af0
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 9•16 years ago
|
||
Gah, helps if you disable the test properly http://hg.mozilla.org/mozilla-central/rev/6e9a29a235ce
You could also disable the test like this: http://hg.mozilla.org/mozilla-central/annotate/c4c46a5f9681/accessible/tests/mochitest/Makefile.in#l66 if you want.
Assignee | ||
Comment 11•16 years ago
|
||
(In reply to comment #8) > Uri left a message on the tinderbox waterfall at 16:32 that he'd pushed a fix > for the test bustage (on linux, mochitest is orange with focus errors after > starting test_word_movement.html; on win32, redness from hanging > test_character_movement.html). That doesn't appear in the pushlog. Sorry. My push failed and I didn't notice. I now pushed the fix to the test and re-enabled it. I'll follow the tree.
Assignee | ||
Comment 12•16 years ago
|
||
Seems to be working now.
Status: REOPENED → RESOLVED
Closed: 16 years ago → 16 years ago
Resolution: --- → FIXED
Updated•16 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•