Crash in mozilla::gfx::DrawTargetD2D1::CreateBrushForPattern when using Canvas
Categories
(Core :: Graphics, defect)
Tracking
()
People
(Reporter: bas.schouten, Assigned: bas.schouten)
References
Details
Crash Data
Attachments
(3 files)
47 bytes,
text/x-phabricator-request
|
lizzard
:
approval-mozilla-beta+
|
Details | Review |
47 bytes,
text/x-phabricator-request
|
lizzard
:
approval-mozilla-beta+
|
Details | Review |
47 bytes,
text/x-phabricator-request
|
Details | Review |
This was an error handling case I hadn't quite predicted. In some situations the device can be reset inbetween a Canvas surface being created and it being first used. This situation should be rare, but there are examples of it happening.
For example 1519e40e-6183-40c7-90a5-7f1e80190201.
Adding a patch which is not superpretty, but minimally invasive so we can upload to beta if this bug turns out to be present in sufficient numbers there.
Assignee | ||
Comment 1•2 years ago
|
||
This isn't the prettiest solution but it's minimally invasive. More long-term a better solution may be to expose a call on DrawTargets to ensure their initialization even if they're on the main thread. IsValid probably isn't a good candidate for this as we want it to be usable freely on the main thread to ensure none of the basic conditions of the surface are still valid.
Sounds like a nice crash fix might be coming to beta 66 soon.... Tracking so I keep an eye on this.
Pushed by bschouten@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/161ce405f2e0 Ensure Canvas surfaces are initialized on the main thread and ensure their validity. r=rhunt
Assignee | ||
Comment 4•2 years ago
|
||
Comment on attachment 9040680 [details]
Bug 1524554: Ensure Canvas surfaces are initialized on the main thread and ensure their validity. r=rhunt
Beta/Release Uplift Approval Request
Feature/Bug causing the regression
User impact if declined
Crashes
Is this code covered by automated tests?
Yes
Has the fix been verified in Nightly?
No
Needs manual test from QE?
No
If yes, steps to reproduce
List of other uplifts needed
None
Risk to taking this patch
Low
Why is the change risky/not risky? (and alternatives if risky)
This just forces on main thread initialization of the Canvas, we've always done this in the past.
String changes made/needed
None
Comment on attachment 9040680 [details]
Bug 1524554: Ensure Canvas surfaces are initialized on the main thread and ensure their validity. r=rhunt
Part of fix for top crash in beta; tests look ok on inbound, I'll take it for beta 5.
[Triage Comment]
Comment 6•2 years ago
|
||
bugherder |
Comment 7•2 years ago
|
||
bugherderuplift |
Updated•2 years ago
|
Assignee | ||
Comment 8•2 years ago
|
||
I still see some of these. Investigating.
Assignee | ||
Comment 9•2 years ago
|
||
Depends on D21900
Updated•2 years ago
|
Comment 10•2 years ago
|
||
Pushed by bschouten@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/fb993d3d4e66 Part 2: Ensure DrawTarget validity for DTs created inside Canvas. r=rhunt
Comment 11•2 years ago
|
||
bugherder |
Assignee | ||
Comment 12•2 years ago
|
||
Still seeing one of these crashes on Trunk, I'll have to keep a close eye on this.
Assignee | ||
Comment 13•2 years ago
|
||
Comment on attachment 9048120 [details]
Bug 1524554 - Part 2: Ensure DrawTarget validity for AdjustedTargets inside Canvas. r=rhunt
Beta/Release Uplift Approval Request
- Feature/Bug causing the regression: Bug 1519760
- User impact if declined: Crashes
- Is this code covered by automated tests?: Yes
- Has the fix been verified in Nightly?: Yes
- Needs manual test from QE?: No
- If yes, steps to reproduce:
- List of other uplifts needed: None
- Risk to taking this patch: Low
- Why is the change risky/not risky? (and alternatives if risky): Just additional verification
- String changes made/needed: None
Comment on attachment 9048120 [details]
Bug 1524554 - Part 2: Ensure DrawTarget validity for AdjustedTargets inside Canvas. r=rhunt
More help for a high volume DrawTarget crash, let's uplift for the RC.
Comment 15•2 years ago
|
||
bugherderuplift |
Assignee | ||
Comment 16•2 years ago
|
||
Comment 18•2 years ago
|
||
Pushed by bschouten@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/c4896662a24a Part 3: Ensure DT validation in internal Canvas fallback path. r=rhunt
Comment 19•2 years ago
|
||
bugherder |
Assignee | ||
Comment 20•2 years ago
|
||
Looks like this last patch has finally eliminated all the crashes, do we want to upload it in some way?
Assignee | ||
Comment 21•2 years ago
|
||
(The overview seems to be lying when it says 67.0b1 has the patch, so those crashes are without the patch)
Wow! That's great news. It is just a little too late to get this into the 66 release. But, I could put it on the list of patches we'd take for a 66 dot release in a couple of weeks.
Can you file a new bug for that, for clarity? Since we have multiple patches that landed at different times here, this bug is confusing, and I'd like to keep the status flags more clear.
Description
•