[Offline Pages] Turn Offliner::Cancel into an async operation to resolve conflicting assumptions be…

chrome/browser/android/offline_pages/background_loader_offliner.cc

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/android/offline_pages/background_loader_offliner.h"
 
 #include "base/metrics/histogram_macros.h"
 #include "base/sys_info.h"
 #include "chrome/browser/android/offline_pages/offline_page_mhtml_archiver.h"
 #include "chrome/browser/android/offline_pages/offliner_helper.h"
@@ skipping 103 matching lines @@
   app_listener_.reset(new base::android::ApplicationStatusListener(
       base::Bind(&BackgroundLoaderOffliner::OnApplicationStateChange,
                  weak_ptr_factory_.GetWeakPtr())));
 
   // Load page attempt.
   loader_.get()->LoadPage(request.url());
 
   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) {
   // TODO(romax) Decide if we want to also take a snapshot on the last timeout
   // for the background loader offliner.
   return false;
 }
 
 void BackgroundLoaderOffliner::DidStopLoading() {
   if (!pending_request_.get()) {
@@ skipping 140 matching lines @@
 void BackgroundLoaderOffliner::OnPageSaved(SavePageResult save_result,
                                            int64_t offline_id) {
   if (!pending_request_)
     return;
 
   SavePageRequest request(*pending_request_.get());
   ResetState();
 
   if (save_state_ == DELETE_AFTER_SAVE) {
     save_state_ = NONE;
+    cancel_callback_.Run(request.request_id());
     return;
   }
 
   save_state_ = NONE;
 
   Offliner::RequestStatus save_status;
   if (save_result == SavePageResult::SUCCESS)
     save_status = RequestStatus::SAVED;
   else
     save_status = RequestStatus::SAVE_FAILED;
@@ skipping 14 matching lines @@
   content::WebContentsObserver::Observe(loader_.get()->web_contents());
 }
 
 void BackgroundLoaderOffliner::OnApplicationStateChange(
     base::android::ApplicationState application_state) {
   if (pending_request_ && is_low_end_device_ &&
       application_state ==
           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