SuggestionsService: implement SyncServiceObserver

components/suggestions/suggestions_service_unittest.cc

 // Copyright 2014 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/suggestions/suggestions_service.h"
 
 #include <stdint.h>
 
-#include <map>
-#include <sstream>
 #include <utility>
 
 #include "base/bind.h"
 #include "base/feature_list.h"
 #include "base/macros.h"
 #include "base/memory/scoped_ptr.h"
 #include "components/signin/core/browser/fake_profile_oauth2_token_service.h"
 #include "components/suggestions/blacklist_store.h"
 #include "components/suggestions/image_manager.h"
 #include "components/suggestions/proto/suggestions.pb.h"
 #include "components/suggestions/suggestions_store.h"
-#include "components/suggestions/suggestions_utils.h"
+#include "components/sync_driver/fake_sync_service.h"
+#include "components/sync_driver/sync_service.h"
 #include "net/base/escape.h"
 #include "net/http/http_response_headers.h"
 #include "net/http/http_status_code.h"
 #include "net/url_request/test_url_fetcher_factory.h"
 #include "net/url_request/url_request_status.h"
 #include "net/url_request/url_request_test_util.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
-using std::string;
 using testing::DoAll;
 using ::testing::AnyNumber;
 using ::testing::Eq;
 using ::testing::Return;
 using testing::SetArgPointee;
 using ::testing::NiceMock;
 using ::testing::StrictMock;
 using ::testing::_;
 
 namespace {
@@ skipping 65 matching lines @@
   suggestion->set_url(kTestUrl);
   suggestion->set_expiry_ts(kTestSetExpiry);
 
   suggestion = profile.add_suggestions();
   suggestion->set_title(kTestTitle);
   suggestion->set_url(kTestUrl);
 
   return profile;
 }
 
