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 5eddd097b2b5b1d356165e4d25e213276f930d92..b4d45ccd713aabfc9f551383a371df7e4abb3711 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/histogram_macros.h"
 #include "base/metrics/sparse_histogram.h"
 #include "base/path_service.h"
 #include "base/strings/string_number_conversions.h"
@@ -287,7 +288,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;
@@ -344,7 +346,6 @@ void NTPSnippetsService::OnSuggestionsChanged(
 
 void NTPSnippetsService::OnSnippetsDownloaded(
     const std::string& snippets_json, const std::string& status) {
-
   if (!snippets_json.empty()) {
     DCHECK(status.empty());
 
@@ -420,6 +421,25 @@ bool NTPSnippetsService::LoadFromListValue(const base::ListValue& list) {
     }
   }
 
+  if (!base::CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kAddIncompleteSnippets)) {
+    int num_new_snippets = new_snippets.size();
+    // 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(),
+                       [](const std::unique_ptr<NTPSnippet>& snippet) {
+                         return !snippet->is_complete();
+                       }),
+        new_snippets.end());
+    int num_snippets_discarded = num_new_snippets - new_snippets.size();
+    UMA_HISTOGRAM_BOOLEAN("NewTabPage.Snippets.SnippetsDiscardedAfterFetch",

[Marc Treib, 2016/05/04 07:42:43]

I wouldn't use "Discarded" here, since that term is also user for "discarded by
the user".
Any particular reason for having a separate boolean histogram, instead of just
unconditionally recording .IncompleteSnippets?

[May, 2016/05/04 09:00:06]

Recommended by jwd@ to just have a separate boolean for this to use as the
denominator for figuring out % of snippet fetches that have issues

Changed name to IncompleteSnippetsAfterFetch, and changed the other one to
NumIncompleteSnippets to clarify that it's a count.

[Marc Treib, 2016/05/04 09:01:36]

Couldn't we do that anyway by comparing the number in the "0" bucket to the
total number of samples? Anyway, I don't have strong feelings, I was just
wondering.
SGTM, thanks!

[Bernhard Bauer, 2016/05/04 09:06:54]

Suggestion for consistency: Use "dismissed" for snippets swiped away by the
user, and "discarded" for the incomplete ones thrown away by us?

[Marc Treib, 2016/05/04 09:08:29]

Ah indeed, I mixed up "dismissed" and "discarded". Still, I like "Incomplete"
more since it's less ambiguous.
Also, the bike shed should be sky blue.

[May, 2016/05/04 09:26:34]

Stop painting my bike shed!! I don't even use a bike!!

Also, executive decision to use "incomplete" as much as grammatically possible
for this scenario, and "dismissed" for the other ones.

+                          num_snippets_discarded > 0);
+    if (num_snippets_discarded > 0)

[Marc Treib, 2016/05/04 07:42:43]

nit: braces if the body doesn't fit on one line

[May, 2016/05/04 09:00:06]

Done.

+      UMA_HISTOGRAM_SPARSE_SLOWLY("NewTabPage.Snippets.IncompleteSnippets",
+                                  num_snippets_discarded);
+  }
+
   // Insert the new snippets at the front.
   snippets_.insert(snippets_.begin(),
                    std::make_move_iterator(new_snippets.begin()),
@@ -442,8 +462,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";
 }