Splits the OfflinePageModel into and interface and and implementation class.

components/offline_pages/offline_page_model_impl.cc

Index: components/offline_pages/offline_page_model_impl.cc
diff --git a/components/offline_pages/offline_page_model.cc b/components/offline_pages/offline_page_model_impl.cc
similarity index 68%
copy from components/offline_pages/offline_page_model.cc
copy to components/offline_pages/offline_page_model_impl.cc
index beb622cafba81da6ccb0c704dd381c956dde9c9b..e8a21ba2dd8c1c314f8a9d885bc0d9fe76bf3a63 100644
--- a/components/offline_pages/offline_page_model.cc
+++ b/components/offline_pages/offline_page_model_impl.cc
@@ -1,11 +1,11 @@
-// Copyright 2015 The Chromium Authors. All rights reserved.
+// 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.
 
-#include "components/offline_pages/offline_page_model.h"
+#include "components/offline_pages/offline_page_model_impl.h"
 
 #include <algorithm>
-#include <utility>
+#include <limits>
 
 #include "base/bind.h"
 #include "base/files/file_util.h"
@@ -41,6 +41,10 @@ enum ClearAllStatus {
   CLEAR_ALL_STATUS_COUNT
 };
 
+int64_t GenerateOfflineId() {
+  return base::RandGenerator(std::numeric_limits<int64_t>::max()) + 1;
+}
+
 // The maximum histogram size for the metrics that measure time between views of
 // a given page.
 const base::TimeDelta kMaxOpenedPageHistogramBucket =
@@ -124,16 +128,11 @@ void ReportStorageHistogramsAfterDelete(
 
 }  // namespace
 
-// static
-bool OfflinePageModel::CanSavePage(const GURL& url) {
-  return url.SchemeIsHTTPOrHTTPS();
-}
-
 // protected
-OfflinePageModel::OfflinePageModel()
-    : is_loaded_(false), weak_ptr_factory_(this) {}
+OfflinePageModelImpl::OfflinePageModelImpl()
+    : OfflinePageModel(), is_loaded_(false), weak_ptr_factory_(this) {}
 
