mirror of
https://github.com/kiwix/kiwix-desktop.git
synced 2025-09-21 02:51:26 -04:00
2-stage pausing/resuming of downloads
Pausing a download now changes its state to PAUSE_REQUESTED and the pause request is enqueued for asynchronous execution. Similarly, resuming a download changes its state to RESUME_REQUESTED and the resume request is enqueued for asynchronous execution. In the PAUSE_REQUESTED and RESUME_REQUESTED states download actions are disabled. Known issues: - The PAUSE_REQUESTED state may be incorrectly restored to DOWNLOADING if at the time of pausing an earlier UPDATE request for the same download preceded it in the queue or was being executed. After the response to the said UPDATE request is received it results in the DownloadState being reset to the previous state. But after the pause request is processed a subsequent UPDATE request will change the state to PAUSED. In GUI this looks as follows (assuming slow responses from aria2c, allowing to observe the events in slow motion): 1. Pause button is pressed 2. Pause button and download progress info (the textual one) disappear 3. Pause button and download progress info (the textual one) re-appear 4. Download switches to paused state without any further user actions A similar problem exists for resuming the download. The solution is to convert the request queue to a priority queue where download actions are given precedence over update actions. However this will not eliminate the problem completely since a pause/resume action may be issued while an update request is being processed by aria2c (the likelyhood of which greatly increases if aria is mostly stuck struggling with slow storage). The latter case will be addressed by tracking the timestamps of the requests and ignoring the download status from those update requests that were issued before the user action requests.
This commit is contained in:
parent
ce2fbbf7fc
commit
0235d0bafa
@ -144,14 +144,18 @@ void ContentManager::onCustomContextMenu(const QPoint &point)
|
||||
const auto bookState = getBookState(id);
|
||||
switch ( bookState ) {
|
||||
case BookState::DOWNLOAD_PAUSED:
|
||||
if ( getDownloadState(id)->getStatus() == DownloadState::PAUSED ) {
|
||||
contextMenu.addAction(&menuResumeBook);
|
||||
contextMenu.addAction(&menuCancelBook);
|
||||
}
|
||||
contextMenu.addAction(&menuPreviewBook);
|
||||
break;
|
||||
|
||||
case BookState::DOWNLOADING:
|
||||
if ( getDownloadState(id)->getStatus() == DownloadState::DOWNLOADING ) {
|
||||
contextMenu.addAction(&menuPauseBook);
|
||||
contextMenu.addAction(&menuCancelBook);
|
||||
}
|
||||
contextMenu.addAction(&menuPreviewBook);
|
||||
break;
|
||||
|
||||
@ -622,13 +626,13 @@ void ContentManager::eraseBook(const QString& id)
|
||||
|
||||
void ContentManager::pauseBook(const QString& id, QModelIndex index)
|
||||
{
|
||||
DownloadManager::addRequest(DownloadManager::PAUSE, id);
|
||||
DownloadManager::addRequest(DownloadState::PAUSE, id);
|
||||
managerModel->triggerDataUpdateAt(index);
|
||||
}
|
||||
|
||||
void ContentManager::resumeBook(const QString& id, QModelIndex index)
|
||||
{
|
||||
DownloadManager::addRequest(DownloadManager::RESUME, id);
|
||||
DownloadManager::addRequest(DownloadState::RESUME, id);
|
||||
managerModel->triggerDataUpdateAt(index);
|
||||
}
|
||||
|
||||
|
@ -134,7 +134,7 @@ void showDownloadProgress(QPainter *painter, QRect box, const DownloadState& dow
|
||||
if (downloadInfo.getStatus() == DownloadState::PAUSED) {
|
||||
createResumeSymbol(painter, dcl.pauseResumeButtonRect);
|
||||
createCancelButton(painter, dcl.cancelButtonRect);
|
||||
} else {
|
||||
} else if (downloadInfo.getStatus() == DownloadState::DOWNLOADING) {
|
||||
createPauseSymbol(painter, dcl.pauseResumeButtonRect);
|
||||
createDownloadStats(painter, box, downloadSpeed, completedLength);
|
||||
}
|
||||
@ -239,6 +239,7 @@ void ContentManagerDelegate::handleLastColumnClicked(const QModelIndex& index, Q
|
||||
{
|
||||
const auto node = static_cast<RowNode*>(index.internalPointer());
|
||||
const auto id = node->getBookId();
|
||||
const auto downloadState = node->getDownloadState();
|
||||
|
||||
const int clickX = portutils::getX(*mouseEvent);
|
||||
const int clickY = portutils::getY(*mouseEvent);
|
||||
@ -254,17 +255,21 @@ void ContentManagerDelegate::handleLastColumnClicked(const QModelIndex& index, Q
|
||||
return contentMgr.downloadBook(id);
|
||||
|
||||
case ContentManager::BookState::DOWNLOADING:
|
||||
if ( downloadState->getStatus() == DownloadState::DOWNLOADING ) {
|
||||
if ( dcl.pauseResumeButtonRect.contains(clickPoint) ) {
|
||||
contentMgr.pauseBook(id, index);
|
||||
}
|
||||
}
|
||||
return;
|
||||
|
||||
case ContentManager::BookState::DOWNLOAD_PAUSED:
|
||||
if ( downloadState->getStatus() == DownloadState::PAUSED ) {
|
||||
if ( dcl.cancelButtonRect.contains(clickPoint) ) {
|
||||
contentMgr.cancelBook(id);
|
||||
} else if ( dcl.pauseResumeButtonRect.contains(clickPoint) ) {
|
||||
contentMgr.resumeBook(id, index);
|
||||
}
|
||||
}
|
||||
return;
|
||||
|
||||
default:
|
||||
|
@ -62,6 +62,24 @@ QString DownloadState::getDownloadSpeed() const
|
||||
return timeSinceLastUpdate() > 2.0 ? "---" : downloadSpeed;
|
||||
}
|
||||
|
||||
void DownloadState::changeState(Action action)
|
||||
{
|
||||
const auto oldStatus = status;
|
||||
if ( action == PAUSE ) {
|
||||
if ( status == DOWNLOADING ) {
|
||||
status = PAUSE_REQUESTED;
|
||||
}
|
||||
} else if ( action == RESUME ) {
|
||||
if ( status == PAUSED ) {
|
||||
status = RESUME_REQUESTED;
|
||||
}
|
||||
}
|
||||
|
||||
if ( status != oldStatus ) {
|
||||
lastUpdated = std::chrono::steady_clock::now();
|
||||
}
|
||||
}
|
||||
|
||||
////////////////////////////////////////////////////////////////////////////////
|
||||
// DowloadManager
|
||||
////////////////////////////////////////////////////////////////////////////////
|
||||
@ -82,7 +100,7 @@ DownloadManager::~DownloadManager()
|
||||
|
||||
// At this point the thread may be stuck waiting for data.
|
||||
// Let's wake it up.
|
||||
m_requestQueue.enqueue({UPDATE, ""});
|
||||
m_requestQueue.enqueue({DownloadState::UPDATE, ""});
|
||||
t->wait();
|
||||
}
|
||||
}
|
||||
@ -98,11 +116,11 @@ void DownloadManager::processDownloadActions()
|
||||
const Request req = m_requestQueue.dequeue();
|
||||
if ( !req.bookId.isEmpty() ) {
|
||||
switch ( req.action ) {
|
||||
case START: /* startDownload(req.bookId); */ break; // API problem
|
||||
case PAUSE: pauseDownload(req.bookId); break;
|
||||
case RESUME: resumeDownload(req.bookId); break;
|
||||
case CANCEL: /* cancelDownload(req.bookId); */ break; // API problem
|
||||
case UPDATE: updateDownload(req.bookId); break;
|
||||
case DownloadState::START: /* startDownload(req.bookId); */ break; // API problem
|
||||
case DownloadState::PAUSE: pauseDownload(req.bookId); break;
|
||||
case DownloadState::RESUME: resumeDownload(req.bookId); break;
|
||||
case DownloadState::CANCEL: /* cancelDownload(req.bookId); */ break; // API problem
|
||||
case DownloadState::UPDATE: updateDownload(req.bookId); break;
|
||||
}
|
||||
}
|
||||
}
|
||||
@ -123,7 +141,7 @@ void DownloadManager::startDownloadUpdaterThread()
|
||||
connect(timer, &QTimer::timeout, [this]() {
|
||||
if ( m_requestQueue.isEmpty() ) {
|
||||
for ( const auto& bookId : m_downloads.keys() ) {
|
||||
addRequest(UPDATE, bookId);
|
||||
addRequest(DownloadState::UPDATE, bookId);
|
||||
}
|
||||
}
|
||||
});
|
||||
@ -248,7 +266,12 @@ std::string DownloadManager::startDownload(const kiwix::Book& book, const QStrin
|
||||
|
||||
void DownloadManager::addRequest(Action action, QString bookId)
|
||||
{
|
||||
m_requestQueue.enqueue({action, bookId});
|
||||
if ( const auto downloadState = getDownloadState(bookId) ) {
|
||||
m_requestQueue.enqueue({action, bookId});
|
||||
if ( action != DownloadState::UPDATE ) {
|
||||
downloadState->changeState(action);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
void DownloadManager::pauseDownload(const QString& bookId)
|
||||
|
@ -54,12 +54,22 @@ private: // data
|
||||
class DownloadState
|
||||
{
|
||||
public: // types
|
||||
enum Action {
|
||||
START,
|
||||
PAUSE,
|
||||
RESUME,
|
||||
CANCEL,
|
||||
UPDATE
|
||||
};
|
||||
|
||||
enum Status {
|
||||
UNKNOWN,
|
||||
WAITING,
|
||||
DOWNLOAD_ERROR,
|
||||
DOWNLOADING,
|
||||
PAUSED
|
||||
PAUSE_REQUESTED,
|
||||
PAUSED,
|
||||
RESUME_REQUESTED
|
||||
};
|
||||
|
||||
public: // data
|
||||
@ -71,6 +81,7 @@ public: // functions
|
||||
void update(const DownloadInfo& info);
|
||||
QString getDownloadSpeed() const;
|
||||
Status getStatus() const { return status; }
|
||||
void changeState(Action action);
|
||||
|
||||
// time in seconds since last update
|
||||
double timeSinceLastUpdate() const;
|
||||
@ -87,15 +98,7 @@ class DownloadManager : public QObject
|
||||
|
||||
public: // types
|
||||
typedef std::shared_ptr<DownloadState> DownloadStatePtr;
|
||||
|
||||
enum Action
|
||||
{
|
||||
START,
|
||||
PAUSE,
|
||||
RESUME,
|
||||
CANCEL,
|
||||
UPDATE
|
||||
};
|
||||
typedef DownloadState::Action Action;
|
||||
|
||||
private:
|
||||
// BookId -> DownloadState map
|
||||
|
Loading…
x
Reference in New Issue
Block a user