[ImportantSites] Adding usage metrics.

chrome/android/java/src/org/chromium/chrome/browser/preferences/privacy/ClearBrowsingDataPreferences.java

Index: chrome/android/java/src/org/chromium/chrome/browser/preferences/privacy/ClearBrowsingDataPreferences.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/preferences/privacy/ClearBrowsingDataPreferences.java b/chrome/android/java/src/org/chromium/chrome/browser/preferences/privacy/ClearBrowsingDataPreferences.java
index ae0ad9c391d31a3321fad1a0a5dab00048ca8ebd..41aecc25fe920ca6035a92a113347ac33f302c3a 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/preferences/privacy/ClearBrowsingDataPreferences.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/preferences/privacy/ClearBrowsingDataPreferences.java
@@ -213,6 +213,7 @@ public class ClearBrowsingDataPreferences extends PreferenceFragment
     private ProgressDialog mProgressDialog;
     private Item[] mItems;
 
+    private int mMaxImportantSites;
     // This is the sorted list of important registerable domains. If null, then we haven't finished
     // fetching them yet.
     private String[] mSortedImportantDomains;
@@ -344,11 +345,17 @@ public class ClearBrowsingDataPreferences extends PreferenceFragment
      * </ol>
      */
     private boolean shouldShowImportantSitesDialog() {
-        if (mSortedImportantDomains == null || mSortedImportantDomains.length == 0) return false;
+        if (!ChromeFeatureList.isEnabled(ChromeFeatureList.IMPORTANT_SITES_IN_CBD)) return false;
         EnumSet<DialogOption> selectedOptions = getSelectedOptions();
-        return ChromeFeatureList.isEnabled(ChromeFeatureList.IMPORTANT_SITES_IN_CBD)
-                && (selectedOptions.contains(DialogOption.CLEAR_CACHE)
-                           || selectedOptions.contains(DialogOption.CLEAR_COOKIES_AND_SITE_DATA));
+        if (!selectedOptions.contains(DialogOption.CLEAR_CACHE)
+                && !selectedOptions.contains(DialogOption.CLEAR_COOKIES_AND_SITE_DATA)) {
+            return false;
+        }
+        boolean haveImportantSites =
+                mSortedImportantDomains != null && mSortedImportantDomains.length != 0;
+        RecordHistogram.recordBooleanHistogram(
+                "History.ClearBrowsingData.ImportantSitesPresent", haveImportantSites);

[Mark P, 2016/06/29 20:20:31]

You forgot to add this to histograms.xml.

[dmurph, 2016/06/29 21:56:05]

Oops, it was supposed to be ImportantDialogShown.

+        return haveImportantSites;
     }
 
     @Override
@@ -395,6 +402,9 @@ public class ClearBrowsingDataPreferences extends PreferenceFragment
     @Override
     public void onCreate(Bundle savedInstanceState) {
         super.onCreate(savedInstanceState);
+        RecordHistogram.recordBooleanHistogram(
+                "History.ClearBrowsingData.OnCreate", savedInstanceState == null);

[Mark P, 2016/06/29 20:20:31]

You forgot to add this to histograms.xml.

[dmurph, 2016/06/29 21:56:06]

Made it an action instead.

+        mMaxImportantSites = PrefServiceBridge.getMaxImportantSites();
         PrefServiceBridge.getInstance().requestInfoAboutOtherFormsOfBrowsingHistory(this);
         getActivity().setTitle(R.string.clear_browsing_data_title);
         addPreferencesFromResource(R.xml.clear_browsing_data_preferences);
@@ -565,6 +575,8 @@ public class ClearBrowsingDataPreferences extends PreferenceFragment
 
     @Override
     public void onImportantRegisterableDomainsReady(String[] domains, String[] exampleOrigins) {
+        RecordHistogram.recordCustomCountHistogram("History.ClearBrowsingData.NumImportant",
+                mSortedImportantDomains.length, 0, mMaxImportantSites + 1, mMaxImportantSites + 1);

[Mark P, 2016/06/29 20:20:31]

I do not think this will work if mMaxImportantSites changes in the course of a
single Chrome session.  What I think happens is that during the first call you
end up creating the histogram and allocating the buckets, and then later calls
cannot expand the histogram size.

Can you make mMaxImportantSites a constant?

ditto below

[dmurph, 2016/06/29 21:56:06]

It's actually a constant, we just have the plumbing to only store it in one
place. I'll add some comments about that, is that alright?

[Mark P, 2016/06/30 05:29:09]

Looks fine to me now.

         mSortedImportantDomains = Arrays.copyOf(domains, domains.length);
         mSortedExampleOrigins = Arrays.copyOf(exampleOrigins, exampleOrigins.length);
     }
@@ -579,6 +591,9 @@ public class ClearBrowsingDataPreferences extends PreferenceFragment
             // Deselected means that the user is excluding the domain from being cleared.
             String[] deselectedDomains = data.getStringArrayExtra(
                     ConfirmImportantSitesDialogFragment.DESELECTED_DOMAINS_TAG);
+            RecordHistogram.recordCustomCountHistogram(
+                    "History.ClearBrowsingData.NumImportantDeselected", deselectedDomains.length, 0,
+                    mMaxImportantSites + 1, mMaxImportantSites + 1);
             clearBrowsingData(getSelectedOptions(), deselectedDomains);
         }
     }