+class MockSyncService : public sync_driver::FakeSyncService {
+ public:
+  MockSyncService() {}
+  virtual ~MockSyncService() {}
+  MOCK_CONST_METHOD0(CanSyncStart, bool());
+  MOCK_CONST_METHOD0(IsSyncActive, bool());
+  MOCK_CONST_METHOD0(ConfigurationDone, bool());
+  MOCK_CONST_METHOD0(GetActiveDataTypes, syncer::ModelTypeSet());
+};
+
 class TestSuggestionsStore : public suggestions::SuggestionsStore {
  public:
   TestSuggestionsStore() {
     cached_suggestions = CreateSuggestionsProfile();
   }
   bool LoadSuggestions(SuggestionsProfile* suggestions) override {
     suggestions->CopyFrom(cached_suggestions);
     return cached_suggestions.suggestions_size();
   }
-  bool StoreSuggestions(const SuggestionsProfile& suggestions)
-      override {
+  bool StoreSuggestions(const SuggestionsProfile& suggestions) override {
     cached_suggestions.CopyFrom(suggestions);
     return true;
   }
   void ClearSuggestions() override {
     cached_suggestions = SuggestionsProfile();
   }
 
   SuggestionsProfile cached_suggestions;
 };
 
@@ skipping 18 matching lines @@
   MOCK_METHOD1(GetCandidateForUpload, bool(GURL*));
   MOCK_METHOD1(RemoveUrl, bool(const GURL&));
   MOCK_METHOD1(FilterSuggestions, void(SuggestionsProfile*));
   MOCK_METHOD0(ClearBlacklist, void());
 };
 
 class SuggestionsServiceTest : public testing::Test {
  public:
   void CheckCallback(const SuggestionsProfile& suggestions_profile) {
     ++suggestions_data_callback_count_;
+    if (suggestions_profile.suggestions_size() == 0)
+      ++suggestions_empty_data_count_;
   }
 
   void CheckSuggestionsData() {
     SuggestionsProfile suggestions_profile;
     test_suggestions_store_->LoadSuggestions(&suggestions_profile);
     EXPECT_EQ(1, suggestions_profile.suggestions_size());
     EXPECT_EQ(kTestTitle, suggestions_profile.suggestions(0).title());
     EXPECT_EQ(kTestUrl, suggestions_profile.suggestions(0).url());
     EXPECT_EQ(kTestFaviconUrl,
               suggestions_profile.suggestions(0).favicon_url());
     EXPECT_EQ(kTestImpressionUrl,
               suggestions_profile.suggestions(0).impression_url());
     EXPECT_EQ(kTestClickUrl, suggestions_profile.suggestions(0).click_url());
   }
 
-  void SetBlacklistFailure() {
-    blacklisting_failed_ = true;
-  }
-
-  void SetUndoBlacklistFailure() {
-    undo_blacklisting_failed_ = true;
-  }
-
-  void ExpectEmptySuggestionsProfile(const SuggestionsProfile& profile) {
-    EXPECT_EQ(0, profile.suggestions_size());
-    ++suggestions_empty_data_count_;
-  }
-
   int suggestions_data_callback_count_;
   int suggestions_empty_data_count_;
   bool blacklisting_failed_;
   bool undo_blacklisting_failed_;
 
  protected:
   SuggestionsServiceTest()
       : suggestions_data_callback_count_(0),
         suggestions_empty_data_count_(0),
         blacklisting_failed_(false),
         undo_blacklisting_failed_(false),
-        factory_(NULL, base::Bind(&CreateURLFetcher)),
-        mock_thumbnail_manager_(NULL),
-        mock_blacklist_store_(NULL),
-        test_suggestions_store_(NULL) {
+        factory_(nullptr, base::Bind(&CreateURLFetcher)),
+        mock_sync_service_(nullptr),
+        mock_thumbnail_manager_(nullptr),
+        mock_blacklist_store_(nullptr),
+        test_suggestions_store_(nullptr) {
     token_service_.UpdateCredentials(kAccountId, "refresh_token");
     token_service_.set_auto_post_fetch_response_on_message_loop(true);
   }
 
   ~SuggestionsServiceTest() override {}
 
   void SetOAuth2FeatureEnabled(bool enabled) {
     base::FeatureList::ClearInstanceForTesting();
     scoped_ptr<base::FeatureList> feature_list(new base::FeatureList);
     if (enabled) {
       feature_list->InitializeFromCommandLine(
           "SuggestionsServiceOAuth2", std::string());
     }
     base::FeatureList::SetInstance(std::move(feature_list));
   }
 
   void SetUp() override {
     request_context_ =
         new net::TestURLRequestContextGetter(io_message_loop_.task_runner());
   }
 
-  void FetchSuggestionsDataHelper(SyncState sync_state) {
-    scoped_ptr<SuggestionsService> suggestions_service(
-        CreateSuggestionsServiceWithMocks());
-    EXPECT_TRUE(suggestions_service != NULL);
+  SuggestionsService* CreateSuggestionsServiceWithMocks() {
+    mock_sync_service_.reset(new MockSyncService);
+    ON_CALL(*mock_sync_service_, CanSyncStart()).WillByDefault(Return(true));
+    ON_CALL(*mock_sync_service_, IsSyncActive()).WillByDefault(Return(true));
+    ON_CALL(*mock_sync_service_, ConfigurationDone())
+        .WillByDefault(Return(true));
+    ON_CALL(*mock_sync_service_, GetActiveDataTypes())
+        .WillByDefault(
+            Return(syncer::ModelTypeSet(syncer::HISTORY_DELETE_DIRECTIVES)));
 
-    SuggestionsProfile suggestions_profile = CreateSuggestionsProfile();
-
-    // Set up net::FakeURLFetcherFactory.
-    factory_.SetFakeResponse(SuggestionsService::BuildSuggestionsURL(),
-                             suggestions_profile.SerializeAsString(),
-                             net::HTTP_OK, net::URLRequestStatus::SUCCESS);
-
-    // Expectations.
-    EXPECT_CALL(*mock_thumbnail_manager_,
-                Initialize(EqualsProto(suggestions_profile)));
-    EXPECT_CALL(*mock_blacklist_store_, FilterSuggestions(_));
-    EXPECT_CALL(*mock_blacklist_store_, GetTimeUntilReadyForUpload(_))
-        .WillOnce(Return(false));
-
-    // Send the request. The data will be returned to the callback.
-    suggestions_service->FetchSuggestionsData(
-        sync_state, base::Bind(&SuggestionsServiceTest::CheckCallback,
-                               base::Unretained(this)));
-
-    // Ensure that CheckCallback() ran once.
-    EXPECT_EQ(1, suggestions_data_callback_count_);
-
-    // Let the network request run.
-    io_message_loop_.RunUntilIdle();
-
-    CheckSuggestionsData();
-  }
-
-  SuggestionsService* CreateSuggestionsServiceWithMocks() {
     // These objects are owned by the returned SuggestionsService, but we keep
     // the pointer around for testing.
     test_suggestions_store_ = new TestSuggestionsStore();
     mock_thumbnail_manager_ = new StrictMock<MockImageManager>();
     mock_blacklist_store_ = new StrictMock<MockBlacklistStore>();
     return new SuggestionsService(
         nullptr /* signin_manager */,
         &token_service_,
+        mock_sync_service_.get(),
         request_context_.get(),
         scoped_ptr<SuggestionsStore>(test_suggestions_store_),
         scoped_ptr<ImageManager>(mock_thumbnail_manager_),
         scoped_ptr<BlacklistStore>(mock_blacklist_store_));
   }
 
   void Blacklist(SuggestionsService* suggestions_service, GURL url) {
-    suggestions_service->BlacklistURL(
-        url, base::Bind(&SuggestionsServiceTest::CheckCallback,
-                        base::Unretained(this)),
-        base::Bind(&SuggestionsServiceTest::SetBlacklistFailure,
-                   base::Unretained(this)));
+    blacklisting_failed_ = !suggestions_service->BlacklistURL(url);
   }
 
   void UndoBlacklist(SuggestionsService* suggestions_service, GURL url) {
-    suggestions_service->UndoBlacklistURL(
-        url, base::Bind(&SuggestionsServiceTest::CheckCallback,
-                        base::Unretained(this)),
-        base::Bind(&SuggestionsServiceTest::SetUndoBlacklistFailure,
-                   base::Unretained(this)));
+    undo_blacklisting_failed_ = !suggestions_service->UndoBlacklistURL(url);
   }
 
   // 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) {
     scoped_ptr<SuggestionsService> suggestions_service(
         CreateSuggestionsServiceWithMocks());
-    EXPECT_TRUE(suggestions_service != NULL);
+    EXPECT_TRUE(suggestions_service != nullptr);
     // 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(_))
@@ skipping 21 matching lines @@
   }
 
   bool HasPendingSuggestionsRequest(SuggestionsService* suggestions_service) {
     return !!suggestions_service->pending_request_.get();
   }
 
  protected:
   base::MessageLoopForIO io_message_loop_;
   net::FakeURLFetcherFactory factory_;
   FakeProfileOAuth2TokenService token_service_;
+  scoped_ptr<MockSyncService> mock_sync_service_;
   // 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) {
-  FetchSuggestionsDataHelper(INITIALIZED_ENABLED_HISTORY);
+  scoped_ptr<SuggestionsService> suggestions_service(
+      CreateSuggestionsServiceWithMocks());
+  ASSERT_TRUE(suggestions_service != nullptr);
+  auto subscription = suggestions_service->AddCallback(base::Bind(
+      &SuggestionsServiceTest::CheckCallback, base::Unretained(this)));
+
+  SuggestionsProfile suggestions_profile = CreateSuggestionsProfile();
+
+  // Set up net::FakeURLFetcherFactory.
+  factory_.SetFakeResponse(SuggestionsService::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(_))
+      .WillOnce(Return(false));
+
+  // Send the request. The data should be returned to the callback.
+  suggestions_service->FetchSuggestionsData();
+
+  // Let the network request run.
+  io_message_loop_.RunUntilIdle();
+
+  // Ensure that CheckCallback() ran once.
+  EXPECT_EQ(1, suggestions_data_callback_count_);
+
+  CheckSuggestionsData();
 }
 
 TEST_F(SuggestionsServiceTest, FetchSuggestionsDataSyncNotInitializedEnabled) {
-  FetchSuggestionsDataHelper(NOT_INITIALIZED_ENABLED);
+  scoped_ptr<SuggestionsService> suggestions_service(
+      CreateSuggestionsServiceWithMocks());
+  ASSERT_TRUE(suggestions_service != nullptr);
+  EXPECT_CALL(*mock_sync_service_, IsSyncActive())
+      .WillRepeatedly(Return(false));
+
+  auto subscription = suggestions_service->AddCallback(base::Bind(
+      &SuggestionsServiceTest::CheckCallback, base::Unretained(this)));
+
+  // Try to fetch suggestions. Since sync is not active, no network request
+  // should be sent.
+  suggestions_service->FetchSuggestionsData();
+
+  // Let any network request run.
+  io_message_loop_.RunUntilIdle();
+
+  // Ensure that CheckCallback() didn't run.
+  EXPECT_EQ(0, suggestions_data_callback_count_);
+
+  // |test_suggestions_store_| should still contain the default values.
+  SuggestionsProfile suggestions;
+  test_suggestions_store_->LoadSuggestions(&suggestions);
+  EXPECT_EQ(CreateSuggestionsProfile().SerializeAsString(),
+            suggestions.SerializeAsString());
 }
 
 TEST_F(SuggestionsServiceTest, FetchSuggestionsDataSyncDisabled) {
   scoped_ptr<SuggestionsService> suggestions_service(
       CreateSuggestionsServiceWithMocks());
-  EXPECT_TRUE(suggestions_service != NULL);
+  ASSERT_TRUE(suggestions_service != nullptr);
+  EXPECT_CALL(*mock_sync_service_, CanSyncStart())
+      .WillRepeatedly(Return(false));
 
-  // Send the request. Cache is cleared and empty data will be returned to the
-  // callback.
-  suggestions_service->FetchSuggestionsData(
-      SYNC_OR_HISTORY_SYNC_DISABLED,
-      base::Bind(&SuggestionsServiceTest::ExpectEmptySuggestionsProfile,
-                 base::Unretained(this)));
+  auto subscription = suggestions_service->AddCallback(base::Bind(
+      &SuggestionsServiceTest::CheckCallback, base::Unretained(this)));
 
-  // Ensure that ExpectEmptySuggestionsProfile ran once.
+  // Tell SuggestionsService that the sync state changed. The cache should be
+  // cleared and empty data returned to the callback.
+  suggestions_service->OnStateChanged();
+
+  // Ensure that CheckCallback ran once with empty data.
+  EXPECT_EQ(1, suggestions_data_callback_count_);
   EXPECT_EQ(1, suggestions_empty_data_count_);
+
+  // Try to fetch suggestions. Since sync is not active, no network request
+  // should be sent.
+  suggestions_service->FetchSuggestionsData();
+
+  // Let any network request run.
+  io_message_loop_.RunUntilIdle();
+
+  // Ensure that CheckCallback didn't run again.
+  EXPECT_EQ(1, suggestions_data_callback_count_);
 }
 
 TEST_F(SuggestionsServiceTest, FetchSuggestionsDataNoAccessToken) {
   SetOAuth2FeatureEnabled(true);
 
   token_service_.RevokeCredentials(kAccountId);
 
   scoped_ptr<SuggestionsService> suggestions_service(
       CreateSuggestionsServiceWithMocks());
-  ASSERT_TRUE(suggestions_service != NULL);
+  ASSERT_TRUE(suggestions_service != nullptr);
 
-  // We should get served from cache.
-  // TODO(treib,mathp): Is this the correct behavior when the credentials have
-  // expired? Currently, the SuggestionsService immediately serves from cache,
-  // before even attempting any online auth.
-  EXPECT_CALL(*mock_thumbnail_manager_, Initialize(_));
-  EXPECT_CALL(*mock_blacklist_store_, FilterSuggestions(_));
+  auto subscription = suggestions_service->AddCallback(base::Bind(
+      &SuggestionsServiceTest::CheckCallback, base::Unretained(this)));
+
   EXPECT_CALL(*mock_blacklist_store_, GetTimeUntilReadyForUpload(_))
       .WillOnce(Return(false));
 
-  suggestions_service->FetchSuggestionsData(
-      INITIALIZED_ENABLED_HISTORY,
-      base::Bind(&SuggestionsServiceTest::CheckCallback,
-                 base::Unretained(this)));
+  suggestions_service->FetchSuggestionsData();
 
-  EXPECT_EQ(1, suggestions_data_callback_count_);
-
-  // But no network request should be sent.
+  // No network request should be sent.
   io_message_loop_.RunUntilIdle();
   EXPECT_FALSE(HasPendingSuggestionsRequest(suggestions_service.get()));
+  EXPECT_EQ(0, suggestions_data_callback_count_);
 }
 
 TEST_F(SuggestionsServiceTest, IssueRequestIfNoneOngoingError) {
   scoped_ptr<SuggestionsService> suggestions_service(
       CreateSuggestionsServiceWithMocks());
-  EXPECT_TRUE(suggestions_service != NULL);
+  ASSERT_TRUE(suggestions_service != nullptr);
 
   // Fake a request error.
   factory_.SetFakeResponse(SuggestionsService::BuildSuggestionsURL(),
                            "irrelevant", net::HTTP_OK,
                            net::URLRequestStatus::FAILED);
 
   EXPECT_CALL(*mock_blacklist_store_, GetTimeUntilReadyForUpload(_))
       .WillOnce(Return(false));
 
   // Send the request. Empty data will be returned to the callback.
   suggestions_service->IssueRequestIfNoneOngoing(
       SuggestionsService::BuildSuggestionsURL());
 
   // (Testing only) wait until suggestion fetch is complete.
   io_message_loop_.RunUntilIdle();
 }
 
 TEST_F(SuggestionsServiceTest, IssueRequestIfNoneOngoingResponseNotOK) {
   scoped_ptr<SuggestionsService> suggestions_service(
       CreateSuggestionsServiceWithMocks());
-  EXPECT_TRUE(suggestions_service != NULL);
+  ASSERT_TRUE(suggestions_service != nullptr);
 
   // Fake a non-200 response code.
   factory_.SetFakeResponse(SuggestionsService::BuildSuggestionsURL(),
                            "irrelevant", net::HTTP_BAD_REQUEST,
                            net::URLRequestStatus::SUCCESS);
 
   // Expect that an upload to the blacklist is scheduled.
   EXPECT_CALL(*mock_blacklist_store_, GetTimeUntilReadyForUpload(_))
       .WillOnce(Return(false));
 
   // Send the request. Empty data will be returned to the callback.
   suggestions_service->IssueRequestIfNoneOngoing(
       SuggestionsService::BuildSuggestionsURL());
 
   // (Testing only) wait until suggestion fetch is complete.
   io_message_loop_.RunUntilIdle();
 
   // Expect no suggestions in the cache.
   SuggestionsProfile empty_suggestions;
   EXPECT_FALSE(test_suggestions_store_->LoadSuggestions(&empty_suggestions));
 }
 
 TEST_F(SuggestionsServiceTest, BlacklistURL) {
   scoped_ptr<SuggestionsService> suggestions_service(
       CreateSuggestionsServiceWithMocks());
-  EXPECT_TRUE(suggestions_service != NULL);
+  EXPECT_TRUE(suggestions_service != nullptr);
   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(
       SuggestionsService::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(EqualsProto(suggestions_profile)));
+  EXPECT_CALL(*mock_thumbnail_manager_, Initialize(_)).Times(2);
 
   // Expected calls to the blacklist store.
   EXPECT_CALL(*mock_blacklist_store_, BlacklistUrl(Eq(blacklisted_url)))
       .WillOnce(Return(true));
-  EXPECT_CALL(*mock_blacklist_store_, FilterSuggestions(_));
+  EXPECT_CALL(*mock_blacklist_store_, FilterSuggestions(_)).Times(2);
   EXPECT_CALL(*mock_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)))
       .WillOnce(Return(true));
 
   Blacklist(suggestions_service.get(), blacklisted_url);
