Looks like there is a new, low volume variant of the SuggestedSize crash even after bug 1397235 landed. The image surface cache is empty somehow. Perhaps as a result of Prune or CollectSizeOfSurfaces. https://crash-stats.mozilla.com/report/index/b13117d4-f2d3-488f-b6ef-eb9fd0170920 https://crash-stats.mozilla.com/report/index/634ca4bd-2019-4e20-b5ff-f8fb80170920 https://crash-stats.mozilla.com/report/index/b13117d4-f2d3-488f-b6ef-eb9fd0170920

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=9308df4712f085c72bb9d9fe67a31f302a14714e

This is showing up on Android as [@ mozilla::image::ImageSurfaceCache::SuggestedSize ]

Pushed by aosmond@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/220a9b1bed87 Ensure SurfaceCache state coherency whenever we perform an operation that may discard surfaces. r=tnikkel

I guess we want to uplift this to 57, right?

(In reply to Sylvestre Ledru [:sylvestre] from comment #5) > I guess we want to uplift this to 57, right? Yes. I wanted to confirm the crashes are gone in 58 first however, and it is only just making it into today's build.

ok, fyi, 57b3 gtb is Monday evening Paris time!

Comment on attachment 8910289 [details] [diff] [review] Ensure SurfaceCache state coherency whenever we perform an operation that may discard surfaces., v1 Approval Request Comment [Feature/Bug causing the regression]: 1380649, 1370412 [User impact if declined]: May experience occasional crash. [Is this code covered by automated tests?]: In part. [Has the fix been verified in Nightly?]: Yes, we stopped getting crash reports from the field. [Needs manual test from QE? If yes, steps to reproduce]: No. [List of other uplifts needed for the feature/fix]: N/A [Is the change risky?]: No. [Why is the change risky/not risky?]: Contained to one module, well understood implications. The only time we will have a behaviour change is when we would have crashed a bit later anyways. [String changes made/needed]: N/A

No crash on the two first signature after 21th builds One on the 23th build bp-0015fd63-4f3b-46a8-a098-98fc30170924 which maybe didn't have the fix. Anyway, it shows a positive improvement, so, taking it.

(In reply to Sylvestre Ledru [:sylvestre] from comment #9) > No crash on the two first signature after 21th builds > One on the 23th build bp-0015fd63-4f3b-46a8-a098-98fc30170924 which maybe > didn't have the fix. > Anyway, it shows a positive improvement, so, taking it. That has the fix, but the crash suggests a different problem. I'm not sure how to explain that one! It claims to be crashing returning a value we have on the stack. Or possible checking an object variable on the heap (which should be valid, we check before using it). We see some very low volume uncommon/weird signatures in imagelib that just seem to be random -- unless it happens with some volume, I'm not terribly worried.

(In reply to Andrew Osmond [:aosmond] from comment #8) > [Is this code covered by automated tests?]: In part. > [Has the fix been verified in Nightly?]: Yes, we stopped getting crash > reports from the field. > [Needs manual test from QE? If yes, steps to reproduce]: No. Setting qe-verify- based on Andrew's assessment on manual testing needs and the fact that this fix has - at least in some measure - automated coverage.