Failed offline snapshots won't erase a successful one from the same page load.

chrome/browser/android/offline_pages/recent_tab_helper.cc

 // Copyright (c) 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/recent_tab_helper.h"
 
 #include <queue>
 #include <vector>
 
 #include "base/bind.h"
@@ skipping 41 matching lines @@
 };
 }  // namespace
 
 namespace offline_pages {
 
 RecentTabHelper::RecentTabHelper(content::WebContents* web_contents)
     : content::WebContentsObserver(web_contents),
       page_model_(nullptr),
       snapshots_enabled_(false),
       is_page_ready_for_snapshot_(false),
+      previous_request_id_for_load_(OfflinePageModel::kInvalidOfflineId),
       delegate_(new DefaultDelegate()),
       weak_ptr_factory_(this) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 }
 
 RecentTabHelper::~RecentTabHelper() {
 }
 
 void RecentTabHelper::SetDelegate(
     std::unique_ptr<RecentTabHelper::Delegate> delegate) {
@@ skipping 73 matching lines @@
   if (!snapshots_enabled_)
     return;
 
   // We navigated to a different page, lets report progress to Background
   // Offliner.
   if (download_info_ && !navigation_handle->IsSamePage()) {
     ReportDownloadStatusToRequestCoordinator();
   }
 
   if (offline_pages::IsOffliningRecentPagesEnabled()) {
-    int64_t proposed_id = OfflinePageModel::kInvalidOfflineId;
     download_info_ = base::MakeUnique<DownloadPageInfo>(
-        GetRecentPagesClientId(), proposed_id);
+        GetRecentPagesClientId(), OfflinePageModel::kInvalidOfflineId);
   } else {
     download_info_.reset();
   }
 
   is_page_ready_for_snapshot_ = false;
+  previous_request_id_for_load_ = OfflinePageModel::kInvalidOfflineId;
 
   // New navigation, new snapshot session.
   snapshot_url_ = web_contents()->GetLastCommittedURL();
 
   // Check for conditions that would cause us not to snapshot.
   bool can_save = !navigation_handle->IsErrorPage() &&
                   OfflinePageModel::CanSaveURL(snapshot_url_) &&
                   OfflinePageUtils::GetOfflinePageFromWebContents(
                       web_contents()) == nullptr;
 
@@ skipping 45 matching lines @@
     GetRecentPagesClientId(),
     base::Bind(&RecentTabHelper::ContinueSnapshotWithIdsToPurge,
                weak_ptr_factory_.GetWeakPtr()));
 }
 
 void RecentTabHelper::ContinueSnapshotWithIdsToPurge(
     const std::vector<int64_t>& page_ids) {
   if (!download_info_)
     return;
 
+  std::vector<int64_t> ids(page_ids);
   // Also remove the download page if this is not a first snapshot.
-  std::vector<int64_t> ids(page_ids);
   ids.push_back(download_info_->request_id_);
+  // Do not delete a previous save from the same page load right now as it's
+  // unknown at this point in time if this snapshot will succeed.
+  for (auto it = ids.begin(); it != ids.end(); it++) {

[fgorski, 2016/12/02 19:07:38]

++it 

[carlosk, 2016/12/02 22:48:51]

Done.

+    if (*it == previous_request_id_for_load_) {
+      ids.erase(it);
+      break;
+    }
+  }
 
   page_model_->DeletePagesByOfflineId(
       ids, base::Bind(&RecentTabHelper::ContinueSnapshotAfterPurge,
                       weak_ptr_factory_.GetWeakPtr()));
 }
 
 void RecentTabHelper::ContinueSnapshotAfterPurge(
     OfflinePageModel::DeletePageResult result) {
   if (!download_info_ ||
       result != OfflinePageModel::DeletePageResult::SUCCESS ||
@@ skipping 11 matching lines @@
                         base::Bind(&RecentTabHelper::SavePageCallback,
                                    weak_ptr_factory_.GetWeakPtr()));
 }
 
 void RecentTabHelper::SavePageCallback(OfflinePageModel::SavePageResult result,
                                        int64_t offline_id) {
   if (!download_info_)
     return;
   download_info_->page_snapshot_completed_ =
       (result == SavePageResult::SUCCESS);
+
+  if (download_info_->page_snapshot_completed_) {
+    int64_t previous_request_id = previous_request_id_for_load_;
+    previous_request_id_for_load_ = offline_id;
+
+    // If there is a previous snapshot saved during the same page load it must
+    // be deleted before finishing the snapshot process.
+    if (previous_request_id != OfflinePageModel::kInvalidOfflineId) {
+      std::vector<int64_t> page_id = {previous_request_id};
+      page_model_->DeletePagesByOfflineId(
+          page_id, base::Bind(&RecentTabHelper::PreviousSaveWasPurged,
+                              weak_ptr_factory_.GetWeakPtr()));
+      return;
+    }
+  }
+
   ReportSnapshotCompleted();
 }
 
+void RecentTabHelper::PreviousSaveWasPurged(
+    OfflinePageModel::DeletePageResult result) {
+  if (!download_info_)
+    return;
+
+  ReportSnapshotCompleted();
+}
+
 void RecentTabHelper::ReportSnapshotCompleted() {
   snapshot_controller_->PendingSnapshotCompleted();
   // Tell RequestCoordinator how the request should be processed further.
   ReportDownloadStatusToRequestCoordinator();
 }
 
 void RecentTabHelper::ReportDownloadStatusToRequestCoordinator() {
   if (!download_info_)
     return;
 
@@ skipping 19 matching lines @@
 bool RecentTabHelper::IsSamePage() const {
   return web_contents() &&
          (web_contents()->GetLastCommittedURL() == snapshot_url_);
 }
 
 ClientId RecentTabHelper::GetRecentPagesClientId() const {
   return ClientId(kLastNNamespace, tab_id_);
 }
 
 }  // namespace offline_pages