| Summary: | Clear failed image loads when an <img> is adopted into a different document | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Adam Klein <adamk> | ||||||
| Component: | New Bugs | Assignee: | Adam Klein <adamk> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | ap, beidson, darin, japhet, rniwa, webkit.review.bot | ||||||
| Priority: | P2 | ||||||||
| Version: | 528+ (Nightly build) | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Attachments: |
|
||||||||
|
Description
Adam Klein
2012-12-07 16:00:58 PST
Created attachment 178298 [details]
Patch
From reading the ways m_failedLoadURL is used, it seems like it's safe to just remove this assert, but I'm not at all an ImageLoader expert so I'm looking for help from the reviewer to decide if I need to do something other than remove the assert. Comment on attachment 178298 [details]
Patch
I think that the gist of this assertion is to say that the function doesn’t do what anything appropriate with the failed load URL, which the original programmer thought was OK because that can never happen. Removing the assertion is clearly OK, but I think that additional work, such as clearing m_failedLoadURL, may be needed.
Clearing m_failedLoadURL seems reasonable in this case, and at worst will just cause a re-fetch, it looks like. Shall I just do that? Created attachment 179514 [details]
Patch
Darin, can you take another look at this? Comment on attachment 179514 [details] Patch Clearing flags on attachment: 179514 Committed r138724: <http://trac.webkit.org/changeset/138724> All reviewed patches have been landed. Closing bug. It appears that this patch broke WebKit1.MemoryCachePruneWithinResourceLoadDelegate: Pass: http://build.webkit.org/builders/Apple%20MountainLion%20Debug%20WK2%20%28Tests%29/builds/5409 Fail: http://build.webkit.org/builders/Apple%20MountainLion%20Debug%20WK2%20%28Tests%29/builds/5410 Blame range: http://trac.webkit.org/log/?verbose=on&rev=138726&stop_rev=138721 On the other hand, http://build.webkit.org/builders/Apple%20MountainLion%20Debug%20WK1%20%28Tests%29?numbuilds=25 says it’s r138726, which is quite inconceivable. Filed https://bugs.webkit.org/show_bug.cgi?id=106051 to track the failure. |