Painting operations with invalid sources in DrawTargetCairo put the DrawTarget in an invalid state

VERIFIED FIXED in Firefox 31

Status

()

VERIFIED FIXED
4 years ago
4 years ago

People

(Reporter: nical, Assigned: nical)

Tracking

unspecified
mozilla33
x86_64
Linux
Points:
---

Firefox Tracking Flags

(firefox31 verified, firefox32 verified, firefox33 verified)

Details

Attachments

(1 attachment)

(Assignee)

Description

4 years ago
It would be better to just cancel the operation and continue with a valid DrawTarget instead.
(Assignee)

Comment 1

4 years ago
Created attachment 8450942 [details] [diff] [review]
Warn and cancel the operation in case of invalid sources
Attachment #8450942 - Flags: review?(bas)
(Assignee)

Comment 2

4 years ago
This fixes the crash in bug 1027103, but the cause of the crash remains (that is, a big canvas2D gets corrupted and we the content of the window stays black until we resize the window (which recreates or resets the canvas's DrawTarget).
Attachment #8450942 - Flags: review?(bas) → review+
https://hg.mozilla.org/mozilla-central/rev/6ce30cc08e84
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
Nical, I know it does not fix bug 1027103 but a bug is better than a crash.
Could you fill the uplift request for both aurora & beta to make sure we have them in tonight builds? Merci!
Flags: needinfo?(nical.bugzilla)
(Assignee)

Comment 6

4 years ago
Comment on attachment 8450942 [details] [diff] [review]
Warn and cancel the operation in case of invalid sources

Approval Request Comment
[Feature/regressing bug #]:
[User impact if declined]: Very easy to crash Firefox on Linux and Windows xp in google streetview.
[Describe test coverage new/current, TBPL]: none
[Risks and why]: Low risk, This patch only adds checks that prevent invalid states from spreading to areas of the code where we don't support them. The patch only affects situations in which we are already crashing.
[String/UUID change made/needed]:
Attachment #8450942 - Flags: approval-mozilla-beta?
Attachment #8450942 - Flags: approval-mozilla-aurora?
Flags: needinfo?(nical.bugzilla)
(Assignee)

Comment 7

4 years ago
(In reply to Sylvestre Ledru [:sylvestre] from comment #5)
> Nical, I know it does not fix bug 1027103 but a bug is better than a crash.
> Could you fill the uplift request for both aurora & beta to make sure we
> have them in tonight builds? Merci!

Yes, plus stable Firefox is already affected by the remaining bug (which is being worked on in bug 1034593).
Attachment #8450942 - Flags: approval-mozilla-beta?
Attachment #8450942 - Flags: approval-mozilla-beta+
Attachment #8450942 - Flags: approval-mozilla-aurora?
Attachment #8450942 - Flags: approval-mozilla-aurora+
https://hg.mozilla.org/releases/mozilla-aurora/rev/48c49053a2e6
https://hg.mozilla.org/releases/mozilla-beta/rev/d8b9f6f0bd20
status-firefox31: --- → fixed
status-firefox32: --- → fixed
status-firefox33: --- → fixed
Florin, please make sure this gets verified in conjunction with bug 1027103.
Flags: needinfo?(florin.mezei)
Keywords: verifyme
So my understanding is this fixes the crash in bug 1027103. So if the fix is good we should no longer see crashes with signature from bug 1027103 in Firefox 31 Beta 8 and 9. Is this correct? Or is there something else that we should verify for this?
Flags: needinfo?(florin.mezei)
(Assignee)

Comment 11

4 years ago
(In reply to Florin Mezei, QA (:FlorinMezei) from comment #10)
> So my understanding is this fixes the crash in bug 1027103. So if the fix is
> good we should no longer see crashes with signature from bug 1027103 in
> Firefox 31 Beta 8 and 9. Is this correct? Or is there something else that we
> should verify for this?

This removes one of the main causes for running into the crash signature of bug 1027103 (in particular the one that is reliably crashing google street view).

If you want to manually verify this patch (on Linux), the STR are: in bug 1027103 comment 11.

Unfortunately there are still non-trivial ways to run into this crash signature, in particular when running out of memory in the middle of a complex drawing operation.
Thank you Nicolas! I've verified this using steps from bug 1027103 comment 11, on Ubuntu 13.04 x64. I was able to easily crash Firefox each time with 31 Beta 5, but could no longer crash it with 31 Beta 8 (image is still black, as described in bug 1027103).

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Firefox/31.0
BuildID: 20140707160635.

Also verified as fixed with latest Firefox 32 Aurora and latest Firefox 33 Nightly (note that on Nightly the image does not stay black after navigating to another position... instead after ~1-2 sec it displays the correct image).
Status: RESOLVED → VERIFIED
status-firefox31: fixed → verified
status-firefox32: fixed → verified
status-firefox33: fixed → verified
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.