Closed Bug 1285942 Opened 4 years ago Closed 4 years ago

RecordedEvent::PlayEvent should return a bool, so that Translators can handle certain errors more gracefully.

Categories

(Core :: Printing: Output, enhancement)

All
Windows
enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla50
Tracking Status
firefox50 --- fixed

People

(Reporter: bobowen, Assigned: bobowen)

References

Details

Attachments

(2 files)

In bug 1273765 an error at [1] meant that we got a null DrawTarget and so this caused a crash in the next event that tried to use it.

The problem was caused by the underlying cairo surface getting into an invalid state.
It would be useful if we could return a bool (or something) from PlayEvent, so that the Translator can handle errors like this more gracefully.

Bas - what are your thoughts on this?

[1] https://hg.mozilla.org/mozilla-central/file/1bee8d2da23e/layout/printing/PrintTranslator.cpp#l72
Flags: needinfo?(bas)
(In reply to Bob Owen (:bobowen) from comment #0)
> In bug 1273765 an error at [1] meant that we got a null DrawTarget and so
> this caused a crash in the next event that tried to use it.
> 
> The problem was caused by the underlying cairo surface getting into an
> invalid state.
> It would be useful if we could return a bool (or something) from PlayEvent,
> so that the Translator can handle errors like this more gracefully.
> 
> Bas - what are your thoughts on this?
> 
> [1]
> https://hg.mozilla.org/mozilla-central/file/1bee8d2da23e/layout/printing/
> PrintTranslator.cpp#l72

Hrm, yes, looking at it that makes sense since some recorded events are indeed fallible.
Flags: needinfo?(bas)
When I added the bool return and null checks for the DrawTarget, I found that on the second print I was hitting this existing low level crash in the child: bug 794910.

So, I've added a fix for that as well.
Assignee: nobody → bobowen.code
Blocks: 794910
Status: NEW → ASSIGNED
Comment on attachment 8771938 [details]
Bug 1285942 Part 1: Make RecordedEvent::PlayEvent return a bool and null check DrawTarget creation.

https://reviewboard.mozilla.org/r/64906/#review62228
Attachment #8771938 - Flags: review?(bas) → review+
Comment on attachment 8771939 [details]
Bug 1285942 Part 2: Check result of CreateGlyphRunAnalysis in _cairo_dwrite_scaled_show_glyphs.

https://reviewboard.mozilla.org/r/64908/#review62230
Attachment #8771939 - Flags: review?(bas) → review+
Pushed by bobowencode@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/e6745f43afe0
Part 1: Make RecordedEvent::PlayEvent return a bool and null check DrawTarget creation. r=bas
https://hg.mozilla.org/integration/autoland/rev/b944fc6619cf
Part 2: Check result of CreateGlyphRunAnalysis in _cairo_dwrite_scaled_show_glyphs. r=bas
https://hg.mozilla.org/mozilla-central/rev/e6745f43afe0
https://hg.mozilla.org/mozilla-central/rev/b944fc6619cf
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
You need to log in before you can comment on or make changes to this bug.