Commit 317d75c6 authored by antti@apple.com's avatar antti@apple.com

2009-06-16 Antti Koivisto <antti@apple.com>

        Reviewed by Brady Eidson.

        <rdar://problem/6660037> CrashTracer: [USER] 46 crashes in Safari at com.apple.WebCore • WebCore::CachedCSSStyleSheet::addClient + 53
        
        When revalidating a resource, calling addClient() on one client might cause another to get removed.
        
        - made CachedResource::addClient() non-virtual and added virtual didAddClient()
        - in CachedResource::switchClientsToRevalidatedResource() add all clients to the client set of the revalidated resource first
        - check if the client is still in the set before invoking didAddClient() for it
        
        No test case, I didn't manage to construct one. You need some combination of 304 revalidation, stylesheets that
        reference each other via @imports and reloading.

        * WebCore.base.exp:
        * loader/CachedCSSStyleSheet.cpp:
        (WebCore::CachedCSSStyleSheet::didAddClient):
        * loader/CachedCSSStyleSheet.h:
        * loader/CachedFont.cpp:
        (WebCore::CachedFont::didAddClient):
        * loader/CachedFont.h:
        * loader/CachedImage.cpp:
        (WebCore::CachedImage::didAddClient):
        * loader/CachedImage.h:
        * loader/CachedResource.cpp:
        (WebCore::CachedResource::addClient):
        (WebCore::CachedResource::addClientToSet):
        (WebCore::CachedResource::switchClientsToRevalidatedResource):
        * loader/CachedResource.h:
        * loader/CachedScript.cpp:
        (WebCore::CachedScript::didAddClient):
        * loader/CachedScript.h:
        * loader/CachedXSLStyleSheet.cpp:
        (WebCore::CachedXSLStyleSheet::didAddClient):
        * loader/CachedXSLStyleSheet.h:



