Remove provider index from Android NewTabPage histograms

BUG=625161

Review-Url: https://codereview.chromium.org/2121133002
Cr-Commit-Position: refs/heads/master@{#405129}

chrome/android/java/src/org/chromium/chrome/browser/ntp/MostVisitedItem.java

@@ -54,7 +54,6 @@ public interface MostVisitedItemManager {
     private int mIndex;
     private int mTileType;
     private int mSource;
-    private int mProviderIndex;
     private View mView;
 
     /**
@@ -69,12 +68,9 @@ public interface MostVisitedItemManager {
      * @param offlineAvailable Whether there is an offline copy of the URL available.
      * @param index The index of this item in the list of most visited items.
      * @param source The {@link MostVisitedSource} that generated this item.
-     * @param providerIndex If this item comes from {@code MostVisitedSource.SUGGESTIONS_SERVICE},
-     *     this is the index of the source of the suggestion.
      */
     public MostVisitedItem(MostVisitedItemManager manager, String title, String url,
-            String whitelistIconPath, boolean offlineAvailable, int index, int source,
-            int providerIndex) {
+            String whitelistIconPath, boolean offlineAvailable, int index, int source) {
         mManager = manager;
         mTitle = title;
         mUrl = url;
@@ -83,7 +79,6 @@ public MostVisitedItem(MostVisitedItemManager manager, String title, String url,
         mIndex = index;
         mTileType = MostVisitedTileType.NONE;
         mSource = source;
-        mProviderIndex = index;
     }
 
     /**
@@ -170,13 +165,6 @@ public int getSource() {
         return mSource;
     }
 
-    /**
-     * @return The provider index of this item.  Used for metrics tracking.
-     */
-    public int getProviderIndex() {
-        return mProviderIndex;
-    }
-
     @Override
     public void onCreateContextMenu(ContextMenu menu, View v, ContextMenuInfo menuInfo) {
         mManager.onCreateContextMenu(menu, this);

chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPage.java

@@ -503,15 +503,13 @@ public void onLoadingComplete(MostVisitedItem[] items) {
 
             int tileTypes[] = new int[items.length];
             int sources[] = new int[items.length];
-            int providerIndices[] = new int[items.length];
 
             for (int i = 0; i < items.length; i++) {
                 tileTypes[i] = items[i].getTileType();
                 sources[i] = items[i].getSource();
-                providerIndices[i] = items[i].getProviderIndex();
             }
 
-            mMostVisitedSites.recordTileTypeMetrics(tileTypes, sources, providerIndices);
+            mMostVisitedSites.recordTileTypeMetrics(tileTypes, sources);
 
             if (isNtpOfflinePagesEnabled()) {
                 final int maxNumTiles = 12;

chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPageView.java

@@ -816,7 +816,7 @@ public void onLayoutChange(View v, int left, int top, int right, int bottom,
 
     @Override
     public void onMostVisitedURLsAvailable(final String[] titles, final String[] urls,
-            final String[] whitelistIconPaths, final int[] sources, final int[] providerIndexes) {
+            final String[] whitelistIconPaths, final int[] sources) {
         Set<String> urlSet = new HashSet<>(Arrays.asList(urls));
 
         // TODO(https://crbug.com/607573): We should show offline-available content in a nonblocking
@@ -825,15 +825,13 @@ public void onMostVisitedURLsAvailable(final String[] titles, final String[] url
         mManager.getUrlsAvailableOffline(urlSet, new Callback<Set<String>>() {
             @Override
             public void onResult(Set<String> offlineUrls) {
-                onOfflineUrlsAvailable(
-                        titles, urls, whitelistIconPaths, offlineUrls, sources, providerIndexes);
+                onOfflineUrlsAvailable(titles, urls, whitelistIconPaths, offlineUrls, sources);
             }
         });
     }
 
     private void onOfflineUrlsAvailable(final String[] titles, final String[] urls,
-            final String[] whitelistIconPaths, final Set<String> offlineUrls, final int[] sources,
-            final int[] providerIndexes) {
+            final String[] whitelistIconPaths, final Set<String> offlineUrls, final int[] sources) {
         mMostVisitedLayout.removeAllViews();
 
         MostVisitedItem[] oldItems = mMostVisitedItems;
@@ -849,7 +847,6 @@ private void onOfflineUrlsAvailable(final String[] titles, final String[] urls,
             final String title = titles[i];
             final String whitelistIconPath = whitelistIconPaths[i];
             final int source = sources[i];
-            final int providerIndex = providerIndexes[i];
 
             boolean offlineAvailable = offlineUrls.contains(url);
 
@@ -871,7 +868,7 @@ private void onOfflineUrlsAvailable(final String[] titles, final String[] urls,
             // If nothing can be reused, create a new item.
             if (item == null) {
                 item = new MostVisitedItem(mManager, title, url, whitelistIconPath,
-                        offlineAvailable, i, source, providerIndex);
+                        offlineAvailable, i, source);
                 View view =
                         mMostVisitedDesign.createMostVisitedItemView(inflater, item, isInitialLoad);
                 item.initView(view);

chrome/android/java/src/org/chromium/chrome/browser/profiles/MostVisitedSites.java

@@ -28,7 +28,7 @@ public interface MostVisitedURLsObserver {
          */
         @CalledByNative("MostVisitedURLsObserver")
         public void onMostVisitedURLsAvailable(String[] titles, String[] urls,
-                String[] whitelistIconPaths, int[] sources, int[] providerIndexes);
+                String[] whitelistIconPaths, int[] sources);
 
         /**
          * This is called when the list of popular URLs is initially available or updated.
@@ -73,11 +73,10 @@ public void setMostVisitedURLsObserver(final MostVisitedURLsObserver observer, i
         MostVisitedURLsObserver wrappedObserver = new MostVisitedURLsObserver() {
             @Override
             public void onMostVisitedURLsAvailable(String[] titles, String[] urls,
-                    String[] whitelistIconPaths, int[] sources, int[] providerIndexes) {
+                    String[] whitelistIconPaths, int[] sources) {
                 // Don't notify observer if we've already been destroyed.
                 if (mNativeMostVisitedSitesBridge != 0) {
-                    observer.onMostVisitedURLsAvailable(
-                            titles, urls, whitelistIconPaths, sources, providerIndexes);
+                    observer.onMostVisitedURLsAvailable(titles, urls, whitelistIconPaths, sources);
                 }
             }
             @Override
@@ -112,9 +111,8 @@ public void removeBlacklistedUrl(String url) {
      * @param tileTypes An array of values from MostVisitedTileType indicating the type of each
      *                  tile that's currently showing.
      */
-    public void recordTileTypeMetrics(int[] tileTypes, int[] sources, int[] providerIndices) {
-        nativeRecordTileTypeMetrics(
-                mNativeMostVisitedSitesBridge, tileTypes, sources, providerIndices);
+    public void recordTileTypeMetrics(int[] tileTypes, int[] sources) {
+        nativeRecordTileTypeMetrics(mNativeMostVisitedSitesBridge, tileTypes, sources);
     }
 
     /**
@@ -134,7 +132,7 @@ private native void nativeAddOrRemoveBlacklistedUrl(
             long nativeMostVisitedSitesBridge, String url,
             boolean addUrl);
     private native void nativeRecordTileTypeMetrics(long nativeMostVisitedSitesBridge,
-            int[] tileTypes, int[] sources, int[] providerIndices);
+            int[] tileTypes, int[] sources);
     private native void nativeRecordOpenedMostVisitedItem(
             long nativeMostVisitedSitesBridge, int index, int tileType);
 

chrome/android/javatests/src/org/chromium/chrome/browser/ntp/FakeMostVisitedSites.java

@@ -20,7 +20,6 @@ public class FakeMostVisitedSites extends MostVisitedSites {
     private final String[] mMostVisitedUrls;
     private final String[] mMostVisitedWhitelistIconPaths;
     private final int[] mMostVisitedSources;
-    private final int[] mMostVisitedProviderIndexes;
 
     private final List<String> mBlacklistedUrls = new ArrayList<String>();
 
@@ -30,15 +29,13 @@ public class FakeMostVisitedSites extends MostVisitedSites {
      * @param mostVisitedUrls The URLs of the fixed list of most visited sites.
      */
     public FakeMostVisitedSites(Profile p, String[] mostVisitedTitles, String[] mostVisitedUrls,
-            String[] mostVisitedWhitelistIconPaths, int[] mostVisitedSources,
-            int[] mostVisitedProviderIndexes) {
+            String[] mostVisitedWhitelistIconPaths, int[] mostVisitedSources) {
         super(p);
         assert mostVisitedTitles.length == mostVisitedUrls.length;
         mMostVisitedTitles = mostVisitedTitles.clone();
         mMostVisitedUrls = mostVisitedUrls.clone();
         mMostVisitedWhitelistIconPaths = mostVisitedWhitelistIconPaths.clone();
         mMostVisitedSources = mostVisitedSources.clone();
-        mMostVisitedProviderIndexes = mostVisitedProviderIndexes.clone();
     }
 
     @Override
@@ -48,7 +45,7 @@ public void setMostVisitedURLsObserver(final MostVisitedURLsObserver observer, i
             public void run() {
                 observer.onMostVisitedURLsAvailable(mMostVisitedTitles.clone(),
                         mMostVisitedUrls.clone(), mMostVisitedWhitelistIconPaths.clone(),
-                        mMostVisitedSources.clone(), mMostVisitedProviderIndexes.clone());
+                        mMostVisitedSources.clone());
             }
         });
     }
@@ -71,7 +68,7 @@ public boolean isUrlBlacklisted(String url) {
     }
 
     @Override
-    public void recordTileTypeMetrics(int[] tileTypes, int[] sources, int[] providerIndices) {
+    public void recordTileTypeMetrics(int[] tileTypes, int[] sources) {
         // Metrics are stubbed out.
     }
 

chrome/android/javatests/src/org/chromium/chrome/browser/ntp/NewTabPageTest.java

@@ -51,7 +51,6 @@ public class NewTabPageTest extends ChromeTabbedActivityTestBase {
     private static final String[] FAKE_MOST_VISITED_TITLES = new String[] { "Simple" };
     private static final String[] FAKE_MOST_VISITED_WHITELIST_ICON_PATHS = new String[] { "" };
     private static final int[] FAKE_MOST_VISITED_SOURCES = new int[] {MostVisitedSource.TOP_SITES};
-    private static final int[] FAKE_MOST_VISITED_PROVIDER_INDEXES = new int[] {0};
 
     private Tab mTab;
     private NewTabPage mNtp;
@@ -89,7 +88,7 @@ public void run() {
                     mFakeMostVisitedSites =
                             new FakeMostVisitedSites(mTab.getProfile(), FAKE_MOST_VISITED_TITLES,
                                     mFakeMostVisitedUrls, FAKE_MOST_VISITED_WHITELIST_ICON_PATHS,
-                                    FAKE_MOST_VISITED_SOURCES, FAKE_MOST_VISITED_PROVIDER_INDEXES);
+                                    FAKE_MOST_VISITED_SOURCES);
                 }
             });
         } catch (Throwable t) {

chrome/browser/android/ntp/most_visited_sites_bridge.cc

@@ -120,26 +120,22 @@ void MostVisitedSitesBridge::JavaObserver::OnMostVisitedURLsAvailable(
   std::vector<std::string> urls;
   std::vector<std::string> whitelist_icon_paths;
   std::vector<int> sources;
-  std::vector<int> provider_indexes;
 
   titles.reserve(suggestions.size());
   urls.reserve(suggestions.size());
   whitelist_icon_paths.reserve(suggestions.size());
   sources.reserve(suggestions.size());
-  provider_indexes.reserve(suggestions.size());
   for (const auto& suggestion : suggestions) {
     titles.emplace_back(suggestion.title);
     urls.emplace_back(suggestion.url.spec());
     whitelist_icon_paths.emplace_back(suggestion.whitelist_icon_path.value());
     sources.emplace_back(suggestion.source);
-    provider_indexes.emplace_back(suggestion.provider_index);
   }
   Java_MostVisitedURLsObserver_onMostVisitedURLsAvailable(
       env, observer_.obj(), ToJavaArrayOfStrings(env, titles).obj(),
       ToJavaArrayOfStrings(env, urls).obj(),
       ToJavaArrayOfStrings(env, whitelist_icon_paths).obj(),
-      ToJavaIntArray(env, sources).obj(),
-      ToJavaIntArray(env, provider_indexes).obj());
+      ToJavaIntArray(env, sources).obj());
 }
 
 void MostVisitedSitesBridge::JavaObserver::OnPopularURLsAvailable(
@@ -204,18 +200,14 @@ void MostVisitedSitesBridge::RecordTileTypeMetrics(
     JNIEnv* env,
     const JavaParamRef<jobject>& obj,
     const JavaParamRef<jintArray>& jtile_types,
-    const JavaParamRef<jintArray>& jsources,
-    const JavaParamRef<jintArray>& jprovider_indices) {
+    const JavaParamRef<jintArray>& jsources) {
   std::vector<int> tile_types;
   std::vector<int> sources;
-  std::vector<int> provider_indices;
 
   base::android::JavaIntArrayToIntVector(env, jtile_types, &tile_types);
   base::android::JavaIntArrayToIntVector(env, jsources, &sources);
-  base::android::JavaIntArrayToIntVector(env, jprovider_indices,
-                                         &provider_indices);
 
-  most_visited_.RecordTileTypeMetrics(tile_types, sources, provider_indices);
+  most_visited_.RecordTileTypeMetrics(tile_types, sources);
 }
 
 void MostVisitedSitesBridge::RecordOpenedMostVisitedItem(

chrome/browser/android/ntp/most_visited_sites_bridge.h

@@ -42,8 +42,7 @@ class MostVisitedSitesBridge {
       JNIEnv* env,
       const base::android::JavaParamRef<jobject>& obj,
       const base::android::JavaParamRef<jintArray>& jtile_types,
-      const base::android::JavaParamRef<jintArray>& jsources,
-      const base::android::JavaParamRef<jintArray>& jprovider_indices);
+      const base::android::JavaParamRef<jintArray>& jsources);
   void RecordOpenedMostVisitedItem(
       JNIEnv* env,
       const base::android::JavaParamRef<jobject>& obj,

components/ntp_tiles/most_visited_sites.cc

@@ -25,7 +25,6 @@
 #include "components/ntp_tiles/switches.h"
 #include "components/pref_registry/pref_registry_syncable.h"
 #include "components/prefs/pref_service.h"
-#include "ui/gfx/codec/jpeg_codec.h"
 #include "url/gurl.h"
 
 using history::TopSites;
@@ -40,7 +39,6 @@ namespace {
 // 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 kHistogramWhitelistName[] = "whitelist";
 
@@ -102,7 +100,7 @@ bool AreURLsEquivalent(const GURL& url1, const GURL& url2) {
   return url1.host() == url2.host() && url1.path() == url2.path();
 }
 
-std::string GetSourceHistogramName(int source, int provider_index) {
+std::string GetSourceHistogramName(int source) {
   switch (source) {
     case MostVisitedSites::TOP_SITES:
       return kHistogramClientName;
@@ -111,19 +109,12 @@ std::string GetSourceHistogramName(int source, int provider_index) {
     case MostVisitedSites::WHITELIST:
       return kHistogramWhitelistName;
     case MostVisitedSites::SUGGESTIONS_SERVICE:
-      return provider_index >= 0
-                 ? base::StringPrintf(kHistogramServerFormat, provider_index)
-                 : kHistogramServerName;
+      return kHistogramServerName;
   }
   NOTREACHED();
   return std::string();
 }
 
-std::string GetSourceHistogramNameFromSuggestion(
-    const MostVisitedSites::Suggestion& suggestion) {
-  return GetSourceHistogramName(suggestion.source, suggestion.provider_index);
-}
-
 void AppendSuggestions(MostVisitedSites::SuggestionsVector src,
                        MostVisitedSites::SuggestionsVector* dst) {
   dst->insert(dst->end(),
@@ -133,7 +124,7 @@ void AppendSuggestions(MostVisitedSites::SuggestionsVector src,
 
 }  // namespace
 
-MostVisitedSites::Suggestion::Suggestion() : provider_index(-1) {}
+MostVisitedSites::Suggestion::Suggestion() {}
 
 MostVisitedSites::Suggestion::~Suggestion() {}
 
@@ -237,15 +228,14 @@ void MostVisitedSites::AddOrRemoveBlacklistedUrl(const GURL& url,
 
 void MostVisitedSites::RecordTileTypeMetrics(
     const std::vector<int>& tile_types,
-    const std::vector<int>& sources,
-    const std::vector<int>& provider_indices) {
+    const std::vector<int>& sources) {
   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",
-        GetSourceHistogramName(sources[i], provider_indices[i]).c_str());
+        GetSourceHistogramName(sources[i]).c_str());
     LogHistogramEvent(histogram, tile_type, NUM_TILE_TYPES);
   }
 
@@ -265,14 +255,12 @@ void MostVisitedSites::RecordOpenedMostVisitedItem(int index, int tile_type) {
   DCHECK_LT(index, static_cast<int>(current_suggestions_.size()));
   std::string histogram = base::StringPrintf(
       "NewTabPage.MostVisited.%s",
-      GetSourceHistogramNameFromSuggestion(current_suggestions_[index])
-          .c_str());
+      GetSourceHistogramName(current_suggestions_[index].source).c_str());
   LogHistogramEvent(histogram, index, num_sites_);
 
   histogram = base::StringPrintf(
       "NewTabPage.TileTypeClicked.%s",
-      GetSourceHistogramNameFromSuggestion(current_suggestions_[index])
-          .c_str());
+      GetSourceHistogramName(current_suggestions_[index].source).c_str());
   LogHistogramEvent(histogram, tile_type, NUM_TILE_TYPES);
 }
 
@@ -369,8 +357,6 @@ void MostVisitedSites::OnSuggestionsProfileAvailable(
     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));
   }
@@ -547,7 +533,7 @@ void MostVisitedSites::RecordImpressionUMAMetrics() {
   for (size_t i = 0; i < current_suggestions_.size(); i++) {
     std::string histogram = base::StringPrintf(
         "NewTabPage.SuggestionsImpression.%s",
-        GetSourceHistogramNameFromSuggestion(current_suggestions_[i]).c_str());
+        GetSourceHistogramName(current_suggestions_[i].source).c_str());
     LogHistogramEvent(histogram, static_cast<int>(i), num_sites_);
   }
 }

components/ntp_tiles/most_visited_sites.h

@@ -143,9 +143,6 @@ class MostVisitedSites : public history::TopSitesObserver,
     // Only valid for source == WHITELIST (empty otherwise).
     base::FilePath whitelist_icon_path;
 
-    // Only valid for source == SUGGESTIONS_SERVICE (-1 otherwise).
-    int provider_index;
-
     Suggestion();
     ~Suggestion();
 
@@ -174,8 +171,7 @@ class MostVisitedSites : public history::TopSitesObserver,
 
   void AddOrRemoveBlacklistedUrl(const GURL& url, bool add_url);
   void RecordTileTypeMetrics(const std::vector<int>& tile_types,
-                             const std::vector<int>& sources,
-                             const std::vector<int>& provider_indices);
+                             const std::vector<int>& sources);
   void RecordOpenedMostVisitedItem(int index, int tile_type);
 
   // MostVisitedSitesSupervisor::Observer implementation.