549110 - Crash [@ nsThebesRenderingContext::GetNativeGraphicData(nsIRenderingContext::GraphicDataType) ] entering text into textbox with D2D / DWrite

Status: RESOLVED INVALID

(Core :: Graphics, defect)

Description

[Jordan Gutterman]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.3a2pre) Gecko/20100226 Minefield/3.7a2pre
Build Identifier: 

Happened while writing a bugzilla comment, haven't been able to reproduce. The signature seems to point to the new D2D / Dwrite code and noone else has hit it yet according to crash-stats, so I thought I'd file anyway.

http://crash-stats.mozilla.com/report/index/6d536f66-6653-4d04-9d2f-4f1062100227

Reproducible: Sometimes

Comment 1

[Bas Schouten (:bas.schouten)]

Wow, yes, that is quite evil code, that just assumes the surface is a gfxWindowsSurface without doing any checking whatsoever. It would definitely crash if it gets hit.

Comment 2

[Bas Schouten (:bas.schouten)]

Attachment: Only return WINDOWS_NATIVE_DC for Win32 surfaces

Comment 3

[Bas Schouten (:bas.schouten)]

We might also want to return something for Win32Printing, but I doubt that's ever used.

Comment 4

[Jeff Muizelaar [:jrmuizel]]

Are you sure returning null is the right thing to do here? Should we even support calling this function on D2D surfaces?

Comment 5

[Jeff Muizelaar [:jrmuizel]]

It looks like we use the HDC for measuring theme stuff with GetThemePartSize. That seems like something we'd still need to do when using D2D so I think we'll need to come up with a different solution than just returning null.

Comment 6

[Bas Schouten (:bas.schouten)]

(In reply to comment #5)
> It looks like we use the HDC for measuring theme stuff with GetThemePartSize.
> That seems like something we'd still need to do when using D2D so I think we'll
> need to come up with a different solution than just returning null.

The code here deals with getting null returned though. Meaning it might lay something out wrong, but we don't crash. And regardless that function should type check before the cast regardless, so I personally think that patch is good anyway.

Comment 7

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 429510 [details] [diff] [review]
Only return WINDOWS_NATIVE_DC for Win32 surfaces

As discussed with Bas we need a better solution. GetNativeGraphicData is only called in 4 places: here, in the video code (bug 549331) and in the os/2 plugin printing code. I'd rather just remove this stuff instead of using a bandaid. I've created bug 549415 for that purpose.

Comment 8

[Ryan Jones-Ward [:sciguyryan]]

I hit this today. First time. But I thought I'd post it here to let people know it still seems to be around:

http://crash-stats.mozilla.com/report/index/bp-9716c98b-1331-4c1b-9bf2-8f63b2100518

Comment 9

[Jeff Muizelaar [:jrmuizel]]

Attachment: Add NativeMeasuring

This gets rid of the use of GetNativeGraphicData(), we still need to figure out what to do for D2D.

Comment 10

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Comment on attachment 447332 [details] [diff] [review]
Add NativeMeasuring


>+    /* Returns a HDC which may be used for native measuring.  This HDC is valid
>+     * until EndNativeDrawing is called; if it is used for drawing after that time,
>+     * the result is undefined. */

This comment refers to drawing; should refer to measuring (both method name and "if it is used for...").

Comment 11

[Doug Sherk (:drs) (inactive)]

Crash reports here no longer work. Please re-open if this is still an issue.