789933 - crash in mozilla::gfx::DrawTargetCairo::Stroke

Status: VERIFIED FIXED

(Core :: Graphics: Canvas2D, defect)

Description

[Marcia Knous [:marcia]]

This bug was filed from the Socorro interface and is 
report bp-a51c4a2d-7f81-498d-add7-e50322120910 .
============================================================= 

Seen while looking at crash stats. Crashes started showing up using the 2012090103 build. One comment mentions "Error occured while printing one page of a pdf."

Possible regression range based on crash stats: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=fcc533f691e9&tochange=a21fd4d085ad

More reports: https://crash-stats.mozilla.com/report/list?signature=mozilla%3A%3Agfx%3A%3ADrawTargetCairo%3A%3AStroke%28mozilla%3A%3Agfx%3A%3APath%20const*%2C%20mozilla%3A%3Agfx%3A%3APattern%20const%26%2C%20mozilla%3A%3Agfx%3A%3AStrokeOptions%20const%26%2C%20mozilla%3A%3Agfx%3A%3ADrawOptions%20const%26%29


Frame 	Module 	Signature 	Source
0 	gkmedias.dll 	mozilla::gfx::DrawTargetCairo::Stroke 	gfx/2d/DrawTargetCairo.cpp:604
1 	xul.dll 	nsCanvasRenderingContext2DAzure::UsedOperation 	content/canvas/src/nsCanvasRenderingContext2DAzure.h:763
2 	xul.dll 	nsCanvasBidiProcessorAzure::DrawText 	content/canvas/src/nsCanvasRenderingContext2DAzure.cpp:3119

Comment 1

[Scoobidiver (away)]

The stack trace in comment 0 is a 64-bit one. Here are the first frames of a 32-bit stack trace:
Frame 	Module 	Signature 	Source
0 	gkmedias.dll 	mozilla::gfx::DrawTargetCairo::Stroke 	gfx/2d/DrawTargetCairo.cpp:604
1 	xul.dll 	nsCanvasBidiProcessorAzure::DrawText 	content/canvas/src/nsCanvasRenderingContext2DAzure.cpp:3119
2 	xul.dll 	nsBidiPresUtils::ProcessText 	layout/base/nsBidiPresUtils.cpp:1860
3 	xul.dll 	nsCanvasRenderingContext2DAzure::DrawOrMeasureText 	content/canvas/src/nsCanvasRenderingContext2DAzure.cpp:3363
4 	xul.dll 	nsCanvasRenderingContext2DAzure::StrokeText 	content/canvas/src/nsCanvasRenderingContext2DAzure.cpp:2928
5 	xul.dll 	mozilla::dom::CanvasRenderingContext2DBinding::strokeText 	obj-firefox/dom/bindings/CanvasRenderingContext2DBinding.cpp:1052
6 	xul.dll 	mozilla::dom::CanvasRenderingContext2DBinding::genericMethod 	obj-firefox/dom/bindings/CanvasRenderingContext2DBinding.cpp:2573
7 	mozjs.dll 	js::InvokeKernel 	js/src/jsinterp.cpp:344
8 	mozjs.dll 	js::Invoke 	js/src/jsinterp.cpp:388

