D28337: Fix unmounting during preview generation

[Méven Car]

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


  >   Solid only sends broadcast notification and doesn't wait for any reply, thus unmounting may fail due to preview jobs not stopping in time.
  
  That points to a solid shortcoming worth fixing. Adding a wait for apps to ask solid to wait or to wait by default for instance 20-50ms between `tearDownRequested` and `teardownDone`

INLINE COMMENTS

> kfileitemmodelrolesupdater.cpp:496
> +        m_state = Paused;
> +        killPreviewJob();
> +        emit previewJobFinished();

The job is finished then, do we really need to kill it ?

> kfileitemmodelrolesupdater.cpp:581
> +        m_state = Paused;
> +        killPreviewJob();
> +        emit previewJobFinished();

In the immediate case, the job should be already killed.
In the not immediate case, this change is not necessary.

> kfileitemmodelrolesupdater.h:131
>       */
> -    void setPaused(bool paused);
> +    void setPaused(bool paused, bool immediate = true);
>      bool isPaused() const;

I would favor a new method `stop()` equivalent to the immediate case here.

> kfileitemmodelrolesupdater.h:288
> +        PreviewJobRunning,
> +        PausePending
>      };

It seems like a "StopPending" or "KillPending" actualy.
I wonder if that is necessary to have a new State.

REPOSITORY
  R318 Dolphin

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

To: alnikiforov, elvisangelaccio, ngraham, meven
Cc: kfm-devel, azyx, nikolaik, pberestov, iasensio, fprice, fbampaloukas, alexde, Codezela, feverfew, meven, spoorun, navarromorales, firef, ngraham, andrebarros, emmanuelp, rdieter, mikesomov
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20200415/e473e9b4/attachment.htm>