Commit e0a5162b authored by eric@webkit.org's avatar eric@webkit.org

Reviewed by Beth Dakin.

        CSS @import statements can cause DocLoader to use
        a dead Frame pointer.
        https://bugs.webkit.org/show_bug.cgi?id=19618

        The fix is to get rid of the Frame pointer on DocLoader.

        I also took this opportunity to clean up Document::detach
        a little to make it clear why we clear the m_frame pointer
        there, and to note that in the future we should stop
        using Node::detach to mean "tear down the whole rendering
        tree and detach from the frame".

        Test: I don't know how to make a good test for this, the test
        we have is network timing dependent and does not make a good
        layout test.

        * dom/Document.cpp:
        (WebCore::Document::Document):
        (WebCore::Document::detach):
        (WebCore::Document::clearFramePointer):
        * dom/Document.h:
        * loader/DocLoader.cpp:
        (WebCore::DocLoader::frame):
        * loader/DocLoader.h:


git-svn-id: http://svn.webkit.org/repository/webkit/trunk@34815 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent 3ca31501
2008-06-26 Eric Seidel <eric@webkit.org>
Reviewed by Beth Dakin.
CSS @import statements can cause DocLoader to use
a dead Frame pointer.
https://bugs.webkit.org/show_bug.cgi?id=19618
The fix is to get rid of the Frame pointer on DocLoader.
I also took this opportunity to clean up Document::detach
a little to make it clear why we clear the m_frame pointer
there, and to note that in the future we should stop
using Node::detach to mean "tear down the whole rendering
tree and detach from the frame".
Test: I don't know how to make a good test for this, the test
we have is network timing dependent and does not make a good
layout test.
* dom/Document.cpp:
(WebCore::Document::Document):
(WebCore::Document::detach):
(WebCore::Document::clearFramePointer):
* dom/Document.h:
* loader/DocLoader.cpp:
(WebCore::DocLoader::frame):
* loader/DocLoader.h:
2008-06-26 Anders Carlsson <andersca@apple.com>
Reviewed by Brady.
......@@ -308,8 +308,7 @@ Document::Document(Frame* frame, bool isXHTML)
m_axObjectCache = 0;
// FIXME: DocLoader probably no longer needs the frame argument
m_docLoader = new DocLoader(frame, this);
m_docLoader = new DocLoader(this);
visuallyOrdered = false;
m_bParsing = false;
......@@ -1285,9 +1284,12 @@ void Document::detach()
if (render)
render->destroy();
// FIXME: is this needed or desirable?
m_frame = 0;
// This is required, as our Frame might delete itself as soon as it detaches
// us. However, this violates Node::detach() symantics, as it's never
// possible to re-attach. Eventually Document::detach() should be renamed
// or this call made explicit in each of the callers of Document::detach().
clearFramePointer();
if (m_renderArena) {
delete m_renderArena;
......@@ -1295,6 +1297,11 @@ void Document::detach()
}
}
void Document::clearFramePointer()
{
m_frame = 0;
}
void Document::removeAllEventListenersFromAllNodes()
{
m_windowEventListeners.clear();
......
......@@ -378,6 +378,8 @@ public:
virtual void attach();
virtual void detach();
void clearFramePointer();
RenderArena* renderArena() { return m_renderArena; }
void clearAXObjectCache();
......
......@@ -46,10 +46,9 @@
namespace WebCore {
DocLoader::DocLoader(Frame *frame, Document* doc)
DocLoader::DocLoader(Document* doc)
: m_cache(cache())
, m_cachePolicy(CachePolicyVerify)
, m_frame(frame)
, m_doc(doc)
, m_requestCount(0)
, m_autoLoadImages(true)
......@@ -68,6 +67,11 @@ DocLoader::~DocLoader()
m_cache->removeDocLoader(this);
}
Frame* DocLoader::frame() const
{
return m_doc->frame();
}
void DocLoader::checkForReload(const KURL& fullURL)
{
if (m_allowStaleResources)
......@@ -180,8 +184,8 @@ CachedResource* DocLoader::requestResource(CachedResource::Type type, const Stri
m_docResources.remove(it);
}
}
if (m_frame && m_frame->loader()->isReloading())
if (frame() && frame()->loader()->isReloading())
setCachePolicy(CachePolicyReload);
checkForReload(fullURL);
......@@ -199,10 +203,10 @@ void DocLoader::printAccessDeniedMessage(const KURL& url) const
if (url.isNull())
return;
if (!m_frame)
if (!frame())
return;
Settings* settings = m_frame->settings();
Settings* settings = frame()->settings();
if (!settings || settings->privateBrowsingEnabled())
return;
......@@ -215,7 +219,7 @@ void DocLoader::printAccessDeniedMessage(const KURL& url) const
m_doc->url().string().utf8().data());
// FIXME: provide a real line number and source URL.
m_frame->domWindow()->console()->addMessage(OtherMessageSource, ErrorMessageLevel, message, 1, String());
frame()->domWindow()->console()->addMessage(OtherMessageSource, ErrorMessageLevel, message, 1, String());
}
void DocLoader::setAutoLoadImages(bool enable)
......@@ -253,14 +257,14 @@ void DocLoader::removeCachedResource(CachedResource* resource) const
void DocLoader::setLoadInProgress(bool load)
{
m_loadInProgress = load;
if (!load && m_frame)
m_frame->loader()->loadDone();
if (!load && frame())
frame()->loader()->loadDone();
}
void DocLoader::checkCacheObjectStatus(CachedResource* resource)
{
// Return from the function for objects that we didn't load from the cache or if we don't have a frame.
if (!resource || !m_frame)
if (!resource || !frame())
return;
switch (resource->status()) {
......@@ -274,7 +278,7 @@ void DocLoader::checkCacheObjectStatus(CachedResource* resource)
}
// FIXME: If the WebKit client changes or cancels the request, WebCore does not respect this and continues the load.
m_frame->loader()->loadedResourceFromMemoryCache(resource);
frame()->loader()->loadedResourceFromMemoryCache(resource);
}
void DocLoader::incrementRequestCount()
......
......@@ -51,7 +51,7 @@ friend class Cache;
friend class HTMLImageLoader;
public:
DocLoader(Frame*, Document*);
DocLoader(Document*);
~DocLoader();
CachedImage* requestImage(const String& url);
......@@ -79,7 +79,7 @@ public:
CachePolicy cachePolicy() const { return m_cachePolicy; }
void setCachePolicy(CachePolicy);
Frame* frame() const { return m_frame; }
Frame* frame() const; // Can be NULL
Document* doc() const { return m_doc; }
void removeCachedResource(CachedResource*) const;
......@@ -111,8 +111,7 @@ private:
HashSet<String> m_reloadedURLs;
mutable HashMap<String, CachedResource*> m_docResources;
CachePolicy m_cachePolicy;
Frame* m_frame;
Document *m_doc;
Document* m_doc;
int m_requestCount;
......
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