Popular sites on the NTP: Extend UMA histograms that track clicks and impressions

chrome/browser/android/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/most_visited_sites.h"
 
 #include "base/android/jni_android.h"
 #include "base/android/jni_array.h"
 #include "base/android/jni_string.h"
 #include "base/android/scoped_java_ref.h"
@@ skipping 41 matching lines @@
 
 namespace {
 
 // Total number of tiles displayed.
 const char kNumTilesHistogramName[] = "NewTabPage.NumberOfTiles";
 // Tracking thumbnails.
 const char kNumLocalThumbnailTilesHistogramName[] =
     "NewTabPage.NumberOfThumbnailTiles";
 const char kNumEmptyTilesHistogramName[] = "NewTabPage.NumberOfGrayTiles";
 const char kNumServerTilesHistogramName[] = "NewTabPage.NumberOfExternalTiles";
-// Client suggestion opened.
-const char kOpenedItemClientHistogramName[] = "NewTabPage.MostVisited.client";
-// Server suggestion opened, no provider.
-const char kOpenedItemServerHistogramName[] = "NewTabPage.MostVisited.server";
-// Server suggestion opened with provider.
-const char kOpenedItemServerProviderHistogramFormat[] =
-    "NewTabPage.MostVisited.server%d";
-// Client impression.
-const char kImpressionClientHistogramName[] =
-    "NewTabPage.SuggestionsImpression.client";
-// Server suggestion impression, no provider.
-const char kImpressionServerHistogramName[] =
-    "NewTabPage.SuggestionsImpression.server";
-// Server suggestion impression with provider.
-const char kImpressionServerHistogramFormat[] =
-    "NewTabPage.SuggestionsImpression.server%d";
+
+// Format for tile clicks histogram.
+const char kOpenedItemHistogramFormat[] = "NewTabPage.MostVisited.%s";
+// Format for tile impressions histogram.
+const char kImpressionHistogramFormat[] = "NewTabPage.SuggestionsImpression.%s";
+// Identifiers for the various tile sources.
+const char kHistogramClientName[] = "client";
+const char kHistogramServerName[] = "server";
+const char kHistogramServerFormat[] = "server%d";
+const char kHistogramPopularName[] = "popular";
 
 const char kPopularSitesFieldTrialName[] = "NTPPopularSites";
 
 scoped_ptr<SkBitmap> MaybeFetchLocalThumbnail(
     const GURL& url,
     const scoped_refptr<TopSites>& top_sites) {
   DCHECK_CURRENTLY_ON(BrowserThread::DB);
   scoped_refptr<base::RefCountedMemory> image;
   scoped_ptr<SkBitmap> bitmap;
   if (top_sites && top_sites->GetPageThumbnail(url, false, &image))
     bitmap.reset(gfx::JPEGCodec::Decode(image->front(), image->size()));
   return bitmap.Pass();
 }
 
 // Log an event for a given |histogram| at a given element |position|. This
 // routine exists because regular histogram macros are cached thus can't be used
 // if the name of the histogram will change at a given call site.
-void LogHistogramEvent(const std::string& histogram, int position,
+void LogHistogramEvent(const std::string& histogram,
+                       int position,
                        int num_sites) {
   base::HistogramBase* counter = base::LinearHistogram::FactoryGet(
       histogram,
       1,
       num_sites,
       num_sites + 1,
       base::Histogram::kUmaTargetedHistogramFlag);
   if (counter)
     counter->Add(position);
 }
@@ skipping 24 matching lines @@
 }
 
 std::string GetPopularSitesFilename() {
   return variations::GetVariationParamValue(kPopularSitesFieldTrialName,
                                             "filename");
 }
 
 }  // namespace
 
 MostVisitedSites::MostVisitedSites(Profile* profile)
