D27148: update d->m_file in ReadOnlyPart::setUrl()

[David Faure]

dfaure requested changes to this revision.
dfaure added a comment.
This revision now requires changes to proceed.


  statJob->exec() creates a nested event loop, which processes timers and socket events etc. -- this can create unexpected re-entrancy and is a nasty source of bugs.
  Therefore it should be avoided as much as possible, *especially* in libraries which don't have the full picture about what happens in the application.
  
  The only safe way to do this is with an async job, i.e. signals and slots. This is tricky in general (when the caller expects things to happen immediately), but by luck this isn't the case here. Apps expect KParts to download remote files. So what could be done, is a statjob before the download, and if stat says there's a local path KParts can *then* set m_file instead of downloading.
  
  But wait.... KParts::ReadOnlyPart::openUrl already does *exactly* that.
  See the code under
  
    // Maybe we can use a "local path", to avoid a temp copy?
  
  which uses KIO::mostLocalUrl() the right way, async.
  
  So I have to ask... what is this fixing, exactly? Why isn't openUrl called, or why doesn't it go into that code path, in your use case?

REPOSITORY
  R306 KParts

REVISION DETAIL
  https://phabricator.kde.org/D27148

To: pdabrowski, elvisangelaccio, ngraham, #frameworks, dfaure
Cc: marten, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20200204/dae985a4/attachment-0001.html>