The default bug view has changed. See this FAQ.

gtk2 nsFilePicker preview uses memory and decodes twice

RESOLVED FIXED in mozilla7

Status

()

Core
Widget: Gtk
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: Rodd Zurcher, Assigned: Rodd Zurcher)

Tracking

unspecified
mozilla7
Other
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

6 years ago
User-Agent:       Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:5.0) Gecko/20100101 Firefox/5.0
Build Identifier: Firefox 4.0.1

The widget/src/gtk2/nsFilePicker.cpp fully decodes the preview image into a pixbuf, solely to get it's image size.

On large images this is a lot of ram (resulting in a OOM and LMK on our device)

Reproducible: Always

Steps to Reproduce:
1. Monitor memory usage (top etc)
2. Add a file attachment to flicker/gmail etc that results in a GTKFilePicker.
3. Choose a very large image (width x height)

Actual Results:  
Briefly a very large amount of memory is consumed; and then the smalle preview image (thumbnail) is displayed. Or if the image is large enough; the shutdown of firefox.

Expected Results:  
an unnoticeable amount of memory be used, the size of a 180x180 (preview) pixbuf 

The current implementation also results in any image larger than 180x180 be decoded twice; wasting CPU as well.
(Assignee)

Comment 1

6 years ago
Created attachment 539007 [details] [diff] [review]
uses gdk_pixbuf_get_file_info() instead of decoding twice

Initial patch for review.

This version calls gdk_pixbuf_get_file_info() to get the image width and height, then if either is larger than 180; calls gdk_pixbuf_new_from_file_at_size, or gdk_pixbuf_new_from_file.
(Assignee)

Comment 2

6 years ago
The above patch (attachment 539007 [details] [diff] [review]) applies cleanly to FIREFOX_4_0_1_BUILD1 (the build I've tested), and mozilla-central.
Comment on attachment 539007 [details] [diff] [review]
uses gdk_pixbuf_get_file_info() instead of decoding twice

Review of attachment 539007 [details] [diff] [review]:
-----------------------------------------------------------------

You need to ask someone specific for review. Fortunately, I found it this time :-).
Attachment #539007 - Flags: review+
Thanks for the patch!
Keywords: checkin-needed

Updated

6 years ago
Assignee: nobody → rodd.zurcher
http://hg.mozilla.org/mozilla-central/rev/6eb2bcc24f76
Status: UNCONFIRMED → RESOLVED
Last Resolved: 6 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla7
The push broke the build, so I backed out all of its changesets.
Status: RESOLVED → REOPENED
Ever confirmed: true
Resolution: FIXED → ---
Landed on inbound.
http://hg.mozilla.org/mozilla-central/rev/c0e225533110
Status: REOPENED → RESOLVED
Last Resolved: 6 years ago6 years ago
Resolution: --- → FIXED

Comment 9

6 years ago
I have tried the steps from the description, I have add a large image in gmail but I didn't get the smalle  preview image (thumbnail), only a field with the attachment. I am doing something wrong? 
The memory wasn't increasing while I've tested this.
You need to log in before you can comment on or make changes to this bug.