RecentTabHelper won't accept overlapping requests from Downloads anymore.

chrome/browser/android/offline_pages/recent_tab_helper.h

 // 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.
 
 #ifndef CHROME_BROWSER_ANDROID_OFFLINE_PAGES_RECENT_TAB_HELPER_H_
 #define CHROME_BROWSER_ANDROID_OFFLINE_PAGES_RECENT_TAB_HELPER_H_
 
 #include <memory>
 #include <vector>
 
@@ skipping 39 matching lines @@
    public:
     virtual ~Delegate() {}
     virtual std::unique_ptr<OfflinePageArchiver> CreatePageArchiver(
         content::WebContents* web_contents) = 0;
     virtual scoped_refptr<base::SingleThreadTaskRunner> GetTaskRunner() = 0;
     // There is no expectations that tab_id is always present.
     virtual bool GetTabId(content::WebContents* web_contents, int* tab_id) = 0;
   };
   void SetDelegate(std::unique_ptr<RecentTabHelper::Delegate> delegate);
 
-  // Support for Download Page feature. The Download Page button does this:
-  // 1. Creates suspended request for Background Offliner.
-  // 2. Calls this method with properly filled ClientId.
-  // 3. This tab helper observes the page load and captures the page
-  //    if the load progresses far enough, as indicated by SnapshotController.
-  // 4. If capture is successful, this tab helper removes the suspended request.
-  //    Otherwise (navigation to other page, close tab) tab helper un-suspends
-  //    the request so the Background Offliner starts working on it.
-  // 5. If Chrome is killed at any point, next time Background Offliner loads
-  //    it converts all suspended requests from last session into active.
+  // Creates a request to download the current page with a properly filled
+  // |client_id| and valid |request_id| issued by RequestCoordinator from a
+  // suspended request. This method might be called multiple times for the same
+  // page at any point after its navigation commits. There are some important
+  // points about how requests are handled:
+  // a) While there is an ongoing request, new requests are ignored (no
+  //    overlapping snapshots).
+  // b) The page has to be sufficiently loaded to be considerer of minimum
+  //    quality for the request to be started immediately.
+  // c) Any calls made before the page is considered to have minimal quality
+  //    will be scheduled to be executed once that happens. The scheduled
+  //    request is considered "ongoing" for a) purposes.
+  // d) If the save operation is successful the dormant request with
+  //    RequestCoordinator is canceled; otherwise it is resumed. This logic is
+  //    robust to crashes.
+  // e) At the moment the page reaches high quality, if there was a successful
+  //    snapshot saved at a lower quality then a new snapshot is automatically
+  //    requested to replace it.
+  // Note #1: Page quality is determined by SnapshotController and is based on
+  // its assessment of "how much loaded" it is.
+  // Note #2: Currently this method only accepts download requests from the
+  // downloads namespace. Multiple clients are not supported.
   void ObserveAndDownloadCurrentPage(const ClientId& client_id,
                                      int64_t request_id);
 
  private:
-  // Keeps client_id/request_id that will be used for the offline snapshot.
-  struct SnapshotProgressInfo {
-   public:
-    // For a downloads snapshot request, where the |request_id| is defined.
-    SnapshotProgressInfo(const ClientId& client_id, int64_t request_id)
-        : client_id(client_id), request_id(request_id) {}
-
-    // For a last_n snapshot request.
-    explicit SnapshotProgressInfo(const ClientId& client_id)
-        : client_id(client_id) {}
-
-    bool IsForLastN();
-
-    // The ClientID to go with the offline page.
-    ClientId client_id;
-
-    // Id of the suspended request in Background Offliner. Used to un-suspend
-    // the request if the capture of the current page was not possible (e.g.
-    // the user navigated to another page before current one was loaded).
-    // 0 if this is a "last_n" info.
-    int64_t request_id = OfflinePageModel::kInvalidOfflineId;
-
-    // True if there was at least one snapshot successfully completed.
-    bool page_snapshot_completed = false;
-
-    // Expected snapshot quality should the saving succeed. This value is only
-    // valid if |page_snapshot_completed| is true.
-    SnapshotController::PageQuality expected_page_quality =
-        SnapshotController::PageQuality::POOR;
-  };
+  struct SnapshotProgressInfo;
 
   explicit RecentTabHelper(content::WebContents* web_contents);
   friend class content::WebContentsUserData<RecentTabHelper>;
 
   bool EnsureInitialized();
   void ContinueSnapshotWithIdsToPurge(SnapshotProgressInfo* snapshot_info,
                                       const std::vector<int64_t>& page_ids);
   void ContinueSnapshotAfterPurge(SnapshotProgressInfo* snapshot_info,
                                   OfflinePageModel::DeletePageResult result);
   void SavePageCallback(SnapshotProgressInfo* snapshot_info,
                         OfflinePageModel::SavePageResult result,
                         int64_t offline_id);
-  void ReportSnapshotCompleted(SnapshotProgressInfo* snapshot_info);
+  void ReportSnapshotCompleted(SnapshotProgressInfo* snapshot_info,
+                               bool success);
   void ReportDownloadStatusToRequestCoordinator(
-      SnapshotProgressInfo* snapshot_info);
+      SnapshotProgressInfo* snapshot_info,
+      bool cancel_background_request);
   ClientId GetRecentPagesClientId() const;
-  void SaveSnapshotForDownloads();
+  void SaveSnapshotForDownloads(bool replace_latest);
 
   // Page model is a service, no ownership. Can be null - for example, in
   // case when tab is in incognito profile.
   OfflinePageModel* page_model_ = nullptr;
 
   // If false, never make snapshots off the attached WebContents.
   // Not page-specific.
   bool snapshots_enabled_ = false;
 
-  // Snapshot progress information for a downloads triggered request. Null if
-  // downloads is not capturing or hasn't captured the current page.
-  std::unique_ptr<SnapshotProgressInfo> downloads_latest_snapshot_info_;
+  // Snapshot progress information for an ongoing snapshot requested by
+  // downloads. Null if there's no ongoing request.
+  std::unique_ptr<SnapshotProgressInfo> downloads_ongoing_snapshot_info_;
+
+  // This is set to true if the ongoing snapshot for downloads is waiting on the
+  // page to reach a minimal quality level to start.
+  bool downloads_snapshot_on_hold_ = false;
+
+  // Snapshot information for the last successful snapshot requested by
+  // downloads. Null no successful one has ever completed.

[Dmitry Titov, 2017/01/27 04:18:22]

nit: "Null no" -> "Null if no"

[carlosk, 2017/01/27 18:47:14]

Done.

+  std::unique_ptr<SnapshotProgressInfo> downloads_latest_saved_snapshot_info_;
 
   // Snapshot progress information for a last_n triggered request. Null if
   // last_n is not currently capturing the current page.
   std::unique_ptr<SnapshotProgressInfo> last_n_ongoing_snapshot_info_;
 
   // If empty, the tab does not have AndroidId and can not capture pages.
   std::string tab_id_;
 
   // The URL of the page that is currently being snapshotted. Used to check,
   // during async operations, that WebContents still contains the same page.
@@ skipping 16 matching lines @@
       SnapshotController::PageQuality::POOR;
 
   base::WeakPtrFactory<RecentTabHelper> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(RecentTabHelper);
 };
 
 }  // namespace offline_pages
 
 #endif  // CHROME_BROWSER_ANDROID_OFFLINE_PAGES_RECENT_TAB_HELPER_H_