REGRESSION(r117501): IconDatabase asserts on startup in synchronousIconForPageURL().

[Andreas Kling]

<rdar://problem/11480012>

[Andreas Kling]

Created attachment 142843 [details]
Pretentious patch

[Anders Carlsson]

Comment on attachment 142843 [details]
Pretentious patch

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

> Source/WebCore/loader/icon/IconDatabase.cpp:1529
> +        toRetain = m_urlsToRetain;
> +        toRelease = m_urlsToRelease;
> +        m_urlsToRetain.clear();
> +        m_urlsToRelease.clear();

I think you could use swap here.

[Darin Adler]

Comment on attachment 142843 [details]
Pretentious patch

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

>> Source/WebCore/loader/icon/IconDatabase.cpp:1529
>> +        toRetain = m_urlsToRetain;
>> +        toRelease = m_urlsToRelease;
>> +        m_urlsToRetain.clear();
>> +        m_urlsToRelease.clear();
> 
> I think you could use swap here.

And a swap would be much more efficient than the assignment operator can be.

Otherwise, you could get significantly better performance using a vector rather than a counted set for the temporary copy, although there is probably not a suitable helper routine to copy the contents of a counted set into a suitably typed vector.

[Andreas Kling]

(In reply to comment #2)
> (From update of attachment 142843 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=142843&action=review
> 
> > Source/WebCore/loader/icon/IconDatabase.cpp:1529
> > +        toRetain = m_urlsToRetain;
> > +        toRelease = m_urlsToRelease;
> > +        m_urlsToRetain.clear();
> > +        m_urlsToRelease.clear();
> 
> I think you could use swap here.

Yeah, you guys win. I'll add a swap() to HashCountedSet.

[Andreas Kling]

Committed r117744: <http://trac.webkit.org/changeset/117744>