RecentTabHelper: Move URL checking logic into DidFinishNavigation.

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 50 matching lines @@
 
 RecentTabHelper::~RecentTabHelper() {
 }
 
 void RecentTabHelper::DidFinishNavigation(
     content::NavigationHandle* navigation_handle) {
   if (navigation_handle->IsInMainFrame() &&
       navigation_handle->HasCommitted()) {
     // Cancel tasks in flight that relate to the previous page.
     weak_ptr_factory_.InvalidateWeakPtrs();
+
     // New navigation, new snapshot session.
+    snapshot_url_ = GURL();
+    GURL last_committed_url = web_contents()->GetLastCommittedURL();
+
+    // Check for conditions that would cause us not to snapshot.
+    bool can_save = !navigation_handle->IsErrorPage() &&
+                    OfflinePageModel::CanSaveURL(last_committed_url);
+
+    // We only want to record UMA if the page is not Off The Record.
+    if (!never_do_snapshots_)
+      UMA_HISTOGRAM_BOOLEAN("OfflinePages.CanSaveRecentPage", can_save);
+
+    // Always reset so that posted tasks get cancelled.
     snapshot_controller_->Reset();
-    snapshot_url_ = GURL::EmptyGURL();
+
+    if (never_do_snapshots_ || !can_save)
+      snapshot_controller_->Stop();
+    else
+      snapshot_url_ = last_committed_url;
   }
 }
 
 void RecentTabHelper::DocumentAvailableInMainFrame() {
   snapshot_controller_->DocumentAvailableInMainFrame();
 }
 
 void RecentTabHelper::DocumentOnLoadCompletedInMainFrame() {
   snapshot_controller_->DocumentOnLoadCompletedInMainFrame();
 }
 
 // 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() {
-  if (never_do_snapshots_)
-    return;
-
-  // Ignores any non-normal pages, like error pages.
-  content::NavigationEntry* entry =
-      web_contents()->GetController().GetLastCommittedEntry();
-  if (!entry || entry->GetPageType() != content::PAGE_TYPE_NORMAL)
-    return;
-
-  GURL url = web_contents()->GetLastCommittedURL();
-  bool can_save = OfflinePageModel::CanSaveURL(url);
-  UMA_HISTOGRAM_BOOLEAN("OfflinePages.CanSaveRecentPage", can_save);
-  if (!can_save)
-    return;
-
-  snapshot_url_ = url;
-
   // TODO(dimich): Implement automatic cleanup as per design doc, based on
   // storage limits and page age.
   // This algorithm (remove pages before making sure the save was successful)
   // prefers keeping the storage use low and under control by potentially
   // sacrificing the current snapshot.
   page_model_->GetOfflineIdsForClientId(
     client_id(),
     base::Bind(&RecentTabHelper::ContinueSnapshotWithIdsToPurge,
                weak_ptr_factory_.GetWeakPtr()));
 }
@@ skipping 86 matching lines @@
     std::unique_ptr<TestArchiveFactory> test_archive_factory) {
   test_archive_factory_ = std::move(test_archive_factory);
 }
 
 void RecentTabHelper::SetTaskRunnerForTest(
     const scoped_refptr<base::SingleThreadTaskRunner>& task_runner) {
   snapshot_controller_.reset(new SnapshotController(task_runner, this));
 }
 
 }  // namespace offline_pages