It looks like the layers refactor regressed bug 829747, where we do async canvas updates. CanvasClient::Updated() calls mForwarder->UpdateTexture, and ShadowLayerForwarder::UpdateTexture calls AddPaint not AddNoSwapPaint.

Ugh, what a pain. I guess I can look into this.

Nick, is there any reason we can't always use AddNoSwapPaint() in ShadowLayerForwarder::UpdateTexture()? Do we ever need a reply?

Not sure if this is really correct for 2d canvas, but should be ok for 3d. Started a try run here: https://tbpl.mozilla.org/?tree=Try&rev=aa301dee400e

(In reply to James Willcox (:snorp) (jwillcox@mozilla.com) from comment #2) > Nick, is there any reason we can't always use AddNoSwapPaint() in > ShadowLayerForwarder::UpdateTexture()? Do we ever need a reply? Yes, there is a reason - I think double buffered image layers need to get their surface descriptors back from the compositor, probably single buffered ones too. See BasicShadowLayerManager::ForwardTransaction

I think we probably need either an UpdateTextureNoSwap method, or UpdateTexture to take a no swap flag. Perhaps better would be if that flag could be attached to aCompositable's flags, rather than being passed explicitly.

Comment on attachment 743808 [details] [diff] [review] Make canvas updates asynchronous once again Review of attachment 743808 [details] [diff] [review]: ----------------------------------------------------------------- r=me with the nits ::: gfx/layers/ipc/ImageBridgeChild.cpp @@ +103,5 @@ > + aDescriptor->type() != SurfaceDescriptor::Tnull_t) { > + MOZ_ASSERT(aCompositable); > + MOZ_ASSERT(aCompositable->GetIPDLActor()); > + mTxn->AddNoSwapEdit(OpPaintTexture(nullptr, aCompositable->GetIPDLActor(), 1, > + SurfaceDescriptor(*aDescriptor))); nit: indent (aligned with wrong opening bracket) ::: gfx/layers/ipc/ShadowLayers.h @@ +276,5 @@ > /** > + * Same as above, but performs an asynchronous layer transaction > + */ > + virtual void UpdateTextureNoSwap(CompositableClient* aCompositable, > + TextureIdentifier aTextureId, nit: indent

Unfortunately this causes us to leak shared memory with Canvas2D. When we send the transaction, we add the Shmem to the changeset and clear our local reference to it. On the compositor side, we give the Shmem to the ImageHost, which uploads to the texture from it and then creates a reply to send it back. Since this is a no-reply transaction, we don't bother to send the reply and we leak instead. What was the intended behaviour here? To create a new shmem every frame (and have the compositor free it), or to keep re-using it (and require that the compositor finish copying from it during the transaction). We should also probably send this intention over IPC so that the compositor knows that it shouldn't be writing a reply. And assert that no reply changesets are generated.

This was just a typo on my part. I meant to do CanvasWebGL async, not 2D. Oops. I'm attaching a patch to 871390