Prefetching: Introduce store commands abstractions to be used by tasks.

Updates AddUniqueUrlsTask (still empty of actual business logic) to use store
commands as an abstraction layer for the execution of operations on the
persistent prefetching store. Only concrete test version of commands are
provided; actual SQL backer implementations will come later.

This also adds all the business logic needed for that task to work as the 1st
step of the prefetching pipeline but for NWake scheduling. The latter will
require a new abstraction to be created and will be left for a follow up patch.

BUG=701939

Review-Url: https://codereview.chromium.org/2920083002
Cr-Commit-Position: refs/heads/master@{#480292}

chrome/browser/offline_pages/prefetch/prefetch_service_factory.cc

@@ -14,7 +14,6 @@
 #include "components/keyed_service/content/browser_context_dependency_manager.h"
 #include "components/offline_pages/core/prefetch/prefetch_dispatcher_impl.h"
 #include "components/offline_pages/core/prefetch/prefetch_gcm_app_handler.h"
-#include "components/offline_pages/core/prefetch/prefetch_in_memory_store.h"
 #include "components/offline_pages/core/prefetch/prefetch_service_impl.h"
 #include "components/offline_pages/core/prefetch/suggested_articles_observer.h"
 #include "content/public/browser/browser_context.h"
@@ -47,12 +46,11 @@ KeyedService* PrefetchServiceFactory::BuildServiceInstanceFor(
       base::MakeUnique<OfflineMetricsCollectorImpl>();
   auto suggested_articles_observer =
       base::MakeUnique<SuggestedArticlesObserver>(prefetch_dispatcher.get());
-  auto in_memory_store = base::MakeUnique<PrefetchInMemoryStore>();
 
-  return new PrefetchServiceImpl(
-      std::move(offline_metrics_collector), std::move(prefetch_dispatcher),
-      std::move(prefetch_gcm_app_handler), std::move(in_memory_store),
-      std::move(suggested_articles_observer));
+  return new PrefetchServiceImpl(std::move(offline_metrics_collector),
+                                 std::move(prefetch_dispatcher),
+                                 std::move(prefetch_gcm_app_handler),
+                                 std::move(suggested_articles_observer));
 }
 
 }  // namespace offline_pages

components/offline_pages/core/prefetch/BUILD.gn

@@ -22,8 +22,6 @@ static_library("prefetch") {
     "prefetch_gcm_app_handler.cc",
     "prefetch_gcm_app_handler.h",
     "prefetch_gcm_handler.h",
-    "prefetch_in_memory_store.cc",
-    "prefetch_in_memory_store.h",
     "prefetch_item.cc",
     "prefetch_item.h",
     "prefetch_proto_utils.cc",
@@ -33,7 +31,6 @@ static_library("prefetch") {
     "prefetch_service.h",
     "prefetch_service_impl.cc",
     "prefetch_service_impl.h",
-    "prefetch_store.h",
     "prefetch_types.cc",
     "prefetch_types.h",
     "suggested_articles_observer.cc",

components/offline_pages/core/prefetch/README.md

-# Prefetching Offline Pages: development guidelines
+# Prefetching Offline Pages
 
-* Implementations that are injected dependencies should always provide
-  lightweight construction and postpone heavier initialization (i.e. DB
+## Architecture overview
+
+### PrefetchService
+
+Is the ownership holder for the main components of the prefetching system and
+controls their lifetime.
+
+### PrefetchDispatcher
+
+Manages the prefetching pipeline tasks. It receives signals from external
+clients and creates the appropriate tasks to execute them. It _might_ at some
+point execute advanced task management operations like canceling queued tasks or
+changing their order of execution.
+
+### \*Task(s) (i.e. AddUniqueUrlsTask)
+
+They are the main wrapper of pipeline steps and interact with different 
+abstracted components (Downloads, persistent store, GCM, etc) to execute them.
+They implement TaskQueue's Task API so that they can be exclusively executed.
+
+## Development guidelines
+
+* Implementations that are injected dependencies during service creation should
+  have lightweight construction and postpone heavier initialization (i.e. DB
   connection) to a later moment. Lazy initialization upon first actual usage is
   recommended.

components/offline_pages/core/prefetch/add_unique_urls_task.cc

@@ -4,21 +4,55 @@
 
 #include "components/offline_pages/core/prefetch/add_unique_urls_task.h"
 
+#include <memory>
+#include <utility>
+
 #include "base/bind.h"
+#include "base/callback.h"
+#include "url/gurl.h"
 
 namespace offline_pages {
 
+namespace {
+
+// Adds new prefetch item entries to the store using the URLs and client IDs
+// from |prefetch_urls| and the client's |name_space|. Also cleans up entries in
+// the Zombie state from the client's |name_space| except for the ones
+// whose URL is contained in |prefetch_urls|.
+// Returns the number of added prefecth items.
+static int AddUrlsAndCleanupZombies(
+    const std::string& name_space,
+    const std::vector<PrefetchURL>& prefetch_urls) {
+  NOTIMPLEMENTED();
+  return 1;
+}
+
+// TODO(fgorski): replace this with the SQL executor.
+static void Execute(base::RepeatingCallback<int()> command_callback,
+                    base::OnceCallback<void(int)> result_callback) {
+  std::move(result_callback).Run(command_callback.Run());
+}
+}
+
 AddUniqueUrlsTask::AddUniqueUrlsTask(
-    PrefetchStore* store,
+    const std::string& name_space,
     const std::vector<PrefetchURL>& prefetch_urls)
-    : prefetch_store_(store),
+    : name_space_(name_space),
       prefetch_urls_(prefetch_urls),
       weak_ptr_factory_(this) {}
 
 AddUniqueUrlsTask::~AddUniqueUrlsTask() {}
 
 void AddUniqueUrlsTask::Run() {
-  CHECK(prefetch_store_);
+  Execute(base::BindRepeating(&AddUrlsAndCleanupZombies, name_space_,
+                              prefetch_urls_),
+          base::BindOnce(&AddUniqueUrlsTask::OnUrlsAdded,
+                         weak_ptr_factory_.GetWeakPtr()));
+}
+
+void AddUniqueUrlsTask::OnUrlsAdded(int added_entry_count) {
+  // TODO(carlosk): schedule NWake here if at least one new entry was added to
+  // the store.
   TaskComplete();
 }
 

components/offline_pages/core/prefetch/add_unique_urls_task.h

@@ -5,13 +5,12 @@
 #ifndef COMPONENTS_OFFLINE_PAGES_CORE_PREFETCH_ADD_UNIQUE_URLS_TASK_H_
 #define COMPONENTS_OFFLINE_PAGES_CORE_PREFETCH_ADD_UNIQUE_URLS_TASK_H_
 
+#include <string>
 #include <vector>
 
 #include "base/memory/weak_ptr.h"
-#include "components/offline_pages/core/prefetch/prefetch_store.h"
 #include "components/offline_pages/core/prefetch/prefetch_types.h"
 #include "components/offline_pages/core/task.h"
-#include "url/gurl.h"
 
 namespace offline_pages {
 
@@ -26,14 +25,16 @@ namespace offline_pages {
 // from the store.
 class AddUniqueUrlsTask : public Task {
  public:
-  AddUniqueUrlsTask(PrefetchStore* store,
+  AddUniqueUrlsTask(const std::string& name_space,
                     const std::vector<PrefetchURL>& prefetch_urls);
   ~AddUniqueUrlsTask() override;
 
   void Run() override;
 
  private:
-  PrefetchStore* prefetch_store_;
+  void OnUrlsAdded(int added_entry_count);
+
+  const std::string& name_space_;
   std::vector<PrefetchURL> prefetch_urls_;
 
   base::WeakPtrFactory<AddUniqueUrlsTask> weak_ptr_factory_;

components/offline_pages/core/prefetch/prefetch_dispatcher.h

@@ -45,6 +45,7 @@ class PrefetchDispatcher {
   // with the client's unique namespace. URLs that are currently in the system
   // for this client are acceptable but ignored.
   virtual void AddCandidatePrefetchURLs(
+      const std::string& name_space,
       const std::vector<PrefetchURL>& prefetch_urls) = 0;
 
   // Called when all existing suggestions are no longer considered valid for a

components/offline_pages/core/prefetch/prefetch_dispatcher_impl.cc

@@ -34,12 +34,13 @@ void PrefetchDispatcherImpl::SetService(PrefetchService* service) {
 }
 
 void PrefetchDispatcherImpl::AddCandidatePrefetchURLs(
+    const std::string& name_space,
     const std::vector<PrefetchURL>& prefetch_urls) {
   if (!IsPrefetchingOfflinePagesEnabled())
     return;
 
-  std::unique_ptr<Task> add_task = base::MakeUnique<AddUniqueUrlsTask>(
-      service_->GetPrefetchStore(), prefetch_urls);
+  std::unique_ptr<Task> add_task =
+      base::MakeUnique<AddUniqueUrlsTask>(name_space, prefetch_urls);
   task_queue_.AddTask(std::move(add_task));
 }
 

components/offline_pages/core/prefetch/prefetch_dispatcher_impl.h

@@ -24,6 +24,7 @@ class PrefetchDispatcherImpl : public PrefetchDispatcher {
   // PrefetchDispatcher implementation:
   void SetService(PrefetchService* service) override;
   void AddCandidatePrefetchURLs(
+      const std::string& name_space,
       const std::vector<PrefetchURL>& prefetch_urls) override;
   void RemoveAllUnprocessedPrefetchURLs(const std::string& name_space) override;
   void RemovePrefetchURLsByClientId(const ClientId& client_id) override;

components/offline_pages/core/prefetch/prefetch_dispatcher_impl_unittest.cc

@@ -12,12 +12,14 @@
 #include "components/offline_pages/core/client_namespace_constants.h"
 #include "components/offline_pages/core/offline_event_logger.h"
 #include "components/offline_pages/core/offline_page_feature.h"
-#include "components/offline_pages/core/prefetch/prefetch_in_memory_store.h"
 #include "components/offline_pages/core/prefetch/prefetch_service.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "url/gurl.h"
 
 namespace offline_pages {
 
+namespace {
+
 class TestScopedBackgroundTask
     : public PrefetchDispatcher::ScopedBackgroundTask {
  public:
@@ -31,8 +33,12 @@ class TestScopedBackgroundTask
   bool needs_reschedule_called = false;
 };
 
+}  // namespace
+
 class PrefetchDispatcherTest : public testing::Test, public PrefetchService {
  public:
+  const std::string TEST_NAMESPACE = "TestPrefetchClientNamespace";
+
   PrefetchDispatcherTest();
 
   // Test implementation.
@@ -44,7 +50,6 @@ class PrefetchDispatcherTest : public testing::Test, public PrefetchService {
   OfflineMetricsCollector* GetOfflineMetricsCollector() override;
   PrefetchDispatcher* GetPrefetchDispatcher() override;
   PrefetchGCMHandler* GetPrefetchGCMHandler() override;
-  PrefetchStore* GetPrefetchStore() override;
   SuggestedArticlesObserver* GetSuggestedArticlesObserver() override;
 
   // KeyedService implementation.
@@ -57,13 +62,13 @@ class PrefetchDispatcherTest : public testing::Test, public PrefetchService {
 
   TaskQueue* dispatcher_task_queue() { return &dispatcher_impl_->task_queue_; }
 
+ protected:
+  std::vector<PrefetchURL> test_urls_;
+
  private:
+  std::unique_ptr<PrefetchDispatcherImpl> dispatcher_impl_;
   OfflineEventLogger logger_;
   base::test::ScopedFeatureList feature_list_;
-
-  std::unique_ptr<PrefetchInMemoryStore> in_memory_store_;
-  std::unique_ptr<PrefetchDispatcherImpl> dispatcher_impl_;
-
   scoped_refptr<base::TestSimpleTaskRunner> task_runner_;
   base::ThreadTaskRunnerHandle task_runner_handle_;
 };
@@ -77,9 +82,12 @@ PrefetchDispatcherTest::PrefetchDispatcherTest()
 void PrefetchDispatcherTest::SetUp() {
   ASSERT_EQ(base::ThreadTaskRunnerHandle::Get(), task_runner_);
   ASSERT_FALSE(task_runner_->HasPendingTask());
-  in_memory_store_ = base::MakeUnique<PrefetchInMemoryStore>();
   dispatcher_impl_ = base::MakeUnique<PrefetchDispatcherImpl>();
   dispatcher_impl_->SetService(this);
+
+  ASSERT_TRUE(test_urls_.empty());
+  test_urls_.push_back({"1", GURL("http://testurl.com/foo")});
+  test_urls_.push_back({"2", GURL("https://testurl.com/bar")});
 }
 
 void PrefetchDispatcherTest::TearDown() {
@@ -104,10 +112,6 @@ PrefetchGCMHandler* PrefetchDispatcherTest::GetPrefetchGCMHandler() {
   return nullptr;
 }
 
-PrefetchStore* PrefetchDispatcherTest::GetPrefetchStore() {
-  return in_memory_store_.get();
-}
-
 SuggestedArticlesObserver*
 PrefetchDispatcherTest::GetSuggestedArticlesObserver() {
   NOTREACHED();
@@ -119,7 +123,7 @@ void PrefetchDispatcherTest::PumpLoop() {
 }
 
 TEST_F(PrefetchDispatcherTest, DispatcherDoesNotCrash) {
-  GetPrefetchDispatcher()->AddCandidatePrefetchURLs(std::vector<PrefetchURL>());
+  GetPrefetchDispatcher()->AddCandidatePrefetchURLs(TEST_NAMESPACE, test_urls_);
   GetPrefetchDispatcher()->RemoveAllUnprocessedPrefetchURLs(
       kSuggestedArticlesNamespace);
   GetPrefetchDispatcher()->RemovePrefetchURLsByClientId(
@@ -127,7 +131,7 @@ TEST_F(PrefetchDispatcherTest, DispatcherDoesNotCrash) {
 }
 
 TEST_F(PrefetchDispatcherTest, AddCandidatePrefetchURLsTask) {
-  GetPrefetchDispatcher()->AddCandidatePrefetchURLs(std::vector<PrefetchURL>());
+  GetPrefetchDispatcher()->AddCandidatePrefetchURLs(TEST_NAMESPACE, test_urls_);
   EXPECT_TRUE(dispatcher_task_queue()->HasPendingTasks());
   EXPECT_TRUE(dispatcher_task_queue()->HasRunningTask());
   PumpLoop();
@@ -140,10 +144,9 @@ TEST_F(PrefetchDispatcherTest, DispatcherDoesNothingIfFeatureNotEnabled) {
   disabled_feature_list.InitAndDisableFeature(kPrefetchingOfflinePagesFeature);
 
   // Don't add a task for new prefetch URLs.
-  ClientId client_id("namespace", "id");
-  PrefetchURL prefetch_url(client_id, GURL("https://www.chromium.org"));
+  PrefetchURL prefetch_url("id", GURL("https://www.chromium.org"));
   GetPrefetchDispatcher()->AddCandidatePrefetchURLs(
-      std::vector<PrefetchURL>(1, prefetch_url));
+      TEST_NAMESPACE, std::vector<PrefetchURL>(1, prefetch_url));
   EXPECT_FALSE(dispatcher_task_queue()->HasRunningTask());
 
   // Do nothing with a new background task.

components/offline_pages/core/prefetch/prefetch_in_memory_store.cc

-// Copyright 2017 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/core/prefetch/prefetch_in_memory_store.h"
-
-namespace offline_pages {
-
-PrefetchInMemoryStore::PrefetchInMemoryStore() = default;
-
-PrefetchInMemoryStore::~PrefetchInMemoryStore() = default;
-
-}  // namespace offline_pages
\ No newline at end of file

components/offline_pages/core/prefetch/prefetch_in_memory_store.h

-// Copyright 2017 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_CORE_PREFETCH_PREFETCH_IN_MEMORY_STORE_H_
-#define COMPONENTS_OFFLINE_PAGES_CORE_PREFETCH_PREFETCH_IN_MEMORY_STORE_H_
-
-#include "components/offline_pages/core/prefetch/prefetch_store.h"
-
-namespace offline_pages {
-
-// A PrefetchStore implementation that keeps all persisted data in memory.
-class PrefetchInMemoryStore : public PrefetchStore {
- public:
-  PrefetchInMemoryStore();
-  ~PrefetchInMemoryStore() override;
-};
-
-}  // namespace offline_pages
-
-#endif  // COMPONENTS_OFFLINE_PAGES_CORE_PREFETCH_PREFETCH_IN_MEMORY_STORE_H_

components/offline_pages/core/prefetch/prefetch_service.h

@@ -12,7 +12,6 @@ class OfflineEventLogger;
 class OfflineMetricsCollector;
 class PrefetchDispatcher;
 class PrefetchGCMHandler;
-class PrefetchStore;
 class SuggestedArticlesObserver;
 
 // Main class and entry point for the Offline Pages Prefetching feature, that
@@ -26,11 +25,10 @@ class PrefetchService : public KeyedService {
   // The service manages lifetime, hookup and initialization of Prefetch
   // system that consists of multiple specialized objects, all vended by this
   // service.
+  virtual OfflineEventLogger* GetLogger() = 0;
   virtual OfflineMetricsCollector* GetOfflineMetricsCollector() = 0;
   virtual PrefetchDispatcher* GetPrefetchDispatcher() = 0;
   virtual PrefetchGCMHandler* GetPrefetchGCMHandler() = 0;
-  virtual PrefetchStore* GetPrefetchStore() = 0;
-  virtual OfflineEventLogger* GetLogger() = 0;
 
   // May be |nullptr| in tests.  The PrefetchService does not depend on the
   // SuggestedArticlesObserver, it merely owns it for lifetime purposes.

components/offline_pages/core/prefetch/prefetch_service_impl.cc

@@ -11,7 +11,6 @@
 #include "components/offline_pages/core/prefetch/offline_metrics_collector.h"
 #include "components/offline_pages/core/prefetch/prefetch_dispatcher.h"
 #include "components/offline_pages/core/prefetch/prefetch_gcm_handler.h"
-#include "components/offline_pages/core/prefetch/prefetch_store.h"
 #include "components/offline_pages/core/prefetch/suggested_articles_observer.h"
 
 namespace offline_pages {
@@ -20,12 +19,10 @@ PrefetchServiceImpl::PrefetchServiceImpl(
     std::unique_ptr<OfflineMetricsCollector> offline_metrics_collector,
     std::unique_ptr<PrefetchDispatcher> dispatcher,
     std::unique_ptr<PrefetchGCMHandler> gcm_handler,
-    std::unique_ptr<PrefetchStore> store,
     std::unique_ptr<SuggestedArticlesObserver> suggested_articles_observer)
     : offline_metrics_collector_(std::move(offline_metrics_collector)),
       prefetch_dispatcher_(std::move(dispatcher)),
       prefetch_gcm_handler_(std::move(gcm_handler)),
-      prefetch_store_(std::move(store)),
       suggested_articles_observer_(std::move(suggested_articles_observer)) {
   prefetch_dispatcher_->SetService(this);
   prefetch_gcm_handler_->SetService(this);
@@ -45,10 +42,6 @@ PrefetchGCMHandler* PrefetchServiceImpl::GetPrefetchGCMHandler() {
   return prefetch_gcm_handler_.get();
 }
 
-PrefetchStore* PrefetchServiceImpl::GetPrefetchStore() {
-  return prefetch_store_.get();
-}
-
 SuggestedArticlesObserver* PrefetchServiceImpl::GetSuggestedArticlesObserver() {
   return suggested_articles_observer_.get();
 }

components/offline_pages/core/prefetch/prefetch_service_impl.h

@@ -23,7 +23,6 @@ class PrefetchServiceImpl : public PrefetchService {
       std::unique_ptr<OfflineMetricsCollector> offline_metrics_collector,
       std::unique_ptr<PrefetchDispatcher> dispatcher,
       std::unique_ptr<PrefetchGCMHandler> gcm_handler,
-      std::unique_ptr<PrefetchStore> store,
       std::unique_ptr<SuggestedArticlesObserver> suggested_articles_observer);
   ~PrefetchServiceImpl() override;
 
@@ -31,7 +30,6 @@ class PrefetchServiceImpl : public PrefetchService {
   OfflineMetricsCollector* GetOfflineMetricsCollector() override;
   PrefetchDispatcher* GetPrefetchDispatcher() override;
   PrefetchGCMHandler* GetPrefetchGCMHandler() override;
-  PrefetchStore* GetPrefetchStore() override;
   SuggestedArticlesObserver* GetSuggestedArticlesObserver() override;
   OfflineEventLogger* GetLogger() override;
 
@@ -44,7 +42,6 @@ class PrefetchServiceImpl : public PrefetchService {
   std::unique_ptr<OfflineMetricsCollector> offline_metrics_collector_;
   std::unique_ptr<PrefetchDispatcher> prefetch_dispatcher_;
   std::unique_ptr<PrefetchGCMHandler> prefetch_gcm_handler_;
-  std::unique_ptr<PrefetchStore> prefetch_store_;
   std::unique_ptr<SuggestedArticlesObserver> suggested_articles_observer_;
 
   DISALLOW_COPY_AND_ASSIGN(PrefetchServiceImpl);

components/offline_pages/core/prefetch/prefetch_service_test_taco.cc

@@ -10,7 +10,6 @@
 #include "components/offline_pages/core/prefetch/prefetch_dispatcher.h"
 #include "components/offline_pages/core/prefetch/prefetch_dispatcher_impl.h"
 #include "components/offline_pages/core/prefetch/prefetch_gcm_handler.h"
-#include "components/offline_pages/core/prefetch/prefetch_in_memory_store.h"
 #include "components/offline_pages/core/prefetch/prefetch_service_impl.h"
 #include "components/offline_pages/core/prefetch/suggested_articles_observer.h"
 #include "components/offline_pages/core/prefetch/test_offline_metrics_collector.h"
@@ -22,7 +21,6 @@ PrefetchServiceTestTaco::PrefetchServiceTestTaco() {
   metrics_collector_ = base::MakeUnique<TestOfflineMetricsCollector>();
   dispatcher_ = base::MakeUnique<PrefetchDispatcherImpl>();
   gcm_handler_ = base::MakeUnique<TestPrefetchGCMHandler>();
-  store_ = base::MakeUnique<PrefetchInMemoryStore>();
 }
 
 PrefetchServiceTestTaco::~PrefetchServiceTestTaco() = default;
@@ -45,12 +43,6 @@ void PrefetchServiceTestTaco::SetPrefetchGCMHandler(
   gcm_handler_ = std::move(gcm_handler);
 }
 
-void PrefetchServiceTestTaco::SetPrefetchStore(
-    std::unique_ptr<PrefetchStore> store) {
-  CHECK(!prefetch_service_);
-  store_ = std::move(store);
-}
-
 void PrefetchServiceTestTaco::SetSuggestedArticlesObserver(
     std::unique_ptr<SuggestedArticlesObserver> suggested_articles_observer) {
   CHECK(!prefetch_service_);
@@ -60,8 +52,7 @@ void PrefetchServiceTestTaco::SetSuggestedArticlesObserver(
 void PrefetchServiceTestTaco::CreatePrefetchService() {
   prefetch_service_ = base::MakeUnique<PrefetchServiceImpl>(
       std::move(metrics_collector_), std::move(dispatcher_),
-      std::move(gcm_handler_), std::move(store_),
-      std::move(suggested_articles_observer_));
+      std::move(gcm_handler_), std::move(suggested_articles_observer_));
 }
 
 }  // namespace offline_page

components/offline_pages/core/prefetch/prefetch_service_test_taco.h

@@ -14,7 +14,6 @@ class OfflineMetricsCollector;
 class PrefetchDispatcher;
 class PrefetchGCMHandler;
 class PrefetchService;
-class PrefetchStore;
 class SuggestedArticlesObserver;
 
 // The taco class acts as a wrapper around the prefetch service making
@@ -27,7 +26,6 @@ class SuggestedArticlesObserver;
 // * TestOfflineMetricsCollector
 // * PrefetchDispatcherImpl
 // * TestPrefetchGCMHandler
-// * PrefetchInMemoryStore
 // * |nullptr| SuggestedArticlesObserver, since this is default just a lifetime
 //   arrangement.
 class PrefetchServiceTestTaco {
@@ -41,7 +39,6 @@ class PrefetchServiceTestTaco {
       std::unique_ptr<OfflineMetricsCollector> metrics_collector);
   void SetPrefetchDispatcher(std::unique_ptr<PrefetchDispatcher> dispatcher);
   void SetPrefetchGCMHandler(std::unique_ptr<PrefetchGCMHandler> gcm_handler);
-  void SetPrefetchStore(std::unique_ptr<PrefetchStore> prefetch_store);
   void SetSuggestedArticlesObserver(
       std::unique_ptr<SuggestedArticlesObserver> suggested_articles_observer);
 
@@ -61,7 +58,6 @@ class PrefetchServiceTestTaco {
   std::unique_ptr<OfflineMetricsCollector> metrics_collector_;
   std::unique_ptr<PrefetchDispatcher> dispatcher_;
   std::unique_ptr<PrefetchGCMHandler> gcm_handler_;
-  std::unique_ptr<PrefetchStore> store_;
   std::unique_ptr<SuggestedArticlesObserver> suggested_articles_observer_;
 
   std::unique_ptr<PrefetchService> prefetch_service_;

components/offline_pages/core/prefetch/prefetch_store.h

-// Copyright 2017 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_CORE_PREFETCH_PREFETCH_STORE_H_
-#define COMPONENTS_OFFLINE_PAGES_CORE_PREFETCH_PREFETCH_STORE_H_
-
-namespace offline_pages {
-
-// Persistent storage access class for prefetching offline pages data.
-class PrefetchStore {
- public:
-  virtual ~PrefetchStore() = default;
-};
-
-}  // namespace offline_pages
-
-#endif  // COMPONENTS_OFFLINE_PAGES_CORE_PREFETCH_PREFETCH_STORE_H_

components/offline_pages/core/prefetch/prefetch_types.h

@@ -109,15 +109,15 @@ using PrefetchRequestFinishedCallback =
                         const std::string& operation_name,
                         const std::vector<RenderPageInfo>& pages)>;
 
-// Holds information about a new URL to be prefetched.
+// Holds information about a suggested URL to be prefetched.
 struct PrefetchURL {
-  PrefetchURL(const ClientId& client_id, const GURL& url)
-      : client_id(client_id), url(url) {}
+  PrefetchURL(const std::string& id, const GURL& url) : id(id), url(url) {}
 
-  // Client provided ID to allow the matching of URLs to the respective work
-  // item in the prefetching system. It can be anything useful to identify the
-  // page . It will not be used internally for de-duplication.
-  ClientId client_id;
+  // Client provided ID to allow the matching of provided URLs to the respective
+  // work item in the prefetching system within that client's assigned
+  // namespace. It can be any string value and it will not be used internally
+  // for de-duplication.
+  std::string id;
 
   // This URL will be prefetched by the service.
   GURL url;

components/offline_pages/core/prefetch/suggested_articles_observer.cc

@@ -28,10 +28,6 @@ const ntp_snippets::Category& ArticlesCategory() {
   return articles;
 }
 
-ClientId CreateClientIDFromSuggestionId(const ContentSuggestion::ID& id) {
-  return ClientId(kSuggestedArticlesNamespace, id.id_within_category());
-}
-
 }  // namespace
 
 SuggestedArticlesObserver::SuggestedArticlesObserver(
@@ -71,10 +67,11 @@ void SuggestedArticlesObserver::OnNewSuggestions(Category category) {
   std::vector<PrefetchURL> prefetch_urls;
   for (const ContentSuggestion& suggestion : suggestions) {
     prefetch_urls.push_back(
-        {CreateClientIDFromSuggestionId(suggestion.id()), suggestion.url()});
+        {suggestion.id().id_within_category(), suggestion.url()});
   }
 
-  prefetch_dispatcher_->AddCandidatePrefetchURLs(prefetch_urls);
+  prefetch_dispatcher_->AddCandidatePrefetchURLs(kSuggestedArticlesNamespace,
+                                                 prefetch_urls);
 }
 
 void SuggestedArticlesObserver::OnCategoryStatusChanged(
@@ -96,8 +93,8 @@ void SuggestedArticlesObserver::OnCategoryStatusChanged(
 
 void SuggestedArticlesObserver::OnSuggestionInvalidated(
     const ContentSuggestion::ID& suggestion_id) {
-  prefetch_dispatcher_->RemovePrefetchURLsByClientId(
-      CreateClientIDFromSuggestionId(suggestion_id));
+  prefetch_dispatcher_->RemovePrefetchURLsByClientId(ClientId(
+      kSuggestedArticlesNamespace, suggestion_id.id_within_category()));
 }
 
 void SuggestedArticlesObserver::OnFullRefreshRequired() {

components/offline_pages/core/prefetch/suggested_articles_observer_unittest.cc

@@ -76,9 +76,8 @@ TEST_F(OfflinePageSuggestedArticlesObserverTest,
   EXPECT_EQ(1U, test_prefetch_dispatcher()->latest_prefetch_urls.size());
   EXPECT_EQ(test_url_1,
             test_prefetch_dispatcher()->latest_prefetch_urls[0].url);
-  EXPECT_EQ(
-      kSuggestedArticlesNamespace,
-      test_prefetch_dispatcher()->latest_prefetch_urls[0].client_id.name_space);
+  EXPECT_EQ(kSuggestedArticlesNamespace,
+            test_prefetch_dispatcher()->latest_name_space);
 }
 
 TEST_F(OfflinePageSuggestedArticlesObserverTest, RemovesAllOnBadStatus) {
@@ -119,9 +118,8 @@ TEST_F(OfflinePageSuggestedArticlesObserverTest, RemovesClientIdOnInvalidated) {
   EXPECT_NE(nullptr, test_prefetch_dispatcher()->last_removed_client_id.get());
   EXPECT_EQ(test_url_1.spec(),
             test_prefetch_dispatcher()->last_removed_client_id->id);
-  EXPECT_EQ(
-      kSuggestedArticlesNamespace,
-      test_prefetch_dispatcher()->latest_prefetch_urls[0].client_id.name_space);
+  EXPECT_EQ(kSuggestedArticlesNamespace,
+            test_prefetch_dispatcher()->latest_name_space);
 }
 
 }  // namespace offline_pages

components/offline_pages/core/prefetch/test_prefetch_dispatcher.cc

@@ -13,8 +13,10 @@ TestPrefetchDispatcher::TestPrefetchDispatcher() = default;
 TestPrefetchDispatcher::~TestPrefetchDispatcher() = default;
 
 void TestPrefetchDispatcher::AddCandidatePrefetchURLs(
-    const std::vector<PrefetchURL>& suggested_urls) {
-  latest_prefetch_urls = suggested_urls;
+    const std::string& name_space,
+    const std::vector<PrefetchURL>& prefetch_urls) {
+  latest_name_space = name_space;
+  latest_prefetch_urls = prefetch_urls;
   new_suggestions_count++;
 }
 

components/offline_pages/core/prefetch/test_prefetch_dispatcher.h

@@ -22,7 +22,8 @@ class TestPrefetchDispatcher : public PrefetchDispatcher {
 
   // PrefetchDispatcher implementation.
   void AddCandidatePrefetchURLs(
-      const std::vector<PrefetchURL>& suggested_urls) override;
+      const std::string& name_space,
+      const std::vector<PrefetchURL>& prefetch_urls) override;
   void RemoveAllUnprocessedPrefetchURLs(const std::string& name_space) override;
   void RemovePrefetchURLsByClientId(const ClientId& client_id) override;
   void BeginBackgroundTask(std::unique_ptr<ScopedBackgroundTask> task) override;
@@ -32,6 +33,7 @@ class TestPrefetchDispatcher : public PrefetchDispatcher {
       const std::string& operation_name) override;
   void RequestFinishBackgroundTaskForTest() override;
 
+  std::string latest_name_space;
   std::vector<PrefetchURL> latest_prefetch_urls;
   std::unique_ptr<ClientId> last_removed_client_id;
   std::vector<std::string> operation_list;