Closed Bug 1413862 Opened 2 years ago Closed 2 years ago

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

Categories

(Core :: Graphics: Layers, defect, critical)

58 Branch
x86
Windows 8
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla58
Tracking Status
firefox-esr52 --- unaffected
firefox56 --- unaffected
firefox57 --- unaffected
firefox58 --- fixed

People

(Reporter: calixte, Assigned: dvander)

References

(Blocks 1 open bug)

Details

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

Crash Data

Attachments

(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.

[1] https://hg.mozilla.org/mozilla-central/rev?node=0f7488658f6afeccb94d0b2da38cb8782ea16196
Flags: needinfo?(dvander)
We requested a Skia draw target but somehow got a D2D one.
Assignee: nobody → dvander
Status: NEW → ASSIGNED
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:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=a1ca421aee5dc288fc2216bb5691a30c85b5d08f
Attachment #8925661 - Flags: review?(matt.woodrow)
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?
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.

[1] https://treeherder.mozilla.org/#/jobs?repo=try&revision=2b19667fea9cdff281b02c52132cfd232f9d3cb1
Attachment #8925661 - Attachment is obsolete: true
Attachment #8926493 - Flags: review?(matt.woodrow)
Attachment #8926493 - Flags: review?(matt.woodrow) → review+
Pushed by danderson@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f44464a40e7d
Don't fallback to DrawTargetCairo in TextureClient. (bug 1413862, r=mattwoodrow)
Duplicate of this bug: 1381722
https://hg.mozilla.org/mozilla-central/rev/f44464a40e7d
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
You need to log in before you can comment on or make changes to this bug.