Reviewed by Geoff and Mitz.
authorandersca <andersca@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 24 Oct 2007 17:56:18 +0000 (17:56 +0000)
committerandersca <andersca@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 24 Oct 2007 17:56:18 +0000 (17:56 +0000)
        <rdar://problem/5493833>
        REGRESSION: ~5MB of image data leaked @ cuteoverload.com (often seen while browsing other sites, too)

        * WebCore.xcodeproj/project.pbxproj:
        * bindings/js/kjs_binding.cpp:
        (KJS::ScriptInterpreter::markDOMNodesForDocument):
        If an image element that is currently loading an image is not in the document,
        it should still be marked.

        * bindings/js/kjs_html.cpp:
        (WebCore::ImageConstructorImp::construct):
        Force the document wrapper to be created.

        * html/HTMLImageElement.h:
        (WebCore::HTMLImageElement::haveFiredLoadEvent):
        New method which calls down to the image loader.

        * html/HTMLImageLoader.cpp:
        (WebCore::HTMLImageLoader::HTMLImageLoader):
        (WebCore::HTMLImageLoader::~HTMLImageLoader):
        (WebCore::HTMLImageLoader::setLoadingImage):
        (WebCore::HTMLImageLoader::dispatchLoadEvent):
        Remove code that's not needed anymore.

        * html/HTMLImageLoader.h:
        (WebCore::HTMLImageLoader::haveFiredLoadEvent):
        Make this public.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@26992 268f45cc-cd09-0410-ab3c-d52691b4dbfc

WebCore/ChangeLog
WebCore/bindings/js/kjs_binding.cpp
WebCore/bindings/js/kjs_html.cpp
WebCore/html/HTMLImageElement.h
WebCore/html/HTMLImageLoader.cpp
WebCore/html/HTMLImageLoader.h

index 444d1ef..f04e830 100644 (file)
@@ -1,3 +1,35 @@
+2007-10-24  Anders Carlsson  <andersca@apple.com>
+
+        Reviewed by Geoff and Mitz.
+
+        <rdar://problem/5493833>
+        REGRESSION: ~5MB of image data leaked @ cuteoverload.com (often seen while browsing other sites, too)
+
+        * WebCore.xcodeproj/project.pbxproj:
+        * bindings/js/kjs_binding.cpp:
+        (KJS::ScriptInterpreter::markDOMNodesForDocument):
+        If an image element that is currently loading an image is not in the document,
+        it should still be marked.
+        
+        * bindings/js/kjs_html.cpp:
+        (WebCore::ImageConstructorImp::construct):
+        Force the document wrapper to be created.
+        
+        * html/HTMLImageElement.h:
+        (WebCore::HTMLImageElement::haveFiredLoadEvent):
+        New method which calls down to the image loader.
+        
+        * html/HTMLImageLoader.cpp:
+        (WebCore::HTMLImageLoader::HTMLImageLoader):
+        (WebCore::HTMLImageLoader::~HTMLImageLoader):
+        (WebCore::HTMLImageLoader::setLoadingImage):
+        (WebCore::HTMLImageLoader::dispatchLoadEvent):
+        Remove code that's not needed anymore.
+        
+        * html/HTMLImageLoader.h:
+        (WebCore::HTMLImageLoader::haveFiredLoadEvent):
+        Make this public.
+
 2007-10-24  Adam Roben  <aroben@apple.com>
 
         Move Windows safe file creation code into WebCore from WebPreferences
index 2e2dd04..92ad1f7 100644 (file)
@@ -31,6 +31,8 @@
 #include "Event.h"
 #include "EventNames.h"
 #include "Frame.h"
+#include "HTMLImageElement.h"
+#include "HTMLNames.h"
 #include "JSNode.h"
 #include "Page.h"
 #include "PlatformString.h"
@@ -52,6 +54,7 @@
 
 using namespace WebCore;
 using namespace EventNames;
+using namespace HTMLNames;
 
 namespace KJS {
 
@@ -210,12 +213,17 @@ void ScriptInterpreter::markDOMNodesForDocument(Document* doc)
         NodeMap* nodeDict = dictIt->second;
         NodeMap::iterator nodeEnd = nodeDict->end();
         for (NodeMap::iterator nodeIt = nodeDict->begin(); nodeIt != nodeEnd; ++nodeIt) {
-            JSNode* node = nodeIt->second;
+            JSNode* jsNode = nodeIt->second;
+            Node* node = jsNode->impl();
+            
             // don't mark wrappers for nodes that are no longer in the
             // document - they should not be saved if the node is not
             // otherwise reachable from JS.
-            if (node->impl()->inDocument() && !node->marked())
-                node->mark();
+            // However, image elements that aren't in the document are also
+            // marked, if they are not done loading yet.
+            if (!jsNode->marked() && (node->inDocument() || (node->hasTagName(imgTag) &&
+                                                             !static_cast<HTMLImageElement*>(node)->haveFiredLoadEvent())))
+                jsNode->mark();
         }
     }
 }
