Review Request: Fix for a couple of KIO put-slave-on-hold bugs

[Dawit Alemayehu]

> On April 29, 2011, 10:51 p.m., David Faure wrote:
> > kio/kio/scheduler.cpp, line 1146
> > <http://git.reviewboard.kde.org/r/101244/diff/3/?file=15605#file15605line1146>
> >
> >     Any risk pq would be null here?

Possibly, but it is inconsistently applied through out the code. In some places it is checked for null, in others it is not. But it does not hurt if there is a check so I will put one there.


> On April 29, 2011, 10:51 p.m., David Faure wrote:
> > kio/kio/scheduler.cpp, line 1165
> > <http://git.reviewboard.kde.org/r/101244/diff/3/?file=15605#file15605line1165>
> >
> >     This if(), on the hand, surprises me. Surely q can't ever be null? I see lots of other methods in the Private class don't test for q being null.

Indeed. Removed.


- Dawit


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/101244/#review2973
-----------------------------------------------------------


On April 29, 2011, 9:20 p.m., Dawit Alemayehu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/101244/
> -----------------------------------------------------------
> 
> (Updated April 29, 2011, 9:20 p.m.)
> 
> 
> Review request for kdelibs.
> 
> 
> Summary
> -------
> 
> 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.
> 
> 
> Diffs
> -----
> 
>   kio/kio/job.cpp e34f562 
>   kio/kio/scheduler.h 9be9db0 
>   kio/kio/scheduler.cpp 4cb33d1 
> 
> Diff: http://git.reviewboard.kde.org/r/101244/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Dawit
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20110430/1c7ce37d/attachment.htm>