git-svn-id: http://svn.webkit.org/repository/webkit/trunk@44749 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent 82319105
2009-06-16 Antti Koivisto <antti@apple.com>
Reviewed by Brady Eidson.
<rdar://problem/6660037> CrashTracer: [USER] 46 crashes in Safari at com.apple.WebCore • WebCore::CachedCSSStyleSheet::addClient + 53
When revalidating a resource, calling addClient() on one client might cause another to get removed.
- made CachedResource::addClient() non-virtual and added virtual didAddClient()
- in CachedResource::switchClientsToRevalidatedResource() add all clients to the client set of the revalidated resource first
- check if the client is still in the set before invoking didAddClient() for it
No test case, I didn't manage to construct one. You need some combination of 304 revalidation, stylesheets that
reference each other via @imports and reloading.
* WebCore.base.exp:
* loader/CachedCSSStyleSheet.cpp:
(WebCore::CachedCSSStyleSheet::didAddClient):
* loader/CachedCSSStyleSheet.h:
* loader/CachedFont.cpp:
(WebCore::CachedFont::didAddClient):
* loader/CachedFont.h:
* loader/CachedImage.cpp:
(WebCore::CachedImage::didAddClient):
* loader/CachedImage.h:
* loader/CachedResource.cpp:
(WebCore::CachedResource::addClient):
(WebCore::CachedResource::addClientToSet):
(WebCore::CachedResource::switchClientsToRevalidatedResource):
* loader/CachedResource.h:
* loader/CachedScript.cpp:
(WebCore::CachedScript::didAddClient):
* loader/CachedScript.h:
* loader/CachedXSLStyleSheet.cpp:
(WebCore::CachedXSLStyleSheet::didAddClient):
* loader/CachedXSLStyleSheet.h:
2009-06-16 Simon Fraser <simon.fraser@apple.com>
No Review
......
......@@ -284,6 +284,7 @@ __ZN7WebCore13TypingCommand39insertParagraphSeparatorInQuotedContentEPNS_8Docume
__ZN7WebCore13toDeviceSpaceERKNS_9FloatRectEP8NSWindow
__ZN7WebCore13toJSDOMWindowEPNS_5FrameE
__ZN7WebCore14CachedResource12removeClientEPNS_20CachedResourceClientE
__ZN7WebCore14CachedResource9addClientEPNS_20CachedResourceClientE
__ZN7WebCore14DocumentLoader13attachToFrameEv
__ZN7WebCore14DocumentLoader15detachFromFrameEv
__ZN7WebCore14DocumentLoader18addArchiveResourceEN3WTF10PassRefPtrINS_15ArchiveResourceEEE
......
......@@ -49,10 +49,8 @@ CachedCSSStyleSheet::~CachedCSSStyleSheet()
{
}
void CachedCSSStyleSheet::addClient(CachedResourceClient *c)
void CachedCSSStyleSheet::didAddClient(CachedResourceClient *c)
{
CachedResource::addClient(c);
if (!m_loading)
c->setCSSStyleSheet(m_url, m_decoder->encoding().name(), this);
}
......
......@@ -42,7 +42,7 @@ namespace WebCore {
const String sheetText(bool enforceMIMEType = true) const;
virtual void addClient(CachedResourceClient*);
virtual void didAddClient(CachedResourceClient*);
virtual void allClientsRemoved();
......
......@@ -71,10 +71,8 @@ void CachedFont::load(DocLoader*)
m_loading = true;
}
void CachedFont::addClient(CachedResourceClient* c)
void CachedFont::didAddClient(CachedResourceClient* c)
{
CachedResource::addClient(c);
if (!m_loading)
c->fontLoaded(this);
}
......
......@@ -51,7 +51,7 @@ public:
virtual void load(DocLoader* docLoader);
virtual void addClient(CachedResourceClient*);
virtual void didAddClient(CachedResourceClient*);
virtual void data(PassRefPtr<SharedBuffer> data, bool allDataReceived);
virtual void error();
......
......@@ -86,10 +86,8 @@ void CachedImage::load(DocLoader* docLoader)
m_loading = false;
}
void CachedImage::addClient(CachedResourceClient* c)
void CachedImage::didAddClient(CachedResourceClient* c)
{
CachedResource::addClient(c);
if (m_decodedDataDeletionTimer.isActive())
m_decodedDataDeletionTimer.stop();
......
......@@ -60,7 +60,7 @@ public:
IntSize imageSize(float multiplier) const; // returns the size of the complete image.
IntRect imageRect(float multiplier) const; // The size of the currently decoded portion of the image.
virtual void addClient(CachedResourceClient*);
virtual void didAddClient(CachedResourceClient*);
virtual void allClientsRemoved();
virtual void destroyDecodedData();
......
......@@ -173,7 +173,13 @@ void CachedResource::setRequest(Request* request)
delete this;
}
void CachedResource::addClient(CachedResourceClient *c)
void CachedResource::addClient(CachedResourceClient* client)
{
addClientToSet(client);
didAddClient(client);
}
void CachedResource::addClientToSet(CachedResourceClient* client)
{
ASSERT(!isPurgeable());
......@@ -187,7 +193,7 @@ void CachedResource::addClient(CachedResourceClient *c)
}
if (!hasClients() && inCache())
cache()->addToLiveResourcesSize(this);
m_clients.add(c);
m_clients.add(client);
}
void CachedResource::removeClient(CachedResourceClient *c)
......@@ -330,10 +336,15 @@ void CachedResource::switchClientsToRevalidatedResource()
}
// Equivalent of calling removeClient() for all clients
m_clients.clear();
unsigned moveCount = clientsToMove.size();
for (unsigned n = 0; n < moveCount; ++n)
m_resourceToRevalidate->addClient(clientsToMove[n]);
m_resourceToRevalidate->addClientToSet(clientsToMove[n]);
for (unsigned n = 0; n < moveCount; ++n) {
// Calling didAddClient for a client may end up removing another client. In that case it won't be in the set anymore.
if (m_resourceToRevalidate->m_clients.contains(clientsToMove[n]))
m_resourceToRevalidate->didAddClient(clientsToMove[n]);
}
}
void CachedResource::updateResponseAfterRevalidation(const ResourceResponse& validatingResponse)
......
......@@ -87,7 +87,7 @@ public:
const String &url() const { return m_url; }
Type type() const { return m_type; }
virtual void addClient(CachedResourceClient*);
void addClient(CachedResourceClient*);
void removeClient(CachedResourceClient*);
bool hasClients() const { return !m_clients.isEmpty(); }
void deleteIfPossible();
......@@ -100,7 +100,8 @@ public:
};
PreloadResult preloadResult() const { return m_preloadResult; }
void setRequestedFromNetworkingLayer() { m_requestedFromNetworkingLayer = true; }
virtual void didAddClient(CachedResourceClient*) = 0;
virtual void allClientsRemoved() { }
unsigned count() const { return m_clients.size(); }
......@@ -204,6 +205,8 @@ protected:
bool m_errorOccurred;
private:
void addClientToSet(CachedResourceClient*);
// These are called by the friendly Cache only
void setResourceToRevalidate(CachedResource*);
void switchClientsToRevalidatedResource();
......
......@@ -49,9 +49,8 @@ CachedScript::~CachedScript()
{
}
void CachedScript::addClient(CachedResourceClient* c)
void CachedScript::didAddClient(CachedResourceClient* c)
{
CachedResource::addClient(c);
if (!m_loading)
c->notifyFinished(this);
}
......
......@@ -41,7 +41,7 @@ namespace WebCore {
const String& script();
virtual void addClient(CachedResourceClient*);
virtual void didAddClient(CachedResourceClient*);
virtual void allClientsRemoved();
virtual void setEncoding(const String&);
......
......@@ -45,10 +45,8 @@ CachedXSLStyleSheet::CachedXSLStyleSheet(const String &url)
setAccept("text/xml, application/xml, application/xhtml+xml, text/xsl, application/rss+xml, application/atom+xml");
}
void CachedXSLStyleSheet::addClient(CachedResourceClient *c)
{
CachedResource::addClient(c);
void CachedXSLStyleSheet::didAddClient(CachedResourceClient* c)
{
if (!m_loading)
c->setXSLStyleSheet(m_url, m_sheet);
}
......
......@@ -41,7 +41,7 @@ namespace WebCore {
const String& sheet() const { return m_sheet; }
virtual void addClient(CachedResourceClient*);
virtual void didAddClient(CachedResourceClient*);
virtual void setEncoding(const String&);
virtual String encoding() const;
......
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