Summary: | Repro crash animating GIF if previously used in a closed window's back/forward list | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Alice Liu <alice.barraclough> | ||||
Component: | Platform | Assignee: | Alice Liu <alice.barraclough> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | darin, mitz, sfalken | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | All | ||||||
OS: | Windows Vista | ||||||
Attachments: |
|
Description
Alice Liu
2009-06-19 23:34:51 PDT
Created attachment 31588 [details]
patch and manual test
Comment on attachment 31588 [details]
patch and manual test
The null checks are good in preventing crashes, but I wonder if it isn’t practical to augment them with ASSERTs, and to add code at a higher level that would stop this crash from happening—one possible place is RenderView::repaintViewRectangle(), but even better would be to change implementations of imageChanged() such that they don’t do any unnecessary work (such as computing a repaint rectangle) when the document is in the back/forward cache.
Comment on attachment 31588 [details]
patch and manual test
I'm going to r+ notwithstanding Mitz's comments, because I think further improvement to fix the problem at a higher level can be done as a separate patch.
Is it possible to make an automated layout test for this? I believe layout tests have the power to open and navigate additional windows. Let's try to make the test into a fully automated LayoutTest if possible.
The crash requires that the back/forward cache be enabled, which afaik is not enabled in the automated layout tests. Fixed by Alice in <http://trac.webkit.org/changeset/44908>. |