Bug 126695

Summary: Possible crash in ApplicationCache::removeResource
Product: WebKit Reporter: Piotr Grad <piotr.grad>
Component: WebCore Misc.Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, japhet
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Description Piotr Grad 2014-01-09 07:09:28 PST 
"Iterator it" variable is used after is removed. Working on patch, will be proposed soon.
Comment 1 Piotr Grad 2014-01-09 07:13:36 PST 
Created attachment 220730 [details]
Patch
Comment 2 Piotr Grad 2014-01-09 08:36:39 PST 
Comment on attachment 220730 [details]
Patch

mac-wk2 is not passing also with other attachments with same output.
Comment 3 Alexey Proskuryakov 2014-01-09 09:16:43 PST 
Comment on attachment 220730 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=220730&action=review

> Source/WebCore/ChangeLog:8
> +        No new tests.

This is not a very helpful line. You could explain _why_ there are no tests, but even that is better to do in the bug, not in ChangeLog I think.
Comment 4 Piotr Grad 2014-01-09 14:15:51 PST 
Created attachment 220768 [details]
Patch
Comment 5 WebKit Commit Bot 2014-01-10 13:04:50 PST 
Comment on attachment 220730 [details]
Patch

Clearing flags on attachment: 220730

Committed r161665: <http://trac.webkit.org/changeset/161665>
Comment 6 WebKit Commit Bot 2014-01-10 13:04:52 PST 
All reviewed patches have been landed.  Closing bug.
Comment 7 Darin Adler 2014-01-10 13:14:40 PST 
Comment on attachment 220730 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=220730&action=review

>> Source/WebCore/ChangeLog:8
>> +        No new tests.
> 
> This is not a very helpful line. You could explain _why_ there are no tests, but even that is better to do in the bug, not in ChangeLog I think.

Given that our iterators are checked, I think we could indeed make a test for this. If we can get this code to run, it should assert in a debug build. This fix should not have gone in without a test!