(In reply to Marcia Knous [:marcia] from comment #0)
> Possible regression range based on crash stats:
> http://hg.mozilla.org/mozilla-central/
> pushloghtml?fromchange=fcc533f691e9&tochange=a21fd4d085ad
I disagree. Indeed, it first appeared in 17.0a1/20120808. The regression range might be:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=1bbc0b65dffb&tochange=e55638d4037a
It might be a regression from bug 777292.

Comment 2

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

The only bug 777292 changesets I see in that range only touch security/manager/.  Also, neither of them should be a functional change.  Why do you think they'd cause a crash in gfx?

Comment 3

[Scoobidiver (away)]

(In reply to :Aryeh Gregor from comment #2)
> The only bug 777292 changesets I see in that range only touch
> security/manager/.  Also, neither of them should be a functional change. 
> Why do you think they'd cause a crash in gfx?
Because of http://hg.mozilla.org/mozilla-central/annotate/1cb30394aa56/content/canvas/src/nsCanvasRenderingContext2DAzure.cpp#l2928 that says the code has been changed by this bug.

Comment 4

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

The changeset summary is actually wrong -- that changeset is for bug 626472.  There are no changesets from bug 626472 in the range.  The only change I made to that file was s/nsnull/nullptr/ (literally just using sed), and at the time nsnull was a #define for nullptr, so it was a no-op as far as the compiler saw.  You'll have to look elsewhere for the culprit, I'm afraid.

(I hope people don't start routinely CCing me on bugs because bug 626472 was the last thing to touch a line.)

Comment 5

[Scoobidiver (away)]

It's #136 top browser crasher in 17.0a2 and #8 in 18.0a1 (without hangs to compare the same way the two channels).

STR:
1. Load the ref. URL
2. Print the document
=> bp-38bbf224-679e-46ac-8c39-4bbba2120929

Comment 6

[Alex Keybl [:akeybl]]

Since this is already reproducible, sending over to Joe to investigate. Feel free to re-assign if you don't have the time to help out.

Comment 7

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

Looks like scaledFont->GetPathForGlyphs() is returning nullptr. It is possibly caused by scaledFont being the wrong font type for the DrawTarget, that is not a Cairo font. This is the code segment from nsCanvasBidiProcessorAzure::DrawText()

        RefPtr<Path> path = scaledFont->GetPathForGlyphs(buffer, mCtx->mTarget);

        const ContextState& state = *mState;
        AdjustedTarget(mCtx, &bounds)->
          Stroke(path, CanvasGeneralPattern().
                   ForStyle(mCtx, nsCanvasRenderingContext2DAzure::STYLE_STROKE, mCtx->mTarget),
                 StrokeOptions(state.lineWidth, state.lineJoin,
                               state.lineCap, state.miterLimit,
                               state.dash.Length(),
                               state.dash.Elements(),
                               state.dashOffset),
                 DrawOptions(state.globalAlpha, mCtx->UsedOperation()));

Comment 8

[Robert Kaiser]

This is pretty low volume on beta or aurora, not happening at all on 16 and older, but it is appearing with constant volume on trunk, be it 18 or 19.

Comment 9

[Scoobidiver (away)]

It's a top crash in the trunk because of 64-bit builds that are more impacted, but indeed it's now a low volume on Beta and Aurora.

Comment 10

[Nick Cameron [:nrc]]

I just tried to reproduce this and couldn't - do I need to set any prefs? I tried opening the URL and printing one page. Do I need to do anything else?

Comment 11

[Nick Cameron [:nrc]]

Marcia of Scoobidiver: is this bug still appearing in our crash stats?

Can anyone reproduce this?

Comment 12

[Scoobidiver (away)]

(In reply to Nick Cameron [:nrc] from comment #11)
> Marcia of Scoobidiver: is this bug still appearing in our crash stats?
Yes. See the link in comment 0.

> Can anyone reproduce this?
Yes with the STR in comment 5: bp-4445f7d5-e644-45b9-821e-fbc7f2121129.

Comment 13

[Scoobidiver (away)]

It's #6 top browser crasher in 19.0a2.

Comment 14

[Robert Kaiser]

And it's the #5 topcrash over builds of the last 3 days on 20.a1 trunk. Nick, any chance of progress here?

Comment 15

[Nick Cameron [:nrc]]

(In reply to Robert Kaiser (:[e-mail]) from comment #14)
> And it's the #5 topcrash over builds of the last 3 days on 20.a1 trunk.
> Nick, any chance of progress here?

Hi, yep just rolled a build for this yesterday and will try to do a bit of investigation today. My issue has been that I haven't been able to reproduce before, but I'll see what I can do today.

Comment 16

[Bas Schouten (:bas.schouten)]

Attachment: Support non-D2D targets for getting a Path off ScaledFontDWrite

This should fix the bug.

Comment 17

[Nick Cameron [:nrc]]

Comment on attachment 688477 [details] [diff] [review]
Support non-D2D targets for getting a Path off ScaledFontDWrite

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

Fixes the bug (which I finally managed to reproduce)

Comment 18

[Bas Schouten (:bas.schouten)]

http://hg.mozilla.org/integration/mozilla-inbound/rev/e9bfe1b6ab08

Comment 19

[Nick Cameron [:nrc]]

leave open for crashtest coming soon...

Comment 20

[Nick Cameron [:nrc]]

Attachment: test

Comment 21

[Ed Morley [:emorley]]

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

Comment 22

[Bas Schouten (:bas.schouten)]

Comment on attachment 688648 [details] [diff] [review]
test

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

We'll soon support 16384 on some platforms. Might want to size up the width or height to 20000, it's fine to half the other dimension to conserve memory.

Comment 23

[Nick Cameron [:nrc]]

crashtest:

https://hg.mozilla.org/integration/mozilla-inbound/rev/6400692c072e

Comment 24

[Nick Cameron [:nrc]]

Comment on attachment 688648 [details] [diff] [review]
test

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: crash on printing large documents
Testing completed (on m-c, etc.): m-c
Risk to taking this patch (and alternatives if risky): low
String or UUID changes made by this patch: none

Comment 25

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/6400692c072e

Comment 26

[Bas Schouten (:bas.schouten)]

Umm, landing the test for Aurora only seems sensible if we also land the bugfix :).

Comment 27

[Nick Cameron [:nrc]]

Comment on attachment 688477 [details] [diff] [review]
Support non-D2D targets for getting a Path off ScaledFontDWrite

see above ^^^

Comment 28

[Nick Cameron [:nrc]]

(In reply to Bas Schouten (:bas.schouten) from comment #26)
> Umm, landing the test for Aurora only seems sensible if we also land the
> bugfix :).

Yeah, may have a? the wrong patch there, d'oh.

Comment 29

[Nick Cameron [:nrc]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/93590c0181c4
https://hg.mozilla.org/releases/mozilla-aurora/rev/78dfb6b50af0

Comment 30

[Robert Kaiser]

It looks like we had no crashes with this signature with Aurora builds from the 10th, but we need to keep watching this.

Comment 31

[u279076]

Flagging for verification using the steps in comment 5.

Comment 32

[Bogdan Maris, Desktop QA]

Mozilla/5.0 (Windows NT 6.1; WOW64; rv:19.0) Gecko/20100101 Firefox/19.0
Build ID: 20130109111322

Verified as fixed on Firefox 19.0b1

Comment 33

[Bogdan Maris, Desktop QA]

Mozilla/5.0 (Windows NT 6.1; WOW64; rv:20.0) Gecko/20100101 Firefox/20.0

Also verified as fixed on Firefox 20 beta 1.