Closed Bug 745818 Opened 8 years ago Closed 6 years ago

Crash with large canvas as source pattern

Categories

(Core :: Graphics, defect)

x86_64
macOS
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla14
Tracking Status
firefox12 - wontfix
firefox13 - affected
firefox14 - ---

People

(Reporter: jruderman, Assigned: jrmuizel)

References

(Blocks 2 open bugs)

Details

(Keywords: crash, testcase)

Crash Data

Attachments

(4 files)

Attached file stack trace
It's very sad, but it might be best to just pref this off for this cycle.
Bas, do you think you could look to see how hard this is to fix properly?
Comment on attachment 615385 [details] [diff] [review]
Turn off coregraphics azure

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

Very sad indeed. I'd love to look at it.
Attachment #615385 - Flags: review?(bas.schouten) → review+
Comment on attachment 615385 [details] [diff] [review]
Turn off coregraphics azure

[Triage Comment]
Pre-approving since this is a simple pref change and we really want to get this into Beta 6 if this is how we need to ship FF12.
Attachment #615385 - Flags: approval-mozilla-beta+
Backed out in https://hg.mozilla.org/releases/mozilla-beta/rev/7446fa39b65c - you'll need to adjust our expectations about a couple of reftests, since that makes one unexpectedly pass and one unexpectedly fail.
One thing worth consider is whether this bug is severe enough to warrant the big pref change. As far as I can see, this is a NULL deference on malicious input which isn't _that_ bad...
(In reply to Jeff Muizelaar [:jrmuizel] from comment #9)
> One thing worth consider is whether this bug is severe enough to warrant the
> big pref change. As far as I can see, this is a NULL deference on malicious
> input which isn't _that_ bad...

Jeff and I discussed this bug in #gfx today. We discussed the fact that the bug has very low crash volume, shouldn't happen in normal web content, and may cause unforeseen regressions since the pref has been set the other way this entire cycle. Untracking for FF12 and wontfixing.
Bas, how do you handle this situation with Direct2D?
(In reply to Jeff Muizelaar [:jrmuizel] from comment #11)
> Bas, how do you handle this situation with Direct2D?

We fallback to Thebes-canvas(and longer term Skia-Azure), when a canvas is more than 8192 pixels in any direction, because SetDimensions fails to create a DrawTarget. It can cause some issues if you make the canvas larger than supported -after- it's already been created, in that case we fail silently having a dummy canvas. All this logic is in CanvasRenderingContext2DAzure so all you really need to do is make CreateDrawTarget return NULL.
With bug 686453, we're not really supposed to get NULL surfaces anymore. Instead we get Invalid ones that we should still ignore.
Attachment #615994 - Flags: review?(bas.schouten)
You could update the comment to something like "Ignore NULL and invalid surfaces".
Attachment #615994 - Flags: review?(bas.schouten) → review+
Comment on attachment 615994 [details] [diff] [review]
Ignore invalid cairo surfaces

[Approval Request Comment]
Regression caused by (bug #): 692879
User impact if declined: Occasional crashes
Testing completed (on m-c, etc.): none yet
Risk to taking this patch (and alternatives if risky): very minimal, we take an already tested bail out earlier
String changes made by this patch: none
Attachment #615994 - Flags: approval-mozilla-central?
Attachment #615994 - Flags: approval-mozilla-aurora?
Attachment #615994 - Flags: approval-mozilla-central? → approval-mozilla-central+
https://hg.mozilla.org/integration/mozilla-inbound/rev/3b31c3121e98
Assignee: nobody → jmuizelaar
Target Milestone: --- → mozilla14
https://hg.mozilla.org/mozilla-central/rev/f474ef88b11c
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Reopening because this crash still happens with other backends.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment on attachment 615994 [details] [diff] [review]
Ignore invalid cairo surfaces

[Triage Comment]
Approved for Aurora 14 since this is low risk and we're still very early in the cycle.
Attachment #615994 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Closing ye olde bug; if there are still crashes with other backends we can file a new bug for them.
Status: REOPENED → RESOLVED
Closed: 8 years ago6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.