Splits the OfflinePageModel into and interface and and implementation class.

components/offline_pages/offline_page_model_impl.h

Index: components/offline_pages/offline_page_model_impl.h
diff --git a/components/offline_pages/offline_page_model.h b/components/offline_pages/offline_page_model_impl.h
similarity index 52%
copy from components/offline_pages/offline_page_model.h
copy to components/offline_pages/offline_page_model_impl.h
index 0e336ccf4144bdf21a1cb799879efa31d07dbb7f..28b106b423a4595af9c956e0d2443ecf8c76dbc3 100644
--- a/components/offline_pages/offline_page_model.h
+++ b/components/offline_pages/offline_page_model_impl.h
@@ -1,15 +1,17 @@
-// 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.
 
-#ifndef COMPONENTS_OFFLINE_PAGES_OFFLINE_PAGE_MODEL_H_
-#define COMPONENTS_OFFLINE_PAGES_OFFLINE_PAGE_MODEL_H_
+#ifndef COMPONENTS_OFFLINE_PAGES_OFFLINE_PAGE_MODEL_IMPL_H_
+#define COMPONENTS_OFFLINE_PAGES_OFFLINE_PAGE_MODEL_IMPL_H_
 
 #include <stdint.h>
 
 #include <map>
 #include <memory>
 #include <set>
+#include <string>
+#include <utility>
 #include <vector>
 
 #include "base/callback.h"
@@ -25,6 +27,7 @@
 #include "components/keyed_service/core/keyed_service.h"
 #include "components/offline_pages/offline_page_archiver.h"
 #include "components/offline_pages/offline_page_metadata_store.h"
+#include "components/offline_pages/offline_page_model.h"
 #include "components/offline_pages/offline_page_storage_manager.h"
 #include "components/offline_pages/offline_page_types.h"
 
