Adds metric to help assess last_n impact on tab restores.

chrome/android/java/src/org/chromium/chrome/browser/offlinepages/OfflinePageUtils.java

Index: chrome/android/java/src/org/chromium/chrome/browser/offlinepages/OfflinePageUtils.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/offlinepages/OfflinePageUtils.java b/chrome/android/java/src/org/chromium/chrome/browser/offlinepages/OfflinePageUtils.java
index e83917082eb0d11a12117af06ff288aab28298e1..584dbdcd991f78f27a78b2483c53e852c875ff5c 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/offlinepages/OfflinePageUtils.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/offlinepages/OfflinePageUtils.java
@@ -36,6 +36,7 @@ import org.chromium.components.offlinepages.SavePageResult;
 import org.chromium.content_public.browser.LoadUrlParams;
 import org.chromium.content_public.browser.WebContents;
 import org.chromium.net.ConnectionType;
+import org.chromium.net.NetError;
 import org.chromium.net.NetworkChangeNotifier;
 import org.chromium.ui.base.PageTransition;
 
@@ -78,6 +79,25 @@ public class OfflinePageUtils {
      */
     private static Map<ChromeActivity, RecentTabTracker> sTabModelObservers = new HashMap<>();
 
+    /**
+     * Contains constant values from the histogram enum OfflinePagesTabRestoreType to be used for
+     * reporting to the OfflinePages.TabRestore metric.
+     */
+    private static class TabRestoreType {
+        public static final int WHILE_ONLINE = 0;
+        public static final int WHILE_ONLINE_COULD_BE_SAVED_OFFLINE = 1;
+        public static final int WHILE_ONLINE_TO_OFFLINE_PAGE = 2;
+        public static final int WHILE_ONLINE_TO_OFFLINE_PAGE_FROM_LAST_N = 3;
+        public static final int WHILE_OFFLINE = 4;
+        public static final int WHILE_OFFLINE_COULD_BE_SAVED_OFFLINE = 5;
+        public static final int WHILE_OFFLINE_TO_OFFLINE_PAGE = 6;
+        public static final int WHILE_OFFLINE_TO_OFFLINE_PAGE_FROM_LAST_N = 7;
+        public static final int FAILED = 8;
+        public static final int FAILED_TO_DINO_PAGE = 9;
+        public static final int CRASHED = 10;

[fgorski, 2017/03/17 03:42:51]

Please add this:
  // NOTE: always keep this entry at the end. Add new result types only
  // immediately above this line. Make sure to update the corresponding
  // histogram enum accordingly.

[carlosk, 2017/03/17 20:37:22]

Done.

+        public static final int COUNT = 11;
+    }
+
     private static OfflinePageUtils getInstance() {
         if (sInstance == null) {
             sInstance = new OfflinePageUtils();
@@ -702,6 +722,57 @@ public class OfflinePageUtils {
         }
     }
 
+    /**
+     * Reports that a tab finished reloading the contents of a tab that was restored.
+     * @param tab the tab that finished being restored.
+     * @param validatedUrl the valid URL that the tab was restored to.
+     */
+    public static void finishedRestorePageLoad(Tab tab, String validatedUrl) {
+        // Note: we're using the knowledge of all OfflinePagesTabRestoreType values to translate the
+        // type from online to the offline values. This makes this code below much simpler.
+        int tabRestoreType;
+        OfflinePageItem page = tab.getOfflinePage();
+        if (page != null) {
+            if (page.getClientId().getNamespace().equals(OfflinePageBridge.LAST_N_NAMESPACE)) {
+                tabRestoreType = TabRestoreType.WHILE_ONLINE_TO_OFFLINE_PAGE_FROM_LAST_N; // == 3
+            } else {
+                tabRestoreType = TabRestoreType.WHILE_ONLINE_TO_OFFLINE_PAGE; // == 2
+            }
+        } else if (OfflinePageBridge.canSavePage(validatedUrl) && !tab.isIncognito()) {
+            tabRestoreType = TabRestoreType.WHILE_ONLINE_COULD_BE_SAVED_OFFLINE; // == 1

[Dmitry Titov, 2017/03/17 01:39:51]

What does this measure? Not clear.

[carlosk, 2017/03/17 20:37:22]

This allows us put in a separate bucket tab restores that succeeded but were to
non offline save-able pages (anything non HTTP/HTTPS or in an incognito tab).
The more important/common case is the NTP in the offline case. I added further
explanations to the histogram enum entries on why these cases are tracked
separately.

I also renamed WHILE_O(N|FF)LINE_CANT_BE_SAVED_OFFLINE and repurpose it to catch
the opposite case so that WHILE_O(N|FF)LINE would track what we actually care
about. This seems easier to read and interpret.

WDYT?

+        } else {
+            tabRestoreType = TabRestoreType.WHILE_ONLINE; // == 0
+        }
+        // The translation to online values happens here if needed.
+        if (!isConnected()) tabRestoreType += TabRestoreType.WHILE_OFFLINE; // == 4

[Dmitry Titov, 2017/03/17 01:39:51]

I'd have this more straightforward, w/o doing math on enum values. It is harder
to read and error-prone in the future...

The alternative to a long chain of 'ifs' is to have a table, like 
| is online | has_offline_page | is last_n | UMA_enum_value

[carlosk, 2017/03/17 20:37:22]

Done.

I had considered a lookup table before too but preferred this one because it was
shorter. I agree readability is better now.

+
+        recordTabRestoreHistogram(tabRestoreType, validatedUrl);
+    }
+
+    /**
+     * Reports that a tab failed reloading the contents of a tab that was being restored.
+     * @param errorCode The error code causing the page to fail reloading.
+     */
+    public static void failedRestorePageLoad(int errorCode) {
+        int tabRestoreType = errorCode == NetError.ERR_INTERNET_DISCONNECTED
+                ? TabRestoreType.FAILED_TO_DINO_PAGE
+                : TabRestoreType.FAILED;
+        recordTabRestoreHistogram(tabRestoreType, null);
+    }
+
+    /**
+     * Reports that a tab crashed while it was being restored.
+     */
+    public static void crashedWhileRestoringPageLoad() {
+        recordTabRestoreHistogram(TabRestoreType.CRASHED, null);
+    }
+
+    private static void recordTabRestoreHistogram(int tabRestoreType, String url) {
+        Log.d(TAG, "Concluded tab restore: type=" + tabRestoreType + ", url=" + url);
+        RecordHistogram.recordEnumeratedHistogram(
+                "OfflinePages.TabRestore", tabRestoreType, TabRestoreType.COUNT);
+    }
+
     @VisibleForTesting
     static void setInstanceForTesting(OfflinePageUtils instance) {
         sInstance = instance;