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

Status: VERIFIED FIXED

(Core :: Graphics, defect)

Description

[Nicolas Silva [:nical]]

It would be better to just cancel the operation and continue with a valid DrawTarget instead.

Comment 1

[Nicolas Silva [:nical]]

Attachment: Warn and cancel the operation in case of invalid sources

Comment 2

[Nicolas Silva [:nical]]

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).

Comment 3

[Nicolas Silva [:nical]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6ce30cc08e84

Comment 4

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/mozilla-central/rev/6ce30cc08e84

Comment 5

[Sylvestre Ledru [:Sylvestre]]

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!

Comment 6

[Nicolas Silva [:nical]]

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]:

Comment 7

[Nicolas Silva [:nical]]

(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).

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/48c49053a2e6
https://hg.mozilla.org/releases/mozilla-beta/rev/d8b9f6f0bd20

Comment 9

[u279076]

Florin, please make sure this gets verified in conjunction with bug 1027103.

Comment 10

[[SV Manager] Florin Mezei, QA (:FlorinMezei)]

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?

Comment 11

[Nicolas Silva [:nical]]

(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.

Comment 12

[[SV Manager] Florin Mezei, QA (:FlorinMezei)]

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).