Add UMA stats for which icon types are shown and clicked on the NTP.

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 38 matching lines @@
 using content::BrowserThread;
 using history::TopSites;
 using suggestions::ChromeSuggestion;
 using suggestions::SuggestionsProfile;
 using suggestions::SuggestionsService;
 using suggestions::SuggestionsServiceFactory;
 using suggestions::SyncState;
 
 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";
-
-// 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";
 
+// The visual type of a most visited tile.
+//
+// These values must stay in sync with the MostVisitedTileType enum
+// in histograms.xml.
+//
+// A Java counterpart will be generated for this enum.
+// GENERATED_JAVA_ENUM_PACKAGE: org.chromium.chrome.browser.ntp
+enum MostVisitedTileType {
+    // The icon or thumbnail hasn't loaded yet.
+    NONE,
+    // The item displays a site's actual favicon or touch icon.
+    ICON_REAL,
+    // The item displays a color derived from the site's favicon or touch icon.
+    ICON_COLOR,
+    // The item displays a default gray box in place of an icon.
+    ICON_DEFAULT,
+    // The item displays a locally-captured thumbnail of the site content.
+    THUMBNAIL_LOCAL,
+    // The item displays a server-provided thumbnail of the site content.
+    THUMBNAIL_SERVER,
+    // The item displays a default graphic in place of a thumbnail.
+    THUMBNAIL_DEFAULT,
+    NUM_TILE_TYPES,
+};
+
 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();
 }
