[NTP Snippets] Re-fetch snippets on ML changes only when the hosts actually change

components/ntp_snippets/ntp_snippets_service.cc

Index: components/ntp_snippets/ntp_snippets_service.cc
diff --git a/components/ntp_snippets/ntp_snippets_service.cc b/components/ntp_snippets/ntp_snippets_service.cc
index b03d0488a29a2bf0a8e1aa985bea6066d9d68813..cd65e192cdf0d74de10c3abf6ca93355c30db165 100644
--- a/components/ntp_snippets/ntp_snippets_service.cc
+++ b/components/ntp_snippets/ntp_snippets_service.cc
@@ -4,6 +4,9 @@
 
 #include "components/ntp_snippets/ntp_snippets_service.h"
 
+#include <algorithm>
+#include <utility>
+
 #include "base/command_line.h"
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
@@ -64,6 +67,7 @@ base::TimeDelta GetFetchingIntervalFallback() {
                              kFetchingIntervalFallbackSeconds);
 }
 
+// Extracts the hosts from |suggestions| and returns them in a sorted vector.
 std::vector<std::string> GetSuggestionsHosts(
     const SuggestionsProfile& suggestions) {
   std::vector<std::string> hosts;
@@ -73,6 +77,7 @@ std::vector<std::string> GetSuggestionsHosts(
     if (url.is_valid())
       hosts.push_back(url.host());
   }
+  std::sort(hosts.begin(), hosts.end());
   return hosts;
 }
 
@@ -139,6 +144,7 @@ NTPSnippetsService::~NTPSnippetsService() {}
 void NTPSnippetsService::RegisterProfilePrefs(PrefRegistrySimple* registry) {
   registry->RegisterListPref(prefs::kSnippets);
   registry->RegisterListPref(prefs::kDiscardedSnippets);
+  registry->RegisterListPref(prefs::kSnippetHosts);
 }
 
 void NTPSnippetsService::Init(bool enabled) {
@@ -214,17 +220,20 @@ void NTPSnippetsService::RemoveObserver(NTPSnippetsServiceObserver* observer) {
 void NTPSnippetsService::OnSuggestionsChanged(
     const SuggestionsProfile& suggestions) {
   std::vector<std::string> hosts = GetSuggestionsHosts(suggestions);
+  if (hosts == GetSnippetHostsFromPrefs())
+    return;
 
   // Remove existing snippets that aren't in the suggestions anymore.
   snippets_.erase(
       std::remove_if(snippets_.begin(), snippets_.end(),
                      [&hosts](const scoped_ptr<NTPSnippet>& snippet) {
-                       return std::find(hosts.begin(), hosts.end(),
-                                        snippet->url().host()) == hosts.end();
+                       return !std::binary_search(hosts.begin(), hosts.end(),

[Marc Treib, 2016/04/06 16:14:55]

Side-effect of the hosts now being sorted :)

[Bernhard Bauer, 2016/04/07 12:28:12]

Neat! :) But could we make the hosts a set instead?

[Marc Treib, 2016/04/07 13:19:54]

Eh.. sure. Done.

+                                                  snippet->url().host());
                      }),
       snippets_.end());
 
   StoreSnippetsToPrefs();
+  StoreSnippetHostsToPrefs(hosts);
 
   FOR_EACH_OBSERVER(NTPSnippetsServiceObserver, observers_,
                     NTPSnippetsServiceLoaded(this));
@@ -336,6 +345,25 @@ void NTPSnippetsService::StoreDiscardedSnippetsToPrefs() {
                      *SnippetsToListValue(discarded_snippets_));
 }
 
+std::vector<std::string> NTPSnippetsService::GetSnippetHostsFromPrefs() const {
+  std::vector<std::string> hosts;
+  const base::ListValue* list = pref_service_->GetList(prefs::kSnippetHosts);
+  for (const base::Value* value : *list) {
+    std::string str;
+    bool success = value->GetAsString(&str);
+    DCHECK(success) << "Failed to parse snippet host from prefs";
+    hosts.push_back(std::move(str));
+  }
+  return hosts;
+}
+
+void NTPSnippetsService::StoreSnippetHostsToPrefs(
+    const std::vector<std::string>& hosts) {
+  base::ListValue list;
+  list.AppendStrings(hosts);
+  pref_service_->Set(prefs::kSnippetHosts, list);
+}
+
 bool NTPSnippetsService::HasDiscardedSnippet(const GURL& url) const {
   auto it = std::find_if(discarded_snippets_.begin(), discarded_snippets_.end(),
                          [&url](const scoped_ptr<NTPSnippet>& snippet) {