<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/101653/">http://git.reviewboard.kde.org/r/101653/</a>
</td>
</tr>
</table>
<br />
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/media/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for kdelibs.</div>
<div>By Thomas Friedrichsmeier.</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I guess this one needs some explaining...
Many KParts::Plugins appear to assume that widgets that they add to a KParts::StatusBarExtension (SE) will effectively be owned by SE, and will be deleted with the extension. E.g. the konqi W3 Validator plugin has this code:
void PluginValidators::removeStatusBarIcon()
{
if (!m_icon)
return;
m_statusBarExt = KParts::StatusBarExtension::childObject(m_part);
if (!m_statusBarExt)
return;
m_statusBarExt->removeStatusBarItem(m_icon);
delete m_icon;
m_icon = 0;
}
Note the return when no SE exists (any more). Checking for the existence of the SE is important, as it may become deleted before the plugins are deleted. In fact, at least for KHTMLPart that is the case. However, the assumption that the icon will have been deleted with the SE is wrong. It is still around. In konqueror this is not visible, because the part is always sent a de-activate event before destruction, and the SE hides the icons, then. In RKWard, parts typically do not receive a de-activate event before destruction, turning the memory leak into a visible icon leak (see this screenshot: http://reaktanz.de/stuff/R/RKWard_icon_soup.png ).
Fixing this in the plugin is simple: Only SE::removeStatusBarItem() would need to be guarded. But the problem is bound to re-surface in the next plugin. Further, I think the assumption that the widget will become owned by the SE is very reasonable. Yet, simply changing that could cause double deletion in plugins which do not make this (currently false) assumption.
Therefore, the only clean way towards fixing this in the mid term, that I could think of is:
1. Keep the current behavior by default, but:
2. Add a parameter "autodelete" to SE::addStatusBarItem(), and deprecate the old version. With autodelete set to true (encouraged by the documentation), icons will be deleted with the SE.
That's what the patch does. Note that this does not actually fix anything, yet, but simply causes deprecation warnings in code that may need appropriate fixing.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>kparts/statusbarextension.h <span style="color: grey">(69691d7f80c55a6193c345f7f514d35af2b20b15)</span></li>
<li>kparts/statusbarextension.cpp <span style="color: grey">(11fdf40681c708bc0f031797083c7f6b6b638f60)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/101653/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>