Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(114)

Unified Diff: chrome/browser/android/offline_pages/background_loader_offliner.cc

Issue 2744553005: [Offline Pages] Turn Offliner::Cancel into an async operation to resolve conflicting assumptions be… (Closed)
Patch Set: Created 3 years, 9 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/browser/android/offline_pages/background_loader_offliner.cc
diff --git a/chrome/browser/android/offline_pages/background_loader_offliner.cc b/chrome/browser/android/offline_pages/background_loader_offliner.cc
index 7fb0f7bfe0c82a9c5a67c9d51ec14a3b4576b97e..540c195dc7091849b869472fbdb17eb3ce87bfd3 100644
--- a/chrome/browser/android/offline_pages/background_loader_offliner.cc
+++ b/chrome/browser/android/offline_pages/background_loader_offliner.cc
@@ -121,18 +121,24 @@ bool BackgroundLoaderOffliner::LoadAndSave(
return true;
}
-void BackgroundLoaderOffliner::Cancel() {
+void BackgroundLoaderOffliner::Cancel(const CancelCallback& callback) {
// TODO(chili): We are not able to cancel a pending
- // OfflinePageModel::SavePage() operation. We just ignore the callback.
- if (!pending_request_)
+ // OfflinePageModel::SavePage() operation. We will notify caller that
+ // cancel completed once the SavePage operation returns.
+ if (!pending_request_) {
+ callback.Run(0LL);
return;
+ }
if (save_state_ != NONE) {
save_state_ = DELETE_AFTER_SAVE;
+ cancel_callback_ = callback;
return;
}
+ int64_t request_id = pending_request_->request_id();
ResetState();
+ callback.Run(request_id);
}
bool BackgroundLoaderOffliner::HandleTimeout(const SavePageRequest& request) {
@@ -293,6 +299,7 @@ void BackgroundLoaderOffliner::OnPageSaved(SavePageResult save_result,
if (save_state_ == DELETE_AFTER_SAVE) {
save_state_ = NONE;
+ cancel_callback_.Run(request.request_id());
return;
}
@@ -327,9 +334,21 @@ void BackgroundLoaderOffliner::OnApplicationStateChange(
base::android::APPLICATION_STATE_HAS_RUNNING_ACTIVITIES) {
DVLOG(1) << "App became active, canceling current offlining request";
SavePageRequest* request = pending_request_.get();
- Cancel();
- completion_callback_.Run(*request, RequestStatus::FOREGROUND_CANCELED);
+ // This works because Bind will make a copy of request, and we
+ // should not have to worry about reset being called before cancel callback.
+ Cancel(base::Bind(
+ &BackgroundLoaderOffliner::HandleApplicationStateChangeCancel,
+ weak_ptr_factory_.GetWeakPtr(), *request));
}
}
+void BackgroundLoaderOffliner::HandleApplicationStateChangeCancel(
+ const SavePageRequest& request,
+ int64_t offline_id) {
+ // If for some reason the request was reset during while waiting for callback
+ // ignore the completion callback.
+ if (pending_request_ && pending_request_->request_id() != offline_id)
+ return;
+ completion_callback_.Run(request, RequestStatus::FOREGROUND_CANCELED);
+}
} // namespace offline_pages

Powered by Google App Engine
This is Rietveld 408576698