+  EXPECT_EQ(1, suggestions_data_callback_count_);
 
   // Wait on the upload 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). Then wait on
   // the blacklist request, then again on the next blacklist scheduling task.
   base::MessageLoop::current()->RunUntilIdle();
   io_message_loop_.RunUntilIdle();
   base::MessageLoop::current()->RunUntilIdle();
 
-  EXPECT_EQ(1, suggestions_data_callback_count_);
+  EXPECT_EQ(2, suggestions_data_callback_count_);
   EXPECT_FALSE(blacklisting_failed_);
   CheckSuggestionsData();
 }
 
 TEST_F(SuggestionsServiceTest, BlacklistURLFails) {
   scoped_ptr<SuggestionsService> suggestions_service(
       CreateSuggestionsServiceWithMocks());
-  EXPECT_TRUE(suggestions_service != NULL);
+  ASSERT_TRUE(suggestions_service != nullptr);
+
+  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)))
       .WillOnce(Return(false));
 
   Blacklist(suggestions_service.get(), blacklisted_url);
 
   EXPECT_TRUE(blacklisting_failed_);
   EXPECT_EQ(0, suggestions_data_callback_count_);
 }
 
 // Initial blacklist request fails, triggering a second which succeeds.
 TEST_F(SuggestionsServiceTest, BlacklistURLRequestFails) {
   scoped_ptr<SuggestionsService> suggestions_service(
       CreateSuggestionsServiceWithMocks());
-  EXPECT_TRUE(suggestions_service != NULL);
+  ASSERT_TRUE(suggestions_service != nullptr);
   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(
       SuggestionsService::BuildSuggestionsBlacklistURL(blacklisted_url));
   GURL blacklisted_url_alt(kBlacklistedUrlAlt);
   GURL request_url_alt(
       SuggestionsService::BuildSuggestionsBlacklistURL(blacklisted_url_alt));
   SuggestionsProfile suggestions_profile = CreateSuggestionsProfile();
 
   // 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(EqualsProto(suggestions_profile)));
+  EXPECT_CALL(*mock_thumbnail_manager_, Initialize(_)).Times(2);
   EXPECT_CALL(*mock_blacklist_store_, BlacklistUrl(Eq(blacklisted_url)))
       .WillOnce(Return(true));
-  EXPECT_CALL(*mock_blacklist_store_, FilterSuggestions(_));
+  EXPECT_CALL(*mock_blacklist_store_, FilterSuggestions(_)).Times(2);
   EXPECT_CALL(*mock_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)))
       .WillOnce(Return(true));
 
