1897201 - Crash in [@ nsWrapperCache::GetWrapperMaybeDead] from CanvasRenderingContext2D_Binding::getTransform

Status: RESOLVED FIXED

(Core :: Graphics: Canvas2D, defect)

Description

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

This is a rather frequent Mobile crash since Fenix 125. The crashstacks are corrupted. The same signature had been reported in the past in bug 1838496, bug 1816975 and bug 1515463.

Crash report: https://crash-stats.mozilla.org/report/index/b7763e9e-ed42-4722-8ef2-28e690240516

Reason: SIGSEGV / SEGV_MAPERR

Top 10 frames:

0  libxul.so  nsWrapperCache::GetWrapperMaybeDead() const  dom/base/nsWrapperCache.h:137
0  libxul.so  nsWrapperCache::GetWrapperPreserveColor() const  dom/base/nsWrapperCacheInlines.h:15
0  libxul.so  nsWrapperCache::GetWrapper() const  dom/base/nsWrapperCacheInlines.h:28
1  libxul.so  mozilla::dom::binding_detail::DoGetOrCreateDOMReflector<mozilla::dom::DOMMatr...  dom/bindings/BindingUtils.h:1159
1  libxul.so  mozilla::dom::GetOrCreateDOMReflector<mozilla::dom::DOMMatrix>(JSContext*, mo...  dom/bindings/BindingUtils.h:1237
1  libxul.so  mozilla::dom::GetOrCreateDOMReflectorHelper<RefPtr<mozilla::dom::DOMMatrix>, ...  dom/bindings/BindingUtils.h:1843
2  libxul.so  mozilla::dom::GetOrCreateDOMReflector<RefPtr<mozilla::dom::DOMMatrix> >(JSCon...  dom/bindings/BindingUtils.h:1861
2  libxul.so  mozilla::dom::CanvasRenderingContext2D_Binding::getTransform(JSContext*, JS::...  dom/bindings/CanvasRenderingContext2DBinding.cpp:8696
3  libxul.so  mozilla::dom::binding_detail::GenericMethod<mozilla::dom::binding_detail::Nor...  dom/bindings/BindingUtils.cpp:3269
4  ?  @0x0000007238be4980

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

The bug is linked to a topcrash signature, which matches the following criteria:

• Top 10 AArch64 and ARM crashes on nightly
• Top 10 AArch64 and ARM crashes on beta
• Top 10 AArch64 and ARM crashes on release

For more information, please visit BugBot documentation.

Comment 2

[Sean Feng [:sefeng211]]

This doesn't look good.

Peter, do you mind to take look? Or redirect this to someone?

Comment 3

[Andrew McCreight [:mccr8]]

These all have CanvasRenderingContext2D_Binding::getTransform in the proto stack.

Comment 4

[Peter Van der Beken [:peterv]]

Bug 1887729 added code in CanvasRenderingContext2D::EnsureTarget that returns false without throwing an error: https://searchfox.org/mozilla-central/rev/c84d3db8d7d6503b1208a0378db640095e106355/dom/canvas/CanvasRenderingContext2D.cpp#1522-1528 (if mIsContextLost is true and mAllowContextRestore is true). This is problematic because CanvasRenderingContext2D::GetTransform then returns a nullptr without an error being thrown (https://searchfox.org/mozilla-central/rev/c84d3db8d7d6503b1208a0378db640095e106355/dom/canvas/CanvasRenderingContext2D.cpp#2209-2211). CanvasRenderingContext2D.getTransform is not marked as returning a nullable DOMMatrix (https://searchfox.org/mozilla-central/rev/c84d3db8d7d6503b1208a0378db640095e106355/dom/webidl/CanvasRenderingContext2D.webidl#168) and then the binding code crashes because it doesn't expect the null.

I don't know what the right fix is, and also haven't checked whether other callers of CanvasRenderingContext2D::EnsureTarget might have similar issues.

Comment 5

[Andrew McCreight [:mccr8]]

As expected, this is also on desktop, in lower volume: bp-f692402d-2bbf-4eb8-aff4-be26b0240520

Comment 6

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

Attachment: Bug 1897201 - Return identity transform while recovering from context loss with canvas.

When we lose the context for a 2D canvas, we need to silently fail any
canvas calls until the context is recovered. Unfortunately that meant we
returned a nullptr in CanvasRenderingContext2D::GetTransform which is
unsupported by the WebIDL. For now, let's just return an identity
transform as a placeholder for these situations.

Comment 7

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

Comment on attachment 9403649 [details]
Bug 1897201 - Return identity transform while recovering from context loss with canvas.

Beta/Release Uplift Approval Request

• User impact if declined: Content process crash when calling CanvasRenderingContext2D::GetTransform after a GPU process crash and before it has recovered
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): We are already silently failing most canvas calls during context loss. The caller isn't supposed to call anything during this time, and after it is restored, the state is reset to default anyways. Change is well contained.
• String changes made/needed:
• Is Android affected?: Yes

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c82f94b99166
Return identity transform while recovering from context loss with canvas. r=gfx-reviewers,lsalzman

Comment 9

[Norisz Fay [:noriszfay]]

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

Comment 10

[Pascal Chevrel:pascalc]

Comment on attachment 9403649 [details]
Bug 1897201 - Return identity transform while recovering from context loss with canvas.

Approved for 127 beta 6, thanks.

Comment 11

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/4fe000c685fd

Comment 12

[Donal Meehan [:dmeehan]]

Comment on attachment 9403649 [details]
Bug 1897201 - Return identity transform while recovering from context loss with canvas.

Approved for 126.0.1

Comment 13

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-release/rev/58013035aac1