<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://svn.reviewboard.kde.org/r/6271/">http://svn.reviewboard.kde.org/r/6271/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On January 4th, 2011, 1:44 p.m., <b>Andreas Hartmetz</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://svn.reviewboard.kde.org/r/6271/diff/3/?file=43485#file43485line924" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/KDE/kdelibs/kio/kio/scheduler.cpp</a>
<span style="font-weight: normal;">
(Diff revision 3)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void SchedulerPrivate::slotReparseSlaveConfiguration(const QString &proto, const QDBusMessage& msg)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">924</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span> <span class="p">(</span><span class="n">cmd</span> <span class="o">==</span> <span class="n">CMD_GET</span> <span class="o">||</span> <span class="p">(</span><span class="n">cmd</span> <span class="o">==</span> <span class="n">CMD_SPECIAL</span> <span class="o">&&</span> <span class="n">canReturnContentOnSpecialCmd</span><span class="p">(</span><span class="n">jobPriv</span><span class="o">-></span><span class="n">m_protocol</span><span class="p">)))</span> <span class="p">{</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Better move everything into a mayReturnContent(KIO::Job *) method and avoid the long function name.</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Indeed... that is much cleaner and descriptive except for the parameter. Need to pass the job comand and the protocol to avoid repeating the protocol lookup in the new function. Fixed.</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On January 4th, 2011, 1:44 p.m., <b>Andreas Hartmetz</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://svn.reviewboard.kde.org/r/6271/diff/3/?file=43485#file43485line1130" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/KDE/kdelibs/kio/kio/scheduler.cpp</a>
<span style="font-weight: normal;">
(Diff revision 3)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void SchedulerPrivate::publishSlaveOnHold()</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">1130</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">m_checkOnHold</span> <span class="o">=</span> <span class="kc">true</span><span class="p">;</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">This is the core of the patch, right...
I don't know if the slave-on-hold mechanism was meant to be used between KParts in the same app or only between applications. In the latter case more changes in KIO will probably be necessary. In the former case, I screwed up something around this line in the scheduler rewrite.</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Well the original intent of the slave-on-hold mechansim was to make an ioslave started in one application available in another. Its creation was mostly intended to handle the scenario where a job in App A needs to be passed to App B because App A cannot handle it. As such the mechanism used to handle both the same app with different KParts and two separate apps is actually the same. The only difference is that for the latter case you would need to call publishSlaveOnHold, which returns the slave-on-hold to klauncher, so that the other application can find and reuse it. Obviously, publishing the presence of a slave-on-hold is not necessary for the same app.
Anyhow, you did not screw up anything that concerns this mechanism during the rewrite. However, my original analysis and patch were wrong. I have amended both the analysis as well as the patch...</pre>
<br />
<p>- Dawit</p>
<br />
<p>On January 4th, 2011, 9:55 a.m., Dawit Alemayehu wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://svn.reviewboard.kde.orgrb/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 Dawit Alemayehu.</div>
<p style="color: grey;"><i>Updated 2011-01-04 09:55:29</i></p>
<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;">The patch attempts to fix the following two oustanding problems in KIO::Scheduler that have been around for a while (see the possibly related bug reports in the Bugs section above):
#1. Set the m_checkOnHold flag to true every time Scheduler::publishSlaveOnHold is invoked. Right now that flag is only set to true when an instance of KIO::Scheduler is created. This results in the flag never being true after the first ioslave has been put on hold and reused unless the programmer explicitly calls KIO::Scheduler::checkSlaveOnHold which is not documented at all. See the description about putting ioslaves on hold in KIO::get's API documentation.
#2. Modify SchedulerPrivate::doJob to correctly set the m_checkOnHold flag for http requests when a job's command is CMD_SPECIAL. That is necessary because HTTP_POST, which is handled as a special command, can return content just like a get request.
</pre>
</td>
</tr>
</table>
<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=123121">123121</a>,
<a href="https://bugs.kde.org/show_bug.cgi?id=148307">148307</a>
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>/trunk/KDE/kdelibs/kio/kio/scheduler.cpp <span style="color: grey">(1211439)</span></li>
</ul>
<p><a href="http://svn.reviewboard.kde.org/r/6271/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>