Closed Bug 945314 Opened 6 years ago Closed 6 years ago

MOZ_CRASH assertion GetBitmapForSurface in DrawTargetSkia makes skia crash always on Linux

Categories

(Core :: Graphics, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla28
Tracking Status
firefox28 --- fixed

People

(Reporter: kevin, Assigned: kevin)

Details

(Whiteboard: [qa-])

Attachments

(1 file, 3 obsolete files)

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.
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.
Attachment #8341161 - Flags: review?(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.
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.
Attachment #8341173 - Flags: review?(mstange)
Attachment #8341161 - Attachment is obsolete: true
Attachment #8341161 - Flags: review?(mstange)
(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?
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!
Attachment #8341173 - Flags: review?(mstange) → review+
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.
Attachment #8341173 - Attachment is obsolete: true
Keywords: checkin-needed
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.
Attachment #8341595 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/efc30ce15377
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.