Closed Bug 1413862 Opened 4 years ago Closed 4 years ago

Crash in mozilla::gfx::DrawTarget::Blur


(Core :: Graphics: Layers, defect)

58 Branch
Windows 8
Not set



Tracking Status
firefox-esr52 --- unaffected
firefox56 --- unaffected
firefox57 --- unaffected
firefox58 --- fixed


(Reporter: calixte, Assigned: dvander)


(Blocks 1 open bug)


(Keywords: crash, regression, Whiteboard: [clouseau])

Crash Data


(1 file, 1 obsolete file)

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.

Flags: needinfo?(dvander)
We requested a Skia draw target but somehow got a D2D one.
Assignee: nobody → dvander
Flags: needinfo?(dvander)
In a few cases we are still creating DrawTargetCairo surfaces, sometimes after device resets, sometimes not.
Attached patch fix (obsolete) — Splinter Review
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:
Attachment #8925661 - Flags: review?(matt.woodrow)
Comment on attachment 8925661 [details] [diff] [review]

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?
Attachment #8925661 - Flags: review?(matt.woodrow) → review+
Attached patch v2Splinter Review
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.

Attachment #8925661 - Attachment is obsolete: true
Attachment #8926493 - Flags: review?(matt.woodrow)
Attachment #8926493 - Flags: review?(matt.woodrow) → review+
Pushed by
Don't fallback to DrawTargetCairo in TextureClient. (bug 1413862, r=mattwoodrow)
Duplicate of this bug: 1381722
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
You need to log in before you can comment on or make changes to this bug.