-    : profile_(profile), num_sites_(0), initial_load_done_(false),
+    : profile_(profile), num_sites_(0), received_most_visited_sites_(false),
+      received_popular_sites_(false), recorded_uma_(false),
       num_local_thumbs_(0), num_server_thumbs_(0), num_empty_thumbs_(0),
       scoped_observer_(this), weak_ptr_factory_(this) {
   // Register the debugging page for the Suggestions Service and the thumbnails
   // debugging page.
   content::URLDataSource::Add(profile_,
                               new suggestions::SuggestionsSource(profile_));
   content::URLDataSource::Add(profile_, new ThumbnailListSource(profile_));
 
   // Register this class as an observer to the sync service. It is important to
   // be notified of changes in the sync state such as initialization, sync
   // being enabled or disabled, etc.
   ProfileSyncService* profile_sync_service =
       ProfileSyncServiceFactory::GetForProfile(profile_);
   if (profile_sync_service)
     profile_sync_service->AddObserver(this);
 
   if (ShouldShowPopularSites()) {
     popular_sites_.reset(new PopularSites(
         GetPopularSitesFilename(),
         profile_->GetRequestContext(),
         base::Bind(&MostVisitedSites::OnPopularSitesAvailable,
                    base::Unretained(this))));
+  } else {
+    received_popular_sites_ = true;
   }
 }
 
 MostVisitedSites::~MostVisitedSites() {
   ProfileSyncService* profile_sync_service =
       ProfileSyncServiceFactory::GetForProfile(profile_);
   if (profile_sync_service && profile_sync_service->HasObserver(this))
     profile_sync_service->RemoveObserver(this);
 }
 
 void MostVisitedSites::Destroy(JNIEnv* env, jobject obj) {
   delete this;
 }
 
 void MostVisitedSites::OnLoadingComplete(JNIEnv* env, jobject obj) {
-  RecordUMAMetrics();
+  RecordThumbnailUMAMetrics();
 }
 
 void MostVisitedSites::SetMostVisitedURLsObserver(JNIEnv* env,
                                                   jobject obj,
                                                   jobject j_observer,
                                                   jint num_sites) {
   observer_.Reset(env, j_observer);
   num_sites_ = num_sites;
 
   QueryMostVisitedURLs();
@@ skipping 96 matching lines @@
     suggestions_service->BlacklistURL(
         url, base::Bind(&MostVisitedSites::OnSuggestionsProfileAvailable,
                         weak_ptr_factory_.GetWeakPtr()),
         base::Closure());
   }
 }
 
 void MostVisitedSites::RecordOpenedMostVisitedItem(JNIEnv* env,
                                                    jobject obj,
                                                    jint index) {
-  switch (mv_source_) {
-    case TOP_SITES: {
-      UMA_HISTOGRAM_SPARSE_SLOWLY(kOpenedItemClientHistogramName, index);
-      break;
-    }
-    case SUGGESTIONS_SERVICE: {
-      if (server_suggestions_.suggestions_size() > index) {
-        if (server_suggestions_.suggestions(index).providers_size()) {
-          std::string histogram = base::StringPrintf(
-              kOpenedItemServerProviderHistogramFormat,
-              server_suggestions_.suggestions(index).providers(0));
-          LogHistogramEvent(histogram, index, num_sites_);
-        } else {
-          UMA_HISTOGRAM_SPARSE_SLOWLY(kOpenedItemServerHistogramName, index);
-        }
-      }
-      break;
-    }
-  }
+  std::string histogram = base::StringPrintf(kOpenedItemHistogramFormat,
+                                             tile_sources_[index].c_str());

[Alexei Svitkine (slow), 2015/08/25 15:01:19]

Nit: Add a DCHECK() that index is within valid range.

[Marc Treib, 2015/08/25 15:19:32]

Done.

+  LogHistogramEvent(histogram, index, num_sites_);
 }
 
 void MostVisitedSites::OnStateChanged() {
   // There have been changes to the sync state. This class cares about a few
   // (just initialized, enabled/disabled or history sync state changed). Re-run
   // the query code which will use the proper state.
   QueryMostVisitedURLs();
 }
 
 // static