@@ -38,9 +41,6 @@ class TimeTicks;
 
 namespace offline_pages {
 
-static const char* const kBookmarkNamespace = "bookmark";
-static const int64_t kInvalidOfflineId = 0;
-
 struct ClientId;
 struct OfflinePageItem;
 
@@ -49,194 +49,78 @@ class ClientPolicyController;
 class OfflinePageMetadataStore;
 class OfflinePageStorageManager;
 
-// Service for saving pages offline, storing the offline copy and metadata, and
-// retrieving them upon request.
-//
-// Example usage:
-//   class ArchiverImpl : public OfflinePageArchiver {
-//     // This is a class that knows how to create archiver
-//     void CreateArchiver(...) override;
-//     ...
-//   }
-//
-//   // In code using the OfflinePagesModel to save a page:
-//   std::unique_ptr<ArchiverImpl> archiver(new ArchiverImpl());
-//   // Callback is of type SavePageCallback.
-//   model->SavePage(url, std::move(archiver), callback);
-//
-// TODO(fgorski): Things to describe:
-// * how to cancel requests and what to expect
-class OfflinePageModel : public KeyedService,
-                         public base::SupportsUserData,
-                         public OfflinePageStorageManager::Client {
+// Implementation of service for saving pages offline, storing the offline
+// copy and metadata, and retrieving them upon request.
+class OfflinePageModelImpl : public OfflinePageModel {
  public:
-  // Observer of the OfflinePageModel.
-  class Observer {
-   public:
-    // Invoked when the model has finished loading.
-    virtual void OfflinePageModelLoaded(OfflinePageModel* model) = 0;
-
-    // Invoked when the model is being updated, due to adding, removing or
-    // updating an offline page.
-    virtual void OfflinePageModelChanged(OfflinePageModel* model) = 0;
-
-    // Invoked when an offline copy related to |offline_id| was deleted.
-    // In can be invoked as a result of |CheckForExternalFileDeletion|, if a
-    // deleted page is detected.
-    virtual void OfflinePageDeleted(int64_t offline_id,
-                                    const ClientId& client_id) = 0;
-
-   protected:
-    virtual ~Observer() {}
-  };
-
-  using CheckPagesExistOfflineResult =
-      offline_pages::CheckPagesExistOfflineResult;
-  using MultipleOfflinePageItemResult =
-      offline_pages::MultipleOfflinePageItemResult;
-  using SingleOfflinePageItemResult =
-      offline_pages::SingleOfflinePageItemResult;
-
-  //using DeletePageCallback = offline_pages::DeletePageCallback;
-  using DeletePageResult = offline_pages::DeletePageResult;
-  using SavePageResult = offline_pages::SavePageResult;
-
-  // Generates a new offline id
-  static int64_t GenerateOfflineId();
-
-  // Returns true if an offline copy can be saved for the given URL.
-  static bool CanSavePage(const GURL& url);
-
-  static base::TimeDelta GetFinalDeletionDelayForTesting();
-
   // All blocking calls/disk access will happen on the provided |task_runner|.
-  OfflinePageModel(std::unique_ptr<OfflinePageMetadataStore> store,
-                   const base::FilePath& archives_dir,
-                   const scoped_refptr<base::SequencedTaskRunner>& task_runner);
-  ~OfflinePageModel() override;
-
-  void AddObserver(Observer* observer);
-  void RemoveObserver(Observer* observer);
-
-  // Attempts to save a page addressed by |url| offline. Requires that the model
-  // is loaded.  Generates a new offline id and returns it.
+  OfflinePageModelImpl(
+      std::unique_ptr<OfflinePageMetadataStore> store,
+      const base::FilePath& archives_dir,
+      const scoped_refptr<base::SequencedTaskRunner>& task_runner);
+  ~OfflinePageModelImpl() override;
+
+  // Implemented methods:
+  void AddObserver(Observer* observer) override;
+  void RemoveObserver(Observer* observer) override;
   void SavePage(const GURL& url,
                 const ClientId& client_id,
                 std::unique_ptr<OfflinePageArchiver> archiver,
-                const SavePageCallback& callback);
-
-  // Marks that the offline page related to the passed |offline_id| has been
-  // accessed. Its access info, including last access time and access count,
-  // will be updated. Requires that the model is loaded.
-  void MarkPageAccessed(int64_t offline_id);
-
-  // Deletes an offline page related to the passed |offline_id|.
-  void DeletePageByOfflineId(int64_t offline_id,

[romax, 2016/05/26 01:26:48]

Why is this removed? I think this is some kind of variant of the
DeletePagesByOfflineId and probably fine to be removed, but just want to make
sure if it's not used by other code.

[dougarnett, 2016/05/26 16:03:22]

I was not able to find any implementation nor usage of it. Maybe another
reviewer will know some history on it.

-                             const DeletePageCallback& callback);
-
-  // Deletes offline pages related to the passed |offline_ids|.
+                const SavePageCallback& callback) override;
+  void MarkPageAccessed(int64_t offline_id) override;
   void DeletePagesByOfflineId(const std::vector<int64_t>& offline_ids,
                               const DeletePageCallback& callback) override;
-
-  // Wipes out all the data by deleting all saved files and clearing the store.
   void ClearAll(const base::Closure& callback);
-
-  // Deletes offline pages matching the URL predicate.
   void DeletePagesByURLPredicate(const UrlPredicate& predicate,
-                                 const DeletePageCallback& callback);
-
-  // Returns true via callback if there are offline pages in the given
-  // |name_space|.
+                                 const DeletePageCallback& callback) override;
   void HasPages(const std::string& name_space,
-                const HasPagesCallback& callback);
-
-  // Returns via callback all GURLs in |urls| that are equal to the online URL
-  // of any offline page.
-  void CheckPagesExistOffline(const std::set<GURL>& urls,
-                              const CheckPagesExistOfflineCallback& callback);
-
-  // Gets all available offline pages.
+                const HasPagesCallback& callback) override;
+  void CheckPagesExistOffline(
+      const std::set<GURL>& urls,
+      const CheckPagesExistOfflineCallback& callback) override;
   void GetAllPages(const MultipleOfflinePageItemCallback& callback) override;