-OfflinePageModel::OfflinePageModel(
+OfflinePageModelImpl::OfflinePageModelImpl(
     std::unique_ptr<OfflinePageMetadataStore> store,
     const base::FilePath& archives_dir,
     const scoped_refptr<base::SequencedTaskRunner>& task_runner)
@@ -144,25 +143,25 @@ OfflinePageModel::OfflinePageModel(
       archive_manager_(new ArchiveManager(archives_dir, task_runner)),
       weak_ptr_factory_(this) {
   archive_manager_->EnsureArchivesDirCreated(
-      base::Bind(&OfflinePageModel::OnEnsureArchivesDirCreatedDone,
+      base::Bind(&OfflinePageModelImpl::OnEnsureArchivesDirCreatedDone,
                  weak_ptr_factory_.GetWeakPtr(), base::TimeTicks::Now()));
 }
 
-OfflinePageModel::~OfflinePageModel() {
-}
+OfflinePageModelImpl::~OfflinePageModelImpl() {}
 
-void OfflinePageModel::AddObserver(Observer* observer) {
+void OfflinePageModelImpl::AddObserver(Observer* observer) {
   observers_.AddObserver(observer);
 }
 
-void OfflinePageModel::RemoveObserver(Observer* observer) {
+void OfflinePageModelImpl::RemoveObserver(Observer* observer) {
   observers_.RemoveObserver(observer);
 }
 
-void OfflinePageModel::SavePage(const GURL& url,
-                                const ClientId& client_id,
-                                std::unique_ptr<OfflinePageArchiver> archiver,
-                                const SavePageCallback& callback) {
+void OfflinePageModelImpl::SavePage(
+    const GURL& url,
+    const ClientId& client_id,
+    std::unique_ptr<OfflinePageArchiver> archiver,
+    const SavePageCallback& callback) {
   DCHECK(is_loaded_);
 
   // Skip saving the page that is not intended to be saved, like local file
@@ -183,15 +182,14 @@ void OfflinePageModel::SavePage(const GURL& url,
   int64_t offline_id = GenerateOfflineId();
 
   archiver->CreateArchive(
-      archives_dir_,
-      offline_id,
-      base::Bind(&OfflinePageModel::OnCreateArchiveDone,
-                 weak_ptr_factory_.GetWeakPtr(), url, offline_id,
-                 client_id, base::Time::Now(), callback));
+      archives_dir_, offline_id,
+      base::Bind(&OfflinePageModelImpl::OnCreateArchiveDone,
+                 weak_ptr_factory_.GetWeakPtr(), url, offline_id, client_id,
+                 base::Time::Now(), callback));
   pending_archivers_.push_back(std::move(archiver));
 }
 
-void OfflinePageModel::MarkPageAccessed(int64_t offline_id) {
+void OfflinePageModelImpl::MarkPageAccessed(int64_t offline_id) {
   DCHECK(is_loaded_);
   auto iter = offline_pages_.find(offline_id);
   if (iter == offline_pages_.end())
@@ -208,11 +206,11 @@ void OfflinePageModel::MarkPageAccessed(int64_t offline_id) {
   // When the access account is still zero, the page is opened for the first
   // time since its creation.
   UMA_HISTOGRAM_CUSTOM_COUNTS(
-      AddHistogramSuffix(
-          offline_page_item.client_id,
-          (offline_page_item.access_count == 0) ?
-              "OfflinePages.FirstOpenSinceCreated" :
-              "OfflinePages.OpenSinceLastOpen").c_str(),
+      AddHistogramSuffix(offline_page_item.client_id,

[dewittj, 2016/05/26 16:38:43]

I'd avoid reformatting code that is unchanged.

[dougarnett, 2016/05/26 20:50:34]

Wasn't my intention - fallout of moving to new file and running git cl format.
If important, I can put back as last update before landing (so I don't run git
cl format again and mess it up).

+                         (offline_page_item.access_count == 0)
+                             ? "OfflinePages.FirstOpenSinceCreated"
+                             : "OfflinePages.OpenSinceLastOpen")
+          .c_str(),
       time_since_last_accessed.InMinutes(), 1,
       kMaxOpenedPageHistogramBucket.InMinutes(), 50);
 
@@ -221,16 +219,16 @@ void OfflinePageModel::MarkPageAccessed(int64_t offline_id) {
 
   store_->AddOrUpdateOfflinePage(
       offline_page_item,
-      base::Bind(&OfflinePageModel::OnMarkPageAccesseDone,
+      base::Bind(&OfflinePageModelImpl::OnMarkPageAccesseDone,
                  weak_ptr_factory_.GetWeakPtr(), offline_page_item));
 }
 
-void OfflinePageModel::DeletePagesByOfflineId(
+void OfflinePageModelImpl::DeletePagesByOfflineId(
     const std::vector<int64_t>& offline_ids,
     const DeletePageCallback& callback) {
   if (!is_loaded_) {
     delayed_tasks_.push_back(
-        base::Bind(&OfflinePageModel::DoDeletePagesByOfflineId,
+        base::Bind(&OfflinePageModelImpl::DoDeletePagesByOfflineId,
                    weak_ptr_factory_.GetWeakPtr(), offline_ids, callback));
 
     return;
@@ -238,7 +236,7 @@ void OfflinePageModel::DeletePagesByOfflineId(
   DoDeletePagesByOfflineId(offline_ids, callback);
 }
 
-void OfflinePageModel::DoDeletePagesByOfflineId(
+void OfflinePageModelImpl::DoDeletePagesByOfflineId(
     const std::vector<int64_t>& offline_ids,
     const DeletePageCallback& callback) {
   DCHECK(is_loaded_);
@@ -258,11 +256,11 @@ void OfflinePageModel::DoDeletePagesByOfflineId(
 
   archive_manager_->DeleteMultipleArchives(
       paths_to_delete,
-      base::Bind(&OfflinePageModel::OnDeleteArchiveFilesDone,
+      base::Bind(&OfflinePageModelImpl::OnDeleteArchiveFilesDone,
                  weak_ptr_factory_.GetWeakPtr(), offline_ids, callback));
 }
 
-void OfflinePageModel::ClearAll(const base::Closure& callback) {
+void OfflinePageModelImpl::ClearAll(const base::Closure& callback) {
   DCHECK(is_loaded_);
 
   std::vector<int64_t> offline_ids;
@@ -270,16 +268,16 @@ void OfflinePageModel::ClearAll(const base::Closure& callback) {
     offline_ids.push_back(id_page_pair.first);
   DeletePagesByOfflineId(
       offline_ids,
-      base::Bind(&OfflinePageModel::OnRemoveAllFilesDoneForClearAll,
+      base::Bind(&OfflinePageModelImpl::OnRemoveAllFilesDoneForClearAll,
                  weak_ptr_factory_.GetWeakPtr(), callback));
 }
 
-void OfflinePageModel::DeletePagesByURLPredicate(
+void OfflinePageModelImpl::DeletePagesByURLPredicate(
     const UrlPredicate& predicate,
     const DeletePageCallback& callback) {
   if (!is_loaded_) {
     delayed_tasks_.push_back(
-        base::Bind(&OfflinePageModel::DoDeletePagesByURLPredicate,
+        base::Bind(&OfflinePageModelImpl::DoDeletePagesByURLPredicate,
                    weak_ptr_factory_.GetWeakPtr(), predicate, callback));
 
     return;
@@ -287,7 +285,7 @@ void OfflinePageModel::DeletePagesByURLPredicate(
   DoDeletePagesByURLPredicate(predicate, callback);
 }
 
-void OfflinePageModel::DoDeletePagesByURLPredicate(
+void OfflinePageModelImpl::DoDeletePagesByURLPredicate(
     const UrlPredicate& predicate,
     const DeletePageCallback& callback) {
   DCHECK(is_loaded_);
@@ -300,14 +298,14 @@ void OfflinePageModel::DoDeletePagesByURLPredicate(
   DoDeletePagesByOfflineId(offline_ids, callback);
 }
 
-void OfflinePageModel::HasPages(const std::string& name_space,
-                                const HasPagesCallback& callback) {
-  RunWhenLoaded(base::Bind(&OfflinePageModel::HasPagesAfterLoadDone,
+void OfflinePageModelImpl::HasPages(const std::string& name_space,
+                                    const HasPagesCallback& callback) {
+  RunWhenLoaded(base::Bind(&OfflinePageModelImpl::HasPagesAfterLoadDone,
                            weak_ptr_factory_.GetWeakPtr(), name_space,
                            callback));
 }
 
-void OfflinePageModel::HasPagesAfterLoadDone(
+void OfflinePageModelImpl::HasPagesAfterLoadDone(
     const std::string& name_space,
     const HasPagesCallback& callback) const {
   DCHECK(is_loaded_);
@@ -324,15 +322,15 @@ void OfflinePageModel::HasPagesAfterLoadDone(
   callback.Run(has_pages);
 }
 
-void OfflinePageModel::CheckPagesExistOffline(
+void OfflinePageModelImpl::CheckPagesExistOffline(
     const std::set<GURL>& urls,
     const CheckPagesExistOfflineCallback& callback) {
   RunWhenLoaded(
-      base::Bind(&OfflinePageModel::CheckPagesExistOfflineAfterLoadDone,
+      base::Bind(&OfflinePageModelImpl::CheckPagesExistOfflineAfterLoadDone,
                  weak_ptr_factory_.GetWeakPtr(), urls, callback));
 }
 
-void OfflinePageModel::CheckPagesExistOfflineAfterLoadDone(
+void OfflinePageModelImpl::CheckPagesExistOfflineAfterLoadDone(
     const std::set<GURL>& urls,
     const CheckPagesExistOfflineCallback& callback) {
   DCHECK(is_loaded_);
@@ -345,13 +343,13 @@ void OfflinePageModel::CheckPagesExistOfflineAfterLoadDone(
   callback.Run(result);
 }
 
-void OfflinePageModel::GetAllPages(
+void OfflinePageModelImpl::GetAllPages(
     const MultipleOfflinePageItemCallback& callback) {
-  RunWhenLoaded(base::Bind(&OfflinePageModel::GetAllPagesAfterLoadDone,
+  RunWhenLoaded(base::Bind(&OfflinePageModelImpl::GetAllPagesAfterLoadDone,
                            weak_ptr_factory_.GetWeakPtr(), callback));
 }
 
-void OfflinePageModel::GetAllPagesAfterLoadDone(
+void OfflinePageModelImpl::GetAllPagesAfterLoadDone(
     const MultipleOfflinePageItemCallback& callback) {
   DCHECK(is_loaded_);
 
@@ -362,21 +360,21 @@ void OfflinePageModel::GetAllPagesAfterLoadDone(
   callback.Run(offline_pages);
 }
 
-void OfflinePageModel::GetOfflineIdsForClientId(
+void OfflinePageModelImpl::GetOfflineIdsForClientId(
     const ClientId& client_id,
     const MultipleOfflineIdCallback& callback) {
   RunWhenLoaded(
-      base::Bind(&OfflinePageModel::GetOfflineIdsForClientIdWhenLoadDone,
+      base::Bind(&OfflinePageModelImpl::GetOfflineIdsForClientIdWhenLoadDone,
                  weak_ptr_factory_.GetWeakPtr(), client_id, callback));
 }
 
-void OfflinePageModel::GetOfflineIdsForClientIdWhenLoadDone(
+void OfflinePageModelImpl::GetOfflineIdsForClientIdWhenLoadDone(
     const ClientId& client_id,
     const MultipleOfflineIdCallback& callback) const {
   callback.Run(MaybeGetOfflineIdsForClientId(client_id));
 }
 
-const std::vector<int64_t> OfflinePageModel::MaybeGetOfflineIdsForClientId(
+const std::vector<int64_t> OfflinePageModelImpl::MaybeGetOfflineIdsForClientId(
     const ClientId& client_id) const {
   DCHECK(is_loaded_);
   std::vector<int64_t> results;
@@ -390,15 +388,15 @@ const std::vector<int64_t> OfflinePageModel::MaybeGetOfflineIdsForClientId(
   return results;
 }
 
-void OfflinePageModel::GetPageByOfflineId(
+void OfflinePageModelImpl::GetPageByOfflineId(
     int64_t offline_id,
     const SingleOfflinePageItemCallback& callback) {
-  RunWhenLoaded(base::Bind(&OfflinePageModel::GetPageByOfflineIdWhenLoadDone,
-                           weak_ptr_factory_.GetWeakPtr(), offline_id,
-                           callback));
+  RunWhenLoaded(
+      base::Bind(&OfflinePageModelImpl::GetPageByOfflineIdWhenLoadDone,
+                 weak_ptr_factory_.GetWeakPtr(), offline_id, callback));
 }
 
-void OfflinePageModel::GetPageByOfflineIdWhenLoadDone(
+void OfflinePageModelImpl::GetPageByOfflineIdWhenLoadDone(
     int64_t offline_id,
     const SingleOfflinePageItemCallback& callback) const {
   SingleOfflinePageItemResult result;
@@ -408,21 +406,21 @@ void OfflinePageModel::GetPageByOfflineIdWhenLoadDone(
   callback.Run(result);
 }
 
-const OfflinePageItem* OfflinePageModel::MaybeGetPageByOfflineId(
+const OfflinePageItem* OfflinePageModelImpl::MaybeGetPageByOfflineId(
     int64_t offline_id) const {
   const auto iter = offline_pages_.find(offline_id);
   return iter != offline_pages_.end() ? &(iter->second) : nullptr;
 }
 
-void OfflinePageModel::GetPageByOfflineURL(
+void OfflinePageModelImpl::GetPageByOfflineURL(
     const GURL& offline_url,
     const SingleOfflinePageItemCallback& callback) {
-  RunWhenLoaded(base::Bind(&OfflinePageModel::GetPageByOfflineURLWhenLoadDone,
-                           weak_ptr_factory_.GetWeakPtr(), offline_url,
-                           callback));
+  RunWhenLoaded(
+      base::Bind(&OfflinePageModelImpl::GetPageByOfflineURLWhenLoadDone,
+                 weak_ptr_factory_.GetWeakPtr(), offline_url, callback));
 }
 
-void OfflinePageModel::GetPageByOfflineURLWhenLoadDone(
+void OfflinePageModelImpl::GetPageByOfflineURLWhenLoadDone(
     const GURL& offline_url,
     const SingleOfflinePageItemCallback& callback) const {
   base::Optional<OfflinePageItem> result;
@@ -437,7 +435,7 @@ void OfflinePageModel::GetPageByOfflineURLWhenLoadDone(
   callback.Run(base::nullopt);
 }
 
-const OfflinePageItem* OfflinePageModel::MaybeGetPageByOfflineURL(
+const OfflinePageItem* OfflinePageModelImpl::MaybeGetPageByOfflineURL(
     const GURL& offline_url) const {
   for (const auto& id_page_pair : offline_pages_) {
     if (id_page_pair.second.GetOfflineURL() == offline_url)
@@ -446,15 +444,15 @@ const OfflinePageItem* OfflinePageModel::MaybeGetPageByOfflineURL(
   return nullptr;
 }
 
-void OfflinePageModel::GetBestPageForOnlineURL(
+void OfflinePageModelImpl::GetBestPageForOnlineURL(
     const GURL& online_url,
     const SingleOfflinePageItemCallback callback) {
   RunWhenLoaded(
-      base::Bind(&OfflinePageModel::GetBestPageForOnlineURLWhenLoadDone,
+      base::Bind(&OfflinePageModelImpl::GetBestPageForOnlineURLWhenLoadDone,
                  weak_ptr_factory_.GetWeakPtr(), online_url, callback));
 }
 
-void OfflinePageModel::GetBestPageForOnlineURLWhenLoadDone(
+void OfflinePageModelImpl::GetBestPageForOnlineURLWhenLoadDone(
     const GURL& online_url,
     const SingleOfflinePageItemCallback& callback) const {
   SingleOfflinePageItemResult result;
@@ -466,15 +464,15 @@ void OfflinePageModel::GetBestPageForOnlineURLWhenLoadDone(
   callback.Run(result);
 }
 
-void OfflinePageModel::GetPagesByOnlineURL(
+void OfflinePageModelImpl::GetPagesByOnlineURL(
     const GURL& online_url,
     const MultipleOfflinePageItemCallback& callback) {
-  RunWhenLoaded(base::Bind(&OfflinePageModel::GetPagesByOnlineURLWhenLoadDone,
-                           weak_ptr_factory_.GetWeakPtr(), online_url,
-                           callback));
+  RunWhenLoaded(
+      base::Bind(&OfflinePageModelImpl::GetPagesByOnlineURLWhenLoadDone,
+                 weak_ptr_factory_.GetWeakPtr(), online_url, callback));
 }
 
-void OfflinePageModel::GetPagesByOnlineURLWhenLoadDone(
+void OfflinePageModelImpl::GetPagesByOnlineURLWhenLoadDone(
     const GURL& online_url,
     const MultipleOfflinePageItemCallback& callback) const {
   std::vector<OfflinePageItem> result;
@@ -487,7 +485,7 @@ void OfflinePageModel::GetPagesByOnlineURLWhenLoadDone(
   callback.Run(result);
 }
 
-const OfflinePageItem* OfflinePageModel::MaybeGetBestPageForOnlineURL(
+const OfflinePageItem* OfflinePageModelImpl::MaybeGetBestPageForOnlineURL(
     const GURL& online_url) const {
   const OfflinePageItem* result = nullptr;
   for (const auto& id_page_pair : offline_pages_) {
@@ -499,16 +497,16 @@ const OfflinePageItem* OfflinePageModel::MaybeGetBestPageForOnlineURL(
   return result;
 }
 
-void OfflinePageModel::CheckForExternalFileDeletion() {
+void OfflinePageModelImpl::CheckForExternalFileDeletion() {
   DCHECK(is_loaded_);
 
   archive_manager_->GetAllArchives(
-      base::Bind(&OfflinePageModel::ScanForMissingArchiveFiles,
+      base::Bind(&OfflinePageModelImpl::ScanForMissingArchiveFiles,
                  weak_ptr_factory_.GetWeakPtr()));
 }
 
-void OfflinePageModel::ExpirePages(const std::vector<int64_t>& offline_ids,
-                                   const base::Time& expiration_time) {
+void OfflinePageModelImpl::ExpirePages(const std::vector<int64_t>& offline_ids,
+                                       const base::Time& expiration_time) {
   for (int64_t offline_id : offline_ids) {
     auto iter = offline_pages_.find(offline_id);
     if (iter == offline_pages_.end())
@@ -518,15 +516,15 @@ void OfflinePageModel::ExpirePages(const std::vector<int64_t>& offline_ids,
     offline_page.expiration_time = expiration_time;
 
     store_->AddOrUpdateOfflinePage(
-        offline_page, base::Bind(&OfflinePageModel::OnExpirePageDone,
+        offline_page, base::Bind(&OfflinePageModelImpl::OnExpirePageDone,
                                  weak_ptr_factory_.GetWeakPtr(), offline_id,
                                  expiration_time));
   }
 }
 
-void OfflinePageModel::OnExpirePageDone(int64_t offline_id,
-                                        const base::Time& expiration_time,
-                                        bool success) {
+void OfflinePageModelImpl::OnExpirePageDone(int64_t offline_id,
+                                            const base::Time& expiration_time,
+                                            bool success) {
   // TODO(romax): Report UMA about successful expiration.
   if (success) {
     auto iter = offline_pages_.find(offline_id);
@@ -535,32 +533,32 @@ void OfflinePageModel::OnExpirePageDone(int64_t offline_id,
   }
 }
 
-ClientPolicyController* OfflinePageModel::GetPolicyController() {
+ClientPolicyController* OfflinePageModelImpl::GetPolicyController() {
   return policy_controller_.get();
 }
 
-OfflinePageMetadataStore* OfflinePageModel::GetStoreForTesting() {
+OfflinePageMetadataStore* OfflinePageModelImpl::GetStoreForTesting() {
   return store_.get();
 }
 
-OfflinePageStorageManager* OfflinePageModel::GetStorageManager() {
+OfflinePageStorageManager* OfflinePageModelImpl::GetStorageManager() {
   return storage_manager_.get();
 }
 
-bool OfflinePageModel::is_loaded() const {
+bool OfflinePageModelImpl::is_loaded() const {
   return is_loaded_;
 }
 
-void OfflinePageModel::OnCreateArchiveDone(const GURL& requested_url,
-                                           int64_t offline_id,
-                                           const ClientId& client_id,
-                                           const base::Time& start_time,
-                                           const SavePageCallback& callback,
-                                           OfflinePageArchiver* archiver,
-                                           ArchiverResult archiver_result,
-                                           const GURL& url,
-                                           const base::FilePath& file_path,
-                                           int64_t file_size) {
+void OfflinePageModelImpl::OnCreateArchiveDone(const GURL& requested_url,
+                                               int64_t offline_id,
+                                               const ClientId& client_id,
+                                               const base::Time& start_time,
+                                               const SavePageCallback& callback,
+                                               OfflinePageArchiver* archiver,
+                                               ArchiverResult archiver_result,
+                                               const GURL& url,
+                                               const base::FilePath& file_path,
+                                               int64_t file_size) {
   if (requested_url != url) {
     DVLOG(1) << "Saved URL does not match requested URL.";
     // TODO(fgorski): We have created an archive for a wrong URL. It should be
@@ -580,27 +578,28 @@ void OfflinePageModel::OnCreateArchiveDone(const GURL& requested_url,
   OfflinePageItem offline_page_item(url, offline_id, client_id, file_path,
                                     file_size, start_time);
   store_->AddOrUpdateOfflinePage(
-      offline_page_item,
-      base::Bind(&OfflinePageModel::OnAddOfflinePageDone,
-                 weak_ptr_factory_.GetWeakPtr(), archiver, callback,
-                 offline_page_item));
+      offline_page_item, base::Bind(&OfflinePageModelImpl::OnAddOfflinePageDone,
+                                    weak_ptr_factory_.GetWeakPtr(), archiver,
+                                    callback, offline_page_item));
 }
 
-void OfflinePageModel::OnAddOfflinePageDone(OfflinePageArchiver* archiver,
-                                            const SavePageCallback& callback,
-                                            const OfflinePageItem& offline_page,
-                                            bool success) {
+void OfflinePageModelImpl::OnAddOfflinePageDone(
+    OfflinePageArchiver* archiver,
+    const SavePageCallback& callback,
+    const OfflinePageItem& offline_page,
+    bool success) {
   SavePageResult result;
   if (success) {
     offline_pages_[offline_page.offline_id] = offline_page;
     result = SavePageResult::SUCCESS;
     UMA_HISTOGRAM_TIMES(
-        AddHistogramSuffix(
-            offline_page.client_id, "OfflinePages.SavePageTime").c_str(),
+        AddHistogramSuffix(offline_page.client_id, "OfflinePages.SavePageTime")
+            .c_str(),
         base::Time::Now() - offline_page.creation_time);
     // 50 buckets capped between 1Kb and 10Mb.
-    UMA_HISTOGRAM_COUNTS_10000(AddHistogramSuffix(
-        offline_page.client_id, "OfflinePages.PageSize").c_str(),
+    UMA_HISTOGRAM_COUNTS_10000(
+        AddHistogramSuffix(offline_page.client_id, "OfflinePages.PageSize")

[dewittj, 2016/05/26 16:38:43]

Avoid reformatting unchanged code

[dougarnett, 2016/05/26 20:50:34]

Acknowledged.

+            .c_str(),
         offline_page.file_size / 1024);
   } else {
     result = SavePageResult::STORE_FAILURE;
@@ -612,8 +611,9 @@ void OfflinePageModel::OnAddOfflinePageDone(OfflinePageArchiver* archiver,
   FOR_EACH_OBSERVER(Observer, observers_, OfflinePageModelChanged(this));
 }
 
-void OfflinePageModel::OnMarkPageAccesseDone(
-    const OfflinePageItem& offline_page_item, bool success) {
+void OfflinePageModelImpl::OnMarkPageAccesseDone(
+    const OfflinePageItem& offline_page_item,
+    bool success) {
   // Update the item in the cache only upon success.
   if (success)
     offline_pages_[offline_page_item.offline_id] = offline_page_item;
@@ -622,16 +622,16 @@ void OfflinePageModel::OnMarkPageAccesseDone(
   // should not have any impact to the UI.
 }
 
-void OfflinePageModel::OnEnsureArchivesDirCreatedDone(
+void OfflinePageModelImpl::OnEnsureArchivesDirCreatedDone(
     const base::TimeTicks& start_time) {
   UMA_HISTOGRAM_TIMES("OfflinePages.Model.ArchiveDirCreationTime",
                       base::TimeTicks::Now() - start_time);
 
-  store_->Load(base::Bind(&OfflinePageModel::OnLoadDone,
+  store_->Load(base::Bind(&OfflinePageModelImpl::OnLoadDone,
                           weak_ptr_factory_.GetWeakPtr(), start_time));
 }
 
-void OfflinePageModel::OnLoadDone(
+void OfflinePageModelImpl::OnLoadDone(
     const base::TimeTicks& start_time,
     OfflinePageMetadataStore::LoadStatus load_status,
     const std::vector<OfflinePageItem>& offline_pages) {
@@ -660,25 +660,25 @@ void OfflinePageModel::OnLoadDone(
   CheckForExternalFileDeletion();
 }
 
-void OfflinePageModel::InformSavePageDone(const SavePageCallback& callback,
-                                          SavePageResult result,
-                                          const ClientId& client_id,
-                                          int64_t offline_id) {
+void OfflinePageModelImpl::InformSavePageDone(const SavePageCallback& callback,
+                                              SavePageResult result,
+                                              const ClientId& client_id,
+                                              int64_t offline_id) {
   UMA_HISTOGRAM_ENUMERATION(
       AddHistogramSuffix(client_id, "OfflinePages.SavePageResult").c_str(),
-      static_cast<int>(result),
-      static_cast<int>(SavePageResult::RESULT_COUNT));
+      static_cast<int>(result), static_cast<int>(SavePageResult::RESULT_COUNT));
   archive_manager_->GetStorageStats(
       base::Bind(&ReportStorageHistogramsAfterSave));
   callback.Run(result, offline_id);
 }
 
-void OfflinePageModel::DeletePendingArchiver(OfflinePageArchiver* archiver) {
-  pending_archivers_.erase(std::find(
-      pending_archivers_.begin(), pending_archivers_.end(), archiver));
+void OfflinePageModelImpl::DeletePendingArchiver(
+    OfflinePageArchiver* archiver) {
+  pending_archivers_.erase(std::find(pending_archivers_.begin(),
+                                     pending_archivers_.end(), archiver));
 }
 
-void OfflinePageModel::OnDeleteArchiveFilesDone(
+void OfflinePageModelImpl::OnDeleteArchiveFilesDone(
     const std::vector<int64_t>& offline_ids,
     const DeletePageCallback& callback,
     bool success) {
@@ -689,11 +689,11 @@ void OfflinePageModel::OnDeleteArchiveFilesDone(
 
   store_->RemoveOfflinePages(
       offline_ids,
-      base::Bind(&OfflinePageModel::OnRemoveOfflinePagesDone,
+      base::Bind(&OfflinePageModelImpl::OnRemoveOfflinePagesDone,
                  weak_ptr_factory_.GetWeakPtr(), offline_ids, callback));
 }
 
-void OfflinePageModel::OnRemoveOfflinePagesDone(
+void OfflinePageModelImpl::OnRemoveOfflinePagesDone(
     const std::vector<int64_t>& offline_ids,
     const DeletePageCallback& callback,
     bool success) {
@@ -709,32 +709,28 @@ void OfflinePageModel::OnRemoveOfflinePagesDone(
     ClientId client_id = iter->second.client_id;
     UMA_HISTOGRAM_CUSTOM_COUNTS(
         AddHistogramSuffix(client_id, "OfflinePages.PageLifetime").c_str(),
-        (now - iter->second.creation_time).InMinutes(),
-        1,
-        base::TimeDelta::FromDays(365).InMinutes(),
-        100);
+        (now - iter->second.creation_time).InMinutes(), 1,
+        base::TimeDelta::FromDays(365).InMinutes(), 100);
     UMA_HISTOGRAM_CUSTOM_COUNTS(
-        AddHistogramSuffix(
-            client_id, "OfflinePages.DeletePage.TimeSinceLastOpen").c_str(),
-        (now - iter->second.last_access_time).InMinutes(),
-        1,
-        base::TimeDelta::FromDays(365).InMinutes(),
-        100);
+        AddHistogramSuffix(client_id,

[dewittj, 2016/05/26 16:38:43]

Same, this code hasn't changed, so don't reformat it.

[dougarnett, 2016/05/26 20:50:34]

Acknowledged.

+                           "OfflinePages.DeletePage.TimeSinceLastOpen")
+            .c_str(),
+        (now - iter->second.last_access_time).InMinutes(), 1,
+        base::TimeDelta::FromDays(365).InMinutes(), 100);
     UMA_HISTOGRAM_CUSTOM_COUNTS(
-        AddHistogramSuffix(
-            client_id, "OfflinePages.DeletePage.LastOpenToCreated").c_str(),
-        (iter->second.last_access_time - iter->second.creation_time).
-            InMinutes(),
-        1,
-        base::TimeDelta::FromDays(365).InMinutes(),
-        100);
+        AddHistogramSuffix(client_id,
+                           "OfflinePages.DeletePage.LastOpenToCreated")
+            .c_str(),
+        (iter->second.last_access_time - iter->second.creation_time)
+            .InMinutes(),
+        1, base::TimeDelta::FromDays(365).InMinutes(), 100);
     UMA_HISTOGRAM_MEMORY_KB(
-        AddHistogramSuffix(
-            client_id, "OfflinePages.DeletePage.PageSize").c_str(),
+        AddHistogramSuffix(client_id, "OfflinePages.DeletePage.PageSize")
+            .c_str(),
         iter->second.file_size / 1024);
     UMA_HISTOGRAM_COUNTS(
-        AddHistogramSuffix(
-            client_id, "OfflinePages.DeletePage.AccessCount").c_str(),
+        AddHistogramSuffix(client_id, "OfflinePages.DeletePage.AccessCount")
+            .c_str(),
         iter->second.access_count);
     FOR_EACH_OBSERVER(
         Observer, observers_,
@@ -744,8 +740,8 @@ void OfflinePageModel::OnRemoveOfflinePagesDone(
   if (offline_ids.size() > 1) {
     UMA_HISTOGRAM_COUNTS("OfflinePages.BatchDelete.Count",
                          static_cast<int32_t>(offline_ids.size()));
-    UMA_HISTOGRAM_MEMORY_KB(
-        "OfflinePages.BatchDelete.TotalPageSize", total_size / 1024);
+    UMA_HISTOGRAM_MEMORY_KB("OfflinePages.BatchDelete.TotalPageSize",
+                            total_size / 1024);
   }
   // Deleting multiple pages always succeeds when it gets to this point.
   InformDeletePageDone(callback, (success || offline_ids.size() > 1)
@@ -753,19 +749,19 @@ void OfflinePageModel::OnRemoveOfflinePagesDone(
                                      : DeletePageResult::STORE_FAILURE);
 }
 
-void OfflinePageModel::InformDeletePageDone(const DeletePageCallback& callback,
-                                            DeletePageResult result) {
-  UMA_HISTOGRAM_ENUMERATION(
-      "OfflinePages.DeletePageResult",
-      static_cast<int>(result),
-      static_cast<int>(DeletePageResult::RESULT_COUNT));
+void OfflinePageModelImpl::InformDeletePageDone(
+    const DeletePageCallback& callback,
+    DeletePageResult result) {
+  UMA_HISTOGRAM_ENUMERATION("OfflinePages.DeletePageResult",
+                            static_cast<int>(result),
+                            static_cast<int>(DeletePageResult::RESULT_COUNT));
   archive_manager_->GetStorageStats(
       base::Bind(&ReportStorageHistogramsAfterDelete));
   if (!callback.is_null())
     callback.Run(result);
 }
 
-void OfflinePageModel::ScanForMissingArchiveFiles(
+void OfflinePageModelImpl::ScanForMissingArchiveFiles(
     const std::set<base::FilePath>& archive_paths) {
   std::vector<int64_t> ids_of_pages_missing_archive_file;
   std::vector<std::pair<int64_t, ClientId>> offline_client_id_pairs;
@@ -781,19 +777,19 @@ void OfflinePageModel::ScanForMissingArchiveFiles(
   if (ids_of_pages_missing_archive_file.empty())
     return;
 
-  DeletePageCallback remove_pages_done_callback(
-      base::Bind(&OfflinePageModel::OnRemoveOfflinePagesMissingArchiveFileDone,
-                 weak_ptr_factory_.GetWeakPtr(), offline_client_id_pairs));
+  DeletePageCallback remove_pages_done_callback(base::Bind(
+      &OfflinePageModelImpl::OnRemoveOfflinePagesMissingArchiveFileDone,
+      weak_ptr_factory_.GetWeakPtr(), offline_client_id_pairs));
 
   store_->RemoveOfflinePages(
       ids_of_pages_missing_archive_file,
-      base::Bind(&OfflinePageModel::OnRemoveOfflinePagesDone,
+      base::Bind(&OfflinePageModelImpl::OnRemoveOfflinePagesDone,
                  weak_ptr_factory_.GetWeakPtr(),
                  ids_of_pages_missing_archive_file,
                  remove_pages_done_callback));
 }
 
-void OfflinePageModel::OnRemoveOfflinePagesMissingArchiveFileDone(
+void OfflinePageModelImpl::OnRemoveOfflinePagesMissingArchiveFileDone(
     const std::vector<std::pair<int64_t, ClientId>>& offline_client_id_pairs,
     DeletePageResult /* result */) {
   for (const auto& id_pair : offline_client_id_pairs) {
@@ -802,56 +798,51 @@ void OfflinePageModel::OnRemoveOfflinePagesMissingArchiveFileDone(
   }
 }
 
-void OfflinePageModel::OnRemoveAllFilesDoneForClearAll(
+void OfflinePageModelImpl::OnRemoveAllFilesDoneForClearAll(
     const base::Closure& callback,
     DeletePageResult result) {
-  store_->Reset(base::Bind(&OfflinePageModel::OnResetStoreDoneForClearAll,
-                           weak_ptr_factory_.GetWeakPtr(),
-                           callback));
+  store_->Reset(base::Bind(&OfflinePageModelImpl::OnResetStoreDoneForClearAll,
+                           weak_ptr_factory_.GetWeakPtr(), callback));
 }
 
-void OfflinePageModel::OnResetStoreDoneForClearAll(
-    const base::Closure& callback, bool success) {
+void OfflinePageModelImpl::OnResetStoreDoneForClearAll(
+    const base::Closure& callback,
+    bool success) {
   DCHECK(success);
   if (!success) {
     UMA_HISTOGRAM_ENUMERATION("OfflinePages.ClearAllStatus2",
-                              STORE_RESET_FAILED,
-                              CLEAR_ALL_STATUS_COUNT);
+                              STORE_RESET_FAILED, CLEAR_ALL_STATUS_COUNT);
   }
 
   offline_pages_.clear();
-  store_->Load(base::Bind(&OfflinePageModel::OnReloadStoreDoneForClearAll,
-                          weak_ptr_factory_.GetWeakPtr(),
-                          callback));
+  store_->Load(base::Bind(&OfflinePageModelImpl::OnReloadStoreDoneForClearAll,
+                          weak_ptr_factory_.GetWeakPtr(), callback));
 }
 
-void OfflinePageModel::OnReloadStoreDoneForClearAll(
+void OfflinePageModelImpl::OnReloadStoreDoneForClearAll(
     const base::Closure& callback,
     OfflinePageMetadataStore::LoadStatus load_status,
     const std::vector<OfflinePageItem>& offline_pages) {
   DCHECK_EQ(OfflinePageMetadataStore::LOAD_SUCCEEDED, load_status);
   UMA_HISTOGRAM_ENUMERATION(
       "OfflinePages.ClearAllStatus2",
-      load_status == OfflinePageMetadataStore::LOAD_SUCCEEDED ?
-          CLEAR_ALL_SUCCEEDED : STORE_RELOAD_FAILED,
+      load_status == OfflinePageMetadataStore::LOAD_SUCCEEDED
+          ? CLEAR_ALL_SUCCEEDED
+          : STORE_RELOAD_FAILED,
       CLEAR_ALL_STATUS_COUNT);
 
   CacheLoadedData(offline_pages);
   callback.Run();
 }
 
-void OfflinePageModel::CacheLoadedData(
+void OfflinePageModelImpl::CacheLoadedData(
     const std::vector<OfflinePageItem>& offline_pages) {
   offline_pages_.clear();
   for (const auto& offline_page : offline_pages)
     offline_pages_[offline_page.offline_id] = offline_page;
 }
 
-int64_t OfflinePageModel::GenerateOfflineId() {
-  return base::RandGenerator(std::numeric_limits<int64_t>::max()) + 1;
-}
-
-void OfflinePageModel::RunWhenLoaded(const base::Closure& task) {
+void OfflinePageModelImpl::RunWhenLoaded(const base::Closure& task) {
   if (!is_loaded_) {
     delayed_tasks_.push_back(task);
     return;