Bug 1284017 - Add telemetry for damaged session store files. r=sebastian, a=sylvestre
authorJan Henning <jh+bugzilla@buttercookie.de>
Tue, 05 Jul 2016 22:40:01 +0200
changeset 342036 8fd80dfc8f7df45c301780c94650e66c06ef077e
parent 342035 99b62d79d2e4f8e8e5cb606891c1df80f0771bd1
child 342037 2f9a3a609582d45b1adee9150ff86d3e3711f759
push id1183
push userraliiev@mozilla.com
push dateMon, 05 Sep 2016 20:01:49 +0000
treeherdermozilla-release@3148731bed45 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssebastian, sylvestre
bugs1284017, 1228593, 1275662, 1261008
milestone49.0a2
Bug 1284017 - Add telemetry for damaged session store files. r=sebastian, a=sylvestre Just watching for a SessionRestoreException during startup can introduce some false positives, because that exception is triggered in any case where we can't restore tabs, not just when the session file has been damaged, e.g.: - on first startup - on builds affected by bug 1228593, users who are (theoretically) restoring their tabs, but clearing their history on exist end up with a deleted sessionstore.js - should we implement bug 1275662, we'd hit that exception in that case, too. Therefore we only send the telemetry event if we hit that exception even though a sessionstore.js file is present. We also exclude the case where the file size of sessionstore.js is 14 bytes, because that is most likely corresponding to a file containing only {"windows":[]}, which means that the session store intentionally wanted to write a file containing no tabs. Currently this is only the case for users who are clearing their history on exit and are also *not* restoring tabs, however if bug 1275662 should get implemented, we'd probably encounter those empty files for users who have their restore setting set to "Always restore", too. Because of bug 1261008, we can also end up with no restored tabs (and a SessionRestoreException) if the session file contains only about:home tabs with no history, because we're skipping those and not restoring them. To detect that case and exclude it from telemetry, we have to include additional logic within the SessionParser instance used during startup and pass those results back to the calling site in GeckoApp. MozReview-Commit-ID: 6pAhDU3d8QA
mobile/android/base/java/org/mozilla/gecko/GeckoApp.java
mobile/android/base/java/org/mozilla/gecko/GeckoProfile.java
toolkit/components/telemetry/Histograms.json
--- a/mobile/android/base/java/org/mozilla/gecko/GeckoApp.java
+++ b/mobile/android/base/java/org/mozilla/gecko/GeckoApp.java
@@ -195,16 +195,97 @@ public abstract class GeckoApp
     private View mFullScreenPluginView;
 
     private final HashMap<String, PowerManager.WakeLock> mWakeLocks = new HashMap<String, PowerManager.WakeLock>();
 
     protected boolean mLastSessionCrashed;
     protected boolean mShouldRestore;
     private boolean mSessionRestoreParsingFinished = false;
 
+    private static final class LastSessionParser extends SessionParser {
+        private JSONArray tabs;
+        private JSONObject windowObject;
+        private boolean isExternalURL;
+
+        private boolean selectNextTab;
+        private boolean tabsWereSkipped;
+        private boolean tabsWereProcessed;
+
+        public LastSessionParser(JSONArray tabs, JSONObject windowObject, boolean isExternalURL) {
+            this.tabs = tabs;
+            this.windowObject = windowObject;
+            this.isExternalURL = isExternalURL;
+        }
+
+        public boolean allTabsSkipped() {
+            return tabsWereSkipped && !tabsWereProcessed;
+        }
+
+        @Override
+        public void onTabRead(final SessionTab sessionTab) {
+            if (sessionTab.isAboutHomeWithoutHistory()) {
+                // This is a tab pointing to about:home with no history. We won't restore
+                // this tab. If we end up restoring no tabs then the browser will decide
+                // whether it needs to open about:home or a different 'homepage'. If we'd
+                // always restore about:home only tabs then we'd never open the homepage.
+                // See bug 1261008.
+
+                if (sessionTab.isSelected()) {
+                    // Unfortunately this tab is the selected tab. Let's just try to select
+                    // the first tab. If we haven't restored any tabs so far then remember
+                    // to select the next tab that gets restored.
+
+                    if (!Tabs.getInstance().selectLastTab()) {
+                        selectNextTab = true;
+                    }
+                }
+
+                // Do not restore this tab.
+                tabsWereSkipped = true;
+                return;
+            }
+
+            tabsWereProcessed = true;
+
+            JSONObject tabObject = sessionTab.getTabObject();
+
+            int flags = Tabs.LOADURL_NEW_TAB;
+            flags |= ((isExternalURL || !sessionTab.isSelected()) ? Tabs.LOADURL_DELAY_LOAD : 0);
+            flags |= (tabObject.optBoolean("desktopMode") ? Tabs.LOADURL_DESKTOP : 0);
+            flags |= (tabObject.optBoolean("isPrivate") ? Tabs.LOADURL_PRIVATE : 0);
+
+            final Tab tab = Tabs.getInstance().loadUrl(sessionTab.getUrl(), flags);
+
+            if (selectNextTab) {
+                // We did not restore the selected tab previously. Now let's select this tab.
+                Tabs.getInstance().selectTab(tab.getId());
+                selectNextTab = false;
+            }
+
+            ThreadUtils.postToUiThread(new Runnable() {
+                @Override
+                public void run() {
+                    tab.updateTitle(sessionTab.getTitle());
+                }
+            });
+
+            try {
+                tabObject.put("tabId", tab.getId());
+            } catch (JSONException e) {
+                Log.e(LOGTAG, "JSON error", e);
+            }
+            tabs.put(tabObject);
+        }
+
+        @Override
+        public void onClosedTabsRead(final JSONArray closedTabData) throws JSONException {
+            windowObject.put("closedTabs", closedTabData);
+        }
+    };
+
     protected boolean mInitialized;
     protected boolean mWindowFocusInitialized;
     private Telemetry.Timer mJavaUiStartupTimer;
     private Telemetry.Timer mGeckoReadyStartupTimer;
 
     private String mPrivateBrowsingSession;
 
     private volatile HealthRecorder mHealthRecorder;
@@ -1290,16 +1371,27 @@ public abstract class GeckoApp
                         // of the tab stubs into the JSON data (which holds the session
                         // history). This JSON data is then sent to Gecko so session
                         // history can be restored for each tab.
                         final SafeIntent intent = new SafeIntent(getIntent());
                         restoreMessage = restoreSessionTabs(invokedWithExternalURL(getIntentURI(intent)));
                     } catch (SessionRestoreException e) {
                         // If restore failed, do a normal startup
                         Log.e(LOGTAG, "An error occurred during restore", e);
+                        // If mShouldRestore was already set to false in restoreSessionTabs(),
+                        // this means that we intentionally skipped all tabs read from the
+                        // session file, so we don't have to report this exception in telemetry
+                        // and can ignore the following bit.
+                        if (mShouldRestore && getProfile().sessionFileExistsAndNotEmptyWindow()) {
+                            // If we got a SessionRestoreException even though the file exists and its
+                            // length doesn't match the known length of an intentionally empty file,
+                            // it's very likely we've encountered a damaged/corrupt session store file.
+                            Log.d(LOGTAG, "Suspecting a damaged session store file.");
+                            Telemetry.addToHistogram("FENNEC_SESSIONSTORE_DAMAGED_SESSION_FILE", 1);
+                        }
                         mShouldRestore = false;
                     }
                 }
 
                 synchronized (GeckoApp.this) {
                     mSessionRestoreParsingFinished = true;
                     GeckoApp.this.notifyAll();
                 }
@@ -1686,88 +1778,35 @@ public abstract class GeckoApp
             }
 
             // If we are doing an OOM restore, parse the session data and
             // stub the restored tabs immediately. This allows the UI to be
             // updated before Gecko has restored.
             if (mShouldRestore) {
                 final JSONArray tabs = new JSONArray();
                 final JSONObject windowObject = new JSONObject();
-                SessionParser parser = new SessionParser() {
-                    private boolean selectNextTab;
-
-                    @Override
-                    public void onTabRead(final SessionTab sessionTab) {
-                        if (sessionTab.isAboutHomeWithoutHistory()) {
-                            // This is a tab pointing to about:home with no history. We won't restore
-                            // this tab. If we end up restoring no tabs then the browser will decide
-                            // whether it needs to open about:home or a different 'homepage'. If we'd
-                            // always restore about:home only tabs then we'd never open the homepage.
-                            // See bug 1261008.
-
-                            if (sessionTab.isSelected()) {
-                                // Unfortunately this tab is the selected tab. Let's just try to select
-                                // the first tab. If we haven't restored any tabs so far then remember
-                                // to select the next tab that gets restored.
-
-                                if (!Tabs.getInstance().selectLastTab()) {
-                                    selectNextTab = true;
-                                }
-                            }
-
-                            // Do not restore this tab.
-                            return;
-                        }
-
-                        JSONObject tabObject = sessionTab.getTabObject();
-
-                        int flags = Tabs.LOADURL_NEW_TAB;
-                        flags |= ((isExternalURL || !sessionTab.isSelected()) ? Tabs.LOADURL_DELAY_LOAD : 0);
-                        flags |= (tabObject.optBoolean("desktopMode") ? Tabs.LOADURL_DESKTOP : 0);
-                        flags |= (tabObject.optBoolean("isPrivate") ? Tabs.LOADURL_PRIVATE : 0);
-
-                        final Tab tab = Tabs.getInstance().loadUrl(sessionTab.getUrl(), flags);
-
-                        if (selectNextTab) {
-                            // We did not restore the selected tab previously. Now let's select this tab.
-                            Tabs.getInstance().selectTab(tab.getId());
-                            selectNextTab = false;
-                        }
-
-                        ThreadUtils.postToUiThread(new Runnable() {
-                            @Override
-                            public void run() {
-                                tab.updateTitle(sessionTab.getTitle());
-                            }
-                        });
-
-                        try {
-                            tabObject.put("tabId", tab.getId());
-                        } catch (JSONException e) {
-                            Log.e(LOGTAG, "JSON error", e);
-                        }
-                        tabs.put(tabObject);
-                    }
-
-                    @Override
-                    public void onClosedTabsRead(final JSONArray closedTabData) throws JSONException {
-                        windowObject.put("closedTabs", closedTabData);
-                    }
-                };
+
+                LastSessionParser parser = new LastSessionParser(tabs, windowObject, isExternalURL);
 
                 if (mPrivateBrowsingSession == null) {
                     parser.parse(sessionString);
                 } else {
                     parser.parse(sessionString, mPrivateBrowsingSession);
                 }
 
                 if (tabs.length() > 0) {
                     windowObject.put("tabs", tabs);
                     sessionString = new JSONObject().put("windows", new JSONArray().put(windowObject)).toString();
                 } else {
+                    if (parser.allTabsSkipped()) {
+                        // If we intentionally skipped all tabs we've read from the session file, we
+                        // set mShouldRestore back to false at this point already, so the calling code
+                        // can infer that the exception wasn't due to a damaged session store file.
+                        mShouldRestore = false;
+                    }
                     throw new SessionRestoreException("No tabs could be read from session file");
                 }
             }
 
             JSONObject restoreData = new JSONObject();
             restoreData.put("sessionString", sessionString);
             return restoreData.toString();
         } catch (JSONException e) {
--- a/mobile/android/base/java/org/mozilla/gecko/GeckoProfile.java
+++ b/mobile/android/base/java/org/mozilla/gecko/GeckoProfile.java
@@ -71,16 +71,17 @@ public final class GeckoProfile {
     // Profile is using a custom directory outside of the Mozilla directory.
     public static final String CUSTOM_PROFILE = "";
     public static final String GUEST_PROFILE_DIR = "guest";
 
     // Session store
     private static final String SESSION_FILE = "sessionstore.js";
     private static final String SESSION_FILE_BACKUP = "sessionstore.bak";
     private static final long MAX_BACKUP_FILE_AGE = 1000 * 3600 * 24; // 24 hours
+    private static final int SESSION_STORE_EMPTY_JSON_LENGTH = 14; // length of {"windows":[]}
 
     private boolean mOldSessionDataProcessed = false;
 
     private static final ConcurrentHashMap<String, GeckoProfile> sProfileCache =
             new ConcurrentHashMap<String, GeckoProfile>(
                     /* capacity */ 4, /* load factor */ 0.75f, /* concurrency */ 2);
     private static String sDefaultProfileName;
 
@@ -649,16 +650,29 @@ public final class GeckoProfile {
             }
         } catch (IOException ioe) {
             Log.e(LOGTAG, "Unable to read session file", ioe);
         }
         return null;
     }
 
     /**
+     * Checks whether the session store file exists and that its length
+     * doesn't match the known length of a session store file containing
+     * only an empty window.
+     */
+    public boolean sessionFileExistsAndNotEmptyWindow() {
+        File sessionFile = getFile(SESSION_FILE);
+
+        return sessionFile != null &&
+               sessionFile.exists() &&
+               sessionFile.length() != SESSION_STORE_EMPTY_JSON_LENGTH;
+    }
+
+    /**
      * Ensures the parent director(y|ies) of the given filename exist by making them
      * if they don't already exist..
      *
      * @param filename The path to the file whose parents should be made directories
      * @return true if the parent directory exists, false otherwise
      */
     @WorkerThread
     protected boolean ensureParentDirs(final String filename) {
--- a/toolkit/components/telemetry/Histograms.json
+++ b/toolkit/components/telemetry/Histograms.json
@@ -10907,10 +10907,18 @@
   },
   "ABOUTCRASHES_OPENED_COUNT": {
     "alert_emails": ["bgirard@mozilla.com"],
     "expires_in_version": "55",
     "kind": "count",
     "bug_numbers": [1276714, 1276716],
     "description": "Number of times about:crashes has been opened.",
     "releaseChannelCollection": "opt-out"
+  },
+  "FENNEC_SESSIONSTORE_DAMAGED_SESSION_FILE": {
+    "alert_emails": ["jh+bugzilla@buttercookie.de"],
+    "expires_in_version": "56",
+    "kind": "flag",
+    "bug_numbers": [1284017],
+    "description": "When restoring tabs on startup, reading from sessionstore.js failed, even though the file exists and is not containing an explicitly empty window.",
+    "cpp_guard": "ANDROID"
   }
 }