@@ skipping 10 matching lines @@
   io_message_loop_.RunUntilIdle();
   base::MessageLoop::current()->RunUntilIdle();
   io_message_loop_.RunUntilIdle();
   base::MessageLoop::current()->RunUntilIdle();
   CheckSuggestionsData();
 }
 
 TEST_F(SuggestionsServiceTest, UndoBlacklistURL) {
   scoped_ptr<SuggestionsService> suggestions_service(
       CreateSuggestionsServiceWithMocks());
-  EXPECT_TRUE(suggestions_service != NULL);
+  ASSERT_TRUE(suggestions_service != nullptr);
   // 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)))
       .Times(AnyNumber());
   EXPECT_CALL(*mock_blacklist_store_, FilterSuggestions(_))
@@ skipping 11 matching lines @@
   UndoBlacklist(suggestions_service.get(), blacklisted_url);
 
   EXPECT_EQ(2, suggestions_data_callback_count_);
   EXPECT_FALSE(blacklisting_failed_);
   EXPECT_FALSE(undo_blacklisting_failed_);
 }
 
 TEST_F(SuggestionsServiceTest, ClearBlacklist) {
   scoped_ptr<SuggestionsService> suggestions_service(
       CreateSuggestionsServiceWithMocks());
-  EXPECT_TRUE(suggestions_service != NULL);
+  ASSERT_TRUE(suggestions_service != nullptr);
   // 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(
       SuggestionsService::BuildSuggestionsBlacklistClearURL(),
       suggestions_profile.SerializeAsString(), net::HTTP_OK,
       net::URLRequestStatus::SUCCESS);
 
   // Blacklist expectations.
   EXPECT_CALL(*mock_blacklist_store_, BlacklistUrl(Eq(blacklisted_url)))
       .WillOnce(Return(true));
   EXPECT_CALL(*mock_thumbnail_manager_,
               Initialize(EqualsProto(suggestions_profile)))
       .Times(AnyNumber());
   EXPECT_CALL(*mock_blacklist_store_, FilterSuggestions(_)).Times(AnyNumber());
   EXPECT_CALL(*mock_blacklist_store_, GetTimeUntilReadyForUpload(_))
       .WillOnce(DoAll(SetArgPointee<0>(delay), Return(true)));
   EXPECT_CALL(*mock_blacklist_store_, ClearBlacklist());
 
   Blacklist(suggestions_service.get(), blacklisted_url);
-  suggestions_service->ClearBlacklist(base::Bind(
-      &SuggestionsServiceTest::CheckCallback, base::Unretained(this)));
+  suggestions_service->ClearBlacklist();
 
   EXPECT_EQ(2, suggestions_data_callback_count_);
   EXPECT_FALSE(blacklisting_failed_);
 }
 
 TEST_F(SuggestionsServiceTest, UndoBlacklistURLFailsIfNotInBlacklist) {
   UndoBlacklistURLFailsHelper(true);
 }
 
 TEST_F(SuggestionsServiceTest, UndoBlacklistURLFailsIfAlreadyCandidate) {
   UndoBlacklistURLFailsHelper(false);
 }
 
 TEST_F(SuggestionsServiceTest, GetBlacklistedUrl) {
   scoped_ptr<GURL> request_url;
   scoped_ptr<net::FakeURLFetcher> fetcher;
   GURL retrieved_url;
 
   // Not a blacklist request.
   request_url.reset(new GURL("http://not-blacklisting.com/a?b=c"));
-  fetcher = CreateURLFetcher(*request_url, NULL, "", net::HTTP_OK,
+  fetcher = CreateURLFetcher(*request_url, nullptr, "", net::HTTP_OK,
                              net::URLRequestStatus::SUCCESS);
   EXPECT_FALSE(SuggestionsService::GetBlacklistedUrl(*fetcher, &retrieved_url));
 
   // An actual blacklist request.
   string blacklisted_url = "http://blacklisted.com/a?b=c&d=e";
   string encoded_blacklisted_url =
       "http%3A%2F%2Fblacklisted.com%2Fa%3Fb%3Dc%26d%3De";
   string blacklist_request_prefix(
       SuggestionsService::BuildSuggestionsBlacklistURLPrefix());
   request_url.reset(
       new GURL(blacklist_request_prefix + encoded_blacklisted_url));
   fetcher.reset();
-  fetcher = CreateURLFetcher(*request_url, NULL, "", net::HTTP_OK,
+  fetcher = CreateURLFetcher(*request_url, nullptr, "", net::HTTP_OK,
                              net::URLRequestStatus::SUCCESS);
   EXPECT_TRUE(SuggestionsService::GetBlacklistedUrl(*fetcher, &retrieved_url));
   EXPECT_EQ(blacklisted_url, retrieved_url.spec());
 }
 
 TEST_F(SuggestionsServiceTest, UpdateBlacklistDelay) {
   scoped_ptr<SuggestionsService> suggestions_service(
       CreateSuggestionsServiceWithMocks());
   base::TimeDelta initial_delay = suggestions_service->blacklist_delay();
 
@@ skipping 17 matching lines @@
       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, GetPageThumbnail) {
   scoped_ptr<SuggestionsService> suggestions_service(
       CreateSuggestionsServiceWithMocks());
-  ASSERT_TRUE(suggestions_service != NULL);
+  ASSERT_TRUE(suggestions_service != nullptr);
 
   GURL test_url(kTestUrl);
   GURL thumbnail_url("https://www.thumbnails.com/thumb.jpg");
   base::Callback<void(const GURL&, const SkBitmap*)> dummy_callback;
 
   EXPECT_CALL(*mock_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);
 
 }
 
 }  // namespace suggestions