BASELINE: Update Chromium to 60.0.3112.70

Change-Id: I9911c2280a014d4632f254857876a395d4baed2d
Reviewed-by: Alexandru Croitor <alexandru.croitor@qt.io>

10 files changed, 635 insertions, 516 deletions

diff --git a/chromium/components/suggestions/BUILD.gn b/chromium/components/suggestions/BUILD.gn
index 9d4a692b276..6d93d045bc4 100644
--- a/chromium/components/suggestions/BUILD.gn
+++ b/chromium/components/suggestions/BUILD.gn
@@ -6,6 +6,8 @@ static_library("suggestions") {
   sources = [
     "blacklist_store.cc",
     "blacklist_store.h",
+    "features.cc",
+    "features.h",
     "image_encoder.h",
     "image_manager.cc",
     "image_manager.h",
diff --git a/chromium/components/suggestions/features.cc b/chromium/components/suggestions/features.cc
new file mode 100644
index 00000000000..2cac42d4bbd
--- /dev/null
+++ b/chromium/components/suggestions/features.cc
@@ -0,0 +1,12 @@
+// Copyright 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "components/suggestions/features.h"
+
+namespace suggestions {
+
+const base::Feature kUseSuggestionsEvenIfFewFeature{
+    "UseSuggestionsEvenIfFew", base::FEATURE_DISABLED_BY_DEFAULT};
+
+}  // namespace suggestions
diff --git a/chromium/components/suggestions/features.h b/chromium/components/suggestions/features.h
new file mode 100644
index 00000000000..8a7945946c2
--- /dev/null
+++ b/chromium/components/suggestions/features.h
@@ -0,0 +1,18 @@
+// Copyright 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef COMPONENTS_SUGGESTIONS_FEATURES_H_
+#define COMPONENTS_SUGGESTIONS_FEATURES_H_
+
+#include "base/feature_list.h"
+
+namespace suggestions {
+
+// If this feature is enabled, we request and use suggestions even if there are
+// only very few of them.
+extern const base::Feature kUseSuggestionsEvenIfFewFeature;
+
+}  // namespace suggestions
+
+#endif  // COMPONENTS_SUGGESTIONS_FEATURES_H_
diff --git a/chromium/components/suggestions/image_encoder.cc b/chromium/components/suggestions/image_encoder.cc
index 73283cf5bbb..8b0fa09fc26 100644
--- a/chromium/components/suggestions/image_encoder.cc
+++ b/chromium/components/suggestions/image_encoder.cc
@@ -17,7 +17,6 @@ std::unique_ptr<SkBitmap> DecodeJPEGToSkBitmap(const void* encoded_data,
 
 bool EncodeSkBitmapToJPEG(const SkBitmap& bitmap,
                           std::vector<unsigned char>* dest) {
-  SkAutoLockPixels bitmap_lock(bitmap);
   if (!bitmap.readyToDraw() || bitmap.isNull()) {
     return false;
   }
diff --git a/chromium/components/suggestions/image_manager.cc b/chromium/components/suggestions/image_manager.cc
index 366a6a2018c..d2fb227f9ac 100644
--- a/chromium/components/suggestions/image_manager.cc
+++ b/chromium/components/suggestions/image_manager.cc
@@ -12,6 +12,7 @@
 #include "base/task_runner_util.h"
 #include "components/image_fetcher/core/image_fetcher.h"
 #include "components/suggestions/image_encoder.h"
+#include "net/traffic_annotation/network_traffic_annotation.h"
 #include "ui/gfx/image/image.h"
 
 using leveldb_proto::ProtoDatabase;
@@ -39,6 +40,39 @@ void WrapCallback(
   wrapped_callback.Run(GURL(url), image);
 }
 
+constexpr net::NetworkTrafficAnnotationTag kTrafficAnnotation =
+    net::DefineNetworkTrafficAnnotation("suggestions_image_manager", R"(
+        semantics {
+          sender: "Suggestions Service Thumbnail Fetch"
+          description:
+            "Retrieves thumbnails for site suggestions based on the user's "
+            "synced browsing history, for use e.g. on the New Tab page."
+          trigger:
+            "Triggered when a thumbnail for a suggestion is required, and no "
+            "local thumbnail is available."
+          data: "The URL for which to retrieve a thumbnail."
+          destination: GOOGLE_OWNED_SERVICE
+        }
+        policy {
+          cookies_allowed: false
+          setting:
+            "Users can disable this feature by signing out of Chrome, or "
+            "disabling Sync or History Sync in Chrome settings under 'Advanced "
+            "sync settings...'. The feature is enabled by default."
+        chrome_policy {
+          SyncDisabled {
+            policy_options {mode: MANDATORY}
+            SyncDisabled: true
+          }
+        }
+        chrome_policy {
+          SigninAllowed {
+            policy_options {mode: MANDATORY}
+            SigninAllowed: false
+          }
+        }
+      })");
+
 }  // namespace
 
 namespace suggestions {
@@ -146,7 +180,8 @@ void ImageManager::OnCacheImageDecoded(
     callback.Run(url, gfx::Image::CreateFrom1xBitmap(*bitmap));
   } else {
     image_fetcher_->StartOrQueueNetworkRequest(
-        url.spec(), image_url, base::Bind(&WrapCallback, callback));
+        url.spec(), image_url, base::Bind(&WrapCallback, callback),
+        kTrafficAnnotation);
   }
 }
 
@@ -173,7 +208,8 @@ void ImageManager::ServeFromCacheOrNetwork(
                    weak_ptr_factory_.GetWeakPtr(), url, image_url, callback));
   } else {
     image_fetcher_->StartOrQueueNetworkRequest(
-        url.spec(), image_url, base::Bind(&WrapCallback, callback));
+        url.spec(), image_url, base::Bind(&WrapCallback, callback),
+        kTrafficAnnotation);
   }
 }
 
diff --git a/chromium/components/suggestions/image_manager_unittest.cc b/chromium/components/suggestions/image_manager_unittest.cc
index 0966a21a7f1..86717dd169b 100644
--- a/chromium/components/suggestions/image_manager_unittest.cc
+++ b/chromium/components/suggestions/image_manager_unittest.cc
@@ -18,6 +18,7 @@
 #include "components/leveldb_proto/testing/fake_db.h"
 #include "components/suggestions/image_encoder.h"
 #include "components/suggestions/proto/suggestions.pb.h"
+#include "net/traffic_annotation/network_traffic_annotation_test_helper.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/gfx/geometry/size.h"
@@ -41,7 +42,7 @@ const char kInvalidImagePath[] = "files/DOESNOTEXIST";
 
 using leveldb_proto::test::FakeDB;
 
-typedef base::hash_map<std::string, ImageData> EntryMap;
+typedef std::map<std::string, ImageData> EntryMap;
 
 void AddEntry(const ImageData& d, EntryMap* map) { (*map)[d.url()] = d; }
 
@@ -49,10 +50,11 @@ class MockImageFetcher : public ImageFetcher {
  public:
   MockImageFetcher() {}
   virtual ~MockImageFetcher() {}
-  MOCK_METHOD3(StartOrQueueNetworkRequest,
+  MOCK_METHOD4(StartOrQueueNetworkRequest,
                void(const std::string&,
                     const GURL&,
-                    const ImageFetcherCallback&));
+                    const ImageFetcherCallback&,
+                    const net::NetworkTrafficAnnotationTag&));
   MOCK_METHOD1(SetImageFetcherDelegate, void(ImageFetcherDelegate*));
   MOCK_METHOD1(SetDataUseServiceName, void(DataUseServiceName));
   MOCK_METHOD1(SetImageDownloadLimit,
@@ -178,7 +180,7 @@ TEST_F(ImageManagerTest, GetImageForURLNetwork) {
   InitializeDefaultImageMapAndDatabase(image_manager_.get(), fake_db_);
 
   // We expect the fetcher to go to network and call the callback.
-  EXPECT_CALL(*mock_image_fetcher_, StartOrQueueNetworkRequest(_, _, _));
+  EXPECT_CALL(*mock_image_fetcher_, StartOrQueueNetworkRequest(_, _, _, _));
 
   // Fetch existing URL.
   base::RunLoop run_loop;
diff --git a/chromium/components/suggestions/suggestions_service.h b/chromium/components/suggestions/suggestions_service.h
index 9773ee07418..76a118290f6 100644
--- a/chromium/components/suggestions/suggestions_service.h
+++ b/chromium/components/suggestions/suggestions_service.h
@@ -17,7 +17,7 @@
 
 namespace gfx {
 class Image;
-}  // namespce gfx
+}  // namespace gfx
 
 namespace suggestions {
 
diff --git a/chromium/components/suggestions/suggestions_service_impl.cc b/chromium/components/suggestions/suggestions_service_impl.cc
index 6e41445d9a6..aacd400d7b5 100644
--- a/chromium/components/suggestions/suggestions_service_impl.cc
+++ b/chromium/components/suggestions/suggestions_service_impl.cc
@@ -10,6 +10,7 @@
 #include "base/feature_list.h"
 #include "base/location.h"
 #include "base/memory/ptr_util.h"
+#include "base/metrics/field_trial_params.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/metrics/sparse_histogram.h"
 #include "base/strings/string_number_conversions.h"
@@ -22,6 +23,7 @@
 #include "components/pref_registry/pref_registry_syncable.h"
 #include "components/signin/core/browser/signin_manager_base.h"
 #include "components/suggestions/blacklist_store.h"
+#include "components/suggestions/features.h"
 #include "components/suggestions/image_manager.h"
 #include "components/suggestions/suggestions_store.h"
 #include "components/sync/driver/sync_service.h"
@@ -46,37 +48,6 @@ namespace suggestions {
 
 namespace {
 
-// Establishes the different sync states that matter to SuggestionsService.
-// There are three different concepts in the sync service: initialized, sync
-// enabled and history sync enabled.
-enum SyncState {
-  // State: Sync service is not initialized, yet not disabled. History sync
-  //     state is unknown (since not initialized).
-  // Behavior: Does not issue a server request, but serves from cache if
-  //     available.
-  NOT_INITIALIZED_ENABLED,
-
-  // State: Sync service is initialized, sync is enabled and history sync is
-  //     enabled.
-  // Behavior: Update suggestions from the server. Serve from cache on timeout.
-  INITIALIZED_ENABLED_HISTORY,
-
-  // State: Sync service is disabled or history sync is disabled.
-  // Behavior: Do not issue a server request. Clear the cache. Serve empty
-  //     suggestions.
-  SYNC_OR_HISTORY_SYNC_DISABLED,
-};
-
-SyncState GetSyncState(syncer::SyncService* sync) {
-  if (!sync || !sync->CanSyncStart() || sync->IsLocalSyncEnabled())
-    return SYNC_OR_HISTORY_SYNC_DISABLED;
-  if (!sync->IsSyncActive() || !sync->ConfigurationDone())
-    return NOT_INITIALIZED_ENABLED;
-  return sync->GetActiveDataTypes().Has(syncer::HISTORY_DELETE_DIRECTIVES)
-             ? INITIALIZED_ENABLED_HISTORY
-             : SYNC_OR_HISTORY_SYNC_DISABLED;
-}
-
 // Used to UMA log the state of the last response from the server.
 enum SuggestionsResponseState {
   RESPONSE_EMPTY,
@@ -113,13 +84,18 @@ GURL GetGoogleBaseURL() {
 // Format strings for the various suggestions URLs. They all have two string
 // params: The Google base URL and the device type.
 // TODO(mathp): Put this in TemplateURL.
-const char kSuggestionsURLFormat[] = "%schromesuggestions?t=%s";
+const char kSuggestionsURLFormat[] = "%schromesuggestions?%s";
 const char kSuggestionsBlacklistURLPrefixFormat[] =
     "%schromesuggestions/blacklist?t=%s&url=";
 const char kSuggestionsBlacklistClearURLFormat[] =
     "%schromesuggestions/blacklist/clear?t=%s";
 
 const char kSuggestionsBlacklistURLParam[] = "url";
+const char kSuggestionsDeviceParam[] = "t=%s";
+const char kSuggestionsMinParam[] = "min=%i";
+
+const char kSuggestionsMinVariationName[] = "min_suggestions";
+const int kSuggestionsMinVariationDefault = 0;
 
 #if defined(OS_ANDROID) || defined(OS_IOS)
 const char kDeviceType[] = "2";
@@ -136,6 +112,12 @@ const char kFaviconURL[] =
 // The default expiry timeout is 168 hours.
 const int64_t kDefaultExpiryUsec = 168 * base::Time::kMicrosecondsPerHour;
 
+int GetMinimumSuggestionsCount() {
+  return base::GetFieldTrialParamByFeatureAsInt(
+      kUseSuggestionsEvenIfFewFeature, kSuggestionsMinVariationName,
+      kSuggestionsMinVariationDefault);
+}
+
 }  // namespace
 
 SuggestionsServiceImpl::SuggestionsServiceImpl(
@@ -150,6 +132,7 @@ SuggestionsServiceImpl::SuggestionsServiceImpl(
       token_service_(token_service),
       sync_service_(sync_service),
       sync_service_observer_(this),
+      sync_state_(INITIALIZED_ENABLED_HISTORY),
       url_request_context_(url_request_context),
       suggestions_store_(std::move(suggestions_store)),
       thumbnail_manager_(std::move(thumbnail_manager)),
@@ -157,8 +140,9 @@ SuggestionsServiceImpl::SuggestionsServiceImpl(
       scheduling_delay_(TimeDelta::FromSeconds(kDefaultSchedulingDelaySec)),
       weak_ptr_factory_(this) {
   // |sync_service_| is null if switches::kDisableSync is set (tests use that).
-  if (sync_service_)
+  if (sync_service_) {
     sync_service_observer_.Add(sync_service_);
+  }
   // Immediately get the current sync state, so we'll flush the cache if
   // necessary.
   OnStateChanged(sync_service_);
@@ -169,8 +153,9 @@ SuggestionsServiceImpl::~SuggestionsServiceImpl() {}
 bool SuggestionsServiceImpl::FetchSuggestionsData() {
   DCHECK(thread_checker_.CalledOnValidThread());
   // If sync state allows, issue a network request to refresh the suggestions.
-  if (GetSyncState(sync_service_) != INITIALIZED_ENABLED_HISTORY)
+  if (sync_state_ != INITIALIZED_ENABLED_HISTORY) {
     return false;
+  }
   IssueRequestIfNoneOngoing(BuildSuggestionsURL());
   return true;
 }
@@ -208,6 +193,8 @@ void SuggestionsServiceImpl::GetPageThumbnailWithURL(
 bool SuggestionsServiceImpl::BlacklistURL(const GURL& candidate_url) {
   DCHECK(thread_checker_.CalledOnValidThread());
 
+  // TODO(treib): Do we need to check |sync_state_| here?
+
   if (!blacklist_store_->BlacklistUrl(candidate_url))
     return false;
 
@@ -224,6 +211,9 @@ bool SuggestionsServiceImpl::BlacklistURL(const GURL& candidate_url) {
 
 bool SuggestionsServiceImpl::UndoBlacklistURL(const GURL& url) {
   DCHECK(thread_checker_.CalledOnValidThread());
+
+  // TODO(treib): Do we need to check |sync_state_| here?
+
   TimeDelta time_delta;
   if (blacklist_store_->GetTimeUntilURLReadyForUpload(url, &time_delta) &&
       time_delta > TimeDelta::FromSeconds(0) &&
@@ -239,6 +229,9 @@ bool SuggestionsServiceImpl::UndoBlacklistURL(const GURL& url) {
 
 void SuggestionsServiceImpl::ClearBlacklist() {
   DCHECK(thread_checker_.CalledOnValidThread());
+
+  // TODO(treib): Do we need to check |sync_state_| here?
+
   blacklist_store_->ClearBlacklist();
   callback_list_.Notify(
       GetSuggestionsDataFromCache().value_or(SuggestionsProfile()));
@@ -276,8 +269,16 @@ void SuggestionsServiceImpl::RegisterProfilePrefs(
 
 // static
 GURL SuggestionsServiceImpl::BuildSuggestionsURL() {
+  std::string device = base::StringPrintf(kSuggestionsDeviceParam, kDeviceType);
+  std::string query = device;
+  if (base::FeatureList::IsEnabled(kUseSuggestionsEvenIfFewFeature)) {
+    std::string min_suggestions =
+        base::StringPrintf(kSuggestionsMinParam, GetMinimumSuggestionsCount());
+    query =
+        base::StringPrintf("%s&%s", device.c_str(), min_suggestions.c_str());
+  }
   return GURL(base::StringPrintf(
-      kSuggestionsURLFormat, GetGoogleBaseURL().spec().c_str(), kDeviceType));
+      kSuggestionsURLFormat, GetGoogleBaseURL().spec().c_str(), query.c_str()));
 }
 
 // static
@@ -300,22 +301,64 @@ GURL SuggestionsServiceImpl::BuildSuggestionsBlacklistClearURL() {
                                  kDeviceType));
 }
 
-void SuggestionsServiceImpl::OnStateChanged(syncer::SyncService* sync) {
-  switch (GetSyncState(sync_service_)) {
+SuggestionsServiceImpl::SyncState SuggestionsServiceImpl::ComputeSyncState()
+    const {
+  if (!sync_service_ || !sync_service_->CanSyncStart() ||
+      sync_service_->IsLocalSyncEnabled()) {
+    return SYNC_OR_HISTORY_SYNC_DISABLED;
+  }
+  if (!sync_service_->IsSyncActive() || !sync_service_->ConfigurationDone()) {
+    return NOT_INITIALIZED_ENABLED;
+  }
+  return sync_service_->GetActiveDataTypes().Has(
+             syncer::HISTORY_DELETE_DIRECTIVES)
+             ? INITIALIZED_ENABLED_HISTORY
+             : SYNC_OR_HISTORY_SYNC_DISABLED;
+}
+
+SuggestionsServiceImpl::RefreshAction
+SuggestionsServiceImpl::RefreshSyncState() {
+  SyncState new_sync_state = ComputeSyncState();
+  if (sync_state_ == new_sync_state) {
+    return NO_ACTION;
+  }
+
+  SyncState old_sync_state = sync_state_;
+  sync_state_ = new_sync_state;
+
+  switch (new_sync_state) {
+    case NOT_INITIALIZED_ENABLED:
+      break;
+    case INITIALIZED_ENABLED_HISTORY:
+      // If the user just signed in, we fetch suggestions, so that hopefully the
+      // next NTP will already get them.
+      if (old_sync_state == SYNC_OR_HISTORY_SYNC_DISABLED) {
+        return FETCH_SUGGESTIONS;
+      }
+      break;
     case SYNC_OR_HISTORY_SYNC_DISABLED:
+      // If the user signed out (or disabled history sync), we have to clear
+      // everything.
+      return CLEAR_SUGGESTIONS;
+  }
+  // Otherwise, there's nothing to do.
+  return NO_ACTION;
+}
+
+void SuggestionsServiceImpl::OnStateChanged(syncer::SyncService* sync) {
+  DCHECK(sync_service_ == sync);
+
+  switch (RefreshSyncState()) {
+    case NO_ACTION:
+      break;
+    case CLEAR_SUGGESTIONS:
       // Cancel any ongoing request, to stop interacting with the server.
       pending_request_.reset(nullptr);
       suggestions_store_->ClearSuggestions();
       callback_list_.Notify(SuggestionsProfile());
       break;
-    case NOT_INITIALIZED_ENABLED:
-      // Keep the cache (if any), but don't refresh.
-      break;
-    case INITIALIZED_ENABLED_HISTORY:
-      // If we have any observers, issue a network request to refresh the
-      // suggestions in the cache.
-      if (!callback_list_.empty())
-        IssueRequestIfNoneOngoing(BuildSuggestionsURL());
+    case FETCH_SUGGESTIONS:
+      IssueRequestIfNoneOngoing(BuildSuggestionsURL());
       break;
   }
 }
@@ -335,6 +378,10 @@ void SuggestionsServiceImpl::SetDefaultExpiryTimestamp(
 
 void SuggestionsServiceImpl::IssueRequestIfNoneOngoing(const GURL& url) {
   // If there is an ongoing request, let it complete.
+  // This will silently swallow blacklist and clearblacklist requests if a
+  // request happens to be ongoing.
+  // TODO(treib): Queue such requests and send them after the current one
+  // completes.
   if (pending_request_.get()) {
     return;
   }
diff --git a/chromium/components/suggestions/suggestions_service_impl.h b/chromium/components/suggestions/suggestions_service_impl.h
index ff31fc84ce4..f01cb854745 100644
--- a/chromium/components/suggestions/suggestions_service_impl.h
+++ b/chromium/components/suggestions/suggestions_service_impl.h
@@ -12,6 +12,7 @@
 
 #include "base/callback.h"
 #include "base/callback_list.h"
+#include "base/compiler_specific.h"
 #include "base/gtest_prod_util.h"
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
@@ -77,6 +78,16 @@ class SuggestionsServiceImpl : public SuggestionsService,
   bool UndoBlacklistURL(const GURL& url) override;
   void ClearBlacklist() override;
 
+  base::TimeDelta blacklist_delay_for_testing() const {
+    return scheduling_delay_;
+  }
+  void set_blacklist_delay_for_testing(base::TimeDelta delay) {
+    scheduling_delay_ = delay;
+  }
+  bool has_pending_request_for_testing() const {
+    return !!pending_request_.get();
+  }
+
   // Determines which URL a blacklist request was for, irrespective of the
   // request's status. Returns false if |request| is not a blacklist request.
   static bool GetBlacklistedUrl(const net::URLFetcher& request, GURL* url);
@@ -85,23 +96,27 @@ class SuggestionsServiceImpl : public SuggestionsService,
   static void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry);
 
  private:
-  friend class SuggestionsServiceTest;
-  FRIEND_TEST_ALL_PREFIXES(SuggestionsServiceTest, FetchSuggestionsData);
-  FRIEND_TEST_ALL_PREFIXES(SuggestionsServiceTest,
-                           FetchSuggestionsDataSyncDisabled);
-  FRIEND_TEST_ALL_PREFIXES(SuggestionsServiceTest,
-                           FetchSuggestionsDataNoAccessToken);
-  FRIEND_TEST_ALL_PREFIXES(SuggestionsServiceTest,
-                           IssueRequestIfNoneOngoingError);
-  FRIEND_TEST_ALL_PREFIXES(SuggestionsServiceTest,
-                           IssueRequestIfNoneOngoingResponseNotOK);
-  FRIEND_TEST_ALL_PREFIXES(SuggestionsServiceTest, BlacklistURL);
-  FRIEND_TEST_ALL_PREFIXES(SuggestionsServiceTest, BlacklistURLRequestFails);
-  FRIEND_TEST_ALL_PREFIXES(SuggestionsServiceTest, ClearBlacklist);
-  FRIEND_TEST_ALL_PREFIXES(SuggestionsServiceTest, UndoBlacklistURL);
-  FRIEND_TEST_ALL_PREFIXES(SuggestionsServiceTest, GetBlacklistedUrl);
-  FRIEND_TEST_ALL_PREFIXES(SuggestionsServiceTest, UpdateBlacklistDelay);
-  FRIEND_TEST_ALL_PREFIXES(SuggestionsServiceTest, CheckDefaultTimeStamps);
+  // Establishes the different sync states that matter to SuggestionsService.
+  enum SyncState {
+    // State: Sync service is not initialized, yet not disabled. History sync
+    //     state is unknown (since not initialized).
+    // Behavior: Do not issue server requests, but serve from cache if
+    //     available.
+    NOT_INITIALIZED_ENABLED,
+
+    // State: Sync service is initialized, sync is enabled and history sync is
+    //     enabled.
+    // Behavior: Update suggestions from the server on FetchSuggestionsData().
+    INITIALIZED_ENABLED_HISTORY,
+
+    // State: Sync service is disabled or history sync is disabled.
+    // Behavior: Do not issue server requests. Clear the cache. Serve empty
+    //     suggestions.
+    SYNC_OR_HISTORY_SYNC_DISABLED,
+  };
+
+  // The action that should be taken as the result of a RefreshSyncState call.
+  enum RefreshAction { NO_ACTION, FETCH_SUGGESTIONS, CLEAR_SUGGESTIONS };
 
   // Helpers to build the various suggestions URLs. These are static members
   // rather than local functions in the .cc file to make them accessible to
@@ -111,6 +126,13 @@ class SuggestionsServiceImpl : public SuggestionsService,
   static GURL BuildSuggestionsBlacklistURL(const GURL& candidate_url);
   static GURL BuildSuggestionsBlacklistClearURL();
 
+  // Computes the appropriate SyncState from |sync_service_|.
+  SyncState ComputeSyncState() const;
+
+  // Re-computes |sync_state_| from the sync service. Returns the action that
+  // should be taken in response.
+  RefreshAction RefreshSyncState() WARN_UNUSED_RESULT;
+
   // syncer::SyncServiceObserver implementation.
   void OnStateChanged(syncer::SyncService* sync) override;
 
@@ -159,10 +181,6 @@ class SuggestionsServiceImpl : public SuggestionsService,
   // Adds extra data to suggestions profile.
   void PopulateExtraData(SuggestionsProfile* suggestions);
 
-  // Test seams.
-  base::TimeDelta blacklist_delay() const { return scheduling_delay_; }
-  void set_blacklist_delay(base::TimeDelta delay) { scheduling_delay_ = delay; }
-
   base::ThreadChecker thread_checker_;
 
   SigninManagerBase* signin_manager_;
@@ -172,6 +190,8 @@ class SuggestionsServiceImpl : public SuggestionsService,
   ScopedObserver<syncer::SyncService, syncer::SyncServiceObserver>
       sync_service_observer_;
 
+  SyncState sync_state_;
+
   net::URLRequestContextGetter* url_request_context_;
 
   // The cache for the suggestions.
diff --git a/chromium/components/suggestions/suggestions_service_impl_unittest.cc b/chromium/components/suggestions/suggestions_service_impl_unittest.cc
index 829c0536912..7aa45beb51e 100644
--- a/chromium/components/suggestions/suggestions_service_impl_unittest.cc
+++ b/chromium/components/suggestions/suggestions_service_impl_unittest.cc
@@ -10,22 +10,25 @@
 #include <utility>
 
 #include "base/bind.h"
-#include "base/feature_list.h"
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
+#include "base/message_loop/message_loop.h"
 #include "base/run_loop.h"
+#include "base/test/mock_callback.h"
+#include "base/test/scoped_feature_list.h"
 #include "components/signin/core/browser/account_tracker_service.h"
 #include "components/signin/core/browser/fake_profile_oauth2_token_service.h"
 #include "components/signin/core/browser/fake_signin_manager.h"
 #include "components/signin/core/browser/test_signin_client.h"
 #include "components/suggestions/blacklist_store.h"
+#include "components/suggestions/features.h"
 #include "components/suggestions/image_manager.h"
 #include "components/suggestions/proto/suggestions.pb.h"
 #include "components/suggestions/suggestions_store.h"
 #include "components/sync/driver/fake_sync_service.h"
 #include "components/sync/driver/sync_service.h"
 #include "components/sync_preferences/testing_pref_service_syncable.h"
-#include "net/base/escape.h"
+#include "net/base/url_util.h"
 #include "net/http/http_response_headers.h"
 #include "net/http/http_status_code.h"
 #include "net/url_request/test_url_fetcher_factory.h"
@@ -36,11 +39,12 @@
 #include "ui/gfx/image/image.h"
 
 using sync_preferences::TestingPrefServiceSyncable;
+using syncer::SyncServiceObserver;
 using testing::_;
 using testing::AnyNumber;
 using testing::DoAll;
 using testing::Eq;
-using testing::NiceMock;
+using testing::Mock;
 using testing::Return;
 using testing::SetArgPointee;
 using testing::StrictMock;
@@ -48,33 +52,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 +84,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;
 }
 
@@ -159,49 +129,22 @@ class MockImageManager : public suggestions::ImageManager {
 class MockBlacklistStore : public suggestions::BlacklistStore {
  public:
   MOCK_METHOD1(BlacklistUrl, bool(const GURL&));
-  MOCK_METHOD0(IsEmpty, bool());
+  MOCK_METHOD0(ClearBlacklist, void());
   MOCK_METHOD1(GetTimeUntilReadyForUpload, bool(base::TimeDelta*));
   MOCK_METHOD2(GetTimeUntilURLReadyForUpload,
                bool(const GURL&, base::TimeDelta*));
   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());
-  }
-
-  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),
-        signin_client_(&pref_service_),
+      : signin_client_(&pref_service_),
         signin_manager_(&signin_client_, &account_tracker_),
-        factory_(nullptr, base::Bind(&CreateURLFetcher)),
-        mock_sync_service_(nullptr),
+        request_context_(new net::TestURLRequestContextGetter(
+            io_message_loop_.task_runner())),
         mock_thumbnail_manager_(nullptr),
         mock_blacklist_store_(nullptr),
         test_suggestions_store_(nullptr) {
@@ -216,308 +159,317 @@ class SuggestionsServiceTest : public testing::Test {
   ~SuggestionsServiceTest() override {}
 
   void SetUp() override {
-    request_context_ =
-        new net::TestURLRequestContextGetter(io_message_loop_.task_runner());
-  }
-
-  std::unique_ptr<SuggestionsServiceImpl> CreateSuggestionsServiceWithMocks() {
-    mock_sync_service_.reset(new MockSyncService);
-    EXPECT_CALL(*mock_sync_service_, CanSyncStart())
+    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)));
-
-    // These objects are owned by the returned SuggestionsService, but we keep
-    // the pointer around for testing.
+    // These objects are owned by the SuggestionsService, but we keep the
+    // pointers around for testing.
     test_suggestions_store_ = new TestSuggestionsStore();
     mock_thumbnail_manager_ = new StrictMock<MockImageManager>();
     mock_blacklist_store_ = new StrictMock<MockBlacklistStore>();
-    return base::MakeUnique<SuggestionsServiceImpl>(
-        &signin_manager_, &token_service_, mock_sync_service_.get(),
+    suggestions_service_ = base::MakeUnique<SuggestionsServiceImpl>(
+        &signin_manager_, &token_service_, &mock_sync_service_,
         request_context_.get(), base::WrapUnique(test_suggestions_store_),
         base::WrapUnique(mock_thumbnail_manager_),
         base::WrapUnique(mock_blacklist_store_));
   }
 
-  void Blacklist(SuggestionsService* suggestions_service, GURL url) {
-    blacklisting_failed_ = !suggestions_service->BlacklistURL(url);
+  GURL GetCurrentlyQueriedUrl() {
+    net::TestURLFetcher* fetcher = factory_.GetFetcherByID(0);
+    if (!fetcher) {
+      return GURL();
+    }
+    return fetcher->GetOriginalURL();
   }
 
-  void UndoBlacklist(SuggestionsService* suggestions_service, GURL url) {
-    undo_blacklisting_failed_ = !suggestions_service->UndoBlacklistURL(url);
+  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);
   }
 
-  // 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());
-    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(_))
-        .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)));
-    }
+  void RespondToFetchWithProfile(const SuggestionsProfile& suggestions) {
+    RespondToFetch(
+        suggestions.SerializeAsString(), net::HTTP_OK,
+        net::URLRequestStatus(net::URLRequestStatus::SUCCESS, net::OK));
+  }
 
-    Blacklist(suggestions_service.get(), blacklisted_url);
-    UndoBlacklist(suggestions_service.get(), blacklisted_url);
+  FakeProfileOAuth2TokenService* token_service() { return &token_service_; }
 
-    EXPECT_EQ(1, suggestions_data_callback_count_);
-    EXPECT_FALSE(blacklisting_failed_);
-    EXPECT_TRUE(undo_blacklisting_failed_);
-  }
+  MockSyncService* sync_service() { return &mock_sync_service_; }
+
+  MockImageManager* thumbnail_manager() { return mock_thumbnail_manager_; }
+
+  MockBlacklistStore* blacklist_store() { return mock_blacklist_store_; }
+
+  TestSuggestionsStore* suggestions_store() { return test_suggestions_store_; }
 
-  bool HasPendingSuggestionsRequest(
-      SuggestionsServiceImpl* suggestions_service) {
-    return !!suggestions_service->pending_request_.get();
+  SuggestionsServiceImpl* suggestions_service() {
+    return suggestions_service_.get();
   }
 
- protected:
+ private:
   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_;
-  // Only used if the SuggestionsService is built with mocks. Not owned.
+  MockSyncService mock_sync_service_;
+  scoped_refptr<net::TestURLRequestContextGetter> request_context_;
+  // Owned by the SuggestionsService.
   MockImageManager* mock_thumbnail_manager_;
   MockBlacklistStore* mock_blacklist_store_;
   TestSuggestionsStore* test_suggestions_store_;
-  scoped_refptr<net::TestURLRequestContextGetter> request_context_;
 
- private:
+  std::unique_ptr<SuggestionsServiceImpl> suggestions_service_;
+
   DISALLOW_COPY_AND_ASSIGN(SuggestionsServiceTest);
 };
 
 TEST_F(SuggestionsServiceTest, FetchSuggestionsData) {
-  std::unique_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(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(_))
+  base::MockCallback<SuggestionsService::ResponseCallback> callback;
+  auto subscription = suggestions_service()->AddCallback(callback.Get());
+
+  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.
+  EXPECT_CALL(callback, Run(_));
+
+  // Wait for the eventual network request.
   base::RunLoop().RunUntilIdle();
+  ASSERT_TRUE(GetCurrentlyQueriedUrl().is_valid());
+  EXPECT_EQ(GetCurrentlyQueriedUrl().path(), kSuggestionsUrlPath);
+  RespondToFetchWithProfile(CreateSuggestionsProfile());
 
-  // Ensure that CheckCallback() ran once.
-  EXPECT_EQ(1, suggestions_data_callback_count_);
+  SuggestionsProfile suggestions;
+  suggestions_store()->LoadSuggestions(&suggestions);
+  ASSERT_EQ(1, suggestions.suggestions_size());
+  EXPECT_EQ(kTestTitle, suggestions.suggestions(0).title());
+  EXPECT_EQ(kTestUrl, suggestions.suggestions(0).url());
+  EXPECT_EQ(kTestFaviconUrl, suggestions.suggestions(0).favicon_url());
+}
+
+TEST_F(SuggestionsServiceTest, IgnoresNoopSyncChange) {
+  base::MockCallback<SuggestionsService::ResponseCallback> callback;
+  EXPECT_CALL(callback, Run(_)).Times(0);
+  auto subscription = suggestions_service()->AddCallback(callback.Get());
+
+  // An no-op change should not result in a suggestions refresh.
+  static_cast<SyncServiceObserver*>(suggestions_service())
+      ->OnStateChanged(sync_service());
+
+  // Wait for eventual (but unexpected) network requests.
+  base::RunLoop().RunUntilIdle();
+  EXPECT_FALSE(suggestions_service()->has_pending_request_for_testing());
+}
 
-  CheckSuggestionsData();
+TEST_F(SuggestionsServiceTest, IgnoresUninterestingSyncChange) {
+  base::MockCallback<SuggestionsService::ResponseCallback> callback;
+  EXPECT_CALL(callback, Run(_)).Times(0);
+  auto subscription = suggestions_service()->AddCallback(callback.Get());
+
+  // An uninteresting change should not result in a network request (the
+  // SyncState is INITIALIZED_ENABLED_HISTORY before and after).
+  EXPECT_CALL(*sync_service(), GetActiveDataTypes())
+      .Times(AnyNumber())
+      .WillRepeatedly(Return(syncer::ModelTypeSet(
+          syncer::HISTORY_DELETE_DIRECTIVES, syncer::BOOKMARKS)));
+  static_cast<SyncServiceObserver*>(suggestions_service())
+      ->OnStateChanged(sync_service());
+
+  // Wait for eventual (but unexpected) network requests.
+  base::RunLoop().RunUntilIdle();
+  EXPECT_FALSE(suggestions_service()->has_pending_request_for_testing());
+}
+
+// During startup, the state changes from NOT_INITIALIZED_ENABLED to
+// INITIALIZED_ENABLED_HISTORY (for a signed-in user with history sync enabled).
+// This should *not* result in an automatic fetch.
+TEST_F(SuggestionsServiceTest, DoesNotFetchOnStartup) {
+  // The sync service starts out inactive.
+  EXPECT_CALL(*sync_service(), IsSyncActive()).WillRepeatedly(Return(false));
+  static_cast<SyncServiceObserver*>(suggestions_service())
+      ->OnStateChanged(sync_service());
+
+  base::RunLoop().RunUntilIdle();
+  ASSERT_FALSE(suggestions_service()->has_pending_request_for_testing());
+
+  // Sync getting enabled should not result in a fetch.
+  EXPECT_CALL(*sync_service(), IsSyncActive()).WillRepeatedly(Return(true));
+  static_cast<SyncServiceObserver*>(suggestions_service())
+      ->OnStateChanged(sync_service());
+
+  // Wait for eventual (but unexpected) network requests.
+  base::RunLoop().RunUntilIdle();
+  EXPECT_FALSE(suggestions_service()->has_pending_request_for_testing());
+}
+
+TEST_F(SuggestionsServiceTest, BuildUrlWithDefaultMinZeroParamForFewFeature) {
+  base::test::ScopedFeatureList scoped_feature_list;
+  scoped_feature_list.InitAndEnableFeature(kUseSuggestionsEvenIfFewFeature);
+
+  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();
+
+  // Wait for the eventual network request.
+  base::RunLoop().RunUntilIdle();
+  ASSERT_TRUE(GetCurrentlyQueriedUrl().is_valid());
+  EXPECT_EQ(GetCurrentlyQueriedUrl().path(), kSuggestionsUrlPath);
+  std::string min_suggestions;
+  EXPECT_TRUE(net::GetValueForKeyInQuery(GetCurrentlyQueriedUrl(), "min",
+                                         &min_suggestions));
+  EXPECT_EQ(min_suggestions, "0");
+  RespondToFetchWithProfile(CreateSuggestionsProfile());
 }
 
 TEST_F(SuggestionsServiceTest, FetchSuggestionsDataSyncNotInitializedEnabled) {
-  std::unique_ptr<SuggestionsService> suggestions_service(
-      CreateSuggestionsServiceWithMocks());
-  ASSERT_TRUE(suggestions_service != nullptr);
-  EXPECT_CALL(*mock_sync_service_, IsSyncActive())
-      .WillRepeatedly(Return(false));
+  EXPECT_CALL(*sync_service(), IsSyncActive()).WillRepeatedly(Return(false));
+  static_cast<SyncServiceObserver*>(suggestions_service())
+      ->OnStateChanged(sync_service());
 
-  auto subscription = suggestions_service->AddCallback(base::Bind(
-      &SuggestionsServiceTest::CheckCallback, base::Unretained(this)));
+  base::MockCallback<SuggestionsService::ResponseCallback> callback;
+  EXPECT_CALL(callback, Run(_)).Times(0);
+  auto subscription = suggestions_service()->AddCallback(callback.Get());
 
   // 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.
+  // Wait for eventual (but unexpected) network requests.
   base::RunLoop().RunUntilIdle();
+  EXPECT_FALSE(suggestions_service()->has_pending_request_for_testing());
 
-  // Ensure that CheckCallback() didn't run.
-  EXPECT_EQ(0, suggestions_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);
-  EXPECT_EQ(CreateSuggestionsProfile().SerializeAsString(),
-            suggestions.SerializeAsString());
+  suggestions_store()->LoadSuggestions(&suggestions);
+  EXPECT_THAT(suggestions, EqualsProto(CreateSuggestionsProfile()));
 }
 
 TEST_F(SuggestionsServiceTest, FetchSuggestionsDataSyncDisabled) {
-  std::unique_ptr<SuggestionsServiceImpl> suggestions_service(
-      CreateSuggestionsServiceWithMocks());
-  ASSERT_TRUE(suggestions_service != nullptr);
-  EXPECT_CALL(*mock_sync_service_, CanSyncStart())
-      .WillRepeatedly(Return(false));
+  EXPECT_CALL(*sync_service(), CanSyncStart()).WillRepeatedly(Return(false));
 
-  auto subscription = suggestions_service->AddCallback(base::Bind(
-      &SuggestionsServiceTest::CheckCallback, base::Unretained(this)));
+  base::MockCallback<SuggestionsService::ResponseCallback> callback;
+  auto subscription = suggestions_service()->AddCallback(callback.Get());
 
   // 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());
-
-  // Ensure that CheckCallback ran once with empty data.
-  EXPECT_EQ(1, suggestions_data_callback_count_);
-  EXPECT_EQ(1, suggestions_empty_data_count_);
+  EXPECT_CALL(callback, Run(EqualsProto(SuggestionsProfile())));
+  static_cast<SyncServiceObserver*>(suggestions_service())
+      ->OnStateChanged(sync_service());
 
   // 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.
+  // Wait for eventual (but unexpected) network requests.
   base::RunLoop().RunUntilIdle();
-
-  // Ensure that CheckCallback didn't run again.
-  EXPECT_EQ(1, suggestions_data_callback_count_);
+  EXPECT_FALSE(suggestions_service()->has_pending_request_for_testing());
 }
 
 TEST_F(SuggestionsServiceTest, FetchSuggestionsDataNoAccessToken) {
-  token_service_.set_auto_post_fetch_response_on_message_loop(false);
+  token_service()->set_auto_post_fetch_response_on_message_loop(false);
 
-  std::unique_ptr<SuggestionsServiceImpl> suggestions_service(
-      CreateSuggestionsServiceWithMocks());
-  ASSERT_TRUE(suggestions_service != nullptr);
+  base::MockCallback<SuggestionsService::ResponseCallback> callback;
+  EXPECT_CALL(callback, Run(_)).Times(0);
+  auto subscription = suggestions_service()->AddCallback(callback.Get());
 
-  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.
+  // Wait for eventual (but unexpected) network requests.
   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());
 }
 
-TEST_F(SuggestionsServiceTest, IssueRequestIfNoneOngoingError) {
-  std::unique_ptr<SuggestionsServiceImpl> suggestions_service(
-      CreateSuggestionsServiceWithMocks());
-  ASSERT_TRUE(suggestions_service != nullptr);
-
-  // 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.
+  // Wait for the eventual network request.
   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());
-  ASSERT_TRUE(suggestions_service != nullptr);
-
-  // 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.
+  // Wait for the eventual network request.
   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());
-  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(
-      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);
-
-  // Expected calls to the blacklist store.
-  EXPECT_CALL(*mock_blacklist_store_, BlacklistUrl(Eq(blacklisted_url)))
+  // Calling RunUntilIdle on the RunLoop only works when the task is not posted
+  // for the future.
+  const base::TimeDelta no_delay = base::TimeDelta::FromSeconds(0);
+  suggestions_service()->set_blacklist_delay_for_testing(no_delay);
+
+  base::MockCallback<SuggestionsService::ResponseCallback> callback;
+  auto subscription = suggestions_service()->AddCallback(callback.Get());
+
+  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(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));
 
-  Blacklist(suggestions_service.get(), blacklisted_url);
-  EXPECT_EQ(1, suggestions_data_callback_count_);
+  EXPECT_CALL(callback, Run(_)).Times(2);
+
+  EXPECT_TRUE(suggestions_service()->BlacklistURL(GURL(kBlacklistedUrl)));
 
   // 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
@@ -525,242 +477,273 @@ TEST_F(SuggestionsServiceTest, BlacklistURL) {
   // BlacklistStore's candidacy delay are zero).
   base::RunLoop().RunUntilIdle();
 
-  EXPECT_EQ(2, suggestions_data_callback_count_);
-  EXPECT_FALSE(blacklisting_failed_);
-  CheckSuggestionsData();
+  EXPECT_EQ(GetCurrentlyQueriedUrl().path(), kBlacklistUrlPath);
+  RespondToFetchWithProfile(CreateSuggestionsProfile());
+
+  SuggestionsProfile suggestions;
+  suggestions_store()->LoadSuggestions(&suggestions);
+  ASSERT_EQ(1, suggestions.suggestions_size());
+  EXPECT_EQ(kTestTitle, suggestions.suggestions(0).title());
+  EXPECT_EQ(kTestUrl, suggestions.suggestions(0).url());
+  EXPECT_EQ(kTestFaviconUrl, suggestions.suggestions(0).favicon_url());
 }
 
 TEST_F(SuggestionsServiceTest, BlacklistURLFails) {
-  std::unique_ptr<SuggestionsService> suggestions_service(
-      CreateSuggestionsServiceWithMocks());
-  ASSERT_TRUE(suggestions_service != nullptr);
+  base::MockCallback<SuggestionsService::ResponseCallback> callback;
+  EXPECT_CALL(callback, Run(_)).Times(0);
+  auto subscription = suggestions_service()->AddCallback(callback.Get());
 
-  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(GURL(kBlacklistedUrl))))
       .WillOnce(Return(false));
 
-  Blacklist(suggestions_service.get(), blacklisted_url);
-
-  EXPECT_TRUE(blacklisting_failed_);
-  EXPECT_EQ(0, suggestions_data_callback_count_);
+  EXPECT_FALSE(suggestions_service()->BlacklistURL(GURL(kBlacklistedUrl)));
 }
 
-// Initial blacklist request fails, triggering a second which succeeds.
-TEST_F(SuggestionsServiceTest, BlacklistURLRequestFails) {
-  std::unique_ptr<SuggestionsServiceImpl> suggestions_service(
-      CreateSuggestionsServiceWithMocks());
-  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(
-      SuggestionsServiceImpl::BuildSuggestionsBlacklistURL(blacklisted_url));
-  GURL blacklisted_url_alt(kBlacklistedUrlAlt);
-  GURL request_url_alt(SuggestionsServiceImpl::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(_)).Times(2);
-  EXPECT_CALL(*mock_blacklist_store_, BlacklistUrl(Eq(blacklisted_url)))
+TEST_F(SuggestionsServiceTest, RetryBlacklistURLRequestAfterFailure) {
+  // Calling RunUntilIdle on the RunLoop only works when the task is not
+  // posted for the future.
+  const base::TimeDelta no_delay = base::TimeDelta::FromSeconds(0);
+  suggestions_service()->set_blacklist_delay_for_testing(no_delay);
+
+  base::MockCallback<SuggestionsService::ResponseCallback> callback;
+  auto subscription = suggestions_service()->AddCallback(callback.Get());
+
+  // Set expectations for first, failing request.
+  EXPECT_CALL(*thumbnail_manager(), Initialize(_));
+  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(_))
-      .WillOnce(DoAll(SetArgPointee<0>(no_delay), Return(true)))
+  EXPECT_CALL(*blacklist_store(), FilterSuggestions(_));
+  EXPECT_CALL(*blacklist_store(), GetTimeUntilReadyForUpload(_))
       .WillOnce(DoAll(SetArgPointee<0>(no_delay), Return(true)))
+      .WillOnce(DoAll(SetArgPointee<0>(no_delay), Return(true)));
+  EXPECT_CALL(*blacklist_store(), GetCandidateForUpload(_))
+      .WillOnce(DoAll(SetArgPointee<0>(GURL(kBlacklistedUrl)), Return(true)));
+
+  EXPECT_CALL(callback, Run(_)).Times(2);
+
+  // Blacklist call, first request attempt.
+  EXPECT_TRUE(suggestions_service()->BlacklistURL(GURL(kBlacklistedUrl)));
+
+  // Wait for the first scheduling receiving a failing response.
+  base::RunLoop().RunUntilIdle();
+  ASSERT_TRUE(GetCurrentlyQueriedUrl().is_valid());
+  EXPECT_EQ(GetCurrentlyQueriedUrl().path(), kBlacklistUrlPath);
+  RespondToFetch("irrelevant", net::HTTP_OK,
+                 net::URLRequestStatus(net::URLRequestStatus::FAILED,
+                                       net::ERR_INVALID_RESPONSE));
+
+  // Assert that the failure was processed as expected.
+  Mock::VerifyAndClearExpectations(thumbnail_manager());
+  Mock::VerifyAndClearExpectations(blacklist_store());
+
+  // Now expect the retried request to succeed.
+  EXPECT_CALL(*thumbnail_manager(), Initialize(_));
+  EXPECT_CALL(*blacklist_store(), FilterSuggestions(_));
+  EXPECT_CALL(*blacklist_store(), GetTimeUntilReadyForUpload(_))
       .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)));
+  EXPECT_CALL(*blacklist_store(), RemoveUrl(Eq(GURL(kBlacklistedUrl))))
       .WillOnce(Return(true));
 
-  // Blacklist call, first request attempt.
-  Blacklist(suggestions_service.get(), blacklisted_url);
-  EXPECT_EQ(1, suggestions_data_callback_count_);
-  EXPECT_FALSE(blacklisting_failed_);
-
-  // 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 second scheduling followed by a successful response.
   base::RunLoop().RunUntilIdle();
-  CheckSuggestionsData();
+  ASSERT_TRUE(GetCurrentlyQueriedUrl().is_valid());
+  EXPECT_EQ(GetCurrentlyQueriedUrl().path(), kBlacklistUrlPath);
+  RespondToFetchWithProfile(CreateSuggestionsProfile());
+
+  SuggestionsProfile suggestions;
+  suggestions_store()->LoadSuggestions(&suggestions);
+  ASSERT_EQ(1, suggestions.suggestions_size());
+  EXPECT_EQ(kTestTitle, suggestions.suggestions(0).title());
+  EXPECT_EQ(kTestUrl, suggestions.suggestions(0).url());
+  EXPECT_EQ(kTestFaviconUrl, suggestions.suggestions(0).favicon_url());
 }
 
 TEST_F(SuggestionsServiceTest, UndoBlacklistURL) {
-  std::unique_ptr<SuggestionsServiceImpl> suggestions_service(
-      CreateSuggestionsServiceWithMocks());
-  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)));
+  const base::TimeDelta delay = base::TimeDelta::FromHours(1);
+  suggestions_service()->set_blacklist_delay_for_testing(delay);
 
-  SuggestionsProfile suggestions_profile = CreateSuggestionsProfile();
-  GURL blacklisted_url(kBlacklistedUrl);
+  base::MockCallback<SuggestionsService::ResponseCallback> callback;
+  auto subscription = suggestions_service()->AddCallback(callback.Get());
 
   // 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)));
   // Undo expectations.
-  EXPECT_CALL(*mock_blacklist_store_,
-              GetTimeUntilURLReadyForUpload(Eq(blacklisted_url), _))
+  EXPECT_CALL(*blacklist_store(),
+              GetTimeUntilURLReadyForUpload(Eq(GURL(kBlacklistedUrl)), _))
       .WillOnce(DoAll(SetArgPointee<1>(delay), Return(true)));
-  EXPECT_CALL(*mock_blacklist_store_, RemoveUrl(Eq(blacklisted_url)))
+  EXPECT_CALL(*blacklist_store(), RemoveUrl(Eq(GURL(kBlacklistedUrl))))
       .WillOnce(Return(true));
 
-  Blacklist(suggestions_service.get(), blacklisted_url);
-  UndoBlacklist(suggestions_service.get(), blacklisted_url);
-
-  EXPECT_EQ(2, suggestions_data_callback_count_);
-  EXPECT_FALSE(blacklisting_failed_);
-  EXPECT_FALSE(undo_blacklisting_failed_);
+  EXPECT_CALL(callback, Run(_)).Times(2);
+  EXPECT_TRUE(suggestions_service()->BlacklistURL(GURL(kBlacklistedUrl)));
+  EXPECT_TRUE(suggestions_service()->UndoBlacklistURL(GURL(kBlacklistedUrl)));
 }
 
 TEST_F(SuggestionsServiceTest, ClearBlacklist) {
-  std::unique_ptr<SuggestionsServiceImpl> suggestions_service(
-      CreateSuggestionsServiceWithMocks());
-  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);
+  const base::TimeDelta delay = base::TimeDelta::FromHours(1);
+  suggestions_service()->set_blacklist_delay_for_testing(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);
+  base::MockCallback<SuggestionsService::ResponseCallback> callback;
+  auto subscription = suggestions_service()->AddCallback(callback.Get());
 
   // 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());
 
-  Blacklist(suggestions_service.get(), blacklisted_url);
-  suggestions_service->ClearBlacklist();
+  EXPECT_CALL(callback, Run(_)).Times(2);
+  EXPECT_TRUE(suggestions_service()->BlacklistURL(GURL(kBlacklistedUrl)));
+  suggestions_service()->ClearBlacklist();
 
-  EXPECT_EQ(2, suggestions_data_callback_count_);
-  EXPECT_FALSE(blacklisting_failed_);
+  // Wait for the eventual network request.
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(GetCurrentlyQueriedUrl().path(), kBlacklistClearUrlPath);
 }
 
 TEST_F(SuggestionsServiceTest, UndoBlacklistURLFailsIfNotInBlacklist) {
-  UndoBlacklistURLFailsHelper(true);
+  // Ensure scheduling the request doesn't happen before undo.
+  const base::TimeDelta delay = base::TimeDelta::FromHours(1);
+  suggestions_service()->set_blacklist_delay_for_testing(delay);
+
+  base::MockCallback<SuggestionsService::ResponseCallback> callback;
+  auto subscription = suggestions_service()->AddCallback(callback.Get());
+
+  // 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_CALL(callback, Run(_));
+
+  EXPECT_TRUE(suggestions_service()->BlacklistURL(GURL(kBlacklistedUrl)));
+  EXPECT_FALSE(suggestions_service()->UndoBlacklistURL(GURL(kBlacklistedUrl)));
 }
 
 TEST_F(SuggestionsServiceTest, UndoBlacklistURLFailsIfAlreadyCandidate) {
-  UndoBlacklistURLFailsHelper(false);
-}
+  // Ensure scheduling the request doesn't happen before undo.
+  const base::TimeDelta delay = base::TimeDelta::FromHours(1);
+  suggestions_service()->set_blacklist_delay_for_testing(delay);
+
+  base::MockCallback<SuggestionsService::ResponseCallback> callback;
+  auto subscription = suggestions_service()->AddCallback(callback.Get());
+
+  // 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)));
+
+  // URL is not yet candidate for upload.
+  const base::TimeDelta negative_delay = base::TimeDelta::FromHours(-1);
+  EXPECT_CALL(*blacklist_store(),
+              GetTimeUntilURLReadyForUpload(Eq(GURL(kBlacklistedUrl)), _))
+      .WillOnce(DoAll(SetArgPointee<1>(negative_delay), Return(true)));
 
-TEST_F(SuggestionsServiceTest, GetBlacklistedUrl) {
-  std::unique_ptr<GURL> request_url;
-  std::unique_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, nullptr, "", net::HTTP_OK,
-                             net::URLRequestStatus::SUCCESS);
-  EXPECT_FALSE(
-      SuggestionsServiceImpl::GetBlacklistedUrl(*fetcher, &retrieved_url));
-
-  // 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());
-  request_url.reset(
-      new GURL(blacklist_request_prefix + encoded_blacklisted_url));
-  fetcher.reset();
-  fetcher = CreateURLFetcher(*request_url, nullptr, "", net::HTTP_OK,
-                             net::URLRequestStatus::SUCCESS);
-  EXPECT_TRUE(
-      SuggestionsServiceImpl::GetBlacklistedUrl(*fetcher, &retrieved_url));
-  EXPECT_EQ(blacklisted_url, retrieved_url.spec());
+  EXPECT_CALL(callback, Run(_));
+
+  EXPECT_TRUE(suggestions_service()->BlacklistURL(GURL(kBlacklistedUrl)));
+  EXPECT_FALSE(suggestions_service()->UndoBlacklistURL(GURL(kBlacklistedUrl)));
 }
 
-TEST_F(SuggestionsServiceTest, UpdateBlacklistDelay) {
-  std::unique_ptr<SuggestionsServiceImpl> suggestions_service(
-      CreateSuggestionsServiceWithMocks());
-  base::TimeDelta initial_delay = suggestions_service->blacklist_delay();
+TEST_F(SuggestionsServiceTest, TemporarilyIncreasesBlacklistDelayOnFailure) {
+  EXPECT_CALL(*thumbnail_manager(), Initialize(_)).Times(AnyNumber());
+  EXPECT_CALL(*blacklist_store(), FilterSuggestions(_)).Times(AnyNumber());
+  EXPECT_CALL(*blacklist_store(), GetTimeUntilReadyForUpload(_))
+      .Times(AnyNumber())
+      .WillRepeatedly(Return(false));
+  const 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;
+  suggestions_store()->LoadSuggestions(&suggestions);
+  ASSERT_EQ(2, suggestions.suggestions_size());
+  // Suggestion[0] had no time stamp and should be ahead of the old suggestion.
+  EXPECT_LT(kTestSetExpiry, suggestions.suggestions(0).expiry_ts());
+  // Suggestion[1] had a very old time stamp but should not be updated.
+  EXPECT_EQ(kTestSetExpiry, suggestions.suggestions(1).expiry_ts());
 }
 
 TEST_F(SuggestionsServiceTest, GetPageThumbnail) {
-  std::unique_ptr<SuggestionsService> suggestions_service(
-      CreateSuggestionsServiceWithMocks());
-  ASSERT_TRUE(suggestions_service != nullptr);
-
-  GURL test_url(kTestUrl);
-  GURL thumbnail_url("https://www.thumbnails.com/thumb.jpg");
+  const GURL test_url(kTestUrl);
+  const 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