@@ skipping 110 matching lines @@
                  ? base::StringPrintf(kHistogramServerFormat, provider_index)
                  : kHistogramServerName;
   }
   NOTREACHED();
   return std::string();
 }
 
 MostVisitedSites::MostVisitedSites(Profile* profile)
     : 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);
 }
 
 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) {
-  RecordThumbnailUMAMetrics();
-}
-
 void MostVisitedSites::SetMostVisitedURLsObserver(JNIEnv* env,
                                                   jobject obj,
                                                   jobject j_observer,
                                                   jint num_sites) {
   observer_.Reset(env, j_observer);
   num_sites_ = num_sites;
 
   if (ShouldShowPopularSites() &&
       NeedPopularSites(profile_->GetPrefs(), num_sites_)) {
     popular_sites_.reset(new PopularSites(
@@ skipping 85 matching lines @@
 }
 
 void MostVisitedSites::OnObtainedThumbnail(
     bool is_local_thumbnail,
     scoped_ptr<ScopedJavaGlobalRef<jobject>> j_callback,
     const GURL& url,
     const SkBitmap* bitmap) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   JNIEnv* env = AttachCurrentThread();
   ScopedJavaLocalRef<jobject> j_bitmap;
-  if (bitmap) {
+  if (bitmap)
     j_bitmap = gfx::ConvertToJavaBitmap(bitmap);
-    if (is_local_thumbnail) {
-      ++num_local_thumbs_;
-    } else {
-      ++num_server_thumbs_;
-    }
-  } else {
-    ++num_empty_thumbs_;
-  }
   Java_ThumbnailCallback_onMostVisitedURLsThumbnailAvailable(
-      env, j_callback->obj(), j_bitmap.obj());
+      env, j_callback->obj(), j_bitmap.obj(), is_local_thumbnail);
 }
 
 void MostVisitedSites::BlacklistUrl(JNIEnv* env,
                                     jobject obj,
                                     jstring j_url) {
   GURL url(ConvertJavaStringToUTF8(env, j_url));
 
   // Always blacklist in the local TopSites.
   scoped_refptr<TopSites> top_sites = TopSitesFactory::GetForProfile(profile_);
   if (top_sites)
     top_sites->AddBlacklistedURL(url);
 
   // Only blacklist in the server-side suggestions service if it's active.
   if (mv_source_ == SUGGESTIONS_SERVICE) {
     SuggestionsService* suggestions_service =
         SuggestionsServiceFactory::GetForProfile(profile_);
     DCHECK(suggestions_service);
     suggestions_service->BlacklistURL(
         url, base::Bind(&MostVisitedSites::OnSuggestionsProfileAvailable,
                         weak_ptr_factory_.GetWeakPtr()),
         base::Closure());
   }
 }
 
+void MostVisitedSites::RecordTileTypeMetrics(JNIEnv* env,
+                                             jobject obj,
+                                             jintArray jtile_types,
+                                             jboolean is_icon_mode) {
+  std::vector<int> tile_types;
+  base::android::JavaIntArrayToIntVector(env, jtile_types, &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());
+    LogHistogramEvent(histogram, tile_type, NUM_TILE_TYPES);
+  }
+
+  if (is_icon_mode) {
+    UMA_HISTOGRAM_COUNTS_100("NewTabPage.IconsReal",
+                             counts_per_type[ICON_REAL]);

[Alexei Svitkine (slow), 2015/09/30 19:52:05]

UMA_HISTOGRAM_COUNTS_100 allocates 50 buckets. I think it's a bit wasteful,
suggest switching to UMA_HISTOGRAM_SPARSE_SLOWLY() like the ones below it.

Looking at your data, all your values are below <12 in the data, which are exact
even in UMA_HISTOGRAM_COUNTS_100(). Suggest making consistent with the else.

[newt (away), 2015/09/30 20:40:54]

Done.

I had assumed that UMA_HISTOGRAM_COUNTS_100 was faster/simpler/more efficient
than UMA_HISTOGRAM_SPARSE_SLOWLY, based on their names, but it sounds like this
isn't the case, at least when only a few values will be reported. What does the
"SLOWLY" mean in the name anyway? 

It would be nice if the UMA_HISTOGRAM_SPARSE_SLOWLY macro had a comment
explaining itself.

[Alexei Svitkine (slow), 2015/09/30 20:59:52]

It is slower because it grabs a lock on that histogram object and uses a map.
It's a tiny bit of overhead, so shouldn't be used in tight loops, but otherwise
fast. But it's more memory efficient.

Agree that comments on the macro would be good.

+    UMA_HISTOGRAM_COUNTS_100("NewTabPage.IconsColor",
+                             counts_per_type[ICON_COLOR]);
+    UMA_HISTOGRAM_COUNTS_100("NewTabPage.IconsGray",
+                             counts_per_type[ICON_DEFAULT]);
+  } else {
+    UMA_HISTOGRAM_SPARSE_SLOWLY("NewTabPage.NumberOfThumbnailTiles",
+                                counts_per_type[THUMBNAIL_LOCAL]);
+    UMA_HISTOGRAM_SPARSE_SLOWLY("NewTabPage.NumberOfExternalTiles",
+                                counts_per_type[THUMBNAIL_SERVER]);
+    UMA_HISTOGRAM_SPARSE_SLOWLY("NewTabPage.NumberOfGrayTiles",
+                                counts_per_type[THUMBNAIL_DEFAULT]);
+  }
+}
+
 void MostVisitedSites::RecordOpenedMostVisitedItem(JNIEnv* env,
                                                    jobject obj,
-                                                   jint index) {
+                                                   jint index,
+                                                   jint tile_type) {
   DCHECK_GE(index, 0);
   DCHECK_LT(index, static_cast<int>(current_suggestions_.size()));
   std::string histogram = base::StringPrintf(
-      kOpenedItemHistogramFormat,
+      "NewTabPage.MostVisited.%s",
       current_suggestions_[index]->GetSourceHistogramName().c_str());
   LogHistogramEvent(histogram, index, num_sites_);
+
+  histogram = base::StringPrintf(
+      "NewTabPage.TileTypeClicked.%s",
+      current_suggestions_[index]->GetSourceHistogramName().c_str());
+  LogHistogramEvent(histogram, tile_type, NUM_TILE_TYPES);
 }
 
 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 259 matching lines @@
   // Return destination postions filled so far which becomes the start_position
   // for future runs.
   return i;
 }
 
 void MostVisitedSites::NotifyMostVisitedURLsObserver() {
   size_t num_suggestions = current_suggestions_.size();
   if (received_most_visited_sites_ && received_popular_sites_ &&
       !recorded_uma_) {
     RecordImpressionUMAMetrics();
-    UMA_HISTOGRAM_SPARSE_SLOWLY(kNumTilesHistogramName, num_suggestions);
+    UMA_HISTOGRAM_SPARSE_SLOWLY("NewTabPage.NumberOfTiles", num_suggestions);
     recorded_uma_ = true;
   }
   std::vector<base::string16> titles;
   std::vector<std::string> urls;
   titles.reserve(num_suggestions);
   urls.reserve(num_suggestions);
   for (const Suggestion* suggestion : current_suggestions_) {
     titles.push_back(suggestion->title);
     urls.push_back(suggestion->url.spec());
   }
@@ skipping 22 matching lines @@
     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::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 < current_suggestions_.size(); i++) {
     std::string histogram = base::StringPrintf(
-        kImpressionHistogramFormat,
+        "NewTabPage.SuggestionsImpression.%s",
         current_suggestions_[i]->GetSourceHistogramName().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();
+    InitiateTopSitesQuery();
   }
 }
 
 static jlong Init(JNIEnv* env,
                   const JavaParamRef<jobject>& obj,
                   const JavaParamRef<jobject>& jprofile) {
   MostVisitedSites* most_visited_sites =
       new MostVisitedSites(ProfileAndroid::FromProfileAndroid(jprofile));
   return reinterpret_cast<intptr_t>(most_visited_sites);
 }