Closed Bug 323532 Opened 18 years ago Closed 18 years ago

[FIX] Leak when using history autocomplete

Categories

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

Product:
Firefox
Component:
Bookmarks & History
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
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)

Patch for real
1.42 KB, patch
mconnor
: review+
benjamin
: approval-branch-1.8.1+
dveditz
: approval1.8.0.2+
Details | Diff | Splinter Review 
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 real
        — 
          — Splinter 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.