-
-  // Gets all offline ids where the offline page has the matching client id.
-  void GetOfflineIdsForClientId(const ClientId& client_id,
-                                const MultipleOfflineIdCallback& callback);
-
-  // Gets all offline ids where the offline page has the matching client id.
-  // Requires that the model is loaded.  May not return matching IDs depending
-  // on the internal state of the model.
-  //
-  // This function is deprecated.  Use |GetOfflineIdsForClientId| instead.
+  void GetOfflineIdsForClientId(
+      const ClientId& client_id,
+      const MultipleOfflineIdCallback& callback) override;
   const std::vector<int64_t> MaybeGetOfflineIdsForClientId(
-      const ClientId& client_id) const;
-
-  // Returns zero or one offline pages associated with a specified |offline_id|.
-  void GetPageByOfflineId(int64_t offline_id,
-                          const SingleOfflinePageItemCallback& callback);
-
-  // Returns an offline page associated with a specified |offline_id|. nullptr
-  // is returned if not found.
-  const OfflinePageItem* MaybeGetPageByOfflineId(int64_t offline_id) const;
-
-  // Returns the offline page that is stored under |offline_url|, if any.
-  void GetPageByOfflineURL(const GURL& offline_url,
-                           const SingleOfflinePageItemCallback& callback);
-
-  // Returns an offline page that is stored as |offline_url|. A nullptr is
-  // returned if not found.
-  //
-  // This function is deprecated, and may return |nullptr| even if a page
-  // exists, depending on the implementation details of OfflinePageModel.
-  // Use |GetPageByOfflineURL| instead.
+      const ClientId& client_id) const override;
+  void GetPageByOfflineId(
+      int64_t offline_id,
+      const SingleOfflinePageItemCallback& callback) override;
+  const OfflinePageItem* MaybeGetPageByOfflineId(
+      int64_t offline_id) const override;
+  void GetPageByOfflineURL(
+      const GURL& offline_url,
+      const SingleOfflinePageItemCallback& callback) override;
   const OfflinePageItem* MaybeGetPageByOfflineURL(
-      const GURL& offline_url) const;
-
-  // Returns the offline pages that are stored under |online_url|.
-  void GetPagesByOnlineURL(const GURL& online_url,
-                           const MultipleOfflinePageItemCallback& callback);
-
-  // Returns via callback an offline page saved for |online_url|, if any. The
-  // best page is chosen based on creation date; a more recently created offline
-  // page will be preferred over an older one. This API function does not
-  // respect namespaces, as it is used to choose which page is rendered in a
-  // tab. Today all namespaces are treated equally for the purposes of this
-  // selection.
-  void GetBestPageForOnlineURL(const GURL& online_url,
-                               const SingleOfflinePageItemCallback callback);
-
-  // Returns an offline page saved for |online_url|. A nullptr is returned if
-  // not found.  See |GetBestPageForOnlineURL| for selection criteria.
+      const GURL& offline_url) const override;
+  void GetPagesByOnlineURL(
+      const GURL& online_url,
+      const MultipleOfflinePageItemCallback& callback) override;
+  void GetBestPageForOnlineURL(
+      const GURL& online_url,
+      const SingleOfflinePageItemCallback callback) override;
   const OfflinePageItem* MaybeGetBestPageForOnlineURL(
-      const GURL& online_url) const;
-
-  // Checks that all of the offline pages have corresponding offline copies.
-  // If a page is discovered to be missing an offline copy, its offline page
-  // metadata will be removed and |OfflinePageDeleted| will be sent to model
-  // observers.
-  void CheckForExternalFileDeletion();
-
-  // Marks pages as expired and removes their respective files from the archive
-  // directory.
+      const GURL& online_url) const override;
+  void CheckForExternalFileDeletion() override;
   void ExpirePages(const std::vector<int64_t>& offline_ids,
-                   const base::Time& expiration_time);
-
-  // Returns the policy controller.
-  ClientPolicyController* GetPolicyController();
+                   const base::Time& expiration_time) override;
+  ClientPolicyController* GetPolicyController() override;
 
   // Methods for testing only:
   OfflinePageMetadataStore* GetStoreForTesting();
 
   OfflinePageStorageManager* GetStorageManager();
 
-  bool is_loaded() const;
+  bool is_loaded() const override;
 
  protected:
   // Adding a protected constructor for testing-only purposes in
   // offline_page_storage_manager_unittest.cc
-  OfflinePageModel();
+  OfflinePageModelImpl();
 
  private:
-  FRIEND_TEST_ALL_PREFIXES(OfflinePageModelTest, MarkPageForDeletion);
+  FRIEND_TEST_ALL_PREFIXES(OfflinePageModelImplTest, MarkPageForDeletion);
 
   typedef ScopedVector<OfflinePageArchiver> PendingArchivers;
 
@@ -371,11 +255,11 @@ class OfflinePageModel : public KeyedService,
   // Manager for the offline archive files and directory.
   std::unique_ptr<ArchiveManager> archive_manager_;
 
-  base::WeakPtrFactory<OfflinePageModel> weak_ptr_factory_;
+  base::WeakPtrFactory<OfflinePageModelImpl> weak_ptr_factory_;
 
-  DISALLOW_COPY_AND_ASSIGN(OfflinePageModel);
+  DISALLOW_COPY_AND_ASSIGN(OfflinePageModelImpl);
 };
 
 }  // namespace offline_pages
 
-#endif  // COMPONENTS_OFFLINE_PAGES_OFFLINE_PAGE_MODEL_H_
+#endif  // COMPONENTS_OFFLINE_PAGES_OFFLINE_PAGE_MODEL_IMPL_H_