SuggestionsService support for Desktop

- support for ClearBlacklist
- support for favicon generation

BUG=514752

Review URL: https://codereview.chromium.org/1259123002

Cr-Commit-Position: refs/heads/master@{#340757}

4 files changed, 114 insertions, 45 deletions

diff --git a/components/suggestions/blacklist_store.h b/components/suggestions/blacklist_store.h
index 6d1bc73..a76a6c9 100644
--- a/components/suggestions/blacklist_store.h
+++ b/components/suggestions/blacklist_store.h
@@ -38,6 +38,9 @@ class BlacklistStore {
   // was already in the blacklist, its blacklisting timestamp gets updated.
   virtual bool BlacklistUrl(const GURL& url);
 
+  // Clears any blacklist data from the profile's preferences.
+  virtual void ClearBlacklist();
+
   // Gets the time until any URL is ready for upload. Returns false if the
   // blacklist is empty.
   virtual bool GetTimeUntilReadyForUpload(base::TimeDelta* delta);
@@ -76,9 +79,6 @@ class BlacklistStore {
   // a base64 encoding of its protobuf serialization.
   bool StoreBlacklist(const SuggestionsBlacklist& blacklist);
 
-  // Clears any blacklist data from the profile's preferences.
-  void ClearBlacklist();
-
  private:
   // The pref service used to persist the suggestions blacklist.
   PrefService* pref_service_;
diff --git a/components/suggestions/suggestions_service.cc b/components/suggestions/suggestions_service.cc
index 2cac424..666ee51 100644
--- a/components/suggestions/suggestions_service.cc
+++ b/components/suggestions/suggestions_service.cc
@@ -13,6 +13,7 @@
 #include "base/single_thread_task_runner.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
+#include "base/strings/stringprintf.h"
 #include "base/thread_task_runner_handle.h"
 #include "base/time/time.h"
 #include "components/pref_registry/pref_registry_syncable.h"
@@ -81,16 +82,30 @@ const int kSchedulingBackoffMultiplier = 2;
 // this are rejected. This means the maximum backoff is at least 5 / 2 minutes.
 const int kSchedulingMaxDelaySec = 5 * 60;
 
+const char kFaviconURL[] =
+    "https://s2.googleusercontent.com/s2/favicons?domain_url=%s&alt=s&sz=32";
+
 }  // namespace
 
 // TODO(mathp): Put this in TemplateURL.
+// TODO(fserb): Add logic to decide the device type of the request.
+#if defined(OS_ANDROID) || defined(OS_IOS)
 const char kSuggestionsURL[] = "https://www.google.com/chromesuggestions?t=2";
 const char kSuggestionsBlacklistURLPrefix[] =
     "https://www.google.com/chromesuggestions/blacklist?t=2&url=";
+const char kSuggestionsBlacklistClearURL[] =
+    "https://www.google.com/chromesuggestions/blacklist/clear?t=2";
+#else
+const char kSuggestionsURL[] = "https://www.google.com/chromesuggestions?t=1";
+const char kSuggestionsBlacklistURLPrefix[] =
+    "https://www.google.com/chromesuggestions/blacklist?t=1&url=";
+const char kSuggestionsBlacklistClearURL[] =
+    "https://www.google.com/chromesuggestions/blacklist/clear?t=1";
+#endif
 const char kSuggestionsBlacklistURLParam[] = "url";
 
-// The default expiry timeout is 72 hours.
-const int64 kDefaultExpiryUsec = 72 * base::Time::kMicrosecondsPerHour;
+// The default expiry timeout is 168 hours.
+const int64 kDefaultExpiryUsec = 168 * base::Time::kMicrosecondsPerHour;
 
 SuggestionsService::SuggestionsService(
     net::URLRequestContextGetter* url_request_context,
@@ -175,6 +190,14 @@ void SuggestionsService::UndoBlacklistURL(
   fail_callback.Run();
 }
 
+void SuggestionsService::ClearBlacklist(const ResponseCallback& callback) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  blacklist_store_->ClearBlacklist();
+  IssueRequestIfNoneOngoing(GURL(kSuggestionsBlacklistClearURL));
+  waiting_requestors_.push_back(callback);
+  ServeFromCache();
+}
+
 // static
 bool SuggestionsService::GetBlacklistedUrl(const net::URLFetcher& request,
                                            GURL* url) {
@@ -296,6 +319,7 @@ void SuggestionsService::OnURLFetchComplete(const net::URLFetcher* source) {
     int64 now_usec = (base::Time::NowFromSystemTime() - base::Time::UnixEpoch())
         .ToInternalValue();
     SetDefaultExpiryTimestamp(&suggestions, now_usec + kDefaultExpiryUsec);
+    PopulateFaviconUrls(&suggestions);
     suggestions_store_->StoreSuggestions(suggestions);
   } else {
     LogResponseState(RESPONSE_INVALID);
@@ -305,6 +329,15 @@ void SuggestionsService::OnURLFetchComplete(const net::URLFetcher* source) {
   ScheduleBlacklistUpload();
 }
 
+void SuggestionsService::PopulateFaviconUrls(SuggestionsProfile* suggestions) {
+  for (int i = 0; i < suggestions->suggestions_size(); ++i) {
+    suggestions::ChromeSuggestion* s = suggestions->mutable_suggestions(i);
+    if (!s->has_favicon_url() || s->favicon_url().empty()) {
+      s->set_favicon_url(base::StringPrintf(kFaviconURL, s->url().c_str()));
+    }
+  }
+}
+
 void SuggestionsService::Shutdown() {
   // Cancel pending request, then serve existing requestors from cache.
   pending_request_.reset(NULL);
diff --git a/components/suggestions/suggestions_service.h b/components/suggestions/suggestions_service.h
index 34e0362..7f0aab3 100644
--- a/components/suggestions/suggestions_service.h
+++ b/components/suggestions/suggestions_service.h
@@ -40,6 +40,7 @@ class SuggestionsStore;
 extern const char kSuggestionsURL[];
 extern const char kSuggestionsBlacklistURLPrefix[];
 extern const char kSuggestionsBlacklistURLParam[];
+extern const char kSuggestionsBlacklistClearURL[];
 extern const int64 kDefaultExpiryUsec;
 
 // An interface to fetch server suggestions asynchronously.
@@ -87,6 +88,9 @@ class SuggestionsService : public KeyedService, public net::URLFetcherDelegate {
                         const ResponseCallback& callback,
                         const base::Closure& fail_callback);
 
+  // Removes all URLs from the blacklist then invokes |callback|.
+  void ClearBlacklist(const ResponseCallback& callback);
+
   // 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);
@@ -106,6 +110,7 @@ class SuggestionsService : public KeyedService, public net::URLFetcherDelegate {
   friend class SuggestionsServiceTest;
   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, UndoBlacklistURLFailsHelper);
   FRIEND_TEST_ALL_PREFIXES(SuggestionsServiceTest, UpdateBlacklistDelay);
@@ -138,6 +143,9 @@ class SuggestionsService : public KeyedService, public net::URLFetcherDelegate {
   // Updates |scheduling_delay_| based on the success of the last request.
   void UpdateBlacklistDelay(bool last_request_successful);
 
+  // Adds favicon URLs to suggestions profile.
+  void PopulateFaviconUrls(SuggestionsProfile* suggestions);
+
   // Test seams.
   base::TimeDelta blacklist_delay() const { return scheduling_delay_; }
   void set_blacklist_delay(base::TimeDelta delay) {
diff --git a/components/suggestions/suggestions_service_unittest.cc b/components/suggestions/suggestions_service_unittest.cc
index dfd0e59..355450c 100644
--- a/components/suggestions/suggestions_service_unittest.cc
+++ b/components/suggestions/suggestions_service_unittest.cc
@@ -36,6 +36,9 @@ namespace {
 
 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 kBlacklistUrl[] = "http://blacklist.com";
 const char kBlacklistUrlAlt[] = "http://blacklist-atl.com";
 const int64 kTestDefaultExpiry = 1402200000000000;
@@ -59,7 +62,7 @@ scoped_ptr<net::FakeURLFetcher> CreateURLFetcher(
 
 std::string GetExpectedBlacklistRequestUrl(const GURL& blacklist_url) {
   std::stringstream request_url;
-  request_url << "https://www.google.com/chromesuggestions/blacklist?t=2&url="
+  request_url << suggestions::kSuggestionsBlacklistURLPrefix
               << net::EscapeQueryParamValue(blacklist_url.spec(), true);
   return request_url.str();
 }
@@ -109,15 +112,12 @@ class TestSuggestionsStore : public suggestions::SuggestionsStore {
     cached_suggestions = CreateSuggestionsProfile();
   }
   bool LoadSuggestions(SuggestionsProfile* suggestions) override {
-    if (cached_suggestions.suggestions_size()) {
-      *suggestions = cached_suggestions;
-      return true;
-    }
-    return false;
+    suggestions->CopyFrom(cached_suggestions);
+    return cached_suggestions.suggestions_size();
   }
   bool StoreSuggestions(const SuggestionsProfile& suggestions)
       override {
-    cached_suggestions = suggestions;
+    cached_suggestions.CopyFrom(suggestions);
     return true;
   }
   void ClearSuggestions() override {
@@ -147,15 +147,23 @@ class MockBlacklistStore : public suggestions::BlacklistStore {
   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 CheckSuggestionsData(const SuggestionsProfile& suggestions_profile) {
+  void CheckCallback(const SuggestionsProfile& suggestions_profile) {
+    ++suggestions_data_callback_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());
-    ++suggestions_data_check_count_;
+    EXPECT_EQ(kTestFaviconUrl,
+              suggestions_profile.suggestions(0).favicon_url());
   }
 
   void SetBlacklistFailure() {
@@ -171,14 +179,14 @@ class SuggestionsServiceTest : public testing::Test {
     ++suggestions_empty_data_count_;
   }
 
-  int suggestions_data_check_count_;
+  int suggestions_data_callback_count_;
   int suggestions_empty_data_count_;
   bool blacklisting_failed_;
   bool undo_blacklisting_failed_;
 
  protected:
   SuggestionsServiceTest()
-      : suggestions_data_check_count_(0),
+      : suggestions_data_callback_count_(0),
         suggestions_empty_data_count_(0),
         blacklisting_failed_(false),
         undo_blacklisting_failed_(false),
@@ -199,10 +207,7 @@ class SuggestionsServiceTest : public testing::Test {
         CreateSuggestionsServiceWithMocks());
     EXPECT_TRUE(suggestions_service != NULL);
 
-    // Add some suggestions in the cache.
-    FillSuggestionsStore();
-    SuggestionsProfile suggestions_profile;
-    test_suggestions_store_->LoadSuggestions(&suggestions_profile);
+    SuggestionsProfile suggestions_profile = CreateSuggestionsProfile();
 
     // Set up net::FakeURLFetcherFactory.
     factory_.SetFakeResponse(GURL(kSuggestionsURL),
@@ -218,15 +223,16 @@ class SuggestionsServiceTest : public testing::Test {
 
     // Send the request. The data will be returned to the callback.
     suggestions_service->FetchSuggestionsData(
-        sync_state,
-        base::Bind(&SuggestionsServiceTest::CheckSuggestionsData,
-                   base::Unretained(this)));
+        sync_state, base::Bind(&SuggestionsServiceTest::CheckCallback,
+                               base::Unretained(this)));
 
     // Ensure that CheckSuggestionsData() ran once.
-    EXPECT_EQ(1, suggestions_data_check_count_);
+    EXPECT_EQ(1, suggestions_data_callback_count_);
 
     // Let the network request run.
     io_message_loop_.RunUntilIdle();
+
+    CheckSuggestionsData();
   }
 
   SuggestionsService* CreateSuggestionsServiceWithMocks() {
@@ -242,24 +248,18 @@ class SuggestionsServiceTest : public testing::Test {
         scoped_ptr<BlacklistStore>(mock_blacklist_store_));
   }
 
-  void FillSuggestionsStore() {
-    test_suggestions_store_->StoreSuggestions(CreateSuggestionsProfile());
-  }
-
   void Blacklist(SuggestionsService* suggestions_service, GURL url) {
     suggestions_service->BlacklistURL(
-        url,
-        base::Bind(&SuggestionsServiceTest::CheckSuggestionsData,
-                   base::Unretained(this)),
+        url, base::Bind(&SuggestionsServiceTest::CheckCallback,
+                        base::Unretained(this)),
         base::Bind(&SuggestionsServiceTest::SetBlacklistFailure,
                    base::Unretained(this)));
   }
 
   void UndoBlacklist(SuggestionsService* suggestions_service, GURL url) {
     suggestions_service->UndoBlacklistURL(
-        url,
-        base::Bind(&SuggestionsServiceTest::CheckSuggestionsData,
-                   base::Unretained(this)),
+        url, base::Bind(&SuggestionsServiceTest::CheckCallback,
+                        base::Unretained(this)),
         base::Bind(&SuggestionsServiceTest::SetUndoBlacklistFailure,
                    base::Unretained(this)));
   }
@@ -302,7 +302,7 @@ class SuggestionsServiceTest : public testing::Test {
     Blacklist(suggestions_service.get(), blacklist_url);
     UndoBlacklist(suggestions_service.get(), blacklist_url);
 
-    EXPECT_EQ(1, suggestions_data_check_count_);
+    EXPECT_EQ(1, suggestions_data_callback_count_);
     EXPECT_FALSE(blacklisting_failed_);
     EXPECT_TRUE(undo_blacklisting_failed_);
   }
@@ -333,8 +333,6 @@ TEST_F(SuggestionsServiceTest, FetchSuggestionsDataSyncDisabled) {
       CreateSuggestionsServiceWithMocks());
   EXPECT_TRUE(suggestions_service != NULL);
 
-  FillSuggestionsStore();
-
   // Send the request. Cache is cleared and empty data will be returned to the
   // callback.
   suggestions_service->FetchSuggestionsData(
@@ -370,9 +368,6 @@ TEST_F(SuggestionsServiceTest, IssueRequestIfNoneOngoingResponseNotOK) {
       CreateSuggestionsServiceWithMocks());
   EXPECT_TRUE(suggestions_service != NULL);
 
-  // Add some suggestions in the cache.
-  FillSuggestionsStore();
-
   // Fake a non-200 response code.
   factory_.SetFakeResponse(GURL(kSuggestionsURL), "irrelevant",
                            net::HTTP_BAD_REQUEST,
@@ -431,9 +426,9 @@ TEST_F(SuggestionsServiceTest, BlacklistURL) {
   io_message_loop_.RunUntilIdle();
   base::MessageLoop::current()->RunUntilIdle();
 
-  // Ensure that CheckSuggestionsData() ran once.
-  EXPECT_EQ(1, suggestions_data_check_count_);
+  EXPECT_EQ(1, suggestions_data_callback_count_);
   EXPECT_FALSE(blacklisting_failed_);
+  CheckSuggestionsData();
 }
 
 TEST_F(SuggestionsServiceTest, BlacklistURLFails) {
@@ -447,7 +442,7 @@ TEST_F(SuggestionsServiceTest, BlacklistURLFails) {
   Blacklist(suggestions_service.get(), blacklist_url);
 
   EXPECT_TRUE(blacklisting_failed_);
-  EXPECT_EQ(0, suggestions_data_check_count_);
+  EXPECT_EQ(0, suggestions_data_callback_count_);
 }
 
 // Initial blacklist request fails, triggering a second which succeeds.
@@ -493,7 +488,7 @@ TEST_F(SuggestionsServiceTest, BlacklistURLRequestFails) {
 
   // Blacklist call, first request attempt.
   Blacklist(suggestions_service.get(), blacklist_url);
-  EXPECT_EQ(1, suggestions_data_check_count_);
+  EXPECT_EQ(1, suggestions_data_callback_count_);
   EXPECT_FALSE(blacklisting_failed_);
 
   // Wait for the first scheduling, the first request, the second scheduling,
@@ -505,6 +500,7 @@ TEST_F(SuggestionsServiceTest, BlacklistURLRequestFails) {
   base::MessageLoop::current()->RunUntilIdle();
   io_message_loop_.RunUntilIdle();
   base::MessageLoop::current()->RunUntilIdle();
+  CheckSuggestionsData();
 }
 
 TEST_F(SuggestionsServiceTest, UndoBlacklistURL) {
@@ -537,11 +533,43 @@ TEST_F(SuggestionsServiceTest, UndoBlacklistURL) {
   Blacklist(suggestions_service.get(), blacklist_url);
   UndoBlacklist(suggestions_service.get(), blacklist_url);
 
-  EXPECT_EQ(2, suggestions_data_check_count_);
+  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);
+  // Ensure scheduling the request doesn't happen before undo.
+  base::TimeDelta delay = base::TimeDelta::FromHours(1);
+  suggestions_service->set_blacklist_delay(delay);
+  SuggestionsProfile suggestions_profile = CreateSuggestionsProfile();
+  GURL blacklist_url(kBlacklistUrl);
+
+  factory_.SetFakeResponse(GURL(suggestions::kSuggestionsBlacklistClearURL),
+                           suggestions_profile.SerializeAsString(),
+                           net::HTTP_OK, net::URLRequestStatus::SUCCESS);
+
+  // Blacklist expectations.
+  EXPECT_CALL(*mock_blacklist_store_, BlacklistUrl(Eq(blacklist_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(), blacklist_url);
+  suggestions_service->ClearBlacklist(base::Bind(
+      &SuggestionsServiceTest::CheckCallback, base::Unretained(this)));
+
+  EXPECT_EQ(2, suggestions_data_callback_count_);
+  EXPECT_FALSE(blacklisting_failed_);
+}
 
 TEST_F(SuggestionsServiceTest, UndoBlacklistURLFailsIfNotInBlacklist) {
   UndoBlacklistURLFailsHelper(true);