@@ skipping 23 matching lines @@
   top_sites->GetMostVisitedURLs(
       base::Bind(&MostVisitedSites::OnMostVisitedURLsAvailable,
                  weak_ptr_factory_.GetWeakPtr()),
       false);
 }
 
 void MostVisitedSites::OnMostVisitedURLsAvailable(
     const history::MostVisitedURLList& visited_list) {
   std::vector<base::string16> titles;
   std::vector<std::string> urls;
+  tile_sources_.clear();
   int num_tiles = std::min(static_cast<int>(visited_list.size()), num_sites_);
   for (int 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.
     }
     titles.push_back(visited.title);
     urls.push_back(visited.url.spec());
+    tile_sources_.push_back(kHistogramClientName);
   }
 
-  // Only log impression metrics on the initial load of the NTP.
-  if (!initial_load_done_) {
-    for (int i = 0; i < num_tiles; ++i) {
-      UMA_HISTOGRAM_SPARSE_SLOWLY(kImpressionClientHistogramName, i);
-    }
-  }
+  received_most_visited_sites_ = true;
   mv_source_ = TOP_SITES;
   AddPopularSites(&titles, &urls);
   NotifyMostVisitedURLsObserver(titles, urls);
 }
 
 void MostVisitedSites::OnSuggestionsProfileAvailable(
     const SuggestionsProfile& suggestions_profile) {
   int num_tiles = suggestions_profile.suggestions_size();
   // With no server suggestions, fall back to local Most Visited.
   if (num_tiles == 0) {
     InitiateTopSitesQuery();
     return;
   }
   if (num_sites_ < num_tiles)
     num_tiles = num_sites_;
 
   std::vector<base::string16> titles;
   std::vector<std::string> urls;
+  tile_sources_.clear();
   for (int i = 0; i < num_tiles; ++i) {
     const ChromeSuggestion& suggestion = suggestions_profile.suggestions(i);
     titles.push_back(base::UTF8ToUTF16(suggestion.title()));
     urls.push_back(suggestion.url());
-    // Only log impression metrics on the initial NTP load.
-    if (!initial_load_done_) {
-      if (suggestion.providers_size()) {
-        std::string histogram = base::StringPrintf(
-            kImpressionServerHistogramFormat, suggestion.providers(0));
-        LogHistogramEvent(histogram, i, num_sites_);
-      } else {
-        UMA_HISTOGRAM_SPARSE_SLOWLY(kImpressionServerHistogramName, i);
-      }
+    std::string tile_source;
+    if (suggestion.providers_size()) {

[Alexei Svitkine (slow), 2015/08/25 15:01:19]

Nit: != 0

[Marc Treib, 2015/08/25 15:19:32]

Done.

+      tile_source =
+          base::StringPrintf(kHistogramServerFormat, suggestion.providers(0));
+    } else {
+      tile_source = kHistogramServerName;
     }
+    tile_sources_.push_back(tile_source);
   }
+
+  received_most_visited_sites_ = true;
   mv_source_ = SUGGESTIONS_SERVICE;
-  // Keep a copy of the suggestions for eventual logging.
-  server_suggestions_ = suggestions_profile;
   AddPopularSites(&titles, &urls);
   NotifyMostVisitedURLsObserver(titles, urls);
 }
 
 void MostVisitedSites::AddPopularSites(std::vector<base::string16>* titles,
-                                       std::vector<std::string>* urls) const {
+                                       std::vector<std::string>* urls) {
   if (!popular_sites_)
     return;
 
   DCHECK_EQ(titles->size(), urls->size());
+  DCHECK_EQ(titles->size(), tile_sources_.size());
   DCHECK_LE(static_cast<int>(titles->size()), num_sites_);
 
   // Collect all non-blacklisted popular suggestions.
-  std::vector<base::string16> new_titles;
-  std::vector<std::string> new_urls;
+  std::vector<base::string16> popular_titles;
+  std::vector<std::string> popular_urls;
   scoped_refptr<TopSites> top_sites(TopSitesFactory::GetForProfile(profile_));
   for (const PopularSites::Site& popular_site : popular_sites_->sites()) {
     // Skip blacklisted sites.
     if (top_sites && top_sites->IsBlacklisted(popular_site.url))
       continue;
 
-    new_titles.push_back(popular_site.title);
-    new_urls.push_back(popular_site.url.spec());
-    if (static_cast<int>(new_titles.size()) >= num_sites_)
+    popular_titles.push_back(popular_site.title);
+    popular_urls.push_back(popular_site.url.spec());
+    if (static_cast<int>(popular_titles.size()) >= num_sites_)
       break;
   }
 
-  AddPopularSitesImpl(num_sites_, titles, urls, new_titles, new_urls);
+  AddPopularSitesImpl(
+      num_sites_, titles, urls, &tile_sources_, popular_titles, popular_urls);
 }
 
 // static
 void MostVisitedSites::AddPopularSitesImpl(
     int num_sites,
     std::vector<base::string16>* titles,
     std::vector<std::string>* urls,
+    std::vector<std::string>* tile_sources,
     const std::vector<base::string16>& popular_titles,
     const std::vector<std::string>& popular_urls) {
   // Start off with the popular suggestions.
   std::vector<base::string16> new_titles(popular_titles);
   std::vector<std::string> new_urls(popular_urls);
+  std::vector<std::string> new_tile_sources(new_titles.size(),
+                                            kHistogramPopularName);
 
   // Now, go over the personalized suggestions and replace matching popular
   // suggestions. This is so that when some of the popular suggestions become
   // personal, they retain their absolute positions.
-  std::vector<bool> new_is_personalized(new_titles.size(), false);
   std::vector<base::string16> titles_to_insert;
   std::vector<std::string> urls_to_insert;
+  std::vector<std::string> tile_sources_to_insert;
   for (size_t site_index = 0; site_index < titles->size(); site_index++) {
     const base::string16& title = (*titles)[site_index];
     const std::string& url = (*urls)[site_index];
+    const std::string& tile_source = (*tile_sources)[site_index];
     // See if we already have a matching popular site.
     bool found = false;
     for (size_t i = 0; i < new_urls.size(); i++) {
-      if (!new_is_personalized[i] &&
+      if (new_tile_sources[i] == kHistogramPopularName &&
           GURL(new_urls[i]).host() == GURL(url).host()) {
         // We have a matching popular sites suggestion. Replace it with the
         // actual URL and title.
         new_titles[i] = title;
         new_urls[i] = url;
-        new_is_personalized[i] = true;
+        new_tile_sources[i] = tile_source;
         found = true;
         break;
       }
     }
     if (!found) {
       titles_to_insert.push_back(title);
       urls_to_insert.push_back(url);
+      tile_sources_to_insert.push_back(tile_source);
     }
   }
 
   // Append personalized suggestions at the end if there's room.
   size_t num_to_append =
       std::min(static_cast<size_t>(num_sites) - new_titles.size(),
                titles_to_insert.size());
   new_titles.insert(new_titles.end(),
                     titles_to_insert.end() - num_to_append,
                     titles_to_insert.end());
   new_urls.insert(new_urls.end(),
                   urls_to_insert.end() - num_to_append,
                   urls_to_insert.end());
-  new_is_personalized.insert(new_is_personalized.end(), num_to_append, true);
+  new_tile_sources.insert(new_tile_sources.end(),
+                          tile_sources_to_insert.end() - num_to_append,
+                          tile_sources_to_insert.end());
 
   // Finally, go over the remaining personalized suggestions and evict popular
   // suggestions to accommodate them. Do it in reverse order, so the least
   // important popular suggestions will be evicted.
   for (size_t i = titles_to_insert.size() - num_to_append; i > 0; --i) {
     const base::string16& title = titles_to_insert[i - 1];
     const std::string& url = urls_to_insert[i - 1];
+    const std::string& tile_source = tile_sources_to_insert[i - 1];
     for (size_t insert_i = new_titles.size(); insert_i > 0; --insert_i) {
       size_t insert_index = insert_i - 1;
-      if (!new_is_personalized[insert_index]) {
+      if (new_tile_sources[insert_index] == kHistogramPopularName) {
         new_titles[insert_index] = title;
         new_urls[insert_index] = url;
-        new_is_personalized[insert_index] = true;
+        new_tile_sources[insert_index] = tile_source;
         break;
       }
     }
   }
 
   titles->swap(new_titles);
   urls->swap(new_urls);
+  tile_sources->swap(new_tile_sources);
 }
 
 void MostVisitedSites::NotifyMostVisitedURLsObserver(
     const std::vector<base::string16>& titles,
     const std::vector<std::string>& urls) {
   DCHECK_EQ(titles.size(), urls.size());
-  if (!initial_load_done_)
+  if (received_most_visited_sites_ && received_popular_sites_ &&
+      !recorded_uma_) {
+    RecordImpressionUMAMetrics();
     UMA_HISTOGRAM_SPARSE_SLOWLY(kNumTilesHistogramName, titles.size());
-  initial_load_done_ = true;
+    recorded_uma_ = true;
+  }
   JNIEnv* env = AttachCurrentThread();
   Java_MostVisitedURLsObserver_onMostVisitedURLsAvailable(
       env, observer_.obj(), ToJavaArrayOfStrings(env, titles).obj(),
       ToJavaArrayOfStrings(env, urls).obj());
 }
 
 void MostVisitedSites::OnPopularSitesAvailable(bool success) {
+  received_popular_sites_ = true;
+
   if (!success) {
     LOG(WARNING) << "Download of popular sites failed";
     return;
   }
 
   if (observer_.is_null())
     return;
 
   std::vector<std::string> urls;
   std::vector<std::string> favicon_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());
   }
   JNIEnv* env = AttachCurrentThread();
   Java_MostVisitedURLsObserver_onPopularURLsAvailable(
       env, observer_.obj(), ToJavaArrayOfStrings(env, urls).obj(),
       ToJavaArrayOfStrings(env, favicon_urls).obj());
 
   QueryMostVisitedURLs();
 }
 
-void MostVisitedSites::RecordUMAMetrics() {
+void MostVisitedSites::RecordThumbnailUMAMetrics() {
   UMA_HISTOGRAM_SPARSE_SLOWLY(kNumLocalThumbnailTilesHistogramName,
                               num_local_thumbs_);
   num_local_thumbs_ = 0;
   UMA_HISTOGRAM_SPARSE_SLOWLY(kNumEmptyTilesHistogramName, num_empty_thumbs_);
   num_empty_thumbs_ = 0;
   UMA_HISTOGRAM_SPARSE_SLOWLY(kNumServerTilesHistogramName, num_server_thumbs_);
   num_server_thumbs_ = 0;
 }
 
+void MostVisitedSites::RecordImpressionUMAMetrics() {
+  for (size_t i = 0; i < tile_sources_.size(); i++) {
+    std::string histogram = base::StringPrintf(kImpressionHistogramFormat,
+                                               tile_sources_[i].c_str());
+    LogHistogramEvent(histogram, static_cast<int>(i), num_sites_);
+  }
+}
+
 void MostVisitedSites::TopSitesLoaded(history::TopSites* top_sites) {
 }
 
 void MostVisitedSites::TopSitesChanged(history::TopSites* top_sites,
                                        ChangeReason change_reason) {
   if (mv_source_ == TOP_SITES) {
     // The displayed suggestions are invalidated.
     QueryMostVisitedURLs();
   }
 }
 
 static jlong Init(JNIEnv* env, jobject obj, jobject jprofile) {
   MostVisitedSites* most_visited_sites =
       new MostVisitedSites(ProfileAndroid::FromProfileAndroid(jprofile));
   return reinterpret_cast<intptr_t>(most_visited_sites);
 }