945314 - MOZ_CRASH assertion GetBitmapForSurface in DrawTargetSkia makes skia crash always on Linux

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Kevin Simons [:kevsim] (Telenor)]

The recently introduced GetBitmapForSurface function has a MOZ_CRASH assertion if we attempt to get the bitmap for a surface of a type other than SURFACE_SKIA or SURFACE_DATA. Since gfxXlibNativeRenderer.cpp and gfxGdkNativeRenderer.cpp (at least) have some hardcoded Cairo surfaces, these means using the skia backend always crashes on Linux.

Previously, these different surface types would just be ignored in DrawTargetSkia::DrawSurface.

Comment 1

[Kevin Simons [:kevsim] (Telenor)]

Attachment: workaround crash with Cairo surfaces in Skia backend

A patch for bug 924102 introduced a MOZ_CRASH assertion for surfaces
of type other than SURFACE_SKIA or SURFACE_DATA being passed to
DrawSurface. Previously surfaces of other types were simply ignored.
On Linux, there are still several hardcoded surfaces of type Cairo
(found in gfxXlibNativeRenderer and friends) and as a result, the
Skia backend crashes always on Linux currently.

This workaround converts the SURFACE_CAIRO surfaces to SURFACE_DATA
surfaces in order to retreive a bitmap of the surface. It is of course
suboptimal and the longterm solution is to remove the hardcoded Cairo
surfaces.

As a side effect, this workaround fixes a lot of Chrome rendering issues
with the Skia backend that were previously caused by the Cairo surfaces
being ignored when they were passed to DrawSurface.

Comment 2

[Markus Stange [:mstange]]

Doesn't this leak? Especially if the call to GetDataSurface allocates a new main-memory buffer.

The easier way out is probably to re-add the early return to DrawSurface that just does nothing if the surface is not a SURFACE_SKIA or a SURFACE_DATA.

Comment 3

[Kevin Simons [:kevsim] (Telenor)]

Attachment: workaround crash with Cairo surfaces in Skia backend

A patch for bug 924102 introduced a MOZ_CRASH assertion for surfaces
of type other than SURFACE_SKIA or SURFACE_DATA being passed to
DrawSurface. Previously surfaces of other types were simply ignored.
On Linux, there are still several hardcoded surfaces of type Cairo
(found in gfxXlibNativeRenderer and friends) and as a result, the
Skia backend crashes always on Linux currently.

This workaround ignores Cairo surfaces passed to DrawSurface.

Comment 4

[Kevin Simons [:kevsim] (Telenor)]

(In reply to Markus Stange [:mstange] from comment #2)
> Doesn't this leak? Especially if the call to GetDataSurface allocates a new
> main-memory buffer.

Yes, you're right of course. Got caught up in the nice result with the fixed chrome rendering ;-)

Submitted a simpler version that works around the problem the "old" way.

Btw, is there a bug to track the issues with the hardcoded cairo usage and is somebody looking into it?

Comment 5

[Markus Stange [:mstange]]

There's bug 882109 as a general tracking bug, but I don't know of a bug specifically about the Linux problem.

Thanks for the patch!

Comment 6

[Kevin Simons [:kevsim] (Telenor)]

Attachment: workaround crash with Cairo surfaces in Skia backend. r=mstange

A patch for bug 924102 introduced a MOZ_CRASH assertion for surfaces
of type other than SURFACE_SKIA or SURFACE_DATA being passed to
DrawSurface. Previously surfaces of other types were simply ignored.
On Linux, there are still several hardcoded surfaces of type Cairo
(found in gfxXlibNativeRenderer and friends) and as a result, the
Skia backend crashes always on Linux currently.

This workaround ignores Cairo surfaces passed to DrawSurface.

Comment 7

[Kevin Simons [:kevsim] (Telenor)]

Attachment: workaround crash with Cairo surfaces in Skia backend. r=mstange

A patch for bug 924102 introduced a MOZ_CRASH assertion for surfaces
of type other than SURFACE_SKIA or SURFACE_DATA being passed to
DrawSurface. Previously surfaces of other types were simply ignored.
On Linux, there are still several hardcoded surfaces of type Cairo
(found in gfxXlibNativeRenderer and friends) and as a result, the
Skia backend crashes always on Linux currently.

This workaround ignores Cairo surfaces passed to DrawSurface.

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/efc30ce15377

Comment 9

[Ryan VanderMeulen [:RyanVM]]

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