Summary: | [Qt] Crash if an scene with accelerated compositing layout during the paint event | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Benjamin Poulain <benjamin> | ||||||
Component: | WebCore Misc. | Assignee: | Benjamin Poulain <benjamin> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Critical | CC: | ademar, commit-queue, kling, menard, noam | ||||||
Priority: | P2 | Keywords: | Qt, QtTriaged | ||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | All | ||||||||
OS: | All | ||||||||
Attachments: |
|
Description
Benjamin Poulain
2010-09-29 08:18:40 PDT
Created attachment 69204 [details]
Clean up QGraphicsWebViewPrivate::overlay (r=ariya)
Let's clean up the QGraphicsWebViewPrivate::overlay pointer before anything else.
Comment on attachment 69204 [details] Clean up QGraphicsWebViewPrivate::overlay (r=ariya) Clearing flags on attachment: 69204 Committed r68653: <http://trac.webkit.org/changeset/68653> Created attachment 69237 [details]
Patch
We have two options:
-keep the overlay around but hide it
-delete the overlay with deleteLater.
I prefer deleting the overlay because:
1) hiding it will not solve the problem that the current frame is gonna be rendered incorrectly
2) QGraphicsItem::setVisible() does not define explicitely what happen to the cached data. A clever algorithm might decide to keep the cached pixmap around and keep track of the updates.
3) accelerated compositing might not be used ever after this page, not point at keeping the memory around
I leave the commit queue blank for now. I hope No'am will have time to look at this patch.
(In reply to comment #3) > Created an attachment (id=69237) [details] > Patch > > We have two options: > -keep the overlay around but hide it > -delete the overlay with deleteLater. > > I prefer deleting the overlay because: Me too. > 1) hiding it will not solve the problem that the current frame is gonna be rendered incorrectly > 2) QGraphicsItem::setVisible() does not define explicitely what happen to the cached data. A clever algorithm might decide to keep the cached pixmap around and keep track of the updates. Really complex to do since all call to update are discarded if the item is not visible...Too much complication for little result. setVisible(true) will trigger a full redraw... > 3) accelerated compositing might not be used ever after this page, not point at keeping the memory around > Good point. > I leave the commit queue blank for now. I hope No'am will have time to look at this patch. Comment on attachment 69237 [details]
Patch
r=me, thanks Alexis for comments.
LGTM Comment on attachment 69237 [details] Patch Clearing flags on attachment: 69237 Committed r68761: <http://trac.webkit.org/changeset/68761> All reviewed patches have been landed. Closing bug. Move the bug as critical for 2.1 now that we know the scope. Revision r68653 cherry-picked into qtwebkit-2.1 with commit 13e454d <http://gitorious.org/webkit/qtwebkit/commit/13e454d> Revision r68761 cherry-picked into qtwebkit-2.1 with commit 35e5a3e <http://gitorious.org/webkit/qtwebkit/commit/35e5a3e> |