Last Comment Bug 663955 - gtk2 nsFilePicker preview uses memory and decodes twice
: gtk2 nsFilePicker preview uses memory and decodes twice
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Widget: Gtk (show other bugs)
: unspecified
: Other Linux
: -- normal (vote)
: mozilla7
Assigned To: Rodd Zurcher
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-06-13 14:02 PDT by Rodd Zurcher
Modified: 2011-09-02 00:55 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
uses gdk_pixbuf_get_file_info() instead of decoding twice (1.87 KB, patch)
2011-06-13 14:09 PDT, Rodd Zurcher
roc: review+
Details | Diff | Splinter Review

Description Rodd Zurcher 2011-06-13 14:02:34 PDT
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.
Comment 1 Rodd Zurcher 2011-06-13 14:09:46 PDT
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.
Comment 2 Rodd Zurcher 2011-06-13 14:12:52 PDT
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 3 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-06-13 16:58:33 PDT
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 :-).
Comment 4 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-06-13 16:58:48 PDT
Thanks for the patch!
Comment 5 Dão Gottwald [:dao] 2011-06-13 21:01:07 PDT
http://hg.mozilla.org/mozilla-central/rev/6eb2bcc24f76
Comment 6 :Ehsan Akhgari 2011-06-13 21:19:28 PDT
The push broke the build, so I backed out all of its changesets.
Comment 7 :Ehsan Akhgari 2011-06-13 21:25:17 PDT
Landed on inbound.
Comment 8 Dão Gottwald [:dao] 2011-06-13 21:30:06 PDT
http://hg.mozilla.org/mozilla-central/rev/c0e225533110
Comment 9 Vlad [QA] 2011-09-02 00:55:38 PDT
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.

Note You need to log in before you can comment on or make changes to this bug.