Closed Bug 1171371 Opened 5 years ago Closed 4 years ago
On memory-pressure, remove any stale images from the visible images list
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.)
Here's the patch, implementing along the lines of our discussion on IRC.
Attachment #8615195 - Flags: review?(tnikkel)
Attachment #8615195 - Flags: review?(tnikkel) → review+
I think you need to bump the NS_IPRESSHELL_IID.
This version of the attach bumps NS_IPRESSHELL_IID.
Attachment #8615195 - Attachment is obsolete: true
Thanks for catching that Mats.
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
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.
Attachment #8615455 - Attachment is obsolete: true
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.
(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 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.
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?
You need to log in before you can comment on or make changes to this bug.