index 4e43590..f4b203f 100644 (file)
@@ -58,6 +58,13 @@ JSObject* ImageConstructorImp::construct(ExecState*  exec, const List& list)
         height = h->toInt32(exec);
     }
 
+    // Calling toJS on the document causes the JS document wrapper to be
+    // added to the window object. This is done to ensure that JSDocument::mark
+    // will be called (which will cause the image element to be marked if necessary).
+    // This is only a problem for elements created using the Image constructor since all
+    // other elements are created through the document, using document.createElement for example.
+    toJS(exec, m_doc.get());
+    
     HTMLImageElement* image = new HTMLImageElement(m_doc.get());
     JSObject* result = static_cast<JSObject*>(toJS(exec, image));
 
index dcc336b..48d0505 100644 (file)
@@ -115,6 +115,7 @@ public:
 
     bool complete() const;
 
+    bool haveFiredLoadEvent() const { return m_imageLoader.haveFiredLoadEvent(); }
 protected:
     HTMLImageLoader m_imageLoader;
     String usemap;
index fe45efd..8d957d8 100644 (file)
@@ -28,8 +28,6 @@
 #include "Document.h"
 #include "Element.h"
 #include "EventNames.h"
-#include "kjs_binding.h"
-#include "JSNode.h"
 #include "HTMLNames.h"
 #include "RenderImage.h"
 
@@ -46,13 +44,11 @@ HTMLImageLoader::HTMLImageLoader(Element* elt)
     , m_firedLoad(true)
     , m_imageComplete(true)
     , m_loadManually(false)
-    , m_elementIsProtected(false)
 {
 }
 
 HTMLImageLoader::~HTMLImageLoader()
 {
-    ASSERT(!m_elementIsProtected);
     if (m_image)
         m_image->deref(this);
     m_element->document()->removeImage(this);
@@ -78,11 +74,6 @@ void HTMLImageLoader::setImage(CachedImage *newImage)
 
 void HTMLImageLoader::setLoadingImage(CachedImage *loadingImage)
 {
-    if (loadingImage)
-        protectElement();
-    else
-        unprotectElement();
-    
     m_firedLoad = false;
     m_imageComplete = false;
     m_image = loadingImage;
@@ -136,8 +127,6 @@ void HTMLImageLoader::dispatchLoadEvent()
         setHaveFiredLoadEvent(true);
         element()->dispatchHTMLEvent(image()->errorOccurred() ? errorEvent : loadEvent, false, false);
     }
-
-    unprotectElement();
 }
 
 void HTMLImageLoader::notifyFinished(CachedResource *image)
@@ -155,29 +144,4 @@ void HTMLImageLoader::notifyFinished(CachedResource *image)
             static_cast<RenderImage*>(renderer)->setCachedImage(m_image);
 }
 
-void HTMLImageLoader::protectElement()
-{
-    if (m_elementIsProtected)
-        return;
-    
-    KJS::JSLock lock;
-    if (JSNode* node = KJS::ScriptInterpreter::getDOMNodeForDocument(m_element->document(), m_element)) {
-        KJS::gcProtect(node);
-        m_elementIsProtected = true;
-    }    
-}
-    
-void HTMLImageLoader::unprotectElement()
-{
-    if (!m_elementIsProtected)
-        return;
-    
-    KJS::JSLock lock;
-    JSNode* node = KJS::ScriptInterpreter::getDOMNodeForDocument(m_element->document(), m_element);
-    ASSERT(node);
-    KJS::gcUnprotect(node);
-    m_elementIsProtected = false;
-}
-    
-
 }
index 916f3d7..b0ebd48 100644 (file)
@@ -51,22 +51,18 @@ public:
     // CachedResourceClient API
     virtual void notifyFinished(CachedResource*);
 
+    bool haveFiredLoadEvent() const { return m_firedLoad; }
 protected:
     void setLoadingImage(CachedImage*);
     
-    bool haveFiredLoadEvent() { return m_firedLoad; }
     void setHaveFiredLoadEvent(bool firedLoad) { m_firedLoad = firedLoad; }
 
 private:
-    void protectElement();
-    void unprotectElement();
-    
     Element* m_element;
     CachedImage* m_image;
     bool m_firedLoad : 1;
     bool m_imageComplete : 1;
     bool m_loadManually : 1;
-    bool m_elementIsProtected : 1;
 };
 
 } //namespace