Test SuggestionsServiceImpl without friending anything

components/suggestions/suggestions_service_impl_unittest.cc

Index: components/suggestions/suggestions_service_impl_unittest.cc
diff --git a/components/suggestions/suggestions_service_impl_unittest.cc b/components/suggestions/suggestions_service_impl_unittest.cc
index 217303e8ba6cdf09c6a4d9bc349a2232ac95c018..c4bfaec7cc6a1e5b718fc6f24bbefaba48840887 100644
--- a/components/suggestions/suggestions_service_impl_unittest.cc
+++ b/components/suggestions/suggestions_service_impl_unittest.cc
@@ -48,33 +48,16 @@ using testing::StrictMock;
 namespace {
 
 const char kAccountId[] = "account";
+const char kSuggestionsUrlPath[] = "/chromesuggestions";
+const char kBlacklistUrlPath[] = "/chromesuggestions/blacklist";
+const char kBlacklistClearUrlPath[] = "/chromesuggestions/blacklist/clear";
 const char kTestTitle[] = "a title";
 const char kTestUrl[] = "http://go.com";
 const char kTestFaviconUrl[] =
     "https://s2.googleusercontent.com/s2/favicons?domain_url="
     "http://go.com&alt=s&sz=32";
 const char kBlacklistedUrl[] = "http://blacklist.com";
-const char kBlacklistedUrlAlt[] = "http://blacklist-atl.com";
-const int64_t kTestDefaultExpiry = 1402200000000000;
-const int64_t kTestSetExpiry = 1404792000000000;
-
-std::unique_ptr<net::FakeURLFetcher> CreateURLFetcher(
-    const GURL& url,
-    net::URLFetcherDelegate* delegate,
-    const std::string& response_data,
-    net::HttpStatusCode response_code,
-    net::URLRequestStatus::Status status) {
-  std::unique_ptr<net::FakeURLFetcher> fetcher(new net::FakeURLFetcher(
-      url, delegate, response_data, response_code, status));
-
-  if (response_code == net::HTTP_OK) {
-    scoped_refptr<net::HttpResponseHeaders> download_headers(
-        new net::HttpResponseHeaders(""));
-    download_headers->AddHeader("Content-Type: text/html");
-    fetcher->set_response_headers(download_headers);
-  }
-  return fetcher;
-}
+const int64_t kTestSetExpiry = 12121212;  // This timestamp lies in the past.
 
 // GMock matcher for protobuf equality.
 MATCHER_P(EqualsProto, message, "") {
@@ -97,23 +80,6 @@ SuggestionsProfile CreateSuggestionsProfile() {
   ChromeSuggestion* suggestion = profile.add_suggestions();
   suggestion->set_title(kTestTitle);
   suggestion->set_url(kTestUrl);
-  suggestion->set_expiry_ts(kTestSetExpiry);
-  return profile;
-}
-
-// Creates one suggestion with expiry timestamp and one without.
-SuggestionsProfile CreateSuggestionsProfileWithExpiryTimestamps() {
-  SuggestionsProfile profile;
-  profile.set_timestamp(123);
-  ChromeSuggestion* suggestion = profile.add_suggestions();
-  suggestion->set_title(kTestTitle);
-  suggestion->set_url(kTestUrl);
-  suggestion->set_expiry_ts(kTestSetExpiry);
-
-  suggestion = profile.add_suggestions();
-  suggestion->set_title(kTestTitle);
-  suggestion->set_url(kTestUrl);
-
   return profile;
 }
 
@@ -171,25 +137,24 @@ class MockBlacklistStore : public suggestions::BlacklistStore {
 class SuggestionsServiceTest : public testing::Test {
  public:
   void CheckCallback(const SuggestionsProfile& suggestions_profile) {
-    ++suggestions_data_callback_count_;
+    ++data_callback_count_;
     if (suggestions_profile.suggestions_size() == 0)
-      ++suggestions_empty_data_count_;
+      ++empty_callback_count_;
   }
 
-  int suggestions_data_callback_count_;
-  int suggestions_empty_data_count_;
-
  protected:
   SuggestionsServiceTest()
-      : suggestions_data_callback_count_(0),
-        suggestions_empty_data_count_(0),
+      : data_callback_count_(0),
+        empty_callback_count_(0),
         signin_client_(&pref_service_),
         signin_manager_(&signin_client_, &account_tracker_),
-        factory_(nullptr, base::Bind(&CreateURLFetcher)),
-        mock_sync_service_(nullptr),
+        factory_(),
+        mock_sync_service_(base::MakeUnique<MockSyncService>()),
         mock_thumbnail_manager_(nullptr),
         mock_blacklist_store_(nullptr),
-        test_suggestions_store_(nullptr) {
+        test_suggestions_store_(nullptr),
+        request_context_(new net::TestURLRequestContextGetter(
+            io_message_loop_.task_runner())) {
     SigninManagerBase::RegisterProfilePrefs(pref_service_.registry());
     SigninManagerBase::RegisterPrefs(pref_service_.registry());
 
@@ -201,26 +166,87 @@ class SuggestionsServiceTest : public testing::Test {
   ~SuggestionsServiceTest() override {}
 
   void SetUp() override {
-    request_context_ =
-        new net::TestURLRequestContextGetter(io_message_loop_.task_runner());
+    IgnoreRepeatedMockCalls();
+    suggestions_service_ = CreateSuggestionsServiceWithMocks();
+    subscriptions_ = suggestions_service_->AddCallback(base::Bind(
+        &SuggestionsServiceTest::CheckCallback, base::Unretained(this)));
   }
 
-  std::unique_ptr<SuggestionsServiceImpl> CreateSuggestionsServiceWithMocks() {
-    mock_sync_service_ = base::MakeUnique<MockSyncService>();
-    EXPECT_CALL(*mock_sync_service_, CanSyncStart())
+  void TearDown() override {
+    testing::Mock::VerifyAndClearExpectations(mock_thumbnail_manager_);
+    testing::Mock::VerifyAndClearExpectations(mock_blacklist_store_);
+    testing::Mock::VerifyAndClearExpectations(mock_sync_service_.get());
+    subscriptions_.reset();
+    suggestions_service_.reset();
+  }
+
+  GURL CurrentlyQueriedUrl() {
+    net::TestURLFetcher* fetcher = factory_.GetFetcherByID(0);
+    if (!fetcher) {
+      return GURL();
+    }
+    return fetcher->GetOriginalURL();
+  }
+
+  void RespondToFetch(const std::string& response_body,
+                      net::HttpStatusCode response_code,
+                      net::URLRequestStatus status) {
+    net::TestURLFetcher* fetcher = factory_.GetFetcherByID(0);
+    ASSERT_TRUE(fetcher) << "Tried to respond to fetch that is not ongoing!";
+    fetcher->SetResponseString(response_body);
+    fetcher->set_response_code(response_code);
+    fetcher->set_status(status);
+    fetcher->delegate()->OnURLFetchComplete(fetcher);
+  }
+
+  void RespondToFetchWithProfile(const SuggestionsProfile& profile) {
+    RespondToFetch(
+        profile.SerializeAsString(), net::HTTP_OK,
+        net::URLRequestStatus(net::URLRequestStatus::SUCCESS, net::OK));
+  }
+
+  void NotifySuggestionsService() {
+    // The implementation of |OnStateChanged| is private in the
+    // SuggestionsService, but public for every SyncServiceObserver.
+    syncer::SyncServiceObserver* observer = suggestions_service();
+    observer->OnStateChanged(sync_service());
+  }
+
+  void IgnoreRepeatedMockCalls() {
+    EXPECT_CALL(*sync_service(), CanSyncStart())
         .Times(AnyNumber())
         .WillRepeatedly(Return(true));
-    EXPECT_CALL(*mock_sync_service_, IsSyncActive())
+    EXPECT_CALL(*sync_service(), IsSyncActive())
         .Times(AnyNumber())
         .WillRepeatedly(Return(true));
-    EXPECT_CALL(*mock_sync_service_, ConfigurationDone())
+    EXPECT_CALL(*sync_service(), ConfigurationDone())
         .Times(AnyNumber())
         .WillRepeatedly(Return(true));
-    EXPECT_CALL(*mock_sync_service_, GetActiveDataTypes())
+    EXPECT_CALL(*sync_service(), GetActiveDataTypes())
         .Times(AnyNumber())
         .WillRepeatedly(
             Return(syncer::ModelTypeSet(syncer::HISTORY_DELETE_DIRECTIVES)));
+  }
 
+  MockBlacklistStore* blacklist_store() { return mock_blacklist_store_; }
+
+  SuggestionsServiceImpl* suggestions_service() {
+    return suggestions_service_.get();
+  }
+
+  TestSuggestionsStore* suggestions_store() { return test_suggestions_store_; }
+
+  MockSyncService* sync_service() { return mock_sync_service_.get(); }
+
+  MockImageManager* thumbnail_manager() { return mock_thumbnail_manager_; }
+
+  FakeProfileOAuth2TokenService* token_service() { return &token_service_; }
+
+  int data_callback_count() { return data_callback_count_; }
+  int empty_callback_count() { return empty_callback_count_; }
+
+ private:
+  std::unique_ptr<SuggestionsServiceImpl> CreateSuggestionsServiceWithMocks() {
     // These objects are owned by the returned SuggestionsService, but we keep
     // the pointer around for testing.
     // TODO(treib): This is broken - if the test destroys the SuggestionsService
@@ -236,103 +262,50 @@ class SuggestionsServiceTest : public testing::Test {
         base::WrapUnique(mock_blacklist_store_));
   }
 
-  // Helper for Undo failure tests. Depending on |is_uploaded|, tests either
-  // the case where the URL is no longer in the local blacklist or the case
-  // in which it's not yet candidate for upload.
-  void UndoBlacklistURLFailsHelper(bool is_uploaded) {
-    std::unique_ptr<SuggestionsServiceImpl> suggestions_service(
-        CreateSuggestionsServiceWithMocks());
-    // Ensure scheduling the request doesn't happen before undo.
-    base::TimeDelta delay = base::TimeDelta::FromHours(1);
-    suggestions_service->set_blacklist_delay(delay);
-
-    auto subscription = suggestions_service->AddCallback(base::Bind(
-        &SuggestionsServiceTest::CheckCallback, base::Unretained(this)));
-
-    SuggestionsProfile suggestions_profile = CreateSuggestionsProfile();
-    GURL blacklisted_url(kBlacklistedUrl);
-
-    // Blacklist expectations.
-    EXPECT_CALL(*mock_blacklist_store_, BlacklistUrl(Eq(blacklisted_url)))
-        .WillOnce(Return(true));
-    EXPECT_CALL(*mock_thumbnail_manager_,
-                Initialize(EqualsProto(suggestions_profile)));
-    EXPECT_CALL(*mock_blacklist_store_, FilterSuggestions(_));
-    EXPECT_CALL(*mock_blacklist_store_, GetTimeUntilReadyForUpload(_))
-        .WillOnce(DoAll(SetArgPointee<0>(delay), Return(true)));
-    // Undo expectations.
-    if (is_uploaded) {
-      // URL is not in local blacklist.
-      EXPECT_CALL(*mock_blacklist_store_,
-                  GetTimeUntilURLReadyForUpload(Eq(blacklisted_url), _))
-          .WillOnce(Return(false));
-    } else {
-      // URL is not yet candidate for upload.
-      base::TimeDelta negative_delay = base::TimeDelta::FromHours(-1);
-      EXPECT_CALL(*mock_blacklist_store_,
-                  GetTimeUntilURLReadyForUpload(Eq(blacklisted_url), _))
-          .WillOnce(DoAll(SetArgPointee<1>(negative_delay), Return(true)));
-    }
-
-    EXPECT_TRUE(suggestions_service->BlacklistURL(blacklisted_url));
-    EXPECT_FALSE(suggestions_service->UndoBlacklistURL(blacklisted_url));
-
-    EXPECT_EQ(1, suggestions_data_callback_count_);
-  }
-
-  bool HasPendingSuggestionsRequest(
-      SuggestionsServiceImpl* suggestions_service) {
-    return !!suggestions_service->pending_request_.get();
-  }
+  int data_callback_count_;
+  int empty_callback_count_;
 
- protected:
+  std::unique_ptr<SuggestionsServiceImpl> suggestions_service_;
   base::MessageLoopForIO io_message_loop_;
   TestingPrefServiceSyncable pref_service_;
   AccountTrackerService account_tracker_;
   TestSigninClient signin_client_;
   FakeSigninManagerBase signin_manager_;
-  net::FakeURLFetcherFactory factory_;
+  net::TestURLFetcherFactory factory_;
   FakeProfileOAuth2TokenService token_service_;
   std::unique_ptr<MockSyncService> mock_sync_service_;
+  std::unique_ptr<SuggestionsService::ResponseCallbackList::Subscription>
+      subscriptions_;
+
   // Only used if the SuggestionsService is built with mocks. Not owned.
   MockImageManager* mock_thumbnail_manager_;
   MockBlacklistStore* mock_blacklist_store_;
   TestSuggestionsStore* test_suggestions_store_;
   scoped_refptr<net::TestURLRequestContextGetter> request_context_;
 
- private:
   DISALLOW_COPY_AND_ASSIGN(SuggestionsServiceTest);
 };
 
 TEST_F(SuggestionsServiceTest, FetchSuggestionsData) {
-  std::unique_ptr<SuggestionsService> suggestions_service(
-      CreateSuggestionsServiceWithMocks());
-  auto subscription = suggestions_service->AddCallback(base::Bind(
-      &SuggestionsServiceTest::CheckCallback, base::Unretained(this)));
-
-  SuggestionsProfile suggestions_profile = CreateSuggestionsProfile();
-
-  // Set up net::FakeURLFetcherFactory.
-  factory_.SetFakeResponse(SuggestionsServiceImpl::BuildSuggestionsURL(),
-                           suggestions_profile.SerializeAsString(),
-                           net::HTTP_OK, net::URLRequestStatus::SUCCESS);
-
-  // Expectations.
-  EXPECT_CALL(*mock_thumbnail_manager_, Initialize(_));
-  EXPECT_CALL(*mock_blacklist_store_, FilterSuggestions(_));
-  EXPECT_CALL(*mock_blacklist_store_, GetTimeUntilReadyForUpload(_))
+  EXPECT_CALL(*thumbnail_manager(), Initialize(_));
+  EXPECT_CALL(*blacklist_store(), FilterSuggestions(_));
+  EXPECT_CALL(*blacklist_store(), GetTimeUntilReadyForUpload(_))
       .WillOnce(Return(false));
 
   // Send the request. The data should be returned to the callback.
-  suggestions_service->FetchSuggestionsData();
+  suggestions_service()->FetchSuggestionsData();
 
-  // Let the network request run.
   base::RunLoop().RunUntilIdle();
+  EXPECT_FALSE(CurrentlyQueriedUrl().is_empty());
+  EXPECT_TRUE(CurrentlyQueriedUrl().is_valid());
+  EXPECT_EQ(CurrentlyQueriedUrl().path(), kSuggestionsUrlPath);
+  RespondToFetchWithProfile(CreateSuggestionsProfile());
 
   // Ensure that CheckCallback() ran once.
-  EXPECT_EQ(1, suggestions_data_callback_count_);
+  EXPECT_EQ(1, data_callback_count());
 
-  test_suggestions_store_->LoadSuggestions(&suggestions_profile);
+  SuggestionsProfile suggestions_profile;
+  suggestions_store()->LoadSuggestions(&suggestions_profile);
   ASSERT_EQ(1, suggestions_profile.suggestions_size());
   EXPECT_EQ(kTestTitle, suggestions_profile.suggestions(0).title());
   EXPECT_EQ(kTestUrl, suggestions_profile.suggestions(0).url());
@@ -340,168 +313,127 @@ TEST_F(SuggestionsServiceTest, FetchSuggestionsData) {
 }
 
 TEST_F(SuggestionsServiceTest, FetchSuggestionsDataSyncNotInitializedEnabled) {
-  std::unique_ptr<SuggestionsService> suggestions_service(
-      CreateSuggestionsServiceWithMocks());
-  EXPECT_CALL(*mock_sync_service_, IsSyncActive())
-      .WillRepeatedly(Return(false));
-
-  auto subscription = suggestions_service->AddCallback(base::Bind(
-      &SuggestionsServiceTest::CheckCallback, base::Unretained(this)));
+  EXPECT_CALL(*sync_service(), IsSyncActive()).WillRepeatedly(Return(false));
 
   // Try to fetch suggestions. Since sync is not active, no network request
   // should be sent.
-  suggestions_service->FetchSuggestionsData();
+  suggestions_service()->FetchSuggestionsData();
 
   // Let any network request run.
   base::RunLoop().RunUntilIdle();
 
   // Ensure that CheckCallback() didn't run.
-  EXPECT_EQ(0, suggestions_data_callback_count_);
+  EXPECT_EQ(0, data_callback_count());
 
-  // |test_suggestions_store_| should still contain the default values.
+  // |suggestions_store()| should still contain the default values.
   SuggestionsProfile suggestions;
-  test_suggestions_store_->LoadSuggestions(&suggestions);
+  suggestions_store()->LoadSuggestions(&suggestions);
   EXPECT_EQ(CreateSuggestionsProfile().SerializeAsString(),
             suggestions.SerializeAsString());
 }
 
-TEST_F(SuggestionsServiceTest, FetchSuggestionsDataSyncDisabled) {
-  std::unique_ptr<SuggestionsServiceImpl> suggestions_service(
-      CreateSuggestionsServiceWithMocks());
-  EXPECT_CALL(*mock_sync_service_, CanSyncStart())
-      .WillRepeatedly(Return(false));
-
-  auto subscription = suggestions_service->AddCallback(base::Bind(
-      &SuggestionsServiceTest::CheckCallback, base::Unretained(this)));
+TEST_F(SuggestionsServiceTest, ClearSuggestionsDataIfSyncDisabled) {
+  EXPECT_CALL(*sync_service(), CanSyncStart()).WillRepeatedly(Return(false));
 
-  // Tell SuggestionsService that the sync state changed. The cache should be
-  // cleared and empty data returned to the callback.
-  suggestions_service->OnStateChanged(mock_sync_service_.get());
+  NotifySuggestionsService();
 
-  // Ensure that CheckCallback ran once with empty data.
-  EXPECT_EQ(1, suggestions_data_callback_count_);
-  EXPECT_EQ(1, suggestions_empty_data_count_);
+  EXPECT_EQ(1, data_callback_count());
+  EXPECT_EQ(1, empty_callback_count());
 
   // Try to fetch suggestions. Since sync is not active, no network request
   // should be sent.
-  suggestions_service->FetchSuggestionsData();
+  suggestions_service()->FetchSuggestionsData();
 
   // Let any network request run.
   base::RunLoop().RunUntilIdle();
 
   // Ensure that CheckCallback didn't run again.
-  EXPECT_EQ(1, suggestions_data_callback_count_);
+  EXPECT_EQ(1, data_callback_count());
+  EXPECT_EQ(1, empty_callback_count());
 }
 
 TEST_F(SuggestionsServiceTest, FetchSuggestionsDataNoAccessToken) {
-  token_service_.set_auto_post_fetch_response_on_message_loop(false);
-
-  std::unique_ptr<SuggestionsServiceImpl> suggestions_service(
-      CreateSuggestionsServiceWithMocks());
+  token_service()->set_auto_post_fetch_response_on_message_loop(false);
 
-  auto subscription = suggestions_service->AddCallback(base::Bind(
-      &SuggestionsServiceTest::CheckCallback, base::Unretained(this)));
-
-  EXPECT_CALL(*mock_blacklist_store_, GetTimeUntilReadyForUpload(_))
+  EXPECT_CALL(*blacklist_store(), GetTimeUntilReadyForUpload(_))
       .WillOnce(Return(false));
 
-  suggestions_service->FetchSuggestionsData();
+  suggestions_service()->FetchSuggestionsData();
 
-  token_service_.IssueErrorForAllPendingRequests(GoogleServiceAuthError(
+  token_service()->IssueErrorForAllPendingRequests(GoogleServiceAuthError(
       GoogleServiceAuthError::State::INVALID_GAIA_CREDENTIALS));
 
-  // No network request should be sent.
   base::RunLoop().RunUntilIdle();
-  EXPECT_FALSE(HasPendingSuggestionsRequest(suggestions_service.get()));
-  EXPECT_EQ(0, suggestions_data_callback_count_);
+  EXPECT_FALSE(suggestions_service()->has_pending_request_for_testing());
+  EXPECT_EQ(0, data_callback_count());
 }
 
-TEST_F(SuggestionsServiceTest, IssueRequestIfNoneOngoingError) {
-  std::unique_ptr<SuggestionsServiceImpl> suggestions_service(
-      CreateSuggestionsServiceWithMocks());
-
-  // Fake a request error.
-  factory_.SetFakeResponse(SuggestionsServiceImpl::BuildSuggestionsURL(),
-                           "irrelevant", net::HTTP_OK,
-                           net::URLRequestStatus::FAILED);
-
-  EXPECT_CALL(*mock_blacklist_store_, GetTimeUntilReadyForUpload(_))
+TEST_F(SuggestionsServiceTest, FetchingSuggestionsIgnoresRequestFailure) {
+  EXPECT_CALL(*blacklist_store(), GetTimeUntilReadyForUpload(_))
       .WillOnce(Return(false));
 
-  // Send the request. Empty data will be returned to the callback.
-  suggestions_service->IssueRequestIfNoneOngoing(
-      SuggestionsServiceImpl::BuildSuggestionsURL());
+  suggestions_service()->FetchSuggestionsData();
 
-  // (Testing only) wait until suggestion fetch is complete.
   base::RunLoop().RunUntilIdle();
-}
+  RespondToFetch("irrelevant", net::HTTP_OK,
+                 net::URLRequestStatus(net::URLRequestStatus::FAILED,
+                                       net::ERR_INVALID_RESPONSE));
 
-TEST_F(SuggestionsServiceTest, IssueRequestIfNoneOngoingResponseNotOK) {
-  std::unique_ptr<SuggestionsServiceImpl> suggestions_service(
-      CreateSuggestionsServiceWithMocks());
+  EXPECT_EQ(0, data_callback_count());
+}
 
-  // Fake a non-200 response code.
-  factory_.SetFakeResponse(SuggestionsServiceImpl::BuildSuggestionsURL(),
-                           "irrelevant", net::HTTP_BAD_REQUEST,
-                           net::URLRequestStatus::SUCCESS);
+TEST_F(SuggestionsServiceTest, FetchingSuggestionsClearsStoreIfResponseNotOK) {
+  suggestions_store()->StoreSuggestions(CreateSuggestionsProfile());
 
   // Expect that an upload to the blacklist is scheduled.
-  EXPECT_CALL(*mock_blacklist_store_, GetTimeUntilReadyForUpload(_))
+  EXPECT_CALL(*blacklist_store(), GetTimeUntilReadyForUpload(_))
       .WillOnce(Return(false));
 
   // Send the request. Empty data will be returned to the callback.
-  suggestions_service->IssueRequestIfNoneOngoing(
-      SuggestionsServiceImpl::BuildSuggestionsURL());
+  suggestions_service()->FetchSuggestionsData();
 
-  // (Testing only) wait until suggestion fetch is complete.
   base::RunLoop().RunUntilIdle();
+  RespondToFetch(
+      "irrelevant", net::HTTP_BAD_REQUEST,
+      net::URLRequestStatus(net::URLRequestStatus::SUCCESS, net::OK));
 
-  // Expect no suggestions in the cache.
   SuggestionsProfile empty_suggestions;
-  EXPECT_FALSE(test_suggestions_store_->LoadSuggestions(&empty_suggestions));
+  EXPECT_FALSE(suggestions_store()->LoadSuggestions(&empty_suggestions));
 }
 
 TEST_F(SuggestionsServiceTest, BlacklistURL) {
-  std::unique_ptr<SuggestionsServiceImpl> suggestions_service(
-      CreateSuggestionsServiceWithMocks());
+  // Calling RunUntilIdle on the MessageLoop only works when the task is not
+  // posted for the future.
   base::TimeDelta no_delay = base::TimeDelta::FromSeconds(0);
-  suggestions_service->set_blacklist_delay(no_delay);
+  suggestions_service()->set_blacklist_delay_for_testing(no_delay);
 
-  auto subscription = suggestions_service->AddCallback(base::Bind(
-      &SuggestionsServiceTest::CheckCallback, base::Unretained(this)));
-
-  GURL blacklisted_url(kBlacklistedUrl);
-  GURL request_url(
-      SuggestionsServiceImpl::BuildSuggestionsBlacklistURL(blacklisted_url));
-  SuggestionsProfile suggestions_profile = CreateSuggestionsProfile();
-  factory_.SetFakeResponse(request_url, suggestions_profile.SerializeAsString(),
-                           net::HTTP_OK, net::URLRequestStatus::SUCCESS);
-  EXPECT_CALL(*mock_thumbnail_manager_, Initialize(_)).Times(2);
+  EXPECT_CALL(*thumbnail_manager(), Initialize(_)).Times(2);
 
-  // Expected calls to the blacklist store.
-  EXPECT_CALL(*mock_blacklist_store_, BlacklistUrl(Eq(blacklisted_url)))
+  EXPECT_CALL(*blacklist_store(), BlacklistUrl(Eq(GURL(kBlacklistedUrl))))
       .WillOnce(Return(true));
-  EXPECT_CALL(*mock_blacklist_store_, FilterSuggestions(_)).Times(2);
-  EXPECT_CALL(*mock_blacklist_store_, GetTimeUntilReadyForUpload(_))
+  EXPECT_CALL(*blacklist_store(), FilterSuggestions(_)).Times(2);
+  EXPECT_CALL(*blacklist_store(), GetTimeUntilReadyForUpload(_))
       .WillOnce(DoAll(SetArgPointee<0>(no_delay), Return(true)))
       .WillOnce(Return(false));
-  EXPECT_CALL(*mock_blacklist_store_, GetCandidateForUpload(_))
-      .WillOnce(DoAll(SetArgPointee<0>(blacklisted_url), Return(true)));
-  EXPECT_CALL(*mock_blacklist_store_, RemoveUrl(Eq(blacklisted_url)))
+  EXPECT_CALL(*blacklist_store(), GetCandidateForUpload(_))
+      .WillOnce(DoAll(SetArgPointee<0>(GURL(kBlacklistedUrl)), Return(true)));
+  EXPECT_CALL(*blacklist_store(), RemoveUrl(Eq(GURL(kBlacklistedUrl))))
       .WillOnce(Return(true));
 
-  EXPECT_TRUE(suggestions_service->BlacklistURL(blacklisted_url));
-  EXPECT_EQ(1, suggestions_data_callback_count_);
+  EXPECT_TRUE(suggestions_service()->BlacklistURL(GURL(kBlacklistedUrl)));
+  EXPECT_EQ(1, data_callback_count());
 
   // Wait on the upload task, the blacklist request and the next blacklist
-  // scheduling task. This only works when the scheduling task is not for future
-  // execution (note how both the SuggestionsService's scheduling delay and the
-  // BlacklistStore's candidacy delay are zero).
+  // scheduling task.
   base::RunLoop().RunUntilIdle();
 
-  EXPECT_EQ(2, suggestions_data_callback_count_);
+  EXPECT_EQ(CurrentlyQueriedUrl().path(), kBlacklistUrlPath);
+  RespondToFetchWithProfile(CreateSuggestionsProfile());
+
+  EXPECT_EQ(2, data_callback_count());
 
-  test_suggestions_store_->LoadSuggestions(&suggestions_profile);
+  SuggestionsProfile suggestions_profile;
+  suggestions_store()->LoadSuggestions(&suggestions_profile);
   ASSERT_EQ(1, suggestions_profile.suggestions_size());
   EXPECT_EQ(kTestTitle, suggestions_profile.suggestions(0).title());
   EXPECT_EQ(kTestUrl, suggestions_profile.suggestions(0).url());
@@ -509,75 +441,54 @@ TEST_F(SuggestionsServiceTest, BlacklistURL) {
 }
 
 TEST_F(SuggestionsServiceTest, BlacklistURLFails) {
-  std::unique_ptr<SuggestionsService> suggestions_service(
-      CreateSuggestionsServiceWithMocks());
-
-  auto subscription = suggestions_service->AddCallback(base::Bind(
-      &SuggestionsServiceTest::CheckCallback, base::Unretained(this)));
-
   GURL blacklisted_url(kBlacklistedUrl);
-  EXPECT_CALL(*mock_blacklist_store_, BlacklistUrl(Eq(blacklisted_url)))
+  EXPECT_CALL(*blacklist_store(), BlacklistUrl(Eq(blacklisted_url)))
       .WillOnce(Return(false));
 
-  EXPECT_FALSE(suggestions_service->BlacklistURL(blacklisted_url));
+  EXPECT_FALSE(suggestions_service()->BlacklistURL(blacklisted_url));
 
-  EXPECT_EQ(0, suggestions_data_callback_count_);
+  EXPECT_EQ(0, data_callback_count());
 }
 
-// Initial blacklist request fails, triggering a second which succeeds.
-TEST_F(SuggestionsServiceTest, BlacklistURLRequestFails) {
-  std::unique_ptr<SuggestionsServiceImpl> suggestions_service(
-      CreateSuggestionsServiceWithMocks());
+TEST_F(SuggestionsServiceTest, RetryBlacklistURLRequestAfterFailure) {
+  // Calling RunUntilIdle on the MessageLoop only works when the task is not
+  // posted for the future.
   base::TimeDelta no_delay = base::TimeDelta::FromSeconds(0);
-  suggestions_service->set_blacklist_delay(no_delay);
-
-  auto subscription = suggestions_service->AddCallback(base::Bind(
-      &SuggestionsServiceTest::CheckCallback, base::Unretained(this)));
-
-  GURL blacklisted_url(kBlacklistedUrl);
-  GURL request_url(
-      SuggestionsServiceImpl::BuildSuggestionsBlacklistURL(blacklisted_url));
-  GURL blacklisted_url_alt(kBlacklistedUrlAlt);
-  GURL request_url_alt(SuggestionsServiceImpl::BuildSuggestionsBlacklistURL(
-      blacklisted_url_alt));
-  SuggestionsProfile suggestions_profile = CreateSuggestionsProfile();
+  suggestions_service()->set_blacklist_delay_for_testing(no_delay);
 
-  // Note: we want to set the response for the blacklist URL to first
-  // succeed, then fail. This doesn't seem possible. For simplicity of testing,
-  // we'll pretend the URL changed in the BlacklistStore between the first and
-  // the second request, and adjust expectations accordingly.
-  factory_.SetFakeResponse(request_url, "irrelevant", net::HTTP_OK,
-                           net::URLRequestStatus::FAILED);
-  factory_.SetFakeResponse(request_url_alt,
-                           suggestions_profile.SerializeAsString(),
-                           net::HTTP_OK, net::URLRequestStatus::SUCCESS);
-
-  // Expectations.
-  EXPECT_CALL(*mock_thumbnail_manager_, Initialize(_)).Times(2);
-  EXPECT_CALL(*mock_blacklist_store_, BlacklistUrl(Eq(blacklisted_url)))
+  EXPECT_CALL(*thumbnail_manager(), Initialize(_)).Times(2);
+  EXPECT_CALL(*blacklist_store(), BlacklistUrl(Eq(GURL(kBlacklistedUrl))))
       .WillOnce(Return(true));
-  EXPECT_CALL(*mock_blacklist_store_, FilterSuggestions(_)).Times(2);
-  EXPECT_CALL(*mock_blacklist_store_, GetTimeUntilReadyForUpload(_))
+  EXPECT_CALL(*blacklist_store(), FilterSuggestions(_)).Times(2);
+  EXPECT_CALL(*blacklist_store(), GetTimeUntilReadyForUpload(_))
       .WillOnce(DoAll(SetArgPointee<0>(no_delay), Return(true)))
       .WillOnce(DoAll(SetArgPointee<0>(no_delay), Return(true)))
       .WillOnce(Return(false));
-  EXPECT_CALL(*mock_blacklist_store_, GetCandidateForUpload(_))
-      .WillOnce(DoAll(SetArgPointee<0>(blacklisted_url), Return(true)))
-      .WillOnce(DoAll(SetArgPointee<0>(blacklisted_url_alt), Return(true)));
-  EXPECT_CALL(*mock_blacklist_store_, RemoveUrl(Eq(blacklisted_url_alt)))
+  EXPECT_CALL(*blacklist_store(), GetCandidateForUpload(_))
+      .WillOnce(DoAll(SetArgPointee<0>(GURL(kBlacklistedUrl)), Return(true)))
+      .WillOnce(DoAll(SetArgPointee<0>(GURL(kBlacklistedUrl)), Return(true)));
+  EXPECT_CALL(*blacklist_store(), RemoveUrl(Eq(GURL(kBlacklistedUrl))))
       .WillOnce(Return(true));
 
   // Blacklist call, first request attempt.
-  EXPECT_TRUE(suggestions_service->BlacklistURL(blacklisted_url));
-  EXPECT_EQ(1, suggestions_data_callback_count_);
+  EXPECT_TRUE(suggestions_service()->BlacklistURL(GURL(kBlacklistedUrl)));
+  EXPECT_EQ(1, data_callback_count());
 
-  // Wait for the first scheduling, the first request, the second scheduling,
-  // second request and the third scheduling. Again, note that calling
-  // RunUntilIdle on the MessageLoop only works when the task is not posted for
-  // the future.
+  // Wait for the first scheduling receiving a failing response.
   base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(CurrentlyQueriedUrl().path(), kBlacklistUrlPath);
+  RespondToFetch("irrelevant", net::HTTP_OK,
+                 net::URLRequestStatus(net::URLRequestStatus::FAILED,
+                                       net::ERR_INVALID_RESPONSE));
 
-  test_suggestions_store_->LoadSuggestions(&suggestions_profile);
+  // Wait for the second scheduling followed by a successful response.
+  base::RunLoop().RunUntilIdle();
+  ASSERT_TRUE(CurrentlyQueriedUrl().is_valid());
+  EXPECT_EQ(CurrentlyQueriedUrl().path(), kBlacklistUrlPath);
+  RespondToFetchWithProfile(CreateSuggestionsProfile());
+
+  SuggestionsProfile suggestions_profile;
+  suggestions_store()->LoadSuggestions(&suggestions_profile);
   ASSERT_EQ(1, suggestions_profile.suggestions_size());
   EXPECT_EQ(kTestTitle, suggestions_profile.suggestions(0).title());
   EXPECT_EQ(kTestUrl, suggestions_profile.suggestions(0).url());
@@ -585,153 +496,183 @@ TEST_F(SuggestionsServiceTest, BlacklistURLRequestFails) {
 }
 
 TEST_F(SuggestionsServiceTest, UndoBlacklistURL) {
-  std::unique_ptr<SuggestionsServiceImpl> suggestions_service(
-      CreateSuggestionsServiceWithMocks());
   // Ensure scheduling the request doesn't happen before undo.
   base::TimeDelta delay = base::TimeDelta::FromHours(1);
-  suggestions_service->set_blacklist_delay(delay);
-
-  auto subscription = suggestions_service->AddCallback(base::Bind(
-      &SuggestionsServiceTest::CheckCallback, base::Unretained(this)));
+  suggestions_service()->set_blacklist_delay_for_testing(delay);
 
   SuggestionsProfile suggestions_profile = CreateSuggestionsProfile();
   GURL blacklisted_url(kBlacklistedUrl);
 
   // Blacklist expectations.
-  EXPECT_CALL(*mock_blacklist_store_, BlacklistUrl(Eq(blacklisted_url)))
+  EXPECT_CALL(*blacklist_store(), BlacklistUrl(Eq(blacklisted_url)))
       .WillOnce(Return(true));
-  EXPECT_CALL(*mock_thumbnail_manager_,
+  EXPECT_CALL(*thumbnail_manager(),
               Initialize(EqualsProto(suggestions_profile)))
       .Times(AnyNumber());
-  EXPECT_CALL(*mock_blacklist_store_, FilterSuggestions(_)).Times(AnyNumber());
-  EXPECT_CALL(*mock_blacklist_store_, GetTimeUntilReadyForUpload(_))
+  EXPECT_CALL(*blacklist_store(), FilterSuggestions(_)).Times(AnyNumber());
+  EXPECT_CALL(*blacklist_store(), GetTimeUntilReadyForUpload(_))
       .WillOnce(DoAll(SetArgPointee<0>(delay), Return(true)));
   // Undo expectations.
-  EXPECT_CALL(*mock_blacklist_store_,
+  EXPECT_CALL(*blacklist_store(),
               GetTimeUntilURLReadyForUpload(Eq(blacklisted_url), _))
       .WillOnce(DoAll(SetArgPointee<1>(delay), Return(true)));
-  EXPECT_CALL(*mock_blacklist_store_, RemoveUrl(Eq(blacklisted_url)))
+  EXPECT_CALL(*blacklist_store(), RemoveUrl(Eq(blacklisted_url)))
       .WillOnce(Return(true));
 
-  EXPECT_TRUE(suggestions_service->BlacklistURL(blacklisted_url));
-  EXPECT_TRUE(suggestions_service->UndoBlacklistURL(blacklisted_url));
+  EXPECT_TRUE(suggestions_service()->BlacklistURL(blacklisted_url));
+  EXPECT_TRUE(suggestions_service()->UndoBlacklistURL(blacklisted_url));
 
-  EXPECT_EQ(2, suggestions_data_callback_count_);
+  EXPECT_EQ(2, data_callback_count());
 }
 
 TEST_F(SuggestionsServiceTest, ClearBlacklist) {
-  std::unique_ptr<SuggestionsServiceImpl> suggestions_service(
-      CreateSuggestionsServiceWithMocks());
   // Ensure scheduling the request doesn't happen before undo.
   base::TimeDelta delay = base::TimeDelta::FromHours(1);
-  suggestions_service->set_blacklist_delay(delay);
-
-  auto subscription = suggestions_service->AddCallback(base::Bind(
-      &SuggestionsServiceTest::CheckCallback, base::Unretained(this)));
-
-  SuggestionsProfile suggestions_profile = CreateSuggestionsProfile();
-  GURL blacklisted_url(kBlacklistedUrl);
-
-  factory_.SetFakeResponse(
-      SuggestionsServiceImpl::BuildSuggestionsBlacklistClearURL(),
-      suggestions_profile.SerializeAsString(), net::HTTP_OK,
-      net::URLRequestStatus::SUCCESS);
+  suggestions_service()->set_blacklist_delay_for_testing(delay);
 
   // Blacklist expectations.
-  EXPECT_CALL(*mock_blacklist_store_, BlacklistUrl(Eq(blacklisted_url)))
+  EXPECT_CALL(*blacklist_store(), BlacklistUrl(Eq(GURL(kBlacklistedUrl))))
       .WillOnce(Return(true));
-  EXPECT_CALL(*mock_thumbnail_manager_,
-              Initialize(EqualsProto(suggestions_profile)))
+  EXPECT_CALL(*thumbnail_manager(),
+              Initialize(EqualsProto(CreateSuggestionsProfile())))
       .Times(AnyNumber());
-  EXPECT_CALL(*mock_blacklist_store_, FilterSuggestions(_)).Times(AnyNumber());
-  EXPECT_CALL(*mock_blacklist_store_, GetTimeUntilReadyForUpload(_))
+  EXPECT_CALL(*blacklist_store(), FilterSuggestions(_)).Times(AnyNumber());
+  EXPECT_CALL(*blacklist_store(), GetTimeUntilReadyForUpload(_))
       .WillOnce(DoAll(SetArgPointee<0>(delay), Return(true)));
-  EXPECT_CALL(*mock_blacklist_store_, ClearBlacklist());
+  EXPECT_CALL(*blacklist_store(), ClearBlacklist());
+
+  EXPECT_TRUE(suggestions_service()->BlacklistURL(GURL(kBlacklistedUrl)));
+  suggestions_service()->ClearBlacklist();
 
-  EXPECT_TRUE(suggestions_service->BlacklistURL(blacklisted_url));
-  suggestions_service->ClearBlacklist();
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(CurrentlyQueriedUrl().path(), kBlacklistClearUrlPath);
 
-  EXPECT_EQ(2, suggestions_data_callback_count_);
+  EXPECT_EQ(2, data_callback_count());
 }
 
 TEST_F(SuggestionsServiceTest, UndoBlacklistURLFailsIfNotInBlacklist) {
-  UndoBlacklistURLFailsHelper(true);
+  // Ensure scheduling the request doesn't happen before undo.
+  base::TimeDelta delay = base::TimeDelta::FromHours(1);
+  suggestions_service()->set_blacklist_delay_for_testing(delay);
+
+  // Blacklist expectations.
+  EXPECT_CALL(*blacklist_store(), BlacklistUrl(Eq(GURL(kBlacklistedUrl))))
+      .WillOnce(Return(true));
+  EXPECT_CALL(*thumbnail_manager(),
+              Initialize(EqualsProto(CreateSuggestionsProfile())));
+  EXPECT_CALL(*blacklist_store(), FilterSuggestions(_));
+  EXPECT_CALL(*blacklist_store(), GetTimeUntilReadyForUpload(_))
+      .WillOnce(DoAll(SetArgPointee<0>(delay), Return(true)));
+  // Undo expectations.
+  // URL is not in local blacklist.
+  EXPECT_CALL(*blacklist_store(),
+              GetTimeUntilURLReadyForUpload(Eq(GURL(kBlacklistedUrl)), _))
+      .WillOnce(Return(false));
+
+  EXPECT_TRUE(suggestions_service()->BlacklistURL(GURL(kBlacklistedUrl)));
+  EXPECT_FALSE(suggestions_service()->UndoBlacklistURL(GURL(kBlacklistedUrl)));
+
+  EXPECT_EQ(1, data_callback_count());
 }
 
 TEST_F(SuggestionsServiceTest, UndoBlacklistURLFailsIfAlreadyCandidate) {
-  UndoBlacklistURLFailsHelper(false);
-}
+  // Ensure scheduling the request doesn't happen before undo.
+  base::TimeDelta delay = base::TimeDelta::FromHours(1);
+  suggestions_service()->set_blacklist_delay_for_testing(delay);
 
-TEST_F(SuggestionsServiceTest, GetBlacklistedUrlNotBlacklistRequest) {
-  // Not a blacklist request.
-  std::unique_ptr<net::FakeURLFetcher> fetcher(
-      CreateURLFetcher(GURL("http://not-blacklisting.com/a?b=c"), nullptr, "",
-                       net::HTTP_OK, net::URLRequestStatus::SUCCESS));
-  GURL retrieved_url;
-  EXPECT_FALSE(
-      SuggestionsServiceImpl::GetBlacklistedUrl(*fetcher, &retrieved_url));
-}
+  SuggestionsProfile suggestions_profile = CreateSuggestionsProfile();
+
+  // Blacklist expectations.
+  EXPECT_CALL(*blacklist_store(), BlacklistUrl(Eq(GURL(kBlacklistedUrl))))
+      .WillOnce(Return(true));
+  EXPECT_CALL(*thumbnail_manager(),
+              Initialize(EqualsProto(suggestions_profile)));
+  EXPECT_CALL(*blacklist_store(), FilterSuggestions(_));
+  EXPECT_CALL(*blacklist_store(), GetTimeUntilReadyForUpload(_))
+      .WillOnce(DoAll(SetArgPointee<0>(delay), Return(true)));
+  // Undo expectations.
+  base::TimeDelta negative_delay = base::TimeDelta::FromHours(-1);
+  EXPECT_CALL(*blacklist_store(),
+              GetTimeUntilURLReadyForUpload(Eq(GURL(kBlacklistedUrl)), _))
+      .WillOnce(DoAll(SetArgPointee<1>(negative_delay), Return(true)));
+
+  EXPECT_TRUE(suggestions_service()->BlacklistURL(GURL(kBlacklistedUrl)));
+  EXPECT_FALSE(suggestions_service()->UndoBlacklistURL(GURL(kBlacklistedUrl)));
 
-TEST_F(SuggestionsServiceTest, GetBlacklistedUrlBlacklistRequest) {
-  // An actual blacklist request.
-  std::string blacklisted_url = "http://blacklisted.com/a?b=c&d=e";
-  std::string encoded_blacklisted_url =
-      "http%3A%2F%2Fblacklisted.com%2Fa%3Fb%3Dc%26d%3De";
-  std::string blacklist_request_prefix(
-      SuggestionsServiceImpl::BuildSuggestionsBlacklistURLPrefix());
-  std::unique_ptr<net::FakeURLFetcher> fetcher(CreateURLFetcher(
-      GURL(blacklist_request_prefix + encoded_blacklisted_url), nullptr, "",
-      net::HTTP_OK, net::URLRequestStatus::SUCCESS));
-  GURL retrieved_url;
-  EXPECT_TRUE(
-      SuggestionsServiceImpl::GetBlacklistedUrl(*fetcher, &retrieved_url));
-  EXPECT_EQ(blacklisted_url, retrieved_url.spec());

[fhorschig, 2017/05/10 08:41:07]

This case is covered by other tests like RetryBlacklistURLRequestAfterFailure
which would not call the mock_blacklist_store_ otherwise.

+  EXPECT_EQ(1, data_callback_count());
 }
 
-TEST_F(SuggestionsServiceTest, UpdateBlacklistDelay) {
-  std::unique_ptr<SuggestionsServiceImpl> suggestions_service(
-      CreateSuggestionsServiceWithMocks());
-  base::TimeDelta initial_delay = suggestions_service->blacklist_delay();
+TEST_F(SuggestionsServiceTest, TemporaryIncreasesBlacklistDelayOnFailure) {
+  EXPECT_CALL(*thumbnail_manager(), Initialize(_)).Times(AnyNumber());
+  EXPECT_CALL(*blacklist_store(), FilterSuggestions(_)).Times(AnyNumber());
+  EXPECT_CALL(*blacklist_store(), GetTimeUntilReadyForUpload(_))
+      .Times(AnyNumber())
+      .WillRepeatedly(Return(false));
+  base::TimeDelta initial_delay =
+      suggestions_service()->blacklist_delay_for_testing();
 
   // Delay unchanged on success.
-  suggestions_service->UpdateBlacklistDelay(true);
-  EXPECT_EQ(initial_delay, suggestions_service->blacklist_delay());
+  suggestions_service()->FetchSuggestionsData();
+  base::RunLoop().RunUntilIdle();
+  RespondToFetchWithProfile(CreateSuggestionsProfile());
+  EXPECT_EQ(initial_delay,
+            suggestions_service()->blacklist_delay_for_testing());
 
   // Delay increases on failure.
-  suggestions_service->UpdateBlacklistDelay(false);
-  EXPECT_GT(suggestions_service->blacklist_delay(), initial_delay);
+  suggestions_service()->FetchSuggestionsData();
+  base::RunLoop().RunUntilIdle();
+  RespondToFetch(
+      "irrelevant", net::HTTP_BAD_REQUEST,
+      net::URLRequestStatus(net::URLRequestStatus::SUCCESS, net::OK));
+  EXPECT_GT(suggestions_service()->blacklist_delay_for_testing(),
+            initial_delay);
 
   // Delay resets on success.
-  suggestions_service->UpdateBlacklistDelay(true);
-  EXPECT_EQ(initial_delay, suggestions_service->blacklist_delay());
+  suggestions_service()->FetchSuggestionsData();
+  base::RunLoop().RunUntilIdle();
+  RespondToFetchWithProfile(CreateSuggestionsProfile());
+  EXPECT_EQ(initial_delay,
+            suggestions_service()->blacklist_delay_for_testing());
 }
 
-TEST_F(SuggestionsServiceTest, CheckDefaultTimeStamps) {
-  std::unique_ptr<SuggestionsServiceImpl> suggestions_service(
-      CreateSuggestionsServiceWithMocks());
-  SuggestionsProfile suggestions =
-      CreateSuggestionsProfileWithExpiryTimestamps();
-  suggestions_service->SetDefaultExpiryTimestamp(&suggestions,
-                                                 kTestDefaultExpiry);
-  EXPECT_EQ(kTestSetExpiry, suggestions.suggestions(0).expiry_ts());
-  EXPECT_EQ(kTestDefaultExpiry, suggestions.suggestions(1).expiry_ts());
+TEST_F(SuggestionsServiceTest, DoesNotOverrideDefaultExpiryTime) {
+  EXPECT_CALL(*thumbnail_manager(), Initialize(_));
+  EXPECT_CALL(*blacklist_store(), FilterSuggestions(_));
+  EXPECT_CALL(*blacklist_store(), GetTimeUntilReadyForUpload(_))
+      .WillOnce(Return(false));
+
+  suggestions_service()->FetchSuggestionsData();
+
+  base::RunLoop().RunUntilIdle();
+  // Creates one suggestion without timestamp and adds a second with timestamp.
+  SuggestionsProfile profile = CreateSuggestionsProfile();
+  ChromeSuggestion* suggestion = profile.add_suggestions();
+  suggestion->set_title(kTestTitle);
+  suggestion->set_url(kTestUrl);
+  suggestion->set_expiry_ts(kTestSetExpiry);
+  RespondToFetchWithProfile(profile);
+
+  SuggestionsProfile suggestions_profile;
+  suggestions_store()->LoadSuggestions(&suggestions_profile);
+  ASSERT_EQ(2, suggestions_profile.suggestions_size());
+  // Suggestion[1] had a very old time stamp but should not be updated.
+  EXPECT_EQ(kTestSetExpiry, suggestions_profile.suggestions(1).expiry_ts());
+  // Suggestion[0] had no time stamp and should be ahead of the old suggestion.
+  EXPECT_GT(kTestSetExpiry, suggestions_profile.suggestions(0).expiry_ts());
 }
 
 TEST_F(SuggestionsServiceTest, GetPageThumbnail) {
-  std::unique_ptr<SuggestionsService> suggestions_service(
-      CreateSuggestionsServiceWithMocks());
-
   GURL test_url(kTestUrl);
   GURL thumbnail_url("https://www.thumbnails.com/thumb.jpg");
   base::Callback<void(const GURL&, const gfx::Image&)> dummy_callback;
 
-  EXPECT_CALL(*mock_thumbnail_manager_, GetImageForURL(test_url, _));
-  suggestions_service->GetPageThumbnail(test_url, dummy_callback);
+  EXPECT_CALL(*thumbnail_manager(), GetImageForURL(test_url, _));
+  suggestions_service()->GetPageThumbnail(test_url, dummy_callback);
 
-  EXPECT_CALL(*mock_thumbnail_manager_, AddImageURL(test_url, thumbnail_url));
-  EXPECT_CALL(*mock_thumbnail_manager_, GetImageForURL(test_url, _));
-  suggestions_service->GetPageThumbnailWithURL(test_url, thumbnail_url,
-                                               dummy_callback);
+  EXPECT_CALL(*thumbnail_manager(), AddImageURL(test_url, thumbnail_url));
+  EXPECT_CALL(*thumbnail_manager(), GetImageForURL(test_url, _));
+  suggestions_service()->GetPageThumbnailWithURL(test_url, thumbnail_url,
+                                                 dummy_callback);
 }
 
 }  // namespace suggestions