Minor improvements to RecentTabHelper.

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 55 matching lines @@
   //    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.
   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 DownloadPageInfo {
    public:
     DownloadPageInfo(const ClientId& client_id,

[dewittj, 2017/01/17 23:21:52]

nit: rename to SnapshotProgressInfo (or something that is not tied to download
exactly)

[carlosk, 2017/01/18 03:06:10]

Done.

                      int64_t request_id)
         : client_id_(client_id),
           request_id_(request_id),
           page_snapshot_completed_(false) {}

[dewittj, 2017/01/17 23:21:52]

Maybe another constructor that just accepts a tab_id would make it clear that
there are really two types of DownloadPageInfo?

[carlosk, 2017/01/18 03:06:10]

Done.

I also made its members follow the correct naming rule for structs and have
their values default initialized.

 
     // 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_1" info.

[dewittj, 2017/01/17 23:21:52]

nit: last_1 is used here and last N is used elsewhere.

[carlosk, 2017/01/18 03:06:10]

Done.

     int64_t request_id_;
     // True if there was at least one snapshot successfully completed.
     bool page_snapshot_completed_;
   };
 
   explicit RecentTabHelper(content::WebContents* web_contents);
   friend class content::WebContentsUserData<RecentTabHelper>;
 
   void EnsureInitialized();
   void ContinueSnapshotWithIdsToPurge(const std::vector<int64_t>& page_ids);
   void ContinueSnapshotAfterPurge(OfflinePageModel::DeletePageResult result);
   void SavePageCallback(OfflinePageModel::SavePageResult result,
                         int64_t offline_id);
   void ReportSnapshotCompleted();
   void ReportDownloadStatusToRequestCoordinator();
   bool IsSamePage() const;
   ClientId GetRecentPagesClientId() const;
+  bool isSnapshottingForLastN() const;

[dewittj, 2017/01/17 23:21:52]

This method feels like it should be part of DownloadPageInfo.

[carlosk, 2017/01/18 03:06:10]

Done.

 
   // Page model is a service, no ownership. Can be null - for example, in
   // case when tab is in incognito profile.
-  OfflinePageModel* page_model_;
+  OfflinePageModel* page_model_ = nullptr;
 
   // If false, never make snapshots off the attached WebContents.
   // Not page-specific.
-  bool snapshots_enabled_;
+  bool snapshots_enabled_ = false;
 
   // Becomes true during navigation if the page is ready for snapshot as
   // indicated by at least one callback from SnapshotController.
-  bool is_page_ready_for_snapshot_;
+  bool is_page_ready_for_snapshot_ = false;
 
   // Info for the offline page to capture. Null if the tab is not capturing
   // current page.
   std::unique_ptr<DownloadPageInfo> download_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.
   GURL snapshot_url_;
   // This starts out null and used as a flag for EnsureInitialized() to do the
   // initialization only once.
   std::unique_ptr<SnapshotController> snapshot_controller_;
 
   std::unique_ptr<Delegate> delegate_;
 
   base::WeakPtrFactory<RecentTabHelper> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(RecentTabHelper);
 };
 
 }  // namespace offline_pages
 #endif  // CHROME_BROWSER_ANDROID_OFFLINE_PAGES_RECENT_TAB_HELPER_H_