<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" /><style type="text/css"><!--
#msg dl { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt;  }
#msg dl a { font-weight: bold}
#msg dl a:link    { color:#fc3; }
#msg dl a:active  { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fc0 solid; padding: 6px; }
#msg ul, pre { overflow: auto; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff  {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<title>[26992] trunk/WebCore</title>
</head>
<body>

<div id="msg">
<dl>
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/26992">26992</a></dd>
<dt>Author</dt> <dd>andersca</dd>
<dt>Date</dt> <dd>2007-10-24 10:56:18 -0700 (Wed, 24 Oct 2007)</dd>
</dl>

<h3>Log Message</h3>
<pre>        Reviewed by Geoff and Mitz.

        &lt;rdar://problem/5493833&gt;
        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.</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkWebCoreChangeLog">trunk/WebCore/ChangeLog</a></li>
<li><a href="#trunkWebCorebindingsjskjs_bindingcpp">trunk/WebCore/bindings/js/kjs_binding.cpp</a></li>
<li><a href="#trunkWebCorebindingsjskjs_htmlcpp">trunk/WebCore/bindings/js/kjs_html.cpp</a></li>
<li><a href="#trunkWebCorehtmlHTMLImageElementh">trunk/WebCore/html/HTMLImageElement.h</a></li>
<li><a href="#trunkWebCorehtmlHTMLImageLoadercpp">trunk/WebCore/html/HTMLImageLoader.cpp</a></li>
<li><a href="#trunkWebCorehtmlHTMLImageLoaderh">trunk/WebCore/html/HTMLImageLoader.h</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/WebCore/ChangeLog (26991 => 26992)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/WebCore/ChangeLog        2007-10-24 17:27:53 UTC (rev 26991)
+++ trunk/WebCore/ChangeLog        2007-10-24 17:56:18 UTC (rev 26992)
</span><span class="lines">@@ -1,3 +1,35 @@
</span><ins>+2007-10-24  Anders Carlsson  &lt;andersca@apple.com&gt;
+
+        Reviewed by Geoff and Mitz.
+
+        &lt;rdar://problem/5493833&gt;
+        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.
+
</ins><span class="cx"> 2007-10-24  Adam Roben  &lt;aroben@apple.com&gt;
</span><span class="cx"> 
</span><span class="cx">         Move Windows safe file creation code into WebCore from WebPreferences
</span></span></pre></div>
<a id="trunkWebCorebindingsjskjs_bindingcpp"></a>
<div class="modfile"><h4>Modified: trunk/WebCore/bindings/js/kjs_binding.cpp (26991 => 26992)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/WebCore/bindings/js/kjs_binding.cpp        2007-10-24 17:27:53 UTC (rev 26991)
+++ trunk/WebCore/bindings/js/kjs_binding.cpp        2007-10-24 17:56:18 UTC (rev 26992)
</span><span class="lines">@@ -31,6 +31,8 @@
</span><span class="cx"> #include &quot;Event.h&quot;
</span><span class="cx"> #include &quot;EventNames.h&quot;
</span><span class="cx"> #include &quot;Frame.h&quot;
</span><ins>+#include &quot;HTMLImageElement.h&quot;
+#include &quot;HTMLNames.h&quot;
</ins><span class="cx"> #include &quot;JSNode.h&quot;
</span><span class="cx"> #include &quot;Page.h&quot;
</span><span class="cx"> #include &quot;PlatformString.h&quot;
</span><span class="lines">@@ -52,6 +54,7 @@
</span><span class="cx"> 
</span><span class="cx"> using namespace WebCore;
</span><span class="cx"> using namespace EventNames;
</span><ins>+using namespace HTMLNames;
</ins><span class="cx"> 
</span><span class="cx"> namespace KJS {
</span><span class="cx"> 
</span><span class="lines">@@ -210,12 +213,17 @@
</span><span class="cx">         NodeMap* nodeDict = dictIt-&gt;second;
</span><span class="cx">         NodeMap::iterator nodeEnd = nodeDict-&gt;end();
</span><span class="cx">         for (NodeMap::iterator nodeIt = nodeDict-&gt;begin(); nodeIt != nodeEnd; ++nodeIt) {
</span><del>-            JSNode* node = nodeIt-&gt;second;
</del><ins>+            JSNode* jsNode = nodeIt-&gt;second;
+            Node* node = jsNode-&gt;impl();
+            
</ins><span class="cx">             // don't mark wrappers for nodes that are no longer in the
</span><span class="cx">             // document - they should not be saved if the node is not
</span><span class="cx">             // otherwise reachable from JS.
</span><del>-            if (node-&gt;impl()-&gt;inDocument() &amp;&amp; !node-&gt;marked())
-                node-&gt;mark();
</del><ins>+            // However, image elements that aren't in the document are also
+            // marked, if they are not done loading yet.
+            if (!jsNode-&gt;marked() &amp;&amp; (node-&gt;inDocument() || (node-&gt;hasTagName(imgTag) &amp;&amp;
+                                                             !static_cast&lt;HTMLImageElement*&gt;(node)-&gt;haveFiredLoadEvent())))
+                jsNode-&gt;mark();
</ins><span class="cx">         }
</span><span class="cx">     }
</span><span class="cx"> }
</span></span></pre></div>
<a id="trunkWebCorebindingsjskjs_htmlcpp"></a>
<div class="modfile"><h4>Modified: trunk/WebCore/bindings/js/kjs_html.cpp (26991 => 26992)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/WebCore/bindings/js/kjs_html.cpp        2007-10-24 17:27:53 UTC (rev 26991)
+++ trunk/WebCore/bindings/js/kjs_html.cpp        2007-10-24 17:56:18 UTC (rev 26992)
</span><span class="lines">@@ -58,6 +58,13 @@
</span><span class="cx">         height = h-&gt;toInt32(exec);
</span><span class="cx">     }
</span><span class="cx"> 
</span><ins>+    // 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());
+    
</ins><span class="cx">     HTMLImageElement* image = new HTMLImageElement(m_doc.get());
</span><span class="cx">     JSObject* result = static_cast&lt;JSObject*&gt;(toJS(exec, image));
</span><span class="cx"> 
</span></span></pre></div>
<a id="trunkWebCorehtmlHTMLImageElementh"></a>
<div class="modfile"><h4>Modified: trunk/WebCore/html/HTMLImageElement.h (26991 => 26992)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/WebCore/html/HTMLImageElement.h        2007-10-24 17:27:53 UTC (rev 26991)
+++ trunk/WebCore/html/HTMLImageElement.h        2007-10-24 17:56:18 UTC (rev 26992)
</span><span class="lines">@@ -115,6 +115,7 @@
</span><span class="cx"> 
</span><span class="cx">     bool complete() const;
</span><span class="cx"> 
</span><ins>+    bool haveFiredLoadEvent() const { return m_imageLoader.haveFiredLoadEvent(); }
</ins><span class="cx"> protected:
</span><span class="cx">     HTMLImageLoader m_imageLoader;
</span><span class="cx">     String usemap;
</span></span></pre></div>
<a id="trunkWebCorehtmlHTMLImageLoadercpp"></a>
<div class="modfile"><h4>Modified: trunk/WebCore/html/HTMLImageLoader.cpp (26991 => 26992)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/WebCore/html/HTMLImageLoader.cpp        2007-10-24 17:27:53 UTC (rev 26991)
+++ trunk/WebCore/html/HTMLImageLoader.cpp        2007-10-24 17:56:18 UTC (rev 26992)
</span><span class="lines">@@ -28,8 +28,6 @@
</span><span class="cx"> #include &quot;Document.h&quot;
</span><span class="cx"> #include &quot;Element.h&quot;
</span><span class="cx"> #include &quot;EventNames.h&quot;
</span><del>-#include &quot;kjs_binding.h&quot;
-#include &quot;JSNode.h&quot;
</del><span class="cx"> #include &quot;HTMLNames.h&quot;
</span><span class="cx"> #include &quot;RenderImage.h&quot;
</span><span class="cx"> 
</span><span class="lines">@@ -46,13 +44,11 @@
</span><span class="cx">     , m_firedLoad(true)
</span><span class="cx">     , m_imageComplete(true)
</span><span class="cx">     , m_loadManually(false)
</span><del>-    , m_elementIsProtected(false)
</del><span class="cx"> {
</span><span class="cx"> }
</span><span class="cx"> 
</span><span class="cx"> HTMLImageLoader::~HTMLImageLoader()
</span><span class="cx"> {
</span><del>-    ASSERT(!m_elementIsProtected);
</del><span class="cx">     if (m_image)
</span><span class="cx">         m_image-&gt;deref(this);
</span><span class="cx">     m_element-&gt;document()-&gt;removeImage(this);
</span><span class="lines">@@ -78,11 +74,6 @@
</span><span class="cx"> 
</span><span class="cx"> void HTMLImageLoader::setLoadingImage(CachedImage *loadingImage)
</span><span class="cx"> {
</span><del>-    if (loadingImage)
-        protectElement();
-    else
-        unprotectElement();
-    
</del><span class="cx">     m_firedLoad = false;
</span><span class="cx">     m_imageComplete = false;
</span><span class="cx">     m_image = loadingImage;
</span><span class="lines">@@ -136,8 +127,6 @@
</span><span class="cx">         setHaveFiredLoadEvent(true);
</span><span class="cx">         element()-&gt;dispatchHTMLEvent(image()-&gt;errorOccurred() ? errorEvent : loadEvent, false, false);
</span><span class="cx">     }
</span><del>-
-    unprotectElement();
</del><span class="cx"> }
</span><span class="cx"> 
</span><span class="cx"> void HTMLImageLoader::notifyFinished(CachedResource *image)
</span><span class="lines">@@ -155,29 +144,4 @@
</span><span class="cx">             static_cast&lt;RenderImage*&gt;(renderer)-&gt;setCachedImage(m_image);
</span><span class="cx"> }
</span><span class="cx"> 
</span><del>-void HTMLImageLoader::protectElement()
-{
-    if (m_elementIsProtected)
-        return;
-    
-    KJS::JSLock lock;
-    if (JSNode* node = KJS::ScriptInterpreter::getDOMNodeForDocument(m_element-&gt;document(), m_element)) {
-        KJS::gcProtect(node);
-        m_elementIsProtected = true;
-    }    
</del><span class="cx"> }
</span><del>-    
-void HTMLImageLoader::unprotectElement()
-{
-    if (!m_elementIsProtected)
-        return;
-    
-    KJS::JSLock lock;
-    JSNode* node = KJS::ScriptInterpreter::getDOMNodeForDocument(m_element-&gt;document(), m_element);
-    ASSERT(node);
-    KJS::gcUnprotect(node);
-    m_elementIsProtected = false;
-}
-    
-
-}
</del></span></pre></div>
<a id="trunkWebCorehtmlHTMLImageLoaderh"></a>
<div class="modfile"><h4>Modified: trunk/WebCore/html/HTMLImageLoader.h (26991 => 26992)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/WebCore/html/HTMLImageLoader.h        2007-10-24 17:27:53 UTC (rev 26991)
+++ trunk/WebCore/html/HTMLImageLoader.h        2007-10-24 17:56:18 UTC (rev 26992)
</span><span class="lines">@@ -51,22 +51,18 @@
</span><span class="cx">     // CachedResourceClient API
</span><span class="cx">     virtual void notifyFinished(CachedResource*);
</span><span class="cx"> 
</span><ins>+    bool haveFiredLoadEvent() const { return m_firedLoad; }
</ins><span class="cx"> protected:
</span><span class="cx">     void setLoadingImage(CachedImage*);
</span><span class="cx">     
</span><del>-    bool haveFiredLoadEvent() { return m_firedLoad; }
</del><span class="cx">     void setHaveFiredLoadEvent(bool firedLoad) { m_firedLoad = firedLoad; }
</span><span class="cx"> 
</span><span class="cx"> private:
</span><del>-    void protectElement();
-    void unprotectElement();
-    
</del><span class="cx">     Element* m_element;
</span><span class="cx">     CachedImage* m_image;
</span><span class="cx">     bool m_firedLoad : 1;
</span><span class="cx">     bool m_imageComplete : 1;
</span><span class="cx">     bool m_loadManually : 1;
</span><del>-    bool m_elementIsProtected : 1;
</del><span class="cx"> };
</span><span class="cx"> 
</span><span class="cx"> } //namespace
</span></span></pre>
</div>
</div>

</body>
</html>