It would be better to just cancel the operation and continue with a valid DrawTarget instead.
Created attachment 8450942 [details] [diff] [review] Warn and cancel the operation in case of invalid sources
Attachment #8450942 - Flags: review?(bas)
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+
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!
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]:
(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).
status-firefox31: --- → fixed
status-firefox32: --- → fixed
status-firefox33: --- → fixed
Florin, please make sure this gets verified in conjunction with bug 1027103.
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?
(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
You need to log in before you can comment on or make changes to this bug.