<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/101244/">http://git.reviewboard.kde.org/r/101244/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On April 28th, 2011, 8:42 a.m., <b>David Faure</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://git.reviewboard.kde.org/r/101244/diff/1/?file=15536#file15536line925" style="color: black; font-weight: bold; text-decoration: underline;">kio/kio/scheduler.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void SchedulerPrivate::slotSlaveOnHoldListChanged()</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">925</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">m_ignoreOnHoldListChanged</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;">Glad to see my idea worked ;)
Just a question, does it really matter that the emitter of the signal would set m_checkOnHold to true (if you remove m_ignoreOnHoldListChanged)? After all there is a slave on hold now, and maybe in some cases we end up opening this url in this process after all?
I'm not sure the m_ignoreOnHoldListChanged bool is necessary.</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, your idea definitely works and is much easier to implement. :) Regarding the flag, I added it simply because I was copying what the other dbus signal, reparseSlaveConfiguration, was doing without really thinking about it. Now that you have raised that point, though I do not see how the current process would end up reusing the ioslave that it caused to be put on hold, it does not really hurt to let its check on hold flag get set to true in this case either ; so I will change that.</pre>
<br />
<p>- Dawit</p>
<br />
<p>On April 27th, 2011, 10:57 p.m., Dawit Alemayehu wrote:</p>
<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 Dawit Alemayehu.</div>
<p style="color: grey;"><i>Updated April 27, 2011, 10:57 p.m.</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 attached patch address a couple of bugs in the KIO put-slave-on-hold functionality:
Problem:
======
#1. If a user clicks a link on a page, e.g. ftp://ftp.kde.org/pub/kde/README_UPLOAD, then KRun will first issue a get (CMD_GET) to determine its content-type and put the ioslave on hold so that the application associated with the returned content-type can handle it. In case of our example that would be kate/kwrite. Unfortunately, the ioslave put on hold will not be reused because almost all applications will use KIO::file_copy (CMD_COPY) to make a local copy before opening it. Even then for ioslaves that do not optimize their copy command, the call to KIO::file_copy will properly reuse the ioslave on hold so long as it do not have an optimized copying method like the ftp ioslave.
#2. If a user types a web address, e.g. www.kde.org, into KRunner to open it in Konqueror and repeats the same process with another address, then whether the ioslave put on hold the second time around will get reused or not depends how the application works. If the application allows multiple documents or tabs and opens the second url as in the already running instance, then the ioslave on hold will NOT be reused the second time around.
Solution:
======
#1. Simply force the KIO::file_copy call not to do the optimized copying if there is a slave on hold for the request. This will force it to use two separate jobs, KIO::get (CMD_GET) and KIO::put (CMD_PUT) ensuring the reuse of the ioslave that was put on hold.
#2. Whenever a call to KIO::Scheduler::publishSlaveOnHold is made, send a dbus message to update all the other scheduler so that they can look for an ioslave-on-hold to service the next request.</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>kio/kio/scheduler.h <span style="color: grey">(9be9db0)</span></li>
<li>kio/kio/scheduler.cpp <span style="color: grey">(4cb33d1)</span></li>
<li>kio/kio/job.cpp <span style="color: grey">(e34f562)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/101244/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>