Downloads that were active during the application shutdown automatically
resume.
Known issues:
- For paused downloads the progress information is lost (but is recovered
when the download is resumed).
Now the downloader paused/active state in UI is set by the updater
thread. This will make it easier to restore downloads after closing
and re-opening kiwix-desktop.
New ZIM files discovered in monitored directories are not added to the
library if they are known to be in the process of being downloaded by
kiwix-desktop. That information is recorded in the library as follows.
When a download is started the book is added to the local library with a
fake path that has a special suffix (".beingdownloadedbykiwix") appended
to the path of the ZIM file (previously, a book for which a download
was initiated had only its download id set and the path was added only
upon completion of the download).
Note that the expected path of the ZIM file is derived from the download
directory and the URL rather than obtained via
`kiwix::Download::getPath()` since the latter seems to return an empty
string until the download is actually started (it may stay queued if the
count of active downloads exceeds the queue size), the file is created
and `kiwix::Download::updateStatus()` is called after that which is too
late for our purposes (we want the information about a new download
to appear in the library before directory monitoring detects creation
of the file).
Known issues:
- If kiwix-desktop is closed while a download is still in progress
(either active or paused) then after reopening the app the information
about the download state is not shown in the UI and attempting to
download that file fails. However this problem is present before this
commit too (though in a slightly different way).
Before this change `ContentManager::reallyCancelBook()` could throw (and
thus result in a crash) if the confirmation to cancel the download was
timed to perform the actual download cancellation on a completed
download. This change fixes that problem. Besides it also fixes a subtle
issue where `ContentManager::reallyCancelBook()` is called on a
completed download and has the effect of removing the downloaded book
from the local library. I considered preserving that dubious behaviour,
but it couldn't be achieved in a simple and consistent fashion (because
of the two ways an actually completed download can be seen by the
`ContentManager::reallyCancelBook()` function).
My experience is that `kiwix::Download::getPath()` may return an empty
string (at least for a fresh download object and probably before Aria2
actually starts downloading and creates the target file). Passing such
an empty string into `eraseBookFilesFromComputer()` might have
disastrous consequences. Now that function should be safer.
Note that this change drops the protection against accidentally removing all
files in a book's directory. The risk of a such a destructive operation
is still present if an invalid path is passed into
`eraseBookFilesFromComputer()` but that will be addressed in a separate
commit.
Eliminated an instance of download removal handling where the conceptual
actions of ContentManager::removeDownload() were performed without
calling the latter.
It looks like the previous version of `ContentManagerModel::rowCount()`
was implemented before the expandable description of books was introduced.
Likely, it kept working without leading to observable bugs only due to some
foolproofness in Qt.
ContentManagerModel is explicitly given access to the list of active downloads
during calls to ContentManagerModel::setBooksData(). This limits by
design the time when the list of downloads can be accessed and
facilitates making it thread-safe.
Now book thumbnails are loaded lazily, triggered by the first attempt to
access them.
`ContentManagerModel::refreshIcons()` seems to exist for that reason
(lazy loading of thumbnails) as it was called from
`ContentManagerModel::fetchMore()` however the latter seems to load data
more eagerly than the new implementation.
This enables the following changes (to be done next):
1. move the thumbnail download initiation into the data access operation.
2. get rid of ContentManagerModel::refreshIcons()
3. get rid of ContentManagerModel::m_data
Now the 0'th field of ContentManagerModel's row representation (which is
used for the book thumbnail) can hold either the favicon image data or its
url (before it held only the image data).
Merged "faviconUrl" entry of `ContentManager::BookInfo` into "favicon".
Now the latter represents either the illustration data (and then it is
of QByteArray type) or the url (in which case it is a QString).
Before this change, thumbnails of local books were obtained by
`ContentManagerModel` (via ugly calls to
`KiwixApp::instance()->getLibrary()`). Now that task is
transferred to `ContentManager::updateModel()` - the thumbnail data, if
available, is loaded by `ContentManager::getBookInfos()` and passed to
`ContentManagerModel::setBooksData()` through a new entry `favicon`.
`ContentManager::getBooksList()` was replaced by
`ContentManager::updateModel()` (which combines the now removed
`ContentManager::getBooksList()` and `ContentManagerModel::setBooksData()`
into a single operation).
Notes:
- The call to `ContentManagerModel::refreshIcons()` could not be
included in `ContentManager::updateModel()`. A few attempts to do
so failed for various reasons, but the outcome of those efforts is
that a better design for thumbnail loading was identified which
enables dropping the `ContentManagerModel::refreshIcons()` operation.
That redesign is pursued in the next few commits.
Renamed `ContentManager::downloadCancelled()` to `downloadDisappeared()`
since the situation handled by that function is unrelated to cancelling
a download (in which case it should still be available with a status of
"removed").
Monitoring of the download dir interferes with the download lifecycle
management. Here are some of the observed effects:
- As soon as the ZIM file is created in the download directory it
is added to the library and overwrites the entry created by download
management logic, thus resetting the download id associated with the book.
- As a result `ContentManager::downloadCompleted()` is never called;
`ContentManager::downloadCancelled()` is called instead (however, for
some reason the user-observable effect is as if the download completion
is normally detected and handled). Looks like this was the main
culprit behind the false bug report kiwix/libkiwix#1049.
- The icon of the book added a time when the file was not yet a valid
ZIM file stays invalid.
I am temporarily disabling this functionality so that the work on
download related code is carried out in a clean room environment.
Once it's complete monitoring of the download directory must be
re-enabled but in a way that doesn't confuse the logic of the
download state machine.
Notes:
- The `getAction()` helper function was introduced in order to keep the
line length within the 80 chars limit and was applied to other usages
of `KiwixApp::getAction()` too.
- The tooltip includes the keyboard shortcut which closes only the
current tab but the tooltip is the same for all the tabs.
