1413862 - Crash in mozilla::gfx::DrawTarget::Blur

Status: RESOLVED FIXED

(Core :: Graphics: Layers, defect)

Description

[Calixte Denizet (:calixte)]

This bug was filed from the Socorro interface and is 
report bp-e945fdc2-5ded-47a7-8894-5a8430171101.
=============================================================

There are 7 crashes in nightly 58 starting with buildid 20171101104430. In analyzing the backtrace, the regression may have been introduced by patch [1] to fix bug 1395478.

[1] https://hg.mozilla.org/mozilla-central/rev?node=0f7488658f6afeccb94d0b2da38cb8782ea16196

Comment 1

[David Anderson [:dvander] - inactive, e-mail if emergency]

We requested a Skia draw target but somehow got a D2D one.

Comment 2

[David Anderson [:dvander] - inactive, e-mail if emergency]

In a few cases we are still creating DrawTargetCairo surfaces, sometimes after device resets, sometimes not.

Comment 3

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: fix

I think what's happening here - as evidenced by lots of failures in the gfxNotes of these crashes - is that a device reset or something similar causes CreateTexture2D to fail in TextureClient. TextureClient is then hardcoded to fallback to Cairo, which does not support blurring.

Of course, we don't support Cairo anymore except for printing. We should force TextureClient to fallback to Skia.

I think it's okay to do this inside CreateForRawBufferAccess itself, but try run just in case:

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

Comment 4

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 8925661 [details] [diff] [review]
fix

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

::: gfx/layers/client/TextureClient.cpp
@@ +1251,4 @@
>    }
>  
> +  // Note that we ignore the backend type if we get here. It should only be D2D
> +  // or Skia, and D2D does not support data surfaces. Therefore it is safe to

Maybe add a soft-assertion/NS_WARNING for this just in case we ever add more backends?

Comment 5

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: v2

Same patch, with a few things added. The original patch bounced on try because - ta da - we were still using Cairo even in normal browser usage. If the backend was D2D, and the SurfaceFormat was A8, the mask would be rendered with Cairo.

The original patch implicitly fixed that and thus some fuzzy-if numbers in reftests had to change [1].

In addition, I found another case where we hardcode BackendType::CAIRO in canvas, and that's fixed here as well.

[1] https://treeherder.mozilla.org/#/jobs?repo=try&revision=2b19667fea9cdff281b02c52132cfd232f9d3cb1

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f44464a40e7d
Don't fallback to DrawTargetCairo in TextureClient. (bug 1413862, r=mattwoodrow)

Comment 8

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

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