1380649 - Gmail images disappear after resume

Status: RESOLVED FIXED

(Core :: Graphics: ImageLib, defect, P1)

Description

[Jeff Muizelaar [:jrmuizel]]

This image https://ssl.gstatic.com/gb/images/v2_ad17f477.png stops working. This is persistent across reloads. If I load this image in a new tab it doesn't show up when scaled at 100% or larger. If I zoom out to 30% it does show up. If I load it in private browsing it does show up.

Comment 1

[Jeff Muizelaar [:jrmuizel]]

This is probably a regression because it didn't happen before and seems unrelated to site behaviour.

Comment 2

[Jeff Muizelaar [:jrmuizel]]

Shift reloading the image fixes the problem.

Comment 3

[Jeff Muizelaar [:jrmuizel]]

This happened to me again.

Comment 4

[Milan Sreckovic [:milan] (needinfo for best results)]

Assuming not running with stylo.

Comment 5

[Jeff Muizelaar [:jrmuizel]]

I'm running with stylo now, but I saw it before I started running with stylo.

Comment 6

[Andrew Osmond [:aosmond] (he/him)]

Attachment: VolatileBuffer should remember if previously purged and the owner should explicitly clear that state., v1

Technically imagelib use of VolatileBuffer relies upon SurfaceCache being the first thing to lock it so that it gets the expected purge state information. This is bad because if something else maps the surface in first and sees the bad state, we would forget it was purged. VolatileBuffer should probably remember that and force the owner to clear that flag explicitly once it has repopulated the data.

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=5381c46a5e3b6219d47bfb19bc5835a8b1400560

Note that this is speculative, no evidence yet that this will fix it :).

Comment 7

[Andrew Osmond [:aosmond] (he/him)]

Attachment: VolatileBuffer should remember if previously purged and the owner should explicitly clear that state., v2

Remove VolatileBufferPtr::mPurged, forgot about that and it broke all the try builds...sigh.

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=2c913db020ba95e12821cdd87cdfb562ddede90d

Comment 8

[Andrew Osmond [:aosmond] (he/him)]

Attachment: VolatileBuffer should remember if previously purged and the owner should explicitly clear that state., v3

I didn't do the right thing on OSX, hence the gtest failure...

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

Comment 9

[Jeff Muizelaar [:jrmuizel]]

I just got this on facebook.com

Comment 10

[Mike Hommey [:glandium]]

Comment on attachment 8896293 [details] [diff] [review]
VolatileBuffer should remember if previously purged and the owner should explicitly clear that state., v3

Review of attachment 8896293 [details] [diff] [review]:
-----------------------------------------------------------------

Can you walk me through what this change does? because it seems idempotent to me, except for exposing a ClearBufferPurged, which I'm not entirely sure has its place here or is useful. I also don't see how it's related to the bug this is attached to.

Specifically, can you say why you think mPurged should be attached to the buffer instead of the ptr? To me, the ptr class can be thought as a tuple, where you get the ptr and whether it points to purged memory or not, and from there the called does what it wants to.

I can however see a problem with VolatileBuffer, but I don't see it addressed in this patch: when two ptr instances are created on the same buffer, and one is destroyed, it unlocks the buffer, meaning the OS is free to drop the corresponding pages, while the user of the first ptr still thinks it's fine. But I don't remember the class having been designed for this use case.

Comment 11

[Andrew Osmond [:aosmond] (he/him)]

(In reply to Mike Hommey [:glandium] from comment #10)
> Comment on attachment 8896293 [details] [diff] [review]
> VolatileBuffer should remember if previously purged and the owner should
> explicitly clear that state., v3
> 
> Review of attachment 8896293 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Can you walk me through what this change does? because it seems idempotent
> to me, except for exposing a ClearBufferPurged, which I'm not entirely sure
> has its place here or is useful. I also don't see how it's related to the
> bug this is attached to.
> 

I poked around the memory reports with Jeff when this happened to him. What it showed was the image was still in the surface cache, with multiple surfaces decoded at different sizes. From the display, I determined that the largest surface was somehow zero filled despite displaying correctly before suspend, and the smaller surfaces contained the expected data.

I have not been able to reproduce myself, but my hunch is that the gmail tab was in the background, meaning its images would be unlocked (i.e. no VolatileBufferPtr is alive) and thus open to purging by the OS. On suspend or reboot the OS took advantage of this for some pages, and thus our reclaimed pages when we did create a VolatileBufferPtr were zero filled. This would be consistent with the transparent display.

If the surface cache in imagelib is the first to try to acquire the volatile buffer, then it will catch this condition and redecode the image at the desired size. It did not do that. My guess is something else acquired it first, saw that it was purged and failed its operation.

To be clear, I don't know for certain if this will work based on how reproducible it is. But it is my best guess right now.

> Specifically, can you say why you think mPurged should be attached to the
> buffer instead of the ptr? To me, the ptr class can be thought as a tuple,
> where you get the ptr and whether it points to purged memory or not, and
> from there the called does what it wants to.
> 

If the surface is passed around, then the end consumer of the buffer might not have anything to do with the producer (imagelib) and thus have no idea or interest in figuring out how to restore it (redecode the image). This shouldn't prevent the producer from finding out later that the data was purged and doing the restoration itself.

(Not only that, imagelib itself doesn't even repopulate the buffer after finding out it is purged -- it discards it and it later on reallocates a new buffer when it is redecoding.)

> I can however see a problem with VolatileBuffer, but I don't see it
> addressed in this patch: when two ptr instances are created on the same
> buffer, and one is destroyed, it unlocks the buffer, meaning the OS is free
> to drop the corresponding pages, while the user of the first ptr still
> thinks it's fine. But I don't remember the class having been designed for
> this use case.

Each platform variant of VolatileBuffer::Lock/Unlock maintains a counter of how many consumers there are of the buffer. imagelib has always been using it in this manner -- the decoders are writing into the buffer, as readers are drawing it (e.g. this effect is visible with large images where you see it draw line-by-line as it decodes). Am I missing something?

Comment 12

[Mike Hommey [:glandium]]

(In reply to Andrew Osmond [:aosmond] (PTO 08/19-08/28) from comment #11)
> If the surface is passed around, then the end consumer of the buffer might
> not have anything to do with the producer (imagelib) and thus have no idea
> or interest in figuring out how to restore it (redecode the image). This
> shouldn't prevent the producer from finding out later that the data was
> purged and doing the restoration itself.

I'm sorry but I don't see how your patch makes things better for that case. Maybe it's missing changes in the consumer to make sense to me.

> Each platform variant of VolatileBuffer::Lock/Unlock maintains a counter of
> how many consumers there are of the buffer.

That's true. Which begs the question: what is it you think is going wrong?
And how does your patch solve it? I'm afraid I don't see it. Not knowing how imagelib is actually using this doesn't help too, and from your description, it would seem the problem would be on the consumer end, not in VolatileBuffer, but again, I don't know what the consumer is doing.

So it would really help if you could detail your thoughts about the series of events/functions calls that you think end up making images blank.

Comment 13

[Jim Mathies [:jimm]]

Based on priority and the fact that we're close to merging 57.

Comment 14

[Andrew Osmond [:aosmond] (he/him)]

Attachment: 0001-Bug-1380649-Assert-if-we-remap-a-purged-SourceSurfac.patch

(In reply to Mike Hommey [:glandium] from comment #12)
> (In reply to Andrew Osmond [:aosmond] (PTO 08/19-08/28) from comment #11)
> > If the surface is passed around, then the end consumer of the buffer might
> > not have anything to do with the producer (imagelib) and thus have no idea
> > or interest in figuring out how to restore it (redecode the image). This
> > shouldn't prevent the producer from finding out later that the data was
> > purged and doing the restoration itself.
> 
> I'm sorry but I don't see how your patch makes things better for that case.
> Maybe it's missing changes in the consumer to make sense to me.
> 
> > Each platform variant of VolatileBuffer::Lock/Unlock maintains a counter of
> > how many consumers there are of the buffer.
> 
> That's true. Which begs the question: what is it you think is going wrong?
> And how does your patch solve it? I'm afraid I don't see it. Not knowing how
> imagelib is actually using this doesn't help too, and from your description,
> it would seem the problem would be on the consumer end, not in
> VolatileBuffer, but again, I don't know what the consumer is doing.
> 
> So it would really help if you could detail your thoughts about the series
> of events/functions calls that you think end up making images blank.

Sorry for the delay on the response. After some thought, you are right. I'm putting the cart before the horse. I should land a diagnostic patch first to confirm the hunch rather than try to fix it and see if that works :).

The attached patch adds a diagnostic assert for when we hit the suspected condition. The stack trace will show us the culprit, if we indeed trip it. I can actually do this inside the SourceSurfaceVolatileData object wrapping the VolatileBuffer.

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=764d7ba829308186a6ccb7458e6468f4b8f1b11a

Comment 15

[Milan Sreckovic [:milan] (needinfo for best results)]

(In reply to Jim Mathies [:jimm] from comment #13)
> Based on priority and the fact that we're close to merging 57.

I'd like to understand how common this is before we do this. This is a type of report that we'd rarely get, so I'd have to guess a high multiplier value for it.

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/722569a89186
Assert if we remap a purged SourceSurfaceVolatileBuffer. r=jrmuizel

Comment 17

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/722569a89186

Comment 18

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b3f6d72c338e
Backed out changeset 722569a89186 due to bug 1400549 and getting the crash signatures we needed.

Comment 19

[Andrew Osmond [:aosmond] (he/him)]

The crash reports confirm that surfaces that were purged volatile buffers are remaining in the cache. It also demostrates the code path that this happens on. I'll set to work on a proper fix now.

Comment 20

[Ryan VanderMeulen [:RyanVM]]

Merge of backout:
https://hg.mozilla.org/mozilla-central/rev/b3f6d72c338e

Comment 21

[Andrew Osmond [:aosmond] (he/him)]

Attachment: Part 1. Ensure SurfaceCache::CollectSizeOfSurfaces removes purged volatile buffer-backed surfaces., v1

Comment 22

[Andrew Osmond [:aosmond] (he/him)]

Attachment: Part 2. Ensure SourceSurfaceVolatileData does not forget its purged state., v1 [carries r=jrmuizel]

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=1154f3641315a9d0692b84266a299f794470aa80

Comment 23

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d4f5756b1d12
Part 1. Ensure SurfaceCache::CollectSizeOfSurfaces removes purged volatile buffer-backed surfaces. r=tnikkel

Comment 24

[Andrew Osmond [:aosmond] (he/him)]

Comment on attachment 8909283 [details] [diff] [review]
Part 2. Ensure SourceSurfaceVolatileData does not forget its purged state., v1 [carries r=jrmuizel]

In retrospect, this is almost identical to attachment 8908717 [details] [diff] [review], save I replaced the diagnostic assert with a graceful failure.

Comment 25

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/58844e6f37b2
Part 2. Ensure SourceSurfaceVolatileData does not forget its purged state. r=jrmuizel

Comment 26

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Backed out for frequently asserting at image/SurfaceCache.cpp:138:

https://hg.mozilla.org/integration/mozilla-inbound/rev/b577f883dc898d203b74f7b02345a0a7dc3d32c0
https://hg.mozilla.org/integration/mozilla-inbound/rev/cc0cd5f70746c4efeb84bb76ce32d19043145341

Push with first failure: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=5fb440d47ab9ebfa9633ff88274f221924f3cc3a&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=running&filter-resultStatus=pending&filter-resultStatus=runnable

Failure log 1: https://treeherder.mozilla.org/logviewer.html#?job_id=131862744&repo=mozilla-inbound

[task 2017-09-19T01:38:34.832Z] 01:38:34     INFO - TEST-START | dom/html/test/browser_bug592641.js
[task 2017-09-19T01:38:34.990Z] 01:38:34     INFO - GECKO(2882) | ++DOCSHELL 0x7f55ecf31000 == 2 [pid = 3029] [id = {0b2b4bdd-c850-480f-9cf3-d5f7e5dbf310}]
[task 2017-09-19T01:38:34.992Z] 01:38:34     INFO - GECKO(2882) | ++DOMWINDOW == 8 (0x7f55ecf33000) [pid = 3029] [serial = 21] [outer = (nil)]
[task 2017-09-19T01:38:35.014Z] 01:38:35     INFO - GECKO(2882) | --DOCSHELL 0x7f64ec728000 == 1 [pid = 2928] [id = {f8db712a-c1e8-4411-9739-e03fa4ba8dae}]
[task 2017-09-19T01:38:35.018Z] 01:38:35     INFO - GECKO(2882) | --DOMWINDOW == 7 (0x7f64ec085000) [pid = 2928] [serial = 15] [outer = (nil)] [url = about:blank]
[task 2017-09-19T01:38:35.022Z] 01:38:35     INFO - GECKO(2882) | --DOMWINDOW == 6 (0x7f64ec740800) [pid = 2928] [serial = 18] [outer = (nil)] [url = about:blank]
[task 2017-09-19T01:38:35.107Z] 01:38:35     INFO - GECKO(2882) | ++DOMWINDOW == 9 (0x7f55ee80b800) [pid = 3029] [serial = 22] [outer = 0x7f55ecf33000]
[task 2017-09-19T01:38:36.317Z] 01:38:36     INFO - GECKO(2882) | [Parent 2882, Main Thread] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80070057: file /builds/worker/workspace/build/src/netwerk/base/nsChannelClassifier.cpp, line 342
[task 2017-09-19T01:38:36.759Z] 01:38:36     INFO - GECKO(2882) | ++DOMWINDOW == 10 (0x7f55ef5c4000) [pid = 3029] [serial = 23] [outer = 0x7f55ecf33000]
[task 2017-09-19T01:38:36.825Z] 01:38:36     INFO - GECKO(2882) | [Child 3029, Main Thread] WARNING: stylo: Web Components not supported yet: file /builds/worker/workspace/build/src/dom/base/nsDocument.cpp, line 6389
[task 2017-09-19T01:38:36.836Z] 01:38:36     INFO - GECKO(2882) | [Child 3029, Main Thread] WARNING: stylo: Web Components not supported yet: file /builds/worker/workspace/build/src/dom/base/nsDocument.cpp, line 6389
[task 2017-09-19T01:38:36.979Z] 01:38:36     INFO - GECKO(2882) | [Child 3029, Main Thread] WARNING: Subsequent listeners are not retargetable: file /builds/worker/workspace/build/src/netwerk/protocol/http/HttpChannelChild.cpp, line 3351
[task 2017-09-19T01:38:37.040Z] 01:38:37     INFO - GECKO(2882) | --DOMWINDOW == 31 (0x7ffa5a9b7800) [pid = 2882] [serial = 17] [outer = (nil)] [url = about:blank]
[task 2017-09-19T01:38:37.246Z] 01:38:37     INFO - GECKO(2882) | ++DOCSHELL 0x7f81dc022800 == 4 [pid = 2978] [id = {24353975-0618-4025-8b4d-baab82ee02af}]
[task 2017-09-19T01:38:37.250Z] 01:38:37     INFO - GECKO(2882) | ++DOMWINDOW == 9 (0x7f81dcdec800) [pid = 2978] [serial = 17] [outer = (nil)]
[task 2017-09-19T01:38:37.375Z] 01:38:37     INFO - GECKO(2882) | ++DOMWINDOW == 10 (0x7f81ddcc7800) [pid = 2978] [serial = 18] [outer = 0x7f81dcdec800]
[task 2017-09-19T01:38:37.675Z] 01:38:37     INFO - GECKO(2882) | [Parent 2882, Main Thread] WARNING: NS_ENSURE_TRUE(aSecondURI) failed: file /builds/worker/workspace/build/src/dom/base/ThirdPartyUtil.cpp, line 98
[task 2017-09-19T01:38:37.845Z] 01:38:37     INFO - GECKO(2882) | [Parent 2882, Main Thread] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80070057: file /builds/worker/workspace/build/src/netwerk/base/nsChannelClassifier.cpp, line 342
[task 2017-09-19T01:38:37.988Z] 01:38:37     INFO - GECKO(2882) | Assertion failure: false (MOZ_ASSERT_UNREACHABLE: Called GetDrawableSurface() on a placeholder), at /builds/worker/workspace/build/src/image/SurfaceCache.cpp:138
[task 2017-09-19T01:38:37.990Z] 01:38:37     INFO - GECKO(2882) | #01: mozilla::image::ImageSurfaceCache::CollectSizeOfSurfaces<mozilla::image::SurfaceCacheImpl::CollectSizeOfSurfaces(mozilla::image::ImageKey, nsTArray<mozilla::image::SurfaceMemoryCounter>&, mozilla::MallocSizeOf, const StaticMutexAutoLock&)::<lambda(mozilla::NotNull<mozilla::image::CachedSurface*>)> > [image/SurfaceCache.cpp:631]
[task 2017-09-19T01:38:37.991Z] 01:38:37     INFO - 
[task 2017-09-19T01:38:38.000Z] 01:38:37     INFO - GECKO(2882) | #02: mozilla::image::SurfaceCache::CollectSizeOfSurfaces [image/SurfaceCache.cpp:1189]
[task 2017-09-19T01:38:38.003Z] 01:38:37     INFO - 
[task 2017-09-19T01:38:38.005Z] 01:38:37     INFO - GECKO(2882) | #03: mozilla::image::RasterImage::CollectSizeOfSurfaces [image/RasterImage.cpp:759]
[task 2017-09-19T01:38:38.005Z] 01:38:37     INFO - 
[task 2017-09-19T01:38:38.006Z] 01:38:38     INFO - GECKO(2882) | #04: mozilla::image::ImageMemoryCounter::ImageMemoryCounter [xpcom/ds/nsTArray.h:398]
[task 2017-09-19T01:38:38.006Z] 01:38:38     INFO - 
[task 2017-09-19T01:38:38.016Z] 01:38:38     INFO - GECKO(2882) | #05: imgMemoryReporter::ImagesContentUsedUncompressedDistinguishedAmount [xpcom/ds/nsTArray.h:2234]
[task 2017-09-19T01:38:38.021Z] 01:38:38     INFO - 
[task 2017-09-19T01:38:38.024Z] 01:38:38     INFO - GECKO(2882) | #06: GetInfallibleAmount [xpcom/base/nsMemoryReporterManager.cpp:2439]
[task 2017-09-19T01:38:38.035Z] 01:38:38     INFO - 
[task 2017-09-19T01:38:38.040Z] 01:38:38     INFO - GECKO(2882) | #07: NS_InvokeByIndex
[task 2017-09-19T01:38:38.043Z] 01:38:38     INFO - 
[task 2017-09-19T01:38:38.046Z] 01:38:38     INFO - GECKO(2882) | #08: CallMethodHelper::Call [js/xpconnect/src/xpcprivate.h:775]
[task 2017-09-19T01:38:38.054Z] 01:38:38     INFO - 
[task 2017-09-19T01:38:38.056Z] 01:38:38     INFO - GECKO(2882) | #09: XPCWrappedNative::CallMethod [js/xpconnect/src/XPCWrappedNative.cpp:1282]
[task 2017-09-19T01:38:38.061Z] 01:38:38     INFO - 
[task 2017-09-19T01:38:38.064Z] 01:38:38     INFO - GECKO(2882) | #10: XPC_WN_GetterSetter [js/xpconnect/src/xpcprivate.h:1685]
[task 2017-09-19T01:38:38.066Z] 01:38:38     INFO - 
[task 2017-09-19T01:38:38.075Z] 01:38:38     INFO - GECKO(2882) | #11: js::CallJSNative [js/src/jscntxtinlines.h:294]
[task 2017-09-19T01:38:38.078Z] 01:38:38     INFO - 
[task 2017-09-19T01:38:38.084Z] 01:38:38     INFO - GECKO(2882) | #12: js::InternalCallOrConstruct [js/src/vm/Interpreter.cpp:495]
[task 2017-09-19T01:38:38.088Z] 01:38:38     INFO - 
[task 2017-09-19T01:38:38.093Z] 01:38:38     INFO - GECKO(2882) | #13: js::Call [js/src/vm/Interpreter.cpp:559]
[task 2017-09-19T01:38:38.095Z] 01:38:38     INFO - 
[task 2017-09-19T01:38:38.100Z] 01:38:38     INFO - GECKO(2882) | #14: js::CallGetter [js/src/jsapi.h:74]
[task 2017-09-19T01:38:38.102Z] 01:38:38     INFO - 
[task 2017-09-19T01:38:38.107Z] 01:38:38     INFO - GECKO(2882) | #15: GetExistingProperty<(js::AllowGC)1u> [js/src/vm/NativeObject.cpp:2122]
[task 2017-09-19T01:38:38.109Z] 01:38:38     INFO - 
[task 2017-09-19T01:38:38.113Z] 01:38:38     INFO - GECKO(2882) | #16: NativeGetPropertyInline<(js::AllowGC)1u> [js/src/vm/NativeObject.cpp:2384]
[task 2017-09-19T01:38:38.118Z] 01:38:38     INFO - 
[task 2017-09-19T01:38:38.121Z] 01:38:38     INFO - GECKO(2882) | #17: js::GetProperty [js/src/vm/NativeObject.h:1541]
[task 2017-09-19T01:38:38.123Z] 01:38:38     INFO - 
[task 2017-09-19T01:38:38.126Z] 01:38:38     INFO - GECKO(2882) | #18: js::GetObjectElementOperation [js/src/vm/Interpreter-inl.h:524]
[task 2017-09-19T01:38:38.131Z] 01:38:38     INFO - 
[task 2017-09-19T01:38:38.134Z] 01:38:38     INFO - GECKO(2882) | #19: js::GetElementOperation [js/src/vm/Interpreter-inl.h:629]
[task 2017-09-19T01:38:38.136Z] 01:38:38     INFO - 
[task 2017-09-19T01:38:38.139Z] 01:38:38     INFO - GECKO(2882) | #20: Interpret [js/src/vm/Interpreter.cpp:2922]
[task 2017-09-19T01:38:38.142Z] 01:38:38     INFO - 
[task 2017-09-19T01:38:38.145Z] 01:38:38     INFO - GECKO(2882) | #21: js::RunScript [js/src/vm/Interpreter.cpp:406]
[task 2017-09-19T01:38:38.150Z] 01:38:38     INFO - 
[task 2017-09-19T01:38:38.154Z] 01:38:38     INFO - GECKO(2882) | #22: js::InternalCallOrConstruct [js/src/vm/Interpreter.cpp:513]
[task 2017-09-19T01:38:38.159Z] 01:38:38     INFO - 
[task 2017-09-19T01:38:38.167Z] 01:38:38     INFO - GECKO(2882) | #23: Interpret [js/src/vm/Interpreter.cpp:3084]
[task 2017-09-19T01:38:38.170Z] 01:38:38     INFO - 
[task 2017-09-19T01:38:38.172Z] 01:38:38     INFO - GECKO(2882) | #24: js::RunScript [js/src/vm/Interpreter.cpp:406]
[task 2017-09-19T01:38:38.174Z] 01:38:38     INFO - 
[task 2017-09-19T01:38:38.180Z] 01:38:38     INFO - GECKO(2882) | #25: js::InternalCallOrConstruct [js/src/vm/Interpreter.cpp:513]
[task 2017-09-19T01:38:38.182Z] 01:38:38     INFO - 
[task 2017-09-19T01:38:38.185Z] 01:38:38     INFO - GECKO(2882) | #26: Interpret [js/src/vm/Interpreter.cpp:3084]
[task 2017-09-19T01:38:38.192Z] 01:38:38     INFO - 
[task 2017-09-19T01:38:38.198Z] 01:38:38     INFO - GECKO(2882) | #27: js::RunScript [js/src/vm/Interpreter.cpp:406]
[task 2017-09-19T01:38:38.201Z] 01:38:38     INFO - 
[task 2017-09-19T01:38:38.203Z] 01:38:38     INFO - GECKO(2882) | #28: js::InternalCallOrConstruct [js/src/vm/Interpreter.cpp:513]
[task 2017-09-19T01:38:38.205Z] 01:38:38     INFO - 
[task 2017-09-19T01:38:38.208Z] 01:38:38     INFO - GECKO(2882) | #29: js::Call [js/src/vm/Interpreter.cpp:559]
[task 2017-09-19T01:38:38.210Z] 01:38:38     INFO - 
[task 2017-09-19T01:38:38.214Z] 01:38:38     INFO - GECKO(2882) | #30: js::jit::InterpretResume [js/src/jsapi.h:74]
[task 2017-09-19T01:38:38.220Z] 01:38:38     INFO - 
[task 2017-09-19T01:38:38.222Z] 01:38:38     INFO - GECKO(2882) | #31: ??? (???:???)

Failure log 2: https://treeherder.mozilla.org/logviewer.html#?job_id=131917420&repo=mozilla-inbound

Comment 28

[Andrew Osmond [:aosmond] (he/him)]

I missed a check to IsPlaceholder. Will add.

Comment 29

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a23a810d9585
Part 1. Ensure SurfaceCache::CollectSizeOfSurfaces removes purged volatile buffer-backed surfaces. r=tnikkel
https://hg.mozilla.org/integration/mozilla-inbound/rev/3ff9cd026e70
Part 2. Ensure SourceSurfaceVolatileData does not forget its purged state. r=jrmuizel

Comment 31

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

https://hg.mozilla.org/mozilla-central/rev/a23a810d9585
https://hg.mozilla.org/mozilla-central/rev/3ff9cd026e70

Comment 35

[Adam Roach [:abr]]

Updated to a fresh nightly, and now I'm hitting your assert. See bp-5361c914-37c9-4f32-890a-c82990170920

Comment 36

[Adam Roach [:abr]]

(In reply to Adam Roach [:abr] from comment #35)
> Updated to a fresh nightly, and now I'm hitting your assert. See
> bp-5361c914-37c9-4f32-890a-c82990170920

Never mind -- my previous update apparently didn't take me all the way to today's build -- this was based on a 09-16 build. I've updated, verified today's date in "About Nightly", and will keep my eyes open for related issues.