1432411 - Crash in mozilla::layers::CanvasClientSharedSurface::UpdateRenderer

Status: RESOLVED FIXED

(Core :: Graphics: Layers, defect)

Description

[[:philipp]]

This bug was filed from the Socorro interface and is
report bp-a2ec6c66-ae81-4510-b465-b1ee20180123.
=============================================================

Top 10 frames of crashing thread:

0 xul.dll mozilla::layers::CanvasClientSharedSurface::UpdateRenderer gfx/layers/client/CanvasClient.cpp:415
1 xul.dll mozilla::layers::CanvasClientSharedSurface::Update gfx/layers/client/CanvasClient.cpp:380
2 xul.dll mozilla::layers::ShareableCanvasRenderer::UpdateCompositableClient gfx/layers/ShareableCanvasRenderer.cpp:234
3 xul.dll mozilla::layers::ClientCanvasLayer::RenderLayer gfx/layers/client/ClientCanvasLayer.cpp:29
4 xul.dll mozilla::layers::ClientLayer::RenderLayerWithReadback gfx/layers/client/ClientLayerManager.h:383
5 xul.dll mozilla::layers::ClientContainerLayer::RenderLayer gfx/layers/client/ClientContainerLayer.h:58
6 xul.dll mozilla::layers::ClientLayerManager::EndTransactionInternal gfx/layers/client/ClientLayerManager.cpp:359
7 xul.dll nsIFrame::ClearInvalidationStateBits layout/generic/nsFrame.cpp:7058
8 xul.dll nsTArray_Impl<mozilla::layout::FrameChildList, nsTArrayInfallibleAllocator>::AppendElement<mozilla::layout::FrameChildList, nsTArrayInfallibleAllocator> xpcom/ds/nsTArray.h:2185
9 xul.dll mozilla::layers::ClientLayerManager::EndTransaction gfx/layers/client/ClientLayerManager.cpp:423

=============================================================

these crashes (mainly in the content process) are regressing in volume cross-platform in firefox 59 since nightly build 20180106100308.

this would be the changelog to the build before:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=2a4362ce2d9fa82f62909532a1b534d214761341&tochange=ece8a96dfaa436c9bcf53105877b3923a264ae31

Comment 1

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

Jeff, there are some error messages with mImplicitMakeCurrent.  Also, does "EGL+" mean we're bypassing ANGLE?

Comment 2

[Kelsey Gilbert [:jgilbert]]

(In reply to Milan Sreckovic [:milan] from comment #1)
> Jeff, there are some error messages with mImplicitMakeCurrent.  Also, does
> "EGL+" mean we're bypassing ANGLE?

EGL+ includes ANGLE. ANGLE's our only EGL on Windows. (for now)
I'll take a look.

Comment 3

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

Any progress on this?

Comment 4

[Kelsey Gilbert [:jgilbert]]

I don't have time for this for the next week.

Comment 5

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

Attachment: Bug 1432411: Wallpaper over the case where there is no Screen.

Review commit: https://reviewboard.mozilla.org/r/218774/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/218774/

Comment 6

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 8949449 [details]
Bug 1432411: Wallpaper over the case where there is no Screen.

https://reviewboard.mozilla.org/r/218774/#review224722

I really want to know why screen could be null. Can you keep an assert that it's non-null?

Comment 7

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

Comment on attachment 8949449 [details]
Bug 1432411: Wallpaper over the case where there is no Screen.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/218774/diff/1-2/

Comment 8

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

(In reply to Jeff Gilbert [:jgilbert] from comment #6)
> Comment on attachment 8949449 [details]
> Bug 1432411: Wallpaper over the case where there is no Screen.
> 
> https://reviewboard.mozilla.org/r/218774/#review224722
> 
> I really want to know why screen could be null. Can you keep an assert that
> it's non-null?

Will do, with gfxCriticalError that asserts in debug.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c13ba6039ceb
Wallpaper over the case where there is no Screen. r=jgilbert

Comment 10

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/c13ba6039ceb

Comment 11

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

https://hg.mozilla.org/mozilla-central/rev/c13ba6039ceb

Comment 12

[Julien Cristau [:jcristau]]

Please request uplift to beta when you get a chance.

Comment 13

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

Comment on attachment 8949449 [details]
Bug 1432411: Wallpaper over the case where there is no Screen.

Approval Request Comment
[Feature/Bug causing the regression]: 1427668
[User impact if declined]: crashes in noticable volume, regression
[Is this code covered by automated tests?]:
[Has the fix been verified in Nightly?]: yes, crashes stopped
[Needs manual test from QE? If yes, steps to reproduce]: 
[List of other uplifts needed for the feature/fix]:
[Is the change risky?]:no, early return to a tested branch
[Why is the change risky/not risky?]:
[String changes made/needed]:

Comment 14

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8949449 [details]
Bug 1432411: Wallpaper over the case where there is no Screen.

Even though the crash volume has gone down with b9 (which doesn't have this fix), this fix is still low risk and good to uplift, Beta59+

Comment 15

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

https://hg.mozilla.org/releases/mozilla-beta/rev/38c1798e2a2e