<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/126385/">https://git.reviewboard.kde.org/r/126385/</a>
</td>
</tr>
</table>
<br />
<p>Ship it!</p>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I agree -- except about the "aside" in the commit log:</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">"since all signal connections to 'this' are removed on destruction anyway". They are indeed, but <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">after</em> destroyed is emitted. So disconnecting does make a difference.</p></pre>
<br />
<p>- David Faure</p>
<br />
<p>On December 16th, 2015, 1:33 p.m. UTC, Jonathan Marten wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
<div>Review request for KDE Frameworks.</div>
<div>By Jonathan Marten.</div>
<p style="color: grey;"><i>Updated Dec. 16, 2015, 1:33 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="https://bugs.kde.org/show_bug.cgi?id=355711">355711</a>
</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kparts
</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This appears to be the cause of a crash when exiting System Settings. More information in the bug report, but basically what happens is that the manager keeps track of widgets that it is managing in d->m_managedTopLevelWidgets. If a widget is a top level widget when it is added, but is no longer top level when it is destroyed, it is not removed from the list which results in an access-to-deleted-object in the destructor.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This change unconditionally removes the widget even if it is no longer top level. Removing the widget from the list has no ill effects, the list is only actually used in Partmanager::eventFilter() which will never get an event for a deleted widget anyway.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Aside: The problematic 'foreach (const QWidget *w, d->m_managedTopLevelWidgets)' loop in PartManager::PartManager() is really superfluous, since all signal connections to 'this' are removed on destruction anyway.</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Built KParts with this change, and also systemsettings5 with the associated change (see associated review). Observed no crash when exiting the application. Also checked correct operation of Konqueror and Kate.</p></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>src/partmanager.cpp <span style="color: grey">(81bf73f)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/126385/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>