[NTP Snippets] Refactor background scheduling for remote suggestions

components/ntp_snippets/remote/remote_suggestions_provider_impl_unittest.cc

Index: components/ntp_snippets/remote/remote_suggestions_provider_impl_unittest.cc
diff --git a/components/ntp_snippets/remote/remote_suggestions_provider_unittest.cc b/components/ntp_snippets/remote/remote_suggestions_provider_impl_unittest.cc
similarity index 85%
rename from components/ntp_snippets/remote/remote_suggestions_provider_unittest.cc
rename to components/ntp_snippets/remote/remote_suggestions_provider_impl_unittest.cc
index 2ad2080a3c4ee90076fabf9980fd8b1616a2dd3e..131ef3c6684551ed0ae99889d04eeefc53fa91e6 100644
--- a/components/ntp_snippets/remote/remote_suggestions_provider_unittest.cc
+++ b/components/ntp_snippets/remote/remote_suggestions_provider_impl_unittest.cc
@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "components/ntp_snippets/remote/remote_suggestions_provider.h"
+#include "components/ntp_snippets/remote/remote_suggestions_provider_impl.h"
 
 #include <memory>
 #include <utility>
@@ -32,7 +32,7 @@
 #include "components/ntp_snippets/pref_names.h"
 #include "components/ntp_snippets/remote/ntp_snippet.h"
 #include "components/ntp_snippets/remote/ntp_snippets_fetcher.h"
-#include "components/ntp_snippets/remote/ntp_snippets_scheduler.h"
+#include "components/ntp_snippets/remote/persistent_scheduler.h"
 #include "components/ntp_snippets/remote/remote_suggestions_database.h"
 #include "components/ntp_snippets/remote/test_utils.h"
 #include "components/ntp_snippets/user_classifier.h"
@@ -264,7 +264,7 @@ void ServeOneByOneImage(
   notify->OnImageDataFetched(id, "1-by-1-image-data");
 }
 
-gfx::Image FetchImage(RemoteSuggestionsProvider* service,
+gfx::Image FetchImage(RemoteSuggestionsProviderImpl* service,
                       const ContentSuggestion::ID& suggestion_id) {
   gfx::Image result;
   base::RunLoop run_loop;
@@ -307,14 +307,6 @@ class FailingFakeURLFetcherFactory : public net::URLFetcherFactory {
   }
 };
 
-class MockScheduler : public NTPSnippetsScheduler {
- public:
-  MOCK_METHOD2(Schedule,
-               bool(base::TimeDelta period_wifi,
-                    base::TimeDelta period_fallback));
-  MOCK_METHOD0(Unschedule, bool());
-};
-
 class MockImageFetcher : public ImageFetcher {
  public:
   MOCK_METHOD1(SetImageFetcherDelegate, void(ImageFetcherDelegate*));
@@ -391,7 +383,7 @@ class FakeImageDecoder : public image_fetcher::ImageDecoder {
 
 }  // namespace
 
-class RemoteSuggestionsProviderTest : public ::testing::Test {
+class RemoteSuggestionsProviderImplTest : public ::testing::Test {
  public:
   RemoteSuggestionsProviderTest()
       : params_manager_(ntp_snippets::kStudyName,
@@ -405,28 +397,28 @@ class RemoteSuggestionsProviderTest : public ::testing::Test {
         image_fetcher_(nullptr),
         image_decoder_(nullptr),
         database_(nullptr) {
-    RemoteSuggestionsProvider::RegisterProfilePrefs(
+    RemoteSuggestionsProviderImpl::RegisterProfilePrefs(
         utils_.pref_service()->registry());
     RequestThrottler::RegisterProfilePrefs(utils_.pref_service()->registry());
 
     EXPECT_TRUE(database_dir_.CreateUniqueTempDir());
   }
 
-  ~RemoteSuggestionsProviderTest() override {
+  ~RemoteSuggestionsProviderImplTest() override {
     // We need to run the message loop after deleting the database, because
     // ProtoDatabaseImpl deletes the actual LevelDB asynchronously on the task
     // runner. Without this, we'd get reports of memory leaks.
     base::RunLoop().RunUntilIdle();
   }
 
-  std::unique_ptr<RemoteSuggestionsProvider> MakeSnippetsService(
+  std::unique_ptr<RemoteSuggestionsProviderImpl> MakeSnippetsService(
       bool set_empty_response = true) {
     auto service = MakeSnippetsServiceWithoutInitialization();
     WaitForSnippetsServiceInitialization(service.get(), set_empty_response);
     return service;
   }
 
-  std::unique_ptr<RemoteSuggestionsProvider>
+  std::unique_ptr<RemoteSuggestionsProviderImpl>
   MakeSnippetsServiceWithoutInitialization() {
     scoped_refptr<base::SingleThreadTaskRunner> task_runner(
         base::ThreadTaskRunnerHandle::Get());
@@ -454,18 +446,19 @@ class RemoteSuggestionsProviderTest : public ::testing::Test {
     auto database = base::MakeUnique<RemoteSuggestionsDatabase>(
         database_dir_.GetPath(), task_runner);
     database_ = database.get();
-    return base::MakeUnique<RemoteSuggestionsProvider>(
+    return base::MakeUnique<RemoteSuggestionsProviderImpl>(
         observer_.get(), &category_factory_, utils_.pref_service(), "fr",
-        &user_classifier_, &scheduler_, std::move(snippets_fetcher),
-        std::move(image_fetcher), std::move(image_decoder),
-        std::move(database),
+        &user_classifier_, std::move(snippets_fetcher),
+        std::move(image_fetcher), std::move(image_decoder), std::move(database),
         base::MakeUnique<RemoteSuggestionsStatusService>(
             utils_.fake_signin_manager(), utils_.pref_service()));
   }
 
-  void WaitForSnippetsServiceInitialization(RemoteSuggestionsProvider* service,
-                                            bool set_empty_response) {
-    EXPECT_EQ(RemoteSuggestionsProvider::State::NOT_INITED, service->state_);
+  void WaitForSnippetsServiceInitialization(
+      RemoteSuggestionsProviderImpl* service,
+      bool set_empty_response) {
+    EXPECT_EQ(RemoteSuggestionsProviderImpl::State::NOT_INITED,
+              service->state_);
 
     // Add an initial fetch response, as the service tries to fetch when there
     // is nothing in the DB.
@@ -475,11 +468,12 @@ class RemoteSuggestionsProviderTest : public ::testing::Test {
 
     // TODO(treib): Find a better way to wait for initialization to finish.
     base::RunLoop().RunUntilIdle();
-    EXPECT_NE(RemoteSuggestionsProvider::State::NOT_INITED, service->state_);
+    EXPECT_NE(RemoteSuggestionsProviderImpl::State::NOT_INITED,
+              service->state_);
   }
 
   void ResetSnippetsService(
-      std::unique_ptr<RemoteSuggestionsProvider>* service) {
+      std::unique_ptr<RemoteSuggestionsProviderImpl>* service) {
     service->reset();
     observer_.reset();
     *service = MakeSnippetsService();
@@ -506,7 +500,6 @@ class RemoteSuggestionsProviderTest : public ::testing::Test {
  protected:
   const GURL& test_url() { return test_url_; }
   FakeContentSuggestionsProviderObserver& observer() { return *observer_; }
-  MockScheduler& mock_scheduler() { return scheduler_; }
   // TODO(tschumann): Make this a strict-mock. We want to avoid unneccesary
   // network requests.
   NiceMock<MockImageFetcher>* image_fetcher() { return image_fetcher_; }
@@ -527,14 +520,14 @@ class RemoteSuggestionsProviderTest : public ::testing::Test {
                                               net::URLRequestStatus::SUCCESS);
   }
 
-  void LoadFromJSONString(RemoteSuggestionsProvider* service,
+  void LoadFromJSONString(RemoteSuggestionsProviderImpl* service,
                           const std::string& json) {
     SetUpFetchResponse(json);
     service->FetchSnippets(true);
     base::RunLoop().RunUntilIdle();
   }
 
-  void LoadMoreFromJSONString(RemoteSuggestionsProvider* service,
+  void LoadMoreFromJSONString(RemoteSuggestionsProviderImpl* service,
                               const Category& category,
                               const std::string& json,
                               const std::set<std::string>& known_ids,
@@ -554,7 +547,6 @@ class RemoteSuggestionsProviderTest : public ::testing::Test {
   const GURL test_url_;
   std::unique_ptr<OAuth2TokenService> fake_token_service_;
   UserClassifier user_classifier_;
-  NiceMock<MockScheduler> scheduler_;
   std::unique_ptr<FakeContentSuggestionsProviderObserver> observer_;
   CategoryFactory category_factory_;
   NiceMock<MockImageFetcher>* image_fetcher_;
@@ -563,127 +555,10 @@ class RemoteSuggestionsProviderTest : public ::testing::Test {
   base::ScopedTempDir database_dir_;
   RemoteSuggestionsDatabase* database_;
 
-  DISALLOW_COPY_AND_ASSIGN(RemoteSuggestionsProviderTest);
+  DISALLOW_COPY_AND_ASSIGN(RemoteSuggestionsProviderImplTest);
 };
 
-TEST_F(RemoteSuggestionsProviderTest, ScheduleOnStart) {
-  // We should get two |Schedule| calls: The first when initialization
-  // completes, the second one after the automatic (since the service doesn't
-  // have any data yet) fetch finishes.
-  EXPECT_CALL(mock_scheduler(), Schedule(_, _)).Times(2);
-  EXPECT_CALL(mock_scheduler(), Unschedule()).Times(0);
-  auto service = MakeSnippetsService();
-
-  // When we have no snippets are all, loading the service initiates a fetch.
-  EXPECT_EQ("OK", service->snippets_fetcher()->last_status());
-}
-
-TEST_F(RemoteSuggestionsProviderTest, DontRescheduleOnStart) {
-  EXPECT_CALL(mock_scheduler(), Schedule(_, _)).Times(2);
-  EXPECT_CALL(mock_scheduler(), Unschedule()).Times(0);
-  SetUpFetchResponse(GetTestJson({GetSnippet()}));
-  auto service = MakeSnippetsService(/*set_empty_response=*/false);
-
-  // When recreating the service, we should not get any |Schedule| calls:
-  // The tasks are already scheduled with the correct intervals, so nothing on
-  // initialization, and the service has data from the DB, so no automatic fetch
-  // should happen.
-  Mock::VerifyAndClearExpectations(&mock_scheduler());
-  EXPECT_CALL(mock_scheduler(), Schedule(_, _)).Times(0);
-  EXPECT_CALL(mock_scheduler(), Unschedule()).Times(0);
-  ResetSnippetsService(&service);
-}
-
-TEST_F(RemoteSuggestionsProviderTest, RescheduleAfterSuccessfulFetch) {
-  // We should get two |Schedule| calls: The first when initialization
-  // completes, the second one after the automatic (since the service doesn't
-  // have any data yet) fetch finishes.
-  EXPECT_CALL(mock_scheduler(), Schedule(_, _)).Times(2);
-  auto service = MakeSnippetsService();
-
-  // A successful fetch should trigger another |Schedule|.
-  EXPECT_CALL(mock_scheduler(), Schedule(_, _));
-  LoadFromJSONString(service.get(), GetTestJson({GetSnippet()}));
-}
-
-TEST_F(RemoteSuggestionsProviderTest, DontRescheduleAfterFailedFetch) {
-  // We should get two |Schedule| calls: The first when initialization
-  // completes, the second one after the automatic (since the service doesn't
-  // have any data yet) fetch finishes.
-  EXPECT_CALL(mock_scheduler(), Schedule(_, _)).Times(2);
-  auto service = MakeSnippetsService();
-
-  // A failed fetch should NOT trigger another |Schedule|.
-  EXPECT_CALL(mock_scheduler(), Schedule(_, _)).Times(0);
-  LoadFromJSONString(service.get(), GetTestJson({GetInvalidSnippet()}));
-}
-
-TEST_F(RemoteSuggestionsProviderTest, IgnoreRescheduleBeforeInit) {
-  // We should get two |Schedule| calls: The first when initialization
-  // completes, the second one after the automatic (since the service doesn't
-  // have any data yet) fetch finishes.
-  EXPECT_CALL(mock_scheduler(), Schedule(_, _)).Times(2);
-  // The |RescheduleFetching| call shouldn't do anything (in particular not
-  // result in an |Unschedule|), since the service isn't initialized yet.
-  EXPECT_CALL(mock_scheduler(), Unschedule()).Times(0);
-  auto service = MakeSnippetsServiceWithoutInitialization();
-  service->RescheduleFetching(false);
-  WaitForSnippetsServiceInitialization(service.get(),
-                                       /*set_empty_response=*/true);
-}
-
-TEST_F(RemoteSuggestionsProviderTest, HandleForcedRescheduleBeforeInit) {
-  {
-    InSequence s;
-    // The |RescheduleFetching| call with force=true should result in an
-    // |Unschedule|, since the service isn't initialized yet.
-    EXPECT_CALL(mock_scheduler(), Unschedule()).Times(1);
-    // We should get two |Schedule| calls: The first when initialization
-    // completes, the second one after the automatic (since the service doesn't
-    // have any data yet) fetch finishes.
-    EXPECT_CALL(mock_scheduler(), Schedule(_, _)).Times(2);
-  }
-  auto service = MakeSnippetsServiceWithoutInitialization();
-  service->RescheduleFetching(true);
-  WaitForSnippetsServiceInitialization(service.get(),
-                                       /*set_empty_response=*/true);
-}
-
-TEST_F(RemoteSuggestionsProviderTest, RescheduleOnStateChange) {
-  {
-    InSequence s;
-    // Initial startup.
-    EXPECT_CALL(mock_scheduler(), Schedule(_, _)).Times(2);
-    // Service gets disabled.
-    EXPECT_CALL(mock_scheduler(), Unschedule());
-    // Service gets enabled again.
-    EXPECT_CALL(mock_scheduler(), Schedule(_, _)).Times(2);
-  }
-  auto service = MakeSnippetsService();
-  ASSERT_TRUE(service->ready());
-
-  service->OnStatusChanged(RemoteSuggestionsStatus::ENABLED_AND_SIGNED_IN,
-                           RemoteSuggestionsStatus::EXPLICITLY_DISABLED);
-  ASSERT_FALSE(service->ready());
-  base::RunLoop().RunUntilIdle();
-
-  service->OnStatusChanged(RemoteSuggestionsStatus::EXPLICITLY_DISABLED,
-                           RemoteSuggestionsStatus::ENABLED_AND_SIGNED_OUT);
-  ASSERT_TRUE(service->ready());
-  base::RunLoop().RunUntilIdle();
-}
-
-TEST_F(RemoteSuggestionsProviderTest, DontUnscheduleOnShutdown) {
-  EXPECT_CALL(mock_scheduler(), Schedule(_, _)).Times(2);
-  EXPECT_CALL(mock_scheduler(), Unschedule()).Times(0);
-
-  auto service = MakeSnippetsService();
-
-  service.reset();
-  base::RunLoop().RunUntilIdle();
-}
-
-TEST_F(RemoteSuggestionsProviderTest, Full) {
+TEST_F(RemoteSuggestionsProviderImplTest, Full) {
   std::string json_str(GetTestJson({GetSnippet()}));
 
   auto service = MakeSnippetsService();
@@ -706,7 +581,7 @@ TEST_F(RemoteSuggestionsProviderTest, Full) {
   EXPECT_EQ(GURL(kSnippetAmpUrl), suggestion.amp_url());
 }
 
-TEST_F(RemoteSuggestionsProviderTest, CategoryTitle) {
+TEST_F(RemoteSuggestionsProviderImplTest, CategoryTitle) {
   const base::string16 test_default_title =
       base::UTF8ToUTF16(kTestJsonDefaultCategoryTitle);
 
@@ -741,7 +616,7 @@ TEST_F(RemoteSuggestionsProviderTest, CategoryTitle) {
   EXPECT_THAT(info_before.show_if_empty(), Eq(true));
 }
 
-TEST_F(RemoteSuggestionsProviderTest, MultipleCategories) {
+TEST_F(RemoteSuggestionsProviderImplTest, MultipleCategories) {
   std::string json_str(
       GetMultiCategoryJson({GetSnippetN(0)}, {GetSnippetN(1)}));
 
@@ -788,7 +663,7 @@ TEST_F(RemoteSuggestionsProviderTest, MultipleCategories) {
   }
 }
 
-TEST_F(RemoteSuggestionsProviderTest, ArticleCategoryInfo) {
+TEST_F(RemoteSuggestionsProviderImplTest, ArticleCategoryInfo) {
   auto service = MakeSnippetsService();
   CategoryInfo article_info = service->GetCategoryInfo(articles_category());
   EXPECT_THAT(article_info.has_more_action(), Eq(true));
@@ -797,7 +672,7 @@ TEST_F(RemoteSuggestionsProviderTest, ArticleCategoryInfo) {
   EXPECT_THAT(article_info.show_if_empty(), Eq(true));
 }
 
-TEST_F(RemoteSuggestionsProviderTest, ExperimentalCategoryInfo) {
+TEST_F(RemoteSuggestionsProviderImplTest, ExperimentalCategoryInfo) {
   auto service = MakeSnippetsService();
 
   // Load data with multiple categories so that a new experimental category gets
@@ -812,7 +687,7 @@ TEST_F(RemoteSuggestionsProviderTest, ExperimentalCategoryInfo) {
   EXPECT_THAT(info.show_if_empty(), Eq(false));
 }
 
-TEST_F(RemoteSuggestionsProviderTest, PersistCategoryInfos) {
+TEST_F(RemoteSuggestionsProviderImplTest, PersistCategoryInfos) {
   auto service = MakeSnippetsService();
 
   LoadFromJSONString(service.get(),
@@ -852,7 +727,7 @@ TEST_F(RemoteSuggestionsProviderTest, PersistCategoryInfos) {
   EXPECT_EQ(info_unknown_before.title(), info_unknown_after.title());
 }
 
-TEST_F(RemoteSuggestionsProviderTest, PersistSuggestions) {
+TEST_F(RemoteSuggestionsProviderImplTest, PersistSuggestions) {
   auto service = MakeSnippetsService();
 
   LoadFromJSONString(service.get(),
@@ -871,7 +746,7 @@ TEST_F(RemoteSuggestionsProviderTest, PersistSuggestions) {
   EXPECT_THAT(observer().SuggestionsForCategory(other_category()), SizeIs(1));
 }
 
-TEST_F(RemoteSuggestionsProviderTest, DontNotifyIfNotAvailable) {
+TEST_F(RemoteSuggestionsProviderImplTest, DontNotifyIfNotAvailable) {
   // Get some suggestions into the database.
   auto service = MakeSnippetsService();
   LoadFromJSONString(service.get(),
@@ -888,7 +763,8 @@ TEST_F(RemoteSuggestionsProviderTest, DontNotifyIfNotAvailable) {
   // Recreate the service to simulate a Chrome start.
   ResetSnippetsService(&service);
 
-  ASSERT_THAT(RemoteSuggestionsProvider::State::DISABLED, Eq(service->state_));
+  ASSERT_THAT(RemoteSuggestionsProviderImpl::State::DISABLED,
+              Eq(service->state_));
 
   // Now the observer should not have received any suggestions.
   EXPECT_THAT(observer().SuggestionsForCategory(articles_category()),
@@ -896,7 +772,7 @@ TEST_F(RemoteSuggestionsProviderTest, DontNotifyIfNotAvailable) {
   EXPECT_THAT(observer().SuggestionsForCategory(other_category()), IsEmpty());
 }
 
-TEST_F(RemoteSuggestionsProviderTest, Clear) {
+TEST_F(RemoteSuggestionsProviderImplTest, Clear) {
   auto service = MakeSnippetsService();
 
   std::string json_str(GetTestJson({GetSnippet()}));
@@ -908,7 +784,7 @@ TEST_F(RemoteSuggestionsProviderTest, Clear) {
   EXPECT_THAT(service->GetSnippetsForTesting(articles_category()), IsEmpty());
 }
 
-TEST_F(RemoteSuggestionsProviderTest, ReplaceSnippets) {
+TEST_F(RemoteSuggestionsProviderImplTest, ReplaceSnippets) {
   auto service = MakeSnippetsService();
 
   std::string first("http://first");
@@ -923,7 +799,7 @@ TEST_F(RemoteSuggestionsProviderTest, ReplaceSnippets) {
               ElementsAre(IdEq(second)));
 }
 
-TEST_F(RemoteSuggestionsProviderTest, LoadsAdditionalSnippets) {
+TEST_F(RemoteSuggestionsProviderImplTest, LoadsAdditionalSnippets) {
   auto service = MakeSnippetsService();
 
   LoadFromJSONString(service.get(),
@@ -973,7 +849,8 @@ TEST_F(RemoteSuggestionsProviderTest, LoadsAdditionalSnippets) {
 // TODO(tschumann): Test step 4) on a higher level instead of peeking into the
 // internal 'dismissed' data. The proper check is to make sure we tell the
 // backend to exclude these snippets.
-TEST_F(RemoteSuggestionsProviderTest, TestMergingFetchedMoreSnippetsFillup) {
+TEST_F(RemoteSuggestionsProviderImplTest,
+       TestMergingFetchedMoreSnippetsFillup) {
   auto service = MakeSnippetsService(/*set_empty_response=*/false);
   LoadFromJSONString(
       service.get(),
@@ -1026,7 +903,7 @@ TEST_F(RemoteSuggestionsProviderTest, TestMergingFetchedMoreSnippetsFillup) {
               ElementsAre(IdEq("http://id-1"), IdEq("http://id-2")));
 }
 
-TEST_F(RemoteSuggestionsProviderTest,
+TEST_F(RemoteSuggestionsProviderImplTest,
        TestMergingFetchedMoreSnippetsReplaceAll) {
   auto service = MakeSnippetsService(/*set_empty_response=*/false);
   LoadFromJSONString(
@@ -1050,17 +927,17 @@ TEST_F(RemoteSuggestionsProviderTest,
 
   auto expect_receiving_ten_new_snippets =
       base::Bind([](Status status, std::vector<ContentSuggestion> suggestions) {
-        EXPECT_THAT(suggestions, ElementsAre(
-            IdWithinCategoryEq("http://more-id-1"),
-            IdWithinCategoryEq("http://more-id-2"),
-            IdWithinCategoryEq("http://more-id-3"),
-            IdWithinCategoryEq("http://more-id-4"),
-            IdWithinCategoryEq("http://more-id-5"),
-            IdWithinCategoryEq("http://more-id-6"),
-            IdWithinCategoryEq("http://more-id-7"),
-            IdWithinCategoryEq("http://more-id-8"),
-            IdWithinCategoryEq("http://more-id-9"),
-            IdWithinCategoryEq("http://more-id-10")));
+        EXPECT_THAT(suggestions,
+                    ElementsAre(IdWithinCategoryEq("http://more-id-1"),
+                                IdWithinCategoryEq("http://more-id-2"),
+                                IdWithinCategoryEq("http://more-id-3"),
+                                IdWithinCategoryEq("http://more-id-4"),
+                                IdWithinCategoryEq("http://more-id-5"),
+                                IdWithinCategoryEq("http://more-id-6"),
+                                IdWithinCategoryEq("http://more-id-7"),
+                                IdWithinCategoryEq("http://more-id-8"),
+                                IdWithinCategoryEq("http://more-id-9"),
+                                IdWithinCategoryEq("http://more-id-10")));
       });
   LoadMoreFromJSONString(
       service.get(), articles_category(),
@@ -1115,7 +992,7 @@ void SuggestionsLoaded(
 
 }  // namespace
 
-TEST_F(RemoteSuggestionsProviderTest, ReturnFetchRequestEmptyBeforeInit) {
+TEST_F(RemoteSuggestionsProviderImplTest, ReturnFetchRequestEmptyBeforeInit) {
   auto service = MakeSnippetsServiceWithoutInitialization();
   MockFunction<void(Status, const std::vector<ContentSuggestion>&)> loaded;
   EXPECT_CALL(loaded, Call(HasCode(StatusCode::TEMPORARY_ERROR), IsEmpty()));
@@ -1124,7 +1001,7 @@ TEST_F(RemoteSuggestionsProviderTest, ReturnFetchRequestEmptyBeforeInit) {
   base::RunLoop().RunUntilIdle();
 }
 
-TEST_F(RemoteSuggestionsProviderTest, ReturnTemporaryErrorForInvalidJson) {
+TEST_F(RemoteSuggestionsProviderImplTest, ReturnTemporaryErrorForInvalidJson) {
   auto service = MakeSnippetsService();
 
   MockFunction<void(Status, const std::vector<ContentSuggestion>&)> loaded;
@@ -1133,11 +1010,13 @@ TEST_F(RemoteSuggestionsProviderTest, ReturnTemporaryErrorForInvalidJson) {
                          "invalid json string}]}",
                          /*known_ids=*/std::set<std::string>(),
                          base::Bind(&SuggestionsLoaded, &loaded));
-  EXPECT_THAT(service->snippets_fetcher()->last_status(),
-              StartsWith("Received invalid JSON"));
+  EXPECT_THAT(
+      service->snippets_fetcher_for_testing_and_debugging()->last_status(),
+      StartsWith("Received invalid JSON"));
 }
 
-TEST_F(RemoteSuggestionsProviderTest, ReturnTemporaryErrorForInvalidSnippet) {
+TEST_F(RemoteSuggestionsProviderImplTest,
+       ReturnTemporaryErrorForInvalidSnippet) {
   auto service = MakeSnippetsService();
 
   MockFunction<void(Status, const std::vector<ContentSuggestion>&)> loaded;
@@ -1146,11 +1025,13 @@ TEST_F(RemoteSuggestionsProviderTest, ReturnTemporaryErrorForInvalidSnippet) {
                          GetTestJson({GetIncompleteSnippet()}),
                          /*known_ids=*/std::set<std::string>(),
                          base::Bind(&SuggestionsLoaded, &loaded));
-  EXPECT_THAT(service->snippets_fetcher()->last_status(),
-              StartsWith("Invalid / empty list"));
+  EXPECT_THAT(
+      service->snippets_fetcher_for_testing_and_debugging()->last_status(),
+      StartsWith("Invalid / empty list"));
 }
 
-TEST_F(RemoteSuggestionsProviderTest, ReturnTemporaryErrorForRequestFailure) {
+TEST_F(RemoteSuggestionsProviderImplTest,
+       ReturnTemporaryErrorForRequestFailure) {
   // Created SnippetsService will fail by default with unsuccessful request.
   auto service = MakeSnippetsService(/*set_empty_response=*/false);
 
@@ -1162,7 +1043,7 @@ TEST_F(RemoteSuggestionsProviderTest, ReturnTemporaryErrorForRequestFailure) {
   base::RunLoop().RunUntilIdle();
 }
 
-TEST_F(RemoteSuggestionsProviderTest, ReturnTemporaryErrorForHttpFailure) {
+TEST_F(RemoteSuggestionsProviderImplTest, ReturnTemporaryErrorForHttpFailure) {
   auto service = MakeSnippetsService();
   SetUpHttpError();
 
@@ -1174,52 +1055,59 @@ TEST_F(RemoteSuggestionsProviderTest, ReturnTemporaryErrorForHttpFailure) {
   base::RunLoop().RunUntilIdle();
 }
 
-TEST_F(RemoteSuggestionsProviderTest, LoadInvalidJson) {
+TEST_F(RemoteSuggestionsProviderImplTest, LoadInvalidJson) {
   auto service = MakeSnippetsService();
 
   LoadFromJSONString(service.get(), GetTestJson({GetInvalidSnippet()}));
-  EXPECT_THAT(service->snippets_fetcher()->last_status(),
-              StartsWith("Received invalid JSON"));
+  EXPECT_THAT(
+      service->snippets_fetcher_for_testing_and_debugging()->last_status(),
+      StartsWith("Received invalid JSON"));
   EXPECT_THAT(service->GetSnippetsForTesting(articles_category()), IsEmpty());
 }
 
-TEST_F(RemoteSuggestionsProviderTest, LoadInvalidJsonWithExistingSnippets) {
+TEST_F(RemoteSuggestionsProviderImplTest, LoadInvalidJsonWithExistingSnippets) {
   auto service = MakeSnippetsService();
 
   LoadFromJSONString(service.get(), GetTestJson({GetSnippet()}));
   ASSERT_THAT(service->GetSnippetsForTesting(articles_category()), SizeIs(1));
-  ASSERT_EQ("OK", service->snippets_fetcher()->last_status());
+  ASSERT_EQ(
+      "OK",
+      service->snippets_fetcher_for_testing_and_debugging()->last_status());
 
   LoadFromJSONString(service.get(), GetTestJson({GetInvalidSnippet()}));
-  EXPECT_THAT(service->snippets_fetcher()->last_status(),
-              StartsWith("Received invalid JSON"));
+  EXPECT_THAT(
+      service->snippets_fetcher_for_testing_and_debugging()->last_status(),
+      StartsWith("Received invalid JSON"));
   // This should not have changed the existing snippets.
   EXPECT_THAT(service->GetSnippetsForTesting(articles_category()), SizeIs(1));
 }
 
-TEST_F(RemoteSuggestionsProviderTest, LoadIncompleteJson) {
+TEST_F(RemoteSuggestionsProviderImplTest, LoadIncompleteJson) {
   auto service = MakeSnippetsService();
 
   LoadFromJSONString(service.get(), GetTestJson({GetIncompleteSnippet()}));
-  EXPECT_EQ("Invalid / empty list.",
-            service->snippets_fetcher()->last_status());
+  EXPECT_EQ(
+      "Invalid / empty list.",
+      service->snippets_fetcher_for_testing_and_debugging()->last_status());
   EXPECT_THAT(service->GetSnippetsForTesting(articles_category()), IsEmpty());
 }
 
-TEST_F(RemoteSuggestionsProviderTest, LoadIncompleteJsonWithExistingSnippets) {
+TEST_F(RemoteSuggestionsProviderImplTest,
+       LoadIncompleteJsonWithExistingSnippets) {
   auto service = MakeSnippetsService();
 
   LoadFromJSONString(service.get(), GetTestJson({GetSnippet()}));
   ASSERT_THAT(service->GetSnippetsForTesting(articles_category()), SizeIs(1));
 
   LoadFromJSONString(service.get(), GetTestJson({GetIncompleteSnippet()}));
-  EXPECT_EQ("Invalid / empty list.",
-            service->snippets_fetcher()->last_status());
+  EXPECT_EQ(
+      "Invalid / empty list.",
+      service->snippets_fetcher_for_testing_and_debugging()->last_status());
   // This should not have changed the existing snippets.
   EXPECT_THAT(service->GetSnippetsForTesting(articles_category()), SizeIs(1));
 }
 
-TEST_F(RemoteSuggestionsProviderTest, Dismiss) {
+TEST_F(RemoteSuggestionsProviderImplTest, Dismiss) {
   auto service = MakeSnippetsService();
 
   std::string json_str(
@@ -1270,7 +1158,7 @@ TEST_F(RemoteSuggestionsProviderTest, Dismiss) {
   EXPECT_THAT(service->GetSnippetsForTesting(articles_category()), SizeIs(1));
 }
 
-TEST_F(RemoteSuggestionsProviderTest, GetDismissed) {
+TEST_F(RemoteSuggestionsProviderImplTest, GetDismissed) {
   auto service = MakeSnippetsService();
 
   LoadFromJSONString(service.get(), GetTestJson({GetSnippet()}));
@@ -1280,8 +1168,8 @@ TEST_F(RemoteSuggestionsProviderTest, GetDismissed) {
   service->GetDismissedSuggestionsForDebugging(
       articles_category(),
       base::Bind(
-          [](RemoteSuggestionsProvider* service,
-             RemoteSuggestionsProviderTest* test,
+          [](RemoteSuggestionsProviderImpl* service,
+             RemoteSuggestionsProviderImplTest* test,
              std::vector<ContentSuggestion> dismissed_suggestions) {
             EXPECT_EQ(1u, dismissed_suggestions.size());
             for (auto& suggestion : dismissed_suggestions) {
@@ -1296,8 +1184,8 @@ TEST_F(RemoteSuggestionsProviderTest, GetDismissed) {
   service->GetDismissedSuggestionsForDebugging(
       articles_category(),
       base::Bind(
-          [](RemoteSuggestionsProvider* service,
-             RemoteSuggestionsProviderTest* test,
+          [](RemoteSuggestionsProviderImpl* service,
+             RemoteSuggestionsProviderImplTest* test,
              std::vector<ContentSuggestion> dismissed_suggestions) {
             EXPECT_EQ(0u, dismissed_suggestions.size());
           },
@@ -1305,7 +1193,7 @@ TEST_F(RemoteSuggestionsProviderTest, GetDismissed) {
   base::RunLoop().RunUntilIdle();
 }
 
-TEST_F(RemoteSuggestionsProviderTest, CreationTimestampParseFail) {
+TEST_F(RemoteSuggestionsProviderImplTest, CreationTimestampParseFail) {
   auto service = MakeSnippetsService();
 
   std::string json =
@@ -1318,7 +1206,7 @@ TEST_F(RemoteSuggestionsProviderTest, CreationTimestampParseFail) {
   EXPECT_THAT(service->GetSnippetsForTesting(articles_category()), IsEmpty());
 }
 
-TEST_F(RemoteSuggestionsProviderTest, RemoveExpiredDismissedContent) {
+TEST_F(RemoteSuggestionsProviderImplTest, RemoveExpiredDismissedContent) {
   auto service = MakeSnippetsService();
 
   std::string json_str1(GetTestJson({GetExpiredSnippet()}));
@@ -1351,7 +1239,7 @@ TEST_F(RemoteSuggestionsProviderTest, RemoveExpiredDismissedContent) {
   EXPECT_TRUE(FetchImage(service.get(), MakeArticleID(kSnippetUrl)).IsEmpty());
 }
 
-TEST_F(RemoteSuggestionsProviderTest, ExpiredContentNotRemoved) {
+TEST_F(RemoteSuggestionsProviderImplTest, ExpiredContentNotRemoved) {
   auto service = MakeSnippetsService();
 
   std::string json_str(GetTestJson({GetExpiredSnippet()}));
@@ -1360,7 +1248,7 @@ TEST_F(RemoteSuggestionsProviderTest, ExpiredContentNotRemoved) {
   EXPECT_THAT(service->GetSnippetsForTesting(articles_category()), SizeIs(1));
 }
 
-TEST_F(RemoteSuggestionsProviderTest, TestSingleSource) {
+TEST_F(RemoteSuggestionsProviderImplTest, TestSingleSource) {
   auto service = MakeSnippetsService();
 
   std::string json_str(GetTestJson({GetSnippetWithSources(
@@ -1376,7 +1264,7 @@ TEST_F(RemoteSuggestionsProviderTest, TestSingleSource) {
   EXPECT_EQ(snippet.amp_url(), GURL("http://source1.amp.com"));
 }
 
-TEST_F(RemoteSuggestionsProviderTest, TestSingleSourceWithMalformedUrl) {
+TEST_F(RemoteSuggestionsProviderImplTest, TestSingleSourceWithMalformedUrl) {
   auto service = MakeSnippetsService();
 
   std::string json_str(GetTestJson({GetSnippetWithSources(
@@ -1386,7 +1274,7 @@ TEST_F(RemoteSuggestionsProviderTest, TestSingleSourceWithMalformedUrl) {
   EXPECT_THAT(service->GetSnippetsForTesting(articles_category()), IsEmpty());
 }
 
-TEST_F(RemoteSuggestionsProviderTest, TestSingleSourceWithMissingData) {
+TEST_F(RemoteSuggestionsProviderImplTest, TestSingleSourceWithMissingData) {
   auto service = MakeSnippetsService();
 
   std::string json_str(
@@ -1396,7 +1284,7 @@ TEST_F(RemoteSuggestionsProviderTest, TestSingleSourceWithMissingData) {
   EXPECT_THAT(service->GetSnippetsForTesting(articles_category()), IsEmpty());
 }
 
-TEST_F(RemoteSuggestionsProviderTest, LogNumArticlesHistogram) {
+TEST_F(RemoteSuggestionsProviderImplTest, LogNumArticlesHistogram) {
   auto service = MakeSnippetsService();
 
   base::HistogramTester tester;
@@ -1473,7 +1361,7 @@ TEST_F(RemoteSuggestionsProviderTest, LogNumArticlesHistogram) {
   tester.ExpectTotalCount("NewTabPage.Snippets.NumArticlesFetched", 6);
 }
 
-TEST_F(RemoteSuggestionsProviderTest, DismissShouldRespectAllKnownUrls) {
+TEST_F(RemoteSuggestionsProviderImplTest, DismissShouldRespectAllKnownUrls) {
   auto service = MakeSnippetsService();
 
   const base::Time creation = GetDefaultCreationTime();
@@ -1504,7 +1392,7 @@ TEST_F(RemoteSuggestionsProviderTest, DismissShouldRespectAllKnownUrls) {
   EXPECT_THAT(service->GetSnippetsForTesting(articles_category()), IsEmpty());
 }
 
-TEST_F(RemoteSuggestionsProviderTest, StatusChanges) {
+TEST_F(RemoteSuggestionsProviderImplTest, StatusChanges) {
   auto service = MakeSnippetsService();
 
   // Simulate user signed out
@@ -1515,7 +1403,8 @@ TEST_F(RemoteSuggestionsProviderTest, StatusChanges) {
   base::RunLoop().RunUntilIdle();
   EXPECT_THAT(observer().StatusForCategory(articles_category()),
               Eq(CategoryStatus::SIGNED_OUT));
-  EXPECT_THAT(RemoteSuggestionsProvider::State::DISABLED, Eq(service->state_));
+  EXPECT_THAT(RemoteSuggestionsProviderImpl::State::DISABLED,
+              Eq(service->state_));
   EXPECT_THAT(service->GetSnippetsForTesting(articles_category()),
               IsEmpty());  // No fetch should be made.
 
@@ -1529,11 +1418,11 @@ TEST_F(RemoteSuggestionsProviderTest, StatusChanges) {
   base::RunLoop().RunUntilIdle();
   EXPECT_THAT(observer().StatusForCategory(articles_category()),
               Eq(CategoryStatus::AVAILABLE));
-  EXPECT_THAT(RemoteSuggestionsProvider::State::READY, Eq(service->state_));
+  EXPECT_THAT(RemoteSuggestionsProviderImpl::State::READY, Eq(service->state_));
   EXPECT_FALSE(service->GetSnippetsForTesting(articles_category()).empty());
 }
 
-TEST_F(RemoteSuggestionsProviderTest, ImageReturnedWithTheSameId) {
+TEST_F(RemoteSuggestionsProviderImplTest, ImageReturnedWithTheSameId) {
   auto service = MakeSnippetsService();
 
   LoadFromJSONString(service.get(), GetTestJson({GetSnippet()}));
@@ -1558,7 +1447,7 @@ TEST_F(RemoteSuggestionsProviderTest, ImageReturnedWithTheSameId) {
   EXPECT_EQ(1, image.Width());
 }
 
-TEST_F(RemoteSuggestionsProviderTest, EmptyImageReturnedForNonExistentId) {
+TEST_F(RemoteSuggestionsProviderImplTest, EmptyImageReturnedForNonExistentId) {
   auto service = MakeSnippetsService();
 
   // Create a non-empty image so that we can test the image gets updated.
@@ -1575,7 +1464,7 @@ TEST_F(RemoteSuggestionsProviderTest, EmptyImageReturnedForNonExistentId) {
   EXPECT_TRUE(image.IsEmpty());
 }
 
-TEST_F(RemoteSuggestionsProviderTest,
+TEST_F(RemoteSuggestionsProviderImplTest,
        FetchingUnknownImageIdShouldNotHitDatabase) {
   // Testing that the provider is not accessing the database is tricky.
   // Therefore, we simply put in some data making sure that if the provider asks
@@ -1601,7 +1490,7 @@ TEST_F(RemoteSuggestionsProviderTest,
   EXPECT_TRUE(image.IsEmpty()) << "got image with width: " << image.Width();
 }
 
-TEST_F(RemoteSuggestionsProviderTest, ClearHistoryRemovesAllSuggestions) {
+TEST_F(RemoteSuggestionsProviderImplTest, ClearHistoryRemovesAllSuggestions) {
   auto service = MakeSnippetsService();
 
   std::string first_snippet = GetSnippetWithUrl("http://url1.com");
@@ -1625,7 +1514,8 @@ TEST_F(RemoteSuggestionsProviderTest, ClearHistoryRemovesAllSuggestions) {
               IsEmpty());
 }
 
-TEST_F(RemoteSuggestionsProviderTest, SuggestionsFetchedOnSignInAndSignOut) {
+TEST_F(RemoteSuggestionsProviderImplTest,
+       SuggestionsFetchedOnSignInAndSignOut) {
   auto service = MakeSnippetsService();
   EXPECT_THAT(service->GetSnippetsForTesting(articles_category()), IsEmpty());
 
@@ -1645,7 +1535,7 @@ TEST_F(RemoteSuggestionsProviderTest, SuggestionsFetchedOnSignInAndSignOut) {
   EXPECT_THAT(service->GetSnippetsForTesting(articles_category()), SizeIs(2));
 }
 
-TEST_F(RemoteSuggestionsProviderTest, ShouldClearOrphanedImagesOnRestart) {
+TEST_F(RemoteSuggestionsProviderImplTest, ShouldClearOrphanedImagesOnRestart) {
   auto service = MakeSnippetsService();
 
   LoadFromJSONString(service.get(), GetTestJson({GetSnippet()}));
@@ -1671,7 +1561,7 @@ TEST_F(RemoteSuggestionsProviderTest, ShouldClearOrphanedImagesOnRestart) {
   EXPECT_TRUE(FetchImage(service.get(), MakeArticleID(kSnippetUrl)).IsEmpty());
 }
 
-TEST_F(RemoteSuggestionsProviderTest,
+TEST_F(RemoteSuggestionsProviderImplTest,
        ShouldHandleMoreThanMaxSnippetsInResponse) {
   auto service = MakeSnippetsService();
 
@@ -1687,8 +1577,10 @@ TEST_F(RemoteSuggestionsProviderTest,
               SizeIs(service->GetMaxSnippetCountForTesting() + 1));
 }
 
-TEST_F(RemoteSuggestionsProviderTest, StoreLastSuccessfullBackgroundFetchTime) {
-  // On initialization of the RemoteSuggestionsProvider a background fetch is
+TEST_F(RemoteSuggestionsProviderImplTest,
+       StoreLastSuccessfullBackgroundFetchTime) {
+  // On initialization of the RemoteSuggestionsProviderImpl a background fetch
+  // is

[Marc Treib, 2016/12/19 12:59:23]

nit: "wrong" line break

[jkrcal, 2016/12/20 16:39:46]

Done.

   // triggered since the snippets DB is empty. Therefore the service must not be
   // initialized until the test clock is set.
   auto service = MakeSnippetsServiceWithoutInitialization();
@@ -1710,7 +1602,9 @@ TEST_F(RemoteSuggestionsProviderTest, StoreLastSuccessfullBackgroundFetchTime) {
   // Advance the time and check whether the time was updated correctly after the
   // background fetch.
   simple_test_clock_ptr->Advance(TimeDelta::FromHours(1));
-  service->FetchSnippetsInTheBackground();
+  // TODO(jkrcal): Move together with the pref storage into the scheduler.
+  static_cast<RemoteSuggestionsScheduler::Updater*>(service.get())
+      ->UpdateRemoteSuggestionsBySchedule();
   base::RunLoop().RunUntilIdle();
   EXPECT_EQ(
       simple_test_clock_ptr->Now().ToInternalValue(),
@@ -1719,4 +1613,57 @@ TEST_F(RemoteSuggestionsProviderTest, StoreLastSuccessfullBackgroundFetchTime) {
   // scheduler refactoring is done (crbug.com/672434).
 }
 
+TEST_F(RemoteSuggestionsProviderImplTest, CallsProviderActiveCallbackIfReady) {
+  // Initiate the service so that it is already READY.
+  auto service = MakeSnippetsService();
+
+  MockFunction<void(bool)> activeness_observer;
+  // The callback should be called on registering.
+  EXPECT_CALL(activeness_observer, Call(true));
+  service->RegisterActivenessObserver(activeness_observer);
+}
+
+TEST_F(RemoteSuggestionsProviderImplTest,
+       DoesNotCallProviderActiveCallbackIfNotInited) {
+  auto service = MakeSnippetsServiceWithoutInitialization();
+
+  MockFunction<void(bool)> activeness_observer;
+  // The provider is not initialized yet, no callback should be called on
+  // registering.
+  EXPECT_CALL(activeness_observer, Call(_)).Times(0);

[tschumann, 2016/12/19 11:07:19]

this is a bit backwards. Instead, I'd recommend a StrictMock<MockFunction<>> and
don't set any expectations (strict-mocks fail on unexpected calls).

[Marc Treib, 2016/12/19 12:59:23]

The ".Times(0)" pattern is somewhat common, though mostly with Mock classes
rather than functions. I guess having a StrictMock in the first place is usually
better. Something to add to our testing guidelines doc?

[jkrcal, 2016/12/20 16:39:46]

Agreed, I've added it into the testing agenda for Jan 12.

[jkrcal, 2016/12/20 16:39:46]

Done.

+  service->RegisterActivenessObserver(activeness_observer);
+}
+
+TEST_F(RemoteSuggestionsProviderImplTest,
+       CallsProviderActiveCallbackWhenReady) {
+  auto service = MakeSnippetsServiceWithoutInitialization();
+  MockFunction<void(bool)> activeness_observer;
+  service->RegisterActivenessObserver(activeness_observer);
+
+  // Call the callback when becoming ready.

[tschumann, 2016/12/19 11:07:19]

nit: The comment being written in "active" form is a bit misleading -- we expect
it to be called.
// Should be called when becoming ready?

Same in the tests below.

[jkrcal, 2016/12/20 16:39:46]

Done.

+  EXPECT_CALL(activeness_observer, Call(true));
+  EnterState(State::READY);
+}
+
+TEST_F(RemoteSuggestionsProviderImplTest, CallsProviderActiveCallbackOnError) {
+  auto service = MakeSnippetsServiceWithoutInitialization();
+  MockFunction<void(bool)> activeness_observer;
+  service->RegisterActivenessObserver(activeness_observer);
+
+  // Call the callback on error.
+  EXPECT_CALL(activeness_observer, Call(false));
+  EnterState(State::ERROR);
+}
+
+TEST_F(RemoteSuggestionsProviderImplTest,
+       CallsProviderActiveCallbackWhenDisabled) {
+  auto service = MakeSnippetsServiceWithoutInitialization();
+  MockFunction<void(bool)> activeness_observer;
+  service->RegisterActivenessObserver(activeness_observer);
+
+  // Call the callback when becoming disabled.
+  EXPECT_CALL(activeness_observer, Call(false));
+  EnterState(State::DISABLED);
+}
+
 }  // namespace ntp_snippets