Closed Bug 323532 Opened 18 years ago Closed 18 years ago

[FIX] Leak when using history autocomplete

Categories

(Firefox :: Bookmarks & History, defect, P3)

defect

Tracking

()

RESOLVED FIXED
Firefox 2

People

(Reporter: bzbarsky, Assigned: bzbarsky)

Details

(Keywords: fixed1.8.0.2, fixed1.8.1, memory-leak, Whiteboard: [nvn-dl])

Attachments

(1 file, 1 obsolete file)

BUILD:  Current Firefox trunk build

STEPS TO REPRODUCE:

1) Start up Firefox
2) Click in URL field
3) Hit Alt+down to open history autocomplete

ACTUAL RESULTS: leak morkobjects

EXPECTED RESULTS: no leaks

This is going to be a safe fix that I'd suggest for 1.8.0.x branch, I think.
Attached patch Patch (obsolete) — Splinter Review
Attachment #208566 - Flags: review?(benjamin)
Priority: -- → P3
Target Milestone: --- → Firefox 2
Attached patch Patch for realSplinter Review
Attachment #208566 - Attachment is obsolete: true
Attachment #208574 - Flags: review?(benjamin)
Attachment #208566 - Flags: review?(benjamin)
Attachment #208574 - Flags: review?(benjamin) → review+
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Comment on attachment 208574 [details] [diff] [review]
Patch for real

This is a very safe and simple leak fix.  We leak a few hundred bytes any time autocomplete happens and there are matches.
Attachment #208574 - Flags: approval1.8.1?
Attachment #208574 - Flags: approval1.8.0.2?
Comment on attachment 208574 [details] [diff] [review]
Patch for real

This code is going away with places, no need for it on the 1.8 branch.
Attachment #208574 - Flags: approval1.8.1? → approval1.8.1-
Comment on attachment 208574 [details] [diff] [review]
Patch for real

approved for 1.8.0 branch, a=dveditz
Attachment #208574 - Flags: approval1.8.0.2? → approval1.8.0.2+
Flags: blocking1.8.0.2+
Fixed for 1.8.0.2.
Keywords: fixed1.8.0.2
marking [nvn-dl], which removes this bug from the "to be verified by QA" list
for Firefox 1.5.0.2.  I'm not sure what we should do to verify leak fixes individually.  I'm open to suggestions
Whiteboard: [nvn-dl]
(In reply to comment #4)
> (From update of attachment 208574 [details] [diff] [review] [edit])
> This code is going away with places, no need for it on the 1.8 branch.
> 
"Places" has been disabled on the 1.8 branch. Do we need this?
Comment on attachment 208574 [details] [diff] [review]
Patch for real

Yes, I think so.
Attachment #208574 - Flags: approval1.8.1- → approval-branch-1.8.1?(benjamin)
Comment on attachment 208574 [details] [diff] [review]
Patch for real

How did this land on 1.8.0 but not 1.8?
Attachment #208574 - Flags: approval-branch-1.8.1?(benjamin) → approval-branch-1.8.1+
Benjamin, comment 4 explains how that happened.
Fixed on 1.8 branch, but I wish this had gotten approved when I asked for it initially; the marging was a little painful.  :(
Keywords: fixed1.8.1
Component: History → Bookmarks & History
QA Contact: history → bookmarks
You need to log in before you can comment on or make changes to this bug.