Add favicon and publisher name to snippet cards

components/ntp_snippets/ntp_snippets_service.cc

 // Copyright 2015 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/ntp_snippets/ntp_snippets_service.h"
 
 #include <algorithm>
 #include <iterator>
 #include <utility>
 
 #include "base/command_line.h"
 #include "base/files/file_path.h"
 #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"
 #include "base/task_runner_util.h"
 #include "base/time/time.h"
 #include "base/values.h"
 #include "components/ntp_snippets/pref_names.h"
 #include "components/ntp_snippets/switches.h"
 #include "components/prefs/pref_registry_simple.h"
 #include "components/prefs/pref_service.h"
@@ skipping 253 matching lines @@
     return std::set<std::string>();
 
   // TODO(treib) this should just call GetSnippetHostsFromPrefs
   return GetSuggestionsHostsImpl(
       suggestions_service_->GetSuggestionsDataFromCache());
 }
 
 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;
   discarded_snippets_.push_back(std::move(*it));
   snippets_.erase(it);
   StoreDiscardedSnippetsToPrefs();
   StoreSnippetsToPrefs();
   FOR_EACH_OBSERVER(NTPSnippetsServiceObserver, observers_,
                     NTPSnippetsServiceLoaded());
   return true;
@@ skipping 36 matching lines @@
   StoreSnippetHostsToPrefs(hosts);
 
   FOR_EACH_OBSERVER(NTPSnippetsServiceObserver, observers_,
                     NTPSnippetsServiceLoaded());
 
   FetchSnippetsFromHosts(hosts);
 }
 
 void NTPSnippetsService::OnSnippetsDownloaded(
     const std::string& snippets_json, const std::string& status) {
-
   if (!snippets_json.empty()) {
     DCHECK(status.empty());
 
     parse_json_callback_.Run(
         snippets_json,
         base::Bind(&NTPSnippetsService::OnJsonParsed,
                    weak_ptr_factory_.GetWeakPtr(), snippets_json),
         base::Bind(&NTPSnippetsService::OnJsonError,
                    weak_ptr_factory_.GetWeakPtr(), snippets_json));
   } else {
@@ skipping 53 matching lines @@
   for (std::unique_ptr<NTPSnippet>& snippet : new_snippets) {
     if (snippet->publish_date().is_null())
       snippet->set_publish_date(base::Time::Now());
     if (snippet->expiry_date().is_null()) {
       snippet->set_expiry_date(
           snippet->publish_date() +
           base::TimeDelta::FromMinutes(kDefaultExpiryTimeMins));
     }
   }
 
+  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()),
                    std::make_move_iterator(new_snippets.end()));
 
   // If there are more snippets now than we want to show, drop the extra ones
   // from the end of the list.
   if (snippets_.size() > kMaxSnippetCount)
     snippets_.resize(kMaxSnippetCount);
 
   return true;
 }
 
 void NTPSnippetsService::LoadSnippetsFromPrefs() {
   bool success = LoadFromListValue(*pref_service_->GetList(prefs::kSnippets));
   DCHECK(success) << "Failed to parse snippets from prefs";
 
   LoadingSnippetsFinished();
 }
 
 void NTPSnippetsService::StoreSnippetsToPrefs() {
   pref_service_->Set(prefs::kSnippets, *SnippetsToListValue(snippets_));
 }
 
 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";
 }
 
 void NTPSnippetsService::StoreDiscardedSnippetsToPrefs() {
   pref_service_->Set(prefs::kDiscardedSnippets,
                      *SnippetsToListValue(discarded_snippets_));
 }
 
 std::set<std::string> NTPSnippetsService::GetSnippetHostsFromPrefs() const {
   std::set<std::string> hosts;
@@ skipping 51 matching lines @@
     if (snippet->expiry_date() < next_expiry)
       next_expiry = snippet->expiry_date();
   }
   DCHECK_GT(next_expiry, expiry);
   expiry_timer_.Start(FROM_HERE, next_expiry - expiry,
                       base::Bind(&NTPSnippetsService::LoadingSnippetsFinished,
                                  base::Unretained(this)));
 }
 
 }  // namespace ntp_snippets