D7671: Fix automatic reload of files saved with QSaveFile

[Albert Astals Cid]

aacid accepted this revision.
aacid added a comment.
This revision is now accepted and ready to land.


  In https://phabricator.kde.org/D7671#143401, @progwolff wrote:
  
  > In https://phabricator.kde.org/D7671#143269, @aacid wrote:
  >
  > > Have you read my email? There clearly says what happens and what the documentation says it should happen (at least to my understanding of reading it).
  >
  >
  > Sure, I read your mail. But I still don't think that KDirWatch behaves wrong.
  
  
  Why?
  
  > On saving a file with QSaveFile or on a plain `mv somefile watchedfile` the watched file is never removed.
  
  According to KDirwatch, it is
  
  > As the docs say, the dirty signal of a directory is emitted when a file in this directory is removed or created. This is not the case here.
  
  According to KDirwatch, it is
  
  > KDirWatch yet does send a "created" signal on inotify's MOVE_TO flag and a "removed" signal on inotify's MOVE_FROM flag. This does not mean, the file actually has been removed or added at any time.
  
  The file may "in reality never be creater or removed", but KDirWatch is sending the removed and created signals, so if the documentation says "dirty is triggered when the file is removed and created", well it should send the signal, otherwise it's pretty confusing, and i don't know why we're discussing this here instead of on the mailing list where i actually asked for clarification.
  
  > So, this behaviour is a little strange and confusing, but from my perspective it's still coherent with the documentation.
  
  I disagree.
  
  > Even if KDirWatch would work as you expect it to, there are cases where Okular still does not react to those signals:
  >  Consider the command `rm watchedfile && touch watchedfile`.
  >  KDirWatch will send the signals: file removed, directory dirty, file added, directory dirty.
  >  Okular receives the first dirty signal asynchronously. Okular checks if the file exists via `QFile::exists`. It is likely that the file has already been added (`touch`) by that time, so Okular will miss to reload the file.
  >  Actually, this case seems to work with your code, but I hope you understand what I want to say. It just doesn't cover every potential case. Same problems apply for `mv`.
  
  This is actually the only good explanation of why the code is not robust enough, you're right there's races involved in the logic. So yeah let's commit this fix then.

REPOSITORY
  R223 Okular

BRANCH
  master

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

To: progwolff, aacid
Cc: sander, rkflx, #okular, aacid
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/okular-devel/attachments/20170906/2a6d16b7/attachment-0001.html>