Update MostVisitedSites observer interface.

chrome/browser/android/ntp/most_visited_sites.cc

 // Copyright 2013 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 "chrome/browser/android/ntp/most_visited_sites.h"
 
 #include <utility>
 
 #include "base/callback.h"
 #include "base/command_line.h"
 #include "base/metrics/field_trial.h"
 #include "base/metrics/histogram.h"
 #include "base/metrics/sparse_histogram.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/time/time.h"
-#include "chrome/browser/android/ntp/popular_sites.h"
 #include "chrome/browser/history/top_sites_factory.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/search/suggestions/suggestions_service_factory.h"
 #include "chrome/browser/supervised_user/supervised_user_service.h"
 #include "chrome/browser/supervised_user/supervised_user_service_factory.h"
 #include "chrome/browser/supervised_user/supervised_user_url_filter.h"
 #include "chrome/browser/thumbnails/thumbnail_list_source.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/pref_names.h"
 #include "components/history/core/browser/top_sites.h"
@@ skipping 112 matching lines @@
   }
   // The whole grid is already filled with personal suggestions, no point in
   // bothering with popular ones.
   return false;
 }
 
 bool AreURLsEquivalent(const GURL& url1, const GURL& url2) {
   return url1.host() == url2.host() && url1.path() == url2.path();
 }
 
-}  // namespace
-
-MostVisitedSites::Suggestion::Suggestion() : provider_index(-1) {}
-
-MostVisitedSites::Suggestion::~Suggestion() {}
-
-std::string MostVisitedSites::Suggestion::GetSourceHistogramName() const {
-  switch (source) {
+std::string GetSourceHistogramName(
+        const MostVisitedSites::Suggestion& suggestion) {
+  switch (suggestion.source) {
     case MostVisitedSites::TOP_SITES:
       return kHistogramClientName;
     case MostVisitedSites::POPULAR:
       return kHistogramPopularName;
     case MostVisitedSites::WHITELIST:
       return kHistogramWhitelistName;
     case MostVisitedSites::SUGGESTIONS_SERVICE:
-      return provider_index >= 0
-                 ? base::StringPrintf(kHistogramServerFormat, provider_index)
+      return suggestion.provider_index >= 0
+                 ? base::StringPrintf(kHistogramServerFormat,
+                                      suggestion.provider_index)
                  : kHistogramServerName;
   }
   NOTREACHED();
   return std::string();
 }
 
+}  // namespace
+
+MostVisitedSites::Suggestion::Suggestion() : provider_index(-1) {}
+
+MostVisitedSites::Suggestion::~Suggestion() {}
+
 MostVisitedSites::MostVisitedSites(Profile* profile)
     : profile_(profile), observer_(nullptr), num_sites_(0),
       received_most_visited_sites_(false), received_popular_sites_(false),
       recorded_uma_(false), scoped_observer_(this), weak_ptr_factory_(this) {
   // Register the thumbnails debugging page.
   content::URLDataSource::Add(profile_, new ThumbnailListSource(profile_));
 
   SupervisedUserService* supervised_user_service =
       SupervisedUserServiceFactory::GetForProfile(profile_);
   supervised_user_service->AddObserver(this);
 }
 
 MostVisitedSites::~MostVisitedSites() {
   SupervisedUserService* supervised_user_service =
       SupervisedUserServiceFactory::GetForProfile(profile_);
   supervised_user_service->RemoveObserver(this);
 }
 
 void MostVisitedSites::SetMostVisitedURLsObserver(
-      MostVisitedSitesObserver* observer, int num_sites) {
+      MostVisitedSites::Observer* observer, int num_sites) {
   observer_ = observer;
   num_sites_ = num_sites;
 
   if (ShouldShowPopularSites() &&
       NeedPopularSites(profile_->GetPrefs(), num_sites_)) {
     popular_sites_.reset(new PopularSites(
         profile_,
         GetPopularSitesCountry(),
         GetPopularSitesVersion(),
         false,
@@ skipping 110 matching lines @@
 
 void MostVisitedSites::RecordTileTypeMetrics(
     const std::vector<int>& tile_types) {
   DCHECK_EQ(current_suggestions_.size(), tile_types.size());
   int counts_per_type[NUM_TILE_TYPES] = {0};
   for (size_t i = 0; i < tile_types.size(); ++i) {
     int tile_type = tile_types[i];
     ++counts_per_type[tile_type];
     std::string histogram = base::StringPrintf(
         "NewTabPage.TileType.%s",
-        current_suggestions_[i]->GetSourceHistogramName().c_str());
+        GetSourceHistogramName(current_suggestions_[i]).c_str());
     LogHistogramEvent(histogram, tile_type, NUM_TILE_TYPES);
   }
 
   UMA_HISTOGRAM_SPARSE_SLOWLY("NewTabPage.IconsReal",
                               counts_per_type[ICON_REAL]);
   UMA_HISTOGRAM_SPARSE_SLOWLY("NewTabPage.IconsColor",
                               counts_per_type[ICON_COLOR]);
   UMA_HISTOGRAM_SPARSE_SLOWLY("NewTabPage.IconsGray",
                               counts_per_type[ICON_DEFAULT]);
 }
 
 void MostVisitedSites::RecordOpenedMostVisitedItem(int index, int tile_type) {
   DCHECK_GE(index, 0);
   DCHECK_LT(index, static_cast<int>(current_suggestions_.size()));
   std::string histogram = base::StringPrintf(
       "NewTabPage.MostVisited.%s",
-      current_suggestions_[index]->GetSourceHistogramName().c_str());
+      GetSourceHistogramName(current_suggestions_[index]).c_str());
   LogHistogramEvent(histogram, index, num_sites_);
 
   histogram = base::StringPrintf(
       "NewTabPage.TileTypeClicked.%s",
-      current_suggestions_[index]->GetSourceHistogramName().c_str());
+      GetSourceHistogramName(current_suggestions_[index]).c_str());
   LogHistogramEvent(histogram, tile_type, NUM_TILE_TYPES);
 }
 
 void MostVisitedSites::OnURLFilterChanged() {
   QueryMostVisitedURLs();
 }
 
 // static
 void MostVisitedSites::RegisterProfilePrefs(
     user_prefs::PrefRegistrySyncable* registry) {
@@ skipping 37 matching lines @@
   }
   return base::FilePath();
 }
 
 void MostVisitedSites::OnMostVisitedURLsAvailable(
     const history::MostVisitedURLList& visited_list) {
   SupervisedUserURLFilter* url_filter =
       SupervisedUserServiceFactory::GetForProfile(profile_)
           ->GetURLFilterForUIThread();
 
-  MostVisitedSites::SuggestionsVector suggestions;
+  MostVisitedSites::SuggestionsPtrVector suggestions;
   size_t num_tiles =
       std::min(visited_list.size(), static_cast<size_t>(num_sites_));
   for (size_t i = 0; i < num_tiles; ++i) {
     const history::MostVisitedURL& visited = visited_list[i];
     if (visited.url.is_empty()) {
       num_tiles = i;
       break;  // This is the signal that there are no more real visited sites.
     }
     if (url_filter->GetFilteringBehaviorForURL(visited.url) ==
         SupervisedUserURLFilter::FilteringBehavior::BLOCK) {
@@ skipping 22 matching lines @@
   if (num_tiles == 0) {
     InitiateTopSitesQuery();
     return;
   }
   if (num_sites_ < num_tiles)
     num_tiles = num_sites_;
 
   SupervisedUserURLFilter* url_filter =
       SupervisedUserServiceFactory::GetForProfile(profile_)
           ->GetURLFilterForUIThread();
-  MostVisitedSites::SuggestionsVector suggestions;
+  MostVisitedSites::SuggestionsPtrVector suggestions;
   for (int i = 0; i < num_tiles; ++i) {
     const ChromeSuggestion& suggestion = suggestions_profile.suggestions(i);
     if (url_filter->GetFilteringBehaviorForURL(GURL(suggestion.url())) ==
         SupervisedUserURLFilter::FilteringBehavior::BLOCK) {
       continue;
     }
 
     std::unique_ptr<Suggestion> generated_suggestion(new Suggestion());
     generated_suggestion->title = base::UTF8ToUTF16(suggestion.title());
     generated_suggestion->url = GURL(suggestion.url());
     generated_suggestion->source = SUGGESTIONS_SERVICE;
     generated_suggestion->whitelist_icon_path = GetWhitelistLargeIconPath(
         GURL(suggestion.url()));
     if (suggestion.providers_size() > 0)
       generated_suggestion->provider_index = suggestion.providers(0);
 
     suggestions.push_back(std::move(generated_suggestion));
   }
 
   received_most_visited_sites_ = true;
   mv_source_ = SUGGESTIONS_SERVICE;
   SaveNewNTPSuggestions(&suggestions);
   NotifyMostVisitedURLsObserver();
 }
 
-MostVisitedSites::SuggestionsVector
+MostVisitedSites::SuggestionsPtrVector
 MostVisitedSites::CreateWhitelistEntryPointSuggestions(
-    const MostVisitedSites::SuggestionsVector& personal_suggestions) {
+    const MostVisitedSites::SuggestionsPtrVector& personal_suggestions) {
   size_t num_personal_suggestions = personal_suggestions.size();
   DCHECK_LE(num_personal_suggestions, static_cast<size_t>(num_sites_));
 
   size_t num_whitelist_suggestions = num_sites_ - num_personal_suggestions;
-  MostVisitedSites::SuggestionsVector whitelist_suggestions;
+  MostVisitedSites::SuggestionsPtrVector whitelist_suggestions;
 
   SupervisedUserService* supervised_user_service =
       SupervisedUserServiceFactory::GetForProfile(profile_);
   SupervisedUserURLFilter* url_filter =
       supervised_user_service->GetURLFilterForUIThread();
 
   std::set<std::string> personal_hosts;
   for (const auto& suggestion : personal_suggestions)
     personal_hosts.insert(suggestion->url.host());
   scoped_refptr<TopSites> top_sites(TopSitesFactory::GetForProfile(profile_));
@@ skipping 21 matching lines @@
     suggestion->whitelist_icon_path = whitelist->large_icon_path();
 
     whitelist_suggestions.push_back(std::move(suggestion));
     if (whitelist_suggestions.size() >= num_whitelist_suggestions)
       break;
   }
 
   return whitelist_suggestions;
 }
 
-MostVisitedSites::SuggestionsVector
+MostVisitedSites::SuggestionsPtrVector
 MostVisitedSites::CreatePopularSitesSuggestions(
-    const MostVisitedSites::SuggestionsVector& personal_suggestions,
-    const MostVisitedSites::SuggestionsVector& whitelist_suggestions) {
+    const MostVisitedSites::SuggestionsPtrVector& personal_suggestions,
+    const MostVisitedSites::SuggestionsPtrVector& whitelist_suggestions) {
   // For child accounts popular sites suggestions will not be added.
   if (profile_->IsChild())
-    return MostVisitedSites::SuggestionsVector();
+    return MostVisitedSites::SuggestionsPtrVector();
 
   size_t num_suggestions =
       personal_suggestions.size() + whitelist_suggestions.size();
   DCHECK_LE(num_suggestions, static_cast<size_t>(num_sites_));
 
   // Collect non-blacklisted popular suggestions, skipping those already present
   // in the personal suggestions.
   size_t num_popular_sites_suggestions = num_sites_ - num_suggestions;
-  MostVisitedSites::SuggestionsVector popular_sites_suggestions;
+  MostVisitedSites::SuggestionsPtrVector popular_sites_suggestions;
 
   if (num_popular_sites_suggestions > 0 && popular_sites_) {
     std::set<std::string> hosts;
     for (const auto& suggestion : personal_suggestions)
       hosts.insert(suggestion->url.host());
     for (const auto& suggestion : whitelist_suggestions)
       hosts.insert(suggestion->url.host());
     scoped_refptr<TopSites> top_sites(TopSitesFactory::GetForProfile(profile_));
     for (const PopularSites::Site& popular_site : popular_sites_->sites()) {
       // Skip blacklisted sites.
@@ skipping 11 matching lines @@
 
       popular_sites_suggestions.push_back(std::move(suggestion));
       if (popular_sites_suggestions.size() >= num_popular_sites_suggestions)
         break;
     }
   }
   return popular_sites_suggestions;
 }
 
 void MostVisitedSites::SaveNewNTPSuggestions(
-    MostVisitedSites::SuggestionsVector* personal_suggestions) {
-  MostVisitedSites::SuggestionsVector whitelist_suggestions =
+    MostVisitedSites::SuggestionsPtrVector* personal_suggestions) {
+  MostVisitedSites::SuggestionsPtrVector whitelist_suggestions =
       CreateWhitelistEntryPointSuggestions(*personal_suggestions);
-  MostVisitedSites::SuggestionsVector popular_sites_suggestions =
+  MostVisitedSites::SuggestionsPtrVector popular_sites_suggestions =
       CreatePopularSitesSuggestions(*personal_suggestions,
                                     whitelist_suggestions);
   size_t num_actual_tiles = personal_suggestions->size() +
                             whitelist_suggestions.size() +
                             popular_sites_suggestions.size();
   std::vector<std::string> old_sites_url;
   std::vector<bool> old_sites_is_personal;
   // TODO(treib): We used to call |GetPreviousNTPSites| here to populate
   // |old_sites_url| and |old_sites_is_personal|, but that caused problems
   // (crbug.com/585391). Either figure out a way to fix them and re-enable,
   // or properly remove the order-persisting code. crbug.com/601734
-  MostVisitedSites::SuggestionsVector merged_suggestions = MergeSuggestions(
+  MostVisitedSites::SuggestionsPtrVector merged_suggestions = MergeSuggestions(
       personal_suggestions, &whitelist_suggestions, &popular_sites_suggestions,
       old_sites_url, old_sites_is_personal);
   DCHECK_EQ(num_actual_tiles, merged_suggestions.size());
-  current_suggestions_.swap(merged_suggestions);
+  current_suggestions_.resize(merged_suggestions.size());
+  for (size_t i = 0; i < merged_suggestions.size(); ++i) {
+    std::swap(*merged_suggestions[i], current_suggestions_[i]);

[Marc Treib, 2016/04/26 09:57:55]

Ah, this took me a while to get :D
I think something like this would also work:
  current_suggestions_.emplace_back(std::move(*merged_suggestions[i]);
(without resizing current_suggestions_ first) which I'd find a bit easier to
read. But since this is temporary anyway, it's fine.

[sfiera, 2016/04/26 10:17:30]

Personally, I find std::swap() a lot easier to think about than std::move(),
though of course that's in part because std::swap() has been around longer. I'll
leave as-is; std::swap() is used this way two other places in this file.

[Marc Treib, 2016/04/26 10:59:22]

What tripped me up wasn't the swap per se, but rather the mix of (owning)
pointer and instance. Though that would apply to the move version as well...
aaanyway. It's fine.

+  }

[Marc Treib, 2016/04/26 09:57:55]

nit: no braces required

[sfiera, 2016/04/26 10:17:30]

Done.

   if (received_popular_sites_)
     SaveCurrentNTPSites();
 }
 
 // static
-MostVisitedSites::SuggestionsVector MostVisitedSites::MergeSuggestions(
-    MostVisitedSites::SuggestionsVector* personal_suggestions,
-    MostVisitedSites::SuggestionsVector* whitelist_suggestions,
-    MostVisitedSites::SuggestionsVector* popular_suggestions,
+MostVisitedSites::SuggestionsPtrVector MostVisitedSites::MergeSuggestions(
+    MostVisitedSites::SuggestionsPtrVector* personal_suggestions,
+    MostVisitedSites::SuggestionsPtrVector* whitelist_suggestions,
+    MostVisitedSites::SuggestionsPtrVector* popular_suggestions,
     const std::vector<std::string>& old_sites_url,
     const std::vector<bool>& old_sites_is_personal) {
   size_t num_personal_suggestions = personal_suggestions->size();
   size_t num_whitelist_suggestions = whitelist_suggestions->size();
   size_t num_popular_suggestions = popular_suggestions->size();
   size_t num_tiles = num_popular_suggestions + num_whitelist_suggestions +
                      num_personal_suggestions;
-  MostVisitedSites::SuggestionsVector merged_suggestions;
+  MostVisitedSites::SuggestionsPtrVector merged_suggestions;
   merged_suggestions.resize(num_tiles);
 
   size_t num_old_tiles = old_sites_url.size();
   DCHECK_LE(num_old_tiles, num_tiles);
   DCHECK_EQ(num_old_tiles, old_sites_is_personal.size());
   std::vector<std::string> old_sites_host;
   old_sites_host.reserve(num_old_tiles);
   // Only populate the hosts for popular suggestions as only they can be
   // replaced by host. Personal suggestions require an exact url match to be
   // replaced.
@@ skipping 52 matching lines @@
     success = source_list->GetBoolean(i, &is_personal);
     DCHECK(success);
     old_sites_is_personal->push_back(is_personal);
   }
 }
 
 void MostVisitedSites::SaveCurrentNTPSites() {
   base::ListValue url_list;
   base::ListValue source_list;
   for (const auto& suggestion : current_suggestions_) {
-    url_list.AppendString(suggestion->url.spec());
-    source_list.AppendBoolean(suggestion->source != MostVisitedSites::POPULAR);
+    url_list.AppendString(suggestion.url.spec());
+    source_list.AppendBoolean(suggestion.source != MostVisitedSites::POPULAR);
   }
   PrefService* prefs = profile_->GetPrefs();
   prefs->Set(prefs::kNTPSuggestionsIsPersonal, source_list);
   prefs->Set(prefs::kNTPSuggestionsURL, url_list);
 }
 
 // static
 std::vector<size_t> MostVisitedSites::InsertMatchingSuggestions(
-    MostVisitedSites::SuggestionsVector* src_suggestions,
-    MostVisitedSites::SuggestionsVector* dst_suggestions,
+    MostVisitedSites::SuggestionsPtrVector* src_suggestions,
+    MostVisitedSites::SuggestionsPtrVector* dst_suggestions,
     const std::vector<std::string>& match_urls,
     const std::vector<std::string>& match_hosts) {
   std::vector<size_t> unmatched_suggestions;
   size_t num_src_suggestions = src_suggestions->size();
   size_t num_matchers = match_urls.size();
   for (size_t i = 0; i < num_src_suggestions; ++i) {
     size_t position;
     for (position = 0; position < num_matchers; ++position) {
       if ((*dst_suggestions)[position] != nullptr)
         continue;
@@ skipping 42 matching lines @@
   if (received_most_visited_sites_ && received_popular_sites_ &&
       !recorded_uma_) {
     RecordImpressionUMAMetrics();
     UMA_HISTOGRAM_SPARSE_SLOWLY("NewTabPage.NumberOfTiles", num_suggestions);
     recorded_uma_ = true;
   }
 
   if (!observer_)
     return;
 
-  std::vector<base::string16> titles;
-  std::vector<std::string> urls;
-  std::vector<std::string> whitelist_icon_paths;
-  titles.reserve(num_suggestions);
-  urls.reserve(num_suggestions);
-  for (const auto& suggestion : current_suggestions_) {
-    titles.push_back(suggestion->title);
-    urls.push_back(suggestion->url.spec());
-    whitelist_icon_paths.push_back(suggestion->whitelist_icon_path.value());
-  }
-
-  observer_->OnMostVisitedURLsAvailable(titles, urls, whitelist_icon_paths);
+  observer_->OnMostVisitedURLsAvailable(current_suggestions_);
 }
 
 void MostVisitedSites::OnPopularSitesAvailable(bool success) {
   received_popular_sites_ = true;
 
   if (!success) {
     LOG(WARNING) << "Download of popular sites failed";
     return;
   }
 
   if (!observer_)
     return;
 
-  std::vector<std::string> urls;
-  std::vector<std::string> favicon_urls;
-  std::vector<std::string> large_icon_urls;
-  for (const PopularSites::Site& popular_site : popular_sites_->sites()) {
-    urls.push_back(popular_site.url.spec());
-    favicon_urls.push_back(popular_site.favicon_url.spec());
-    large_icon_urls.push_back(popular_site.large_icon_url.spec());
-  }
-  observer_->OnPopularURLsAvailable(urls, favicon_urls, large_icon_urls);
+  observer_->OnPopularURLsAvailable(popular_sites_->sites());
   QueryMostVisitedURLs();
 }
 
 void MostVisitedSites::RecordImpressionUMAMetrics() {
   for (size_t i = 0; i < current_suggestions_.size(); i++) {
     std::string histogram = base::StringPrintf(
         "NewTabPage.SuggestionsImpression.%s",
-        current_suggestions_[i]->GetSourceHistogramName().c_str());
+        GetSourceHistogramName(current_suggestions_[i]).c_str());
     LogHistogramEvent(histogram, static_cast<int>(i), num_sites_);
   }
 }
 
 void MostVisitedSites::TopSitesLoaded(TopSites* top_sites) {}
 
 void MostVisitedSites::TopSitesChanged(TopSites* top_sites,
                                        ChangeReason change_reason) {
   if (mv_source_ == TOP_SITES) {
     // The displayed suggestions are invalidated.
     InitiateTopSitesQuery();
   }
 }