663955 - gtk2 nsFilePicker preview uses memory and decodes twice

Status: RESOLVED FIXED

(Core :: Widget: Gtk, defect)

Description

[Rodd Zurcher]

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]

Attachment: 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]

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) (email my personal email if necessary)]

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) (email my personal email if necessary)]

Thanks for the patch!

Comment 5

[Dão Gottwald [:dao]]

http://hg.mozilla.org/mozilla-central/rev/6eb2bcc24f76

Comment 6

[(no longer active)]

The push broke the build, so I backed out all of its changesets.

Comment 7

[(no longer active)]

Landed on inbound.

Comment 8

[Dão Gottwald [:dao]]

http://hg.mozilla.org/mozilla-central/rev/c0e225533110

Comment 9

[Vlad [QA]]

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.