<!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" />
<title>[184793] trunk/Source/WebCore</title>
</head>
<body>

<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { 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 #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#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>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/184793">184793</a></dd>
<dt>Author</dt> <dd>cdumez@apple.com</dd>
<dt>Date</dt> <dd>2015-05-22 13:47:53 -0700 (Fri, 22 May 2015)</dd>
</dl>

<h3>Log Message</h3>
<pre>[CG] Regression(<a href="http://trac.webkit.org/projects/webkit/changeset/78652">r78652</a>): Partially decoded images are not properly removed from MemoryCache when pruning
https://bugs.webkit.org/show_bug.cgi?id=145310

Reviewed by Antti Koivisto.

<a href="http://trac.webkit.org/projects/webkit/changeset/78652">r78652</a> added partially decoded images to the MemoryCache's list of live
decoded resources so that they can be pruned on memory pressure. This
was needed because CG decodes part of the image to determine its
properties (e.g. its size). On memory pressure, we call
BitmapImage::destroyDecodedData() which clears the ImageSource and
frees up this extra decoded data.

However, we would fail to remove such partially decoded images from the
MemoryCache's list of live resources when pruning. This is because
BitmapImage::destroyMetadataAndNotify() fails to take into account the
decoded properties size when no frame has been cleared. We would thus
fail to detect a decoded size change and not call
CachedImage::decodedSizeChanged(). As a result, the CachedImage's
decoded size is not reset to 0 and we don't remove it from live decoded
resources.

This patch updates BitmapImage::destroyMetadataAndNotify() to account
for m_decodedPropertiesSize even if frameBytesCleared is 0. This way,
images for which we have't decoded any frames yet will correctly report
that we cleared the decoded data used to determine the image properties
and their decoded size will be properly reset to 0. As a result, these
will be removed from the MemoryCache's list of live decoded resources.

* platform/graphics/BitmapImage.cpp:
(WebCore::BitmapImage::destroyDecodedData):
(WebCore::BitmapImage::destroyMetadataAndNotify):
(WebCore::BitmapImage::dataChanged):
* platform/graphics/BitmapImage.h:</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoreplatformgraphicsBitmapImagecpp">trunk/Source/WebCore/platform/graphics/BitmapImage.cpp</a></li>
<li><a href="#trunkSourceWebCoreplatformgraphicsBitmapImageh">trunk/Source/WebCore/platform/graphics/BitmapImage.h</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (184792 => 184793)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog        2015-05-22 20:31:47 UTC (rev 184792)
+++ trunk/Source/WebCore/ChangeLog        2015-05-22 20:47:53 UTC (rev 184793)
</span><span class="lines">@@ -1,3 +1,39 @@
</span><ins>+2015-05-22  Chris Dumez  &lt;cdumez@apple.com&gt;
+
+        [CG] Regression(r78652): Partially decoded images are not properly removed from MemoryCache when pruning
+        https://bugs.webkit.org/show_bug.cgi?id=145310
+
+        Reviewed by Antti Koivisto.
+
+        r78652 added partially decoded images to the MemoryCache's list of live
+        decoded resources so that they can be pruned on memory pressure. This
+        was needed because CG decodes part of the image to determine its
+        properties (e.g. its size). On memory pressure, we call
+        BitmapImage::destroyDecodedData() which clears the ImageSource and
+        frees up this extra decoded data.
+
+        However, we would fail to remove such partially decoded images from the
+        MemoryCache's list of live resources when pruning. This is because
+        BitmapImage::destroyMetadataAndNotify() fails to take into account the
+        decoded properties size when no frame has been cleared. We would thus
+        fail to detect a decoded size change and not call
+        CachedImage::decodedSizeChanged(). As a result, the CachedImage's
+        decoded size is not reset to 0 and we don't remove it from live decoded
+        resources.
+
+        This patch updates BitmapImage::destroyMetadataAndNotify() to account
+        for m_decodedPropertiesSize even if frameBytesCleared is 0. This way,
+        images for which we have't decoded any frames yet will correctly report
+        that we cleared the decoded data used to determine the image properties
+        and their decoded size will be properly reset to 0. As a result, these
+        will be removed from the MemoryCache's list of live decoded resources.
+
+        * platform/graphics/BitmapImage.cpp:
+        (WebCore::BitmapImage::destroyDecodedData):
+        (WebCore::BitmapImage::destroyMetadataAndNotify):
+        (WebCore::BitmapImage::dataChanged):
+        * platform/graphics/BitmapImage.h:
+
</ins><span class="cx"> 2015-05-22  Joseph Pecoraro  &lt;pecoraro@apple.com&gt;
</span><span class="cx"> 
</span><span class="cx">         Web Inspector: Unable to get cursor in new Rule section after creating multiple New Rules
</span></span></pre></div>
<a id="trunkSourceWebCoreplatformgraphicsBitmapImagecpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/platform/graphics/BitmapImage.cpp (184792 => 184793)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/platform/graphics/BitmapImage.cpp        2015-05-22 20:31:47 UTC (rev 184792)
+++ trunk/Source/WebCore/platform/graphics/BitmapImage.cpp        2015-05-22 20:47:53 UTC (rev 184793)
</span><span class="lines">@@ -135,10 +135,8 @@
</span><span class="cx">             frameBytesCleared += frameBytes;
</span><span class="cx">     }
</span><span class="cx"> 
</span><del>-    destroyMetadataAndNotify(frameBytesCleared);
-
</del><span class="cx">     m_source.clear(destroyAll, clearBeforeFrame, data(), m_allDataReceived);
</span><del>-    return;
</del><ins>+    destroyMetadataAndNotify(frameBytesCleared, ClearedSource::Yes);
</ins><span class="cx"> }
</span><span class="cx"> 
</span><span class="cx"> void BitmapImage::destroyDecodedDataIfNecessary(bool destroyAll)
</span><span class="lines">@@ -169,7 +167,7 @@
</span><span class="cx">         destroyDecodedData(destroyAll);
</span><span class="cx"> }
</span><span class="cx"> 
</span><del>-void BitmapImage::destroyMetadataAndNotify(unsigned frameBytesCleared)
</del><ins>+void BitmapImage::destroyMetadataAndNotify(unsigned frameBytesCleared, ClearedSource clearedSource)
</ins><span class="cx"> {
</span><span class="cx">     m_isSolidColor = false;
</span><span class="cx">     m_checkedForSolidColor = false;
</span><span class="lines">@@ -177,10 +175,13 @@
</span><span class="cx"> 
</span><span class="cx">     ASSERT(m_decodedSize &gt;= frameBytesCleared);
</span><span class="cx">     m_decodedSize -= frameBytesCleared;
</span><del>-    if (frameBytesCleared &gt; 0) {
</del><ins>+
+    // Clearing the ImageSource destroys the extra decoded data used for determining image properties.
+    if (clearedSource == ClearedSource::Yes) {
</ins><span class="cx">         frameBytesCleared += m_decodedPropertiesSize;
</span><span class="cx">         m_decodedPropertiesSize = 0;
</span><span class="cx">     }
</span><ins>+
</ins><span class="cx">     if (frameBytesCleared &amp;&amp; imageObserver())
</span><span class="cx">         imageObserver()-&gt;decodedSizeChanged(this, -safeCast&lt;int&gt;(frameBytesCleared));
</span><span class="cx"> }
</span><span class="lines">@@ -313,7 +314,7 @@
</span><span class="cx">         if (m_frames[i].m_haveMetadata &amp;&amp; !m_frames[i].m_isComplete)
</span><span class="cx">             frameBytesCleared += (m_frames[i].clear(true) ? frameBytes : 0);
</span><span class="cx">     }
</span><del>-    destroyMetadataAndNotify(frameBytesCleared);
</del><ins>+    destroyMetadataAndNotify(frameBytesCleared, ClearedSource::No);
</ins><span class="cx"> #else
</span><span class="cx">     // FIXME: why is this different for iOS?
</span><span class="cx">     int deltaBytes = 0;
</span><span class="lines">@@ -325,7 +326,7 @@
</span><span class="cx">             m_decodedPropertiesSize = 0;
</span><span class="cx">         }
</span><span class="cx">     }
</span><del>-    destroyMetadataAndNotify(deltaBytes);
</del><ins>+    destroyMetadataAndNotify(deltaBytes, ClearedSource::No);
</ins><span class="cx"> #endif
</span><span class="cx">     
</span><span class="cx">     // Feed all the data we've seen so far to the image decoder.
</span></span></pre></div>
<a id="trunkSourceWebCoreplatformgraphicsBitmapImageh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/platform/graphics/BitmapImage.h (184792 => 184793)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/platform/graphics/BitmapImage.h        2015-05-22 20:31:47 UTC (rev 184792)
+++ trunk/Source/WebCore/platform/graphics/BitmapImage.h        2015-05-22 20:47:53 UTC (rev 184793)
</span><span class="lines">@@ -243,7 +243,8 @@
</span><span class="cx">     // Generally called by destroyDecodedData(), destroys whole-image metadata
</span><span class="cx">     // and notifies observers that the memory footprint has (hopefully)
</span><span class="cx">     // decreased by |frameBytesCleared|.
</span><del>-    void destroyMetadataAndNotify(unsigned frameBytesCleared);
</del><ins>+    enum class ClearedSource { No, Yes };
+    void destroyMetadataAndNotify(unsigned frameBytesCleared, ClearedSource);
</ins><span class="cx"> 
</span><span class="cx">     // Whether or not size is available yet.
</span><span class="cx">     bool isSizeAvailable();
</span></span></pre>
</div>
</div>

</body>
</html>