Commit 7ca1166d authored by darin's avatar darin

Reviewed by John.

	- fixed 3132382 -- crash in khtml::CachedImage

	The source of this bug was my long-ago fix to bug 3079499.
	I changed the code to copy the clients list. But this doesn't work if
	one of the clients is removed while iterating because it's deleted.
	So I made a new class, CachedObjectClientWalker, that does the iterating safely.
	Now both this new bug and the original are fixed.

        * khtml/misc/loader.cpp:
        (CachedCSSStyleSheet::checkNotify): Use CachedObjectClientWalker to walk the list.
        (CachedScript::checkNotify): Ditto.
        (CachedImage::do_notify): Ditto.
        (CachedImage::movieStatus): Ditto.
        (CachedImage::checkNotify): Ditto.
        (CachedObjectClientWalker::next): Walk the list using a list iterator, which is
	safe against the current item being removed. But go that safety one better by making
	sure you don't miss the item after one that's removed.


git-svn-id: http://svn.webkit.org/repository/webkit/trunk@3155 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent 5c764a80
2002-12-20 Darin Adler <darin@apple.com>
Reviewed by John.
- fixed 3132382 -- crash in khtml::CachedImage
The source of this bug was my long-ago fix to bug 3079499.
I changed the code to copy the clients list. But this doesn't work if
one of the clients is removed while iterating because it's deleted.
So I made a new class, CachedObjectClientWalker, that does the iterating safely.
Now both this new bug and the original are fixed.
* khtml/misc/loader.cpp:
(CachedCSSStyleSheet::checkNotify): Use CachedObjectClientWalker to walk the list.
(CachedScript::checkNotify): Ditto.
(CachedImage::do_notify): Ditto.
(CachedImage::movieStatus): Ditto.
(CachedImage::checkNotify): Ditto.
(CachedObjectClientWalker::next): Walk the list using a list iterator, which is
safe against the current item being removed. But go that safety one better by making
sure you don't miss the item after one that's removed.
2002-12-20 Ken Kocienda <kocienda@apple.com>
Reviewed by Darin
......
2002-12-20 Darin Adler <darin@apple.com>
Reviewed by John.
- fixed 3132382 -- crash in khtml::CachedImage
The source of this bug was my long-ago fix to bug 3079499.
I changed the code to copy the clients list. But this doesn't work if
one of the clients is removed while iterating because it's deleted.
So I made a new class, CachedObjectClientWalker, that does the iterating safely.
Now both this new bug and the original are fixed.
* khtml/misc/loader.cpp:
(CachedCSSStyleSheet::checkNotify): Use CachedObjectClientWalker to walk the list.
(CachedScript::checkNotify): Ditto.
(CachedImage::do_notify): Ditto.
(CachedImage::movieStatus): Ditto.
(CachedImage::checkNotify): Ditto.
(CachedObjectClientWalker::next): Walk the list using a list iterator, which is
safe against the current item being removed. But go that safety one better by making
sure you don't miss the item after one that's removed.
2002-12-20 Ken Kocienda <kocienda@apple.com>
Reviewed by Darin
......
......@@ -69,6 +69,17 @@ using namespace DOM;
static bool cacheDisabled;
#endif
// Call this "walker" instead of iterator so people won't expect Qt or STL-style iterator interface.
// Just keep calling next() on this. It's safe from deletions of the current item
class CachedObjectClientWalker {
public:
CachedObjectClientWalker(const QPtrList<CachedObjectClient> &clients) : _current(0), _iterator(clients) { }
CachedObjectClient *next();
private:
CachedObjectClient *_current;
QPtrListIterator<CachedObjectClient> _iterator;
};
CachedObject::~CachedObject()
{
if(m_deleted) abort();
......@@ -237,10 +248,9 @@ void CachedCSSStyleSheet::checkNotify()
kdDebug( 6060 ) << "CachedCSSStyleSheet:: finishedLoading " << m_url.string() << endl;
#endif
QPtrList<CachedObjectClient> clients(m_clients);
CachedObjectClient *c;
for ( c = clients.first(); c != 0; c = clients.next() )
c->setStyleSheet( m_url, m_sheet );
CachedObjectClientWalker w(m_clients);
while (CachedObjectClient *c = w.next())
c->setStyleSheet(m_url, m_sheet);
}
......@@ -312,9 +322,8 @@ void CachedScript::checkNotify()
{
if(m_loading) return;
QPtrList<CachedObjectClient> clients(m_clients);
CachedObjectClient *c;
for ( c = clients.first(); c != 0; c = clients.next() )
CachedObjectClientWalker w(m_clients);
while (CachedObjectClient *c = w.next())
c->notifyFinished(this);
}
......@@ -695,10 +704,9 @@ QRect CachedImage::valid_rect() const
void CachedImage::do_notify(const QPixmap& p, const QRect& r)
{
QPtrList<CachedObjectClient> clients(m_clients);
CachedObjectClient *c;
for ( c = clients.first(); c != 0; c = clients.next() )
c->setPixmap( p, r, this);
CachedObjectClientWalker w(m_clients);
while (CachedObjectClient *c = w.next())
c->setPixmap(p, r, this);
}
#if !APPLE_CHANGES
......@@ -779,9 +787,8 @@ void CachedImage::movieStatus(int status)
}
}
QPtrList<CachedObjectClient> clients(m_clients);
CachedObjectClient *c;
for ( c = clients.first(); c != 0; c = clients.next() )
CachedObjectClientWalker w(m_clients);
while (CachedObjectClient *c = w.next())
c->notifyFinished(this);
}
......@@ -965,13 +972,11 @@ void CachedImage::checkNotify()
{
if(m_loading) return;
QPtrList<CachedObjectClient> clients(m_clients);
CachedObjectClient *c;
for ( c = clients.first(); c != 0; c = clients.next() )
CachedObjectClientWalker w(m_clients);
while (CachedObjectClient *c = w.next())
c->notifyFinished(this);
}
// ------------------------------------------------------------------------------------------
Request::Request(DocLoader* dl, CachedObject *_object, bool _incremental)
......@@ -1944,11 +1949,23 @@ bool Cache::adjustSize(CachedObject *object, int delta)
// --------------------------------------
CachedObjectClient *CachedObjectClientWalker::next()
{
// Only advance if we already returned this item.
// This handles cases where the current item is removed, and prevents us from skipping the next item.
// The iterator automatically gets advanced to the next item, and we make sure we return it.
if (_current == _iterator.current())
++_iterator;
_current = _iterator.current();
return _current;
}
// --------------------------------------
void CachedObjectClient::setPixmap(const QPixmap &, const QRect&, CachedImage *) {}
void CachedObjectClient::setStyleSheet(const DOM::DOMString &/*url*/, const DOM::DOMString &/*sheet*/) {}
void CachedObjectClient::notifyFinished(CachedObject * /*finishedObj*/) {}
#include "loader.moc"
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment