Minor improvements to RecentTabHelper.

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 36 matching lines @@
     return base::ThreadTaskRunnerHandle::Get();
   }
   bool GetTabId(content::WebContents* web_contents, int* tab_id) override {
     return offline_pages::OfflinePageUtils::GetTabId(web_contents, tab_id);
   }
 };
 }  // namespace
 
 namespace offline_pages {
 
+bool RecentTabHelper::SnapshotProgressInfo::IsForLastN() {
+  // A last_n snapshot always has an invalid request id.
+  return request_id == OfflinePageModel::kInvalidOfflineId;
+}
+
 RecentTabHelper::RecentTabHelper(content::WebContents* web_contents)
     : content::WebContentsObserver(web_contents),
-      page_model_(nullptr),
-      snapshots_enabled_(false),
-      is_page_ready_for_snapshot_(false),
       delegate_(new DefaultDelegate()),
       weak_ptr_factory_(this) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 }
 
 RecentTabHelper::~RecentTabHelper() {
 }
 
 void RecentTabHelper::SetDelegate(
     std::unique_ptr<RecentTabHelper::Delegate> delegate) {
   DCHECK(delegate);
   delegate_ = std::move(delegate);
 }
 
 void RecentTabHelper::ObserveAndDownloadCurrentPage(
     const ClientId& client_id, int64_t request_id) {
   EnsureInitialized();
-  download_info_ = base::MakeUnique<DownloadPageInfo>(client_id, request_id);
+  snapshot_info_ =
+      base::MakeUnique<SnapshotProgressInfo>(client_id, request_id);
 
   // If this tab helper is not enabled, immediately give the job back to
   // RequestCoordinator.
   if (!snapshots_enabled_ || !page_model_) {
     ReportDownloadStatusToRequestCoordinator();
-    download_info_.reset();
+    weak_ptr_factory_.InvalidateWeakPtrs();
+    snapshot_info_.reset();
     return;
   }
 
   // No snapshots yet happened on the current page - return and wait for some.
   if (!is_page_ready_for_snapshot_)
     return;
 
   // If snapshot already happened and we missed it, go ahead and snapshot now.
   OfflinePageModel::SavePageParams save_page_params;
   save_page_params.url = web_contents()->GetLastCommittedURL();
@@ skipping 30 matching lines @@
 
   if (!snapshots_enabled_)
     return;
 
   page_model_ = OfflinePageModelFactory::GetForBrowserContext(
       web_contents()->GetBrowserContext());
 }
 
 void RecentTabHelper::DidFinishNavigation(
     content::NavigationHandle* navigation_handle) {
-  if (!navigation_handle->IsInMainFrame() ||
+  if (!navigation_handle->IsInMainFrame() || navigation_handle->IsSamePage() ||
       !navigation_handle->HasCommitted()) {
     return;
   }
 
-  // Cancel tasks in flight that relate to the previous page.
-  weak_ptr_factory_.InvalidateWeakPtrs();
-
   EnsureInitialized();
   if (!snapshots_enabled_)
     return;
 
+  // Cancel tasks in flight that relate to the previous page.
+  weak_ptr_factory_.InvalidateWeakPtrs();
+
   // We navigated to a different page, lets report progress to Background
   // Offliner.
-  if (download_info_ && !navigation_handle->IsSamePage()) {
-    ReportDownloadStatusToRequestCoordinator();
-  }
+  ReportDownloadStatusToRequestCoordinator();
 
   if (offline_pages::IsOffliningRecentPagesEnabled()) {
-    download_info_ = base::MakeUnique<DownloadPageInfo>(
-        GetRecentPagesClientId(), OfflinePageModel::kInvalidOfflineId);
+    // Note: at any point in time |snapshot_info_| might be replaced with one
+    // created by ObserveAndDownloadCurrentPage for a download snapshot request.
+    snapshot_info_ =
+        base::MakeUnique<SnapshotProgressInfo>(GetRecentPagesClientId());
   } else {
-    download_info_.reset();
+    snapshot_info_.reset();
   }
 
   is_page_ready_for_snapshot_ = false;
 
   // 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_) &&
@@ skipping 14 matching lines @@
   snapshot_controller_->DocumentAvailableInMainFrame();
 }
 
 void RecentTabHelper::DocumentOnLoadCompletedInMainFrame() {
   EnsureInitialized();
   snapshot_controller_->DocumentOnLoadCompletedInMainFrame();
 }
 
 void RecentTabHelper::WebContentsDestroyed() {
   // WebContents (and maybe Tab) is destroyed, report status to Offliner.
-  if (!download_info_)
-    return;
   ReportDownloadStatusToRequestCoordinator();
 }
 
 
 // This starts a sequence of async operations chained through callbacks:
 // - compute the set of old 'last_n' pages that have to be purged
 // - delete the pages found in the previous step
 // - snapshot the current web contents
 // Along the chain, the original URL is passed and compared, to detect
 // possible navigation and cancel snapshot in that case.
 void RecentTabHelper::StartSnapshot() {
   is_page_ready_for_snapshot_ = true;
 
-  if (!snapshots_enabled_ ||
-      !page_model_ ||
-      !download_info_) {
+  if (!snapshots_enabled_ || !page_model_ || !snapshot_info_) {
     ReportSnapshotCompleted();
     return;
   }
 
   // Remove previously captured pages for this tab.
   page_model_->GetOfflineIdsForClientId(
     GetRecentPagesClientId(),
     base::Bind(&RecentTabHelper::ContinueSnapshotWithIdsToPurge,
                weak_ptr_factory_.GetWeakPtr()));
 }
 
 void RecentTabHelper::ContinueSnapshotWithIdsToPurge(
     const std::vector<int64_t>& page_ids) {
-  if (!download_info_)
-    return;
+  DCHECK(snapshot_info_);
 
   // 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_);
+  ids.push_back(snapshot_info_->request_id);
 
   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 ||
-      !IsSamePage()) {
+  DCHECK(snapshot_info_);
+  DCHECK_EQ(snapshot_url_, web_contents()->GetLastCommittedURL());
+  if (result != OfflinePageModel::DeletePageResult::SUCCESS) {
     ReportSnapshotCompleted();
     return;
   }
 
   OfflinePageModel::SavePageParams save_page_params;
   save_page_params.url = snapshot_url_;
-  save_page_params.client_id = download_info_->client_id_;
-  save_page_params.proposed_offline_id = download_info_->request_id_;
+  save_page_params.client_id = snapshot_info_->client_id;
+  save_page_params.proposed_offline_id = snapshot_info_->request_id;
   page_model_->SavePage(save_page_params,
                         delegate_->CreatePageArchiver(web_contents()),
                         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);
+  DCHECK(snapshot_info_);
+  snapshot_info_->page_snapshot_completed = (result == SavePageResult::SUCCESS);
   ReportSnapshotCompleted();
 }
 
 void RecentTabHelper::ReportSnapshotCompleted() {
   snapshot_controller_->PendingSnapshotCompleted();
   // Tell RequestCoordinator how the request should be processed further.
   ReportDownloadStatusToRequestCoordinator();
 }
 
 void RecentTabHelper::ReportDownloadStatusToRequestCoordinator() {
-  if (!download_info_)
-    return;
-
-  if (download_info_->request_id_ == OfflinePageModel::kInvalidOfflineId)
+  if (!snapshot_info_ || snapshot_info_->IsForLastN())
     return;
 
   RequestCoordinator* request_coordinator =
       RequestCoordinatorFactory::GetForBrowserContext(
           web_contents()->GetBrowserContext());
   if (!request_coordinator)
     return;
 
   // It is OK to call these methods more then once, depending on
   // number of snapshots attempted in this tab helper. If the request_id is not
   // in the list of RequestCoordinator, these calls have no effect.
-  if (download_info_->page_snapshot_completed_)
-    request_coordinator->MarkRequestCompleted(download_info_->request_id_);
+  if (snapshot_info_->page_snapshot_completed)
+    request_coordinator->MarkRequestCompleted(snapshot_info_->request_id);
   else
-    request_coordinator->EnableForOffliner(download_info_->request_id_,
-                                           download_info_->client_id_);
-}
-
-bool RecentTabHelper::IsSamePage() const {
-  return web_contents() &&
-         (web_contents()->GetLastCommittedURL() == snapshot_url_);
+    request_coordinator->EnableForOffliner(snapshot_info_->request_id,
+                                           snapshot_info_->client_id);
 }
 
 ClientId RecentTabHelper::GetRecentPagesClientId() const {
   return ClientId(kLastNNamespace, tab_id_);
 }
 
 }  // namespace offline_pages