1109438 - Re-disable discarding of chrome images

Status: RESOLVED WONTFIX

(Core :: Graphics: ImageLib, defect)

Description

[Seth Fowler [:seth] [:s2h]]

I changed ImageLib's behavior to retain source data for chrome images and allow them to be discarded in bug 1060869 because there was a chicken-and-egg situation going on where I needed to land bug 1060869 in order to land bug 1104622, and I couldn't easily *not* retain the source data for chrome images without landing bug 1104622. I also hadn't tested that configuration, and it was close to the merge.

Note that this bug depends on bug 1104622, but I can't express that in Bugzilla due to cycles in the dependency graph. We need a Bugzilla cycle collector. =)

So: it's time to re-disable discarding of chrome images, and stop retaining their source data. We should uplift this to Aurora as well.

Comment 1

[Seth Fowler [:seth] [:s2h]]

Attachment: Re-disable discarding of chrome images

Here's the patch.

Comment 2

[Seth Fowler [:seth] [:s2h]]

Try job here:

https://tbpl.mozilla.org/?tree=Try&rev=79c63f764d0b

Comment 3

[Seth Fowler [:seth] [:s2h]]

Thanks for the review!

It looks like something is going wrong on try, unfortunately, and I can't easily reproduce it locally. I'll investigate tomorrow.

Comment 4

[Joel Maher ( :jmaher ) (UTC -8)]

any status on this?

Comment 5

[Seth Fowler [:seth] [:s2h]]

(In reply to Joel Maher (:jmaher) from comment #4)
> any status on this?

I'm not sure we even want this anymore.

The reason we wanted this originally was that we wanted to reduce memory usage of chrome images by dropping their source data.

However, if we do that, we can't use downscale-during-decode for chrome images, and it's actually not clear to me that discarding the source data is more valuable than enabling downscale-during-decode. It's true that when chrome images are displayed at 1:1, DDD gains us nothing. However, particularly on Windows chrome images are often *not* displayed 1:1, and many popular addons also do not display their images at 1:1. 

Since the decoded version of images is generally *much* larger than the source data, I think we are probably better off favoring optimizations that reduce the decoded size, rather than reducing the source data size.

So that's pretty much what the status is: I haven't work on this because the closer we get to flipping on DDD for all images types, the less sense this change seems to make.

Comment 6

[Seth Fowler [:seth] [:s2h]]

Actually, there is a second upcoming feature which also argues against supporting this.

Eventually we are going to support higher-priority image locking for synchronous operations like canvas.drawImage(). In a scenario like that, we'd discard even visible/locked images if necessary to ensure that a high-priority image can be decoded. If we disable discarding for chrome images, we've reduced the amount of memory that we can free using this technique, and on low-memory B2G devices we really need every byte we can get.

I think these concerns, taken together, include me to WONTFIX this bug, at least for now.