Add favicon and publisher name to snippet cards

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 af047ac3af7da517dda41c378aa711cfd3cecd9c..556f45cc23614649ab75d8831889d244d78f0592 100644
--- a/components/ntp_snippets/ntp_snippets_service.cc
+++ b/components/ntp_snippets/ntp_snippets_service.cc
@@ -13,6 +13,7 @@
 #include "base/files/file_util.h"
 #include "base/json/json_reader.h"
 #include "base/location.h"
+#include "base/metrics/sparse_histogram.h"
 #include "base/path_service.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/stringprintf.h"
@@ -286,7 +287,8 @@ std::set<std::string> NTPSnippetsService::GetSuggestionsHosts() const {
 bool NTPSnippetsService::DiscardSnippet(const GURL& url) {
   auto it = std::find_if(snippets_.begin(), snippets_.end(),
                          [&url](const std::unique_ptr<NTPSnippet>& snippet) {
-                           return snippet->url() == url;
+                           return snippet->url() == url ||
+                                  snippet->best_source().url == url;
                          });
   if (it == snippets_.end())
     return false;
@@ -343,7 +345,6 @@ void NTPSnippetsService::OnSuggestionsChanged(
 
 void NTPSnippetsService::OnSnippetsDownloaded(
     const std::string& snippets_json, const std::string& status) {
-
   if (!snippets_json.empty()) {
     DCHECK(status.empty());
 
@@ -417,6 +418,24 @@ bool NTPSnippetsService::LoadFromListValue(const base::ListValue& list) {
     }
   }
 
+  int num_new_snippets = new_snippets.size();
+
+  if (!base::CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kAddIncompleteSnippets)) {
+    // Remove snippets that do not have all the info we need to display it to
+    // the user.
+    new_snippets.erase(
+        std::remove_if(new_snippets.begin(), new_snippets.end(),
+                       [this](const std::unique_ptr<NTPSnippet>& snippet) {

[Marc Treib, 2016/04/30 13:51:19]

No need to capture |this| here.

[May, 2016/05/03 17:11:00]

Done.

+                         return !snippet->is_complete();
+                       }),
+        new_snippets.end());
+    int num_snippets_discarded = num_new_snippets - new_snippets.size();
+    if (num_snippets_discarded > 0)

[Marc Treib, 2016/04/30 13:51:19]

I think we want to record this even if it's zero, so that we can compute the
percentage of cases where something goes wrong?

[jwd, 2016/05/02 16:00:28]

I'd suggest keeping this histogram as it is, and adding a 2nd histogram that's a
boolean histogram and records if snippets were discarded or not. You can then
compute ratios from the dashboard side.

I suggest this so you can more easily do analysis on only the data when things
were discarded.

[May, 2016/05/03 17:11:00]

Done.

+      UMA_HISTOGRAM_SPARSE_SLOWLY("NewTabPage.Snippets.IncompleteSnippets",

[jwd, 2016/05/02 16:00:28]

How many unique num_snippets_discarded do you expect to see in the full
population? If it's too many, the aggregated data will be unmanageable.

[May, 2016/05/03 17:11:00]

Up to kMaxSnippetCount (=10 at the moment) can be discarded. Discarding any
snippets should be a rare event, we expect to not discard any most of the time
but would like to know when it does happen.

[jwd, 2016/05/03 19:36:46]

Sounds fine, as long as you're ok with the overhead from the sparse histogram
(I'm assuming you are).

+                                  num_snippets_discarded);
+  }
+
   // Insert the new snippets at the front.
   snippets_.insert(snippets_.begin(),
                    std::make_move_iterator(new_snippets.begin()),
@@ -444,8 +463,7 @@ void NTPSnippetsService::StoreSnippetsToPrefs() {
 void NTPSnippetsService::LoadDiscardedSnippetsFromPrefs() {
   discarded_snippets_.clear();
   bool success = AddSnippetsFromListValue(
-      *pref_service_->GetList(prefs::kDiscardedSnippets),
-      &discarded_snippets_);
+      *pref_service_->GetList(prefs::kDiscardedSnippets), &discarded_snippets_);
   DCHECK(success) << "Failed to parse discarded snippets from prefs";
 }