1171371 - On memory-pressure, remove any stale images from the visible images list

Status: RESOLVED FIXED

(Core :: Layout: Images, Video, and HTML Frames, defect)

Description

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

On low memory B2G devices, memory-pressure is not an uncommon situation. We should remove stale images from the visible images list when we enter memory-pressure, because that will allow us to discard those images and hopefully ease the memory pressure.

(When I say "stale images" here, I mean images which are in the visible images list, but which are no longer visible.)

Comment 1

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

Attachment: On memory-pressure, remove any stale images from the visible images list

Here's the patch, implementing along the lines of our discussion on IRC.

Comment 2

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

https://treeherder.mozilla.org/#/jobs?repo=try&revision=369cfedea741

Comment 3

[Mats Palmgren (inactive)]

I think you need to bump the NS_IPRESSHELL_IID.

Comment 4

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

Attachment: On memory-pressure, remove any stale images from the visible images list

This version of the attach bumps NS_IPRESSHELL_IID.

Comment 5

[Timothy Nikkel (:tnikkel)]

Thanks for catching that Mats.

Comment 6

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/061363215959

Comment 7

[Ryan VanderMeulen [:RyanVM]]

Backed out for Android testAboutPage crashes.
https://treeherder.mozilla.org/logviewer.html#?job_id=10440825&repo=mozilla-inbound

https://hg.mozilla.org/integration/mozilla-inbound/rev/227d356ac030

Comment 8

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

Attachment: On memory-pressure, remove any stale images from the visible images list

OK, we just needed an additional null check for mPresContext.

I have to say I was a little surprised at this, as it looks like other code
assumes that mPresContext is non-null, but I confirmed with dbaron that it *can*
be non-null in some situations - for example, during page teardown.

Comment 9

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/60963a019905

Comment 10

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

Comment on attachment 8615728 [details] [diff] [review]
On memory-pressure, remove any stale images from the visible images list

[Approval Request Comment]
Bug caused by (feature/regressing bug #): This patch helps us recover from memory pressure with image locking enabled. We enabled image locking on B2G in bug 1148696.
User impact if declined: Images may fail to decode due to low memory, causing blank thumbnails in the gallery which may persist across reboots (making this effectively a data loss issue). This patch is one of several that together fix bug 1166136, which is nominated for blocking 2.2, and bug 1164164, which is a 2.2 blocker. Additionally, this patch should help us recover from memory pressure more easily, no matter what the cause is.
Testing completed: Extensively tested locally. On mozilla-inbound and anticipated to land on mozilla-central as of now.
Risk to taking this patch (and alternatives if risky): This is a pretty low risk patch.
String or UUID changes made by this patch: None.

Comment 11

[Phil Ringnalda (:philor)]

(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #7)
> Backed out for Android testAboutPage crashes.

What he said. https://treeherder.mozilla.org/logviewer.html#?job_id=10453971&repo=mozilla-inbound, backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/ca87dfad14cf

Comment 12

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

Comment on attachment 8615728 [details] [diff] [review]
On memory-pressure, remove any stale images from the visible images list

Hmm, frustrating. Not really sure what's going on here, and unfortunately we're out of time to debug it.

Comment 13

[Timothy Nikkel (:tnikkel)]

I rebased and pushed to try
https://treeherder.mozilla.org/#/jobs?repo=try&revision=809473283162

Looks to be green now.

Seth, any reason I shouldn't land this now?

Comment 14

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9bf8c3d6ec19

Comment 15

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/9bf8c3d6ec19