Bug 1247782 - Remove GeckoApp.createHealthRecorder & related code. r=rnewman draft
authorMichael Comella <michael.l.comella@gmail.com>
Thu, 11 Feb 2016 15:54:59 -0800
changeset 330593 56074874d411b6c056678c9a9154cc90ca42ab96
parent 330448 306cf0271d3e3a344fcbfd2baf75e0450c288cf1
child 514191 a6d7a5f0e757b456483eda0afd3a9f7d8dbbfad2
push id10775
push usermichael.l.comella@gmail.com
push dateFri, 12 Feb 2016 00:05:15 +0000
reviewersrnewman
bugs1247782, 1183320
milestone47.0a1
Bug 1247782 - Remove GeckoApp.createHealthRecorder & related code. r=rnewman It seems we missed this when we implemented bug 1183320. There was a FENNEC_WAS_KILLED histogram that used the SessionInformation - this changeset maintains this functionality by pulling the access out of SessionInformation and directly into GeckoApp, removing the SessionInformation class. MozReview-Commit-ID: GKTf5lAExFq
mobile/android/base/java/org/mozilla/gecko/GeckoApp.java
mobile/android/base/java/org/mozilla/gecko/health/HealthRecorder.java
mobile/android/base/java/org/mozilla/gecko/health/SessionInformation.java
mobile/android/base/java/org/mozilla/gecko/health/StubbedHealthRecorder.java
mobile/android/base/moz.build
--- a/mobile/android/base/java/org/mozilla/gecko/GeckoApp.java
+++ b/mobile/android/base/java/org/mozilla/gecko/GeckoApp.java
@@ -11,19 +11,16 @@ import org.mozilla.gecko.AppConstants.Ve
 import org.mozilla.gecko.GeckoProfileDirectories.NoMozillaDirectoryException;
 import org.mozilla.gecko.db.BrowserDB;
 import org.mozilla.gecko.favicons.Favicons;
 import org.mozilla.gecko.gfx.BitmapUtils;
 import org.mozilla.gecko.gfx.FullScreenState;
 import org.mozilla.gecko.gfx.Layer;
 import org.mozilla.gecko.gfx.LayerView;
 import org.mozilla.gecko.gfx.PluginLayer;
-import org.mozilla.gecko.health.HealthRecorder;
-import org.mozilla.gecko.health.SessionInformation;
-import org.mozilla.gecko.health.StubbedHealthRecorder;
 import org.mozilla.gecko.menu.GeckoMenu;
 import org.mozilla.gecko.menu.GeckoMenuInflater;
 import org.mozilla.gecko.menu.MenuPanel;
 import org.mozilla.gecko.mozglue.ContextUtils;
 import org.mozilla.gecko.mozglue.ContextUtils.SafeIntent;
 import org.mozilla.gecko.mozglue.GeckoLoader;
 import org.mozilla.gecko.permissions.Permissions;
 import org.mozilla.gecko.preferences.ClearOnShutdownPref;
@@ -205,17 +202,16 @@ public abstract class GeckoApp
     protected boolean mShouldRestore;
     protected boolean mInitialized;
     protected boolean mWindowFocusInitialized;
     private Telemetry.Timer mJavaUiStartupTimer;
     private Telemetry.Timer mGeckoReadyStartupTimer;
 
     private String mPrivateBrowsingSession;
 
-    private volatile HealthRecorder mHealthRecorder;
     private volatile Locale mLastLocale;
 
     private EventListener mWebappEventListener;
 
     private Intent mRestartIntent;
 
     abstract public int getLayout();
 
@@ -691,25 +687,16 @@ public abstract class GeckoApp
     public void handleMessage(String event, JSONObject message) {
         try {
             if (event.equals("Gecko:DelayedStartup")) {
                 ThreadUtils.postToBackgroundThread(new UninstallListener.DelayedStartupTask(this));
             } else if (event.equals("Gecko:Ready")) {
                 mGeckoReadyStartupTimer.stop();
                 geckoConnected();
 
-                // This method is already running on the background thread, so we
-                // know that mHealthRecorder will exist. That doesn't stop us being
-                // paranoid.
-                // This method is cheap, so don't spawn a new runnable.
-                final HealthRecorder rec = mHealthRecorder;
-                if (rec != null) {
-                  rec.recordGeckoStartupTime(mGeckoReadyStartupTimer.getElapsed());
-                }
-
             } else if (event.equals("Gecko:Exited")) {
                 // Gecko thread exited first; let GeckoApp die too.
                 doShutdown();
                 return;
 
             } else if ("NativeApp:IsDebuggable".equals(event)) {
                 JSONObject ret = new JSONObject();
                 ret.put("isDebuggable", getIsDebuggable());
@@ -1357,55 +1344,41 @@ public abstract class GeckoApp
             public void run() {
                 final SharedPreferences prefs = GeckoApp.this.getSharedPreferences();
 
                 // Wait until now to set this, because we'd rather throw an exception than
                 // have a caller of BrowserLocaleManager regress startup.
                 final LocaleManager localeManager = BrowserLocaleManager.getInstance();
                 localeManager.initialize(getApplicationContext());
 
-                SessionInformation previousSession = SessionInformation.fromSharedPrefs(prefs);
-                if (previousSession.wasKilled()) {
+                if (wasPreviousSessionKilled(prefs)) {
                     Telemetry.addToHistogram("FENNEC_WAS_KILLED", 1);
                 }
 
                 SharedPreferences.Editor editor = prefs.edit();
                 editor.putBoolean(GeckoApp.PREFS_OOM_EXCEPTION, false);
 
                 // Put a flag to check if we got a normal `onSaveInstanceState`
                 // on exit, or if we were suddenly killed (crash or native OOM).
                 editor.putBoolean(GeckoApp.PREFS_WAS_STOPPED, false);
 
                 editor.apply();
 
-                // The lifecycle of mHealthRecorder is "shortly after onCreate"
-                // through "onDestroy" -- essentially the same as the lifecycle
-                // of the activity itself.
-                final String profilePath = getProfile().getDir().getAbsolutePath();
-                final EventDispatcher dispatcher = EventDispatcher.getInstance();
-
                 // This is the locale prior to fixing it up.
                 final Locale osLocale = Locale.getDefault();
 
                 // Both of these are Java-format locale strings: "en_US", not "en-US".
                 final String osLocaleString = osLocale.toString();
                 String appLocaleString = localeManager.getAndApplyPersistedLocale(GeckoApp.this);
                 Log.d(LOGTAG, "OS locale is " + osLocaleString + ", app locale is " + appLocaleString);
 
                 if (appLocaleString == null) {
                     appLocaleString = osLocaleString;
                 }
 
-                mHealthRecorder = GeckoApp.this.createHealthRecorder(GeckoApp.this,
-                                                                     profilePath,
-                                                                     dispatcher,
-                                                                     osLocaleString,
-                                                                     appLocaleString,
-                                                                     previousSession);
-
                 final String uiLocale = appLocaleString;
                 ThreadUtils.postToUiThread(new Runnable() {
                     @Override
                     public void run() {
                         GeckoApp.this.onLocaleReady(uiLocale);
                     }
                 });
 
@@ -1414,16 +1387,22 @@ public abstract class GeckoApp
                 BrowserLocaleManager.storeAndNotifyOSLocale(GeckoSharedPrefs.forProfile(GeckoApp.this), osLocale);
             }
         });
 
         GeckoAppShell.setNotificationClient(makeNotificationClient());
         IntentHelper.init(this);
     }
 
+    private static boolean wasPreviousSessionKilled(final SharedPreferences prefs) {
+        final boolean wasOOM = prefs.getBoolean(GeckoApp.PREFS_OOM_EXCEPTION, false);
+        final boolean wasStopped = prefs.getBoolean(GeckoApp.PREFS_WAS_STOPPED, true);
+        return wasOOM || !wasStopped;
+    }
+
     /**
      * At this point, the resource system and the rest of the browser are
      * aware of the locale.
      *
      * Now we can display strings!
      *
      * You can think of this as being something like a second phase of onCreate,
      * where you can do string-related operations. Use this in place of embedding
@@ -1617,21 +1596,16 @@ public abstract class GeckoApp
         // Trigger the completion of the telemetry timer that wraps activity startup,
         // then grab the duration to give to FHR.
         mJavaUiStartupTimer.stop();
         final long javaDuration = mJavaUiStartupTimer.getElapsed();
 
         ThreadUtils.getBackgroundHandler().postDelayed(new Runnable() {
             @Override
             public void run() {
-                final HealthRecorder rec = mHealthRecorder;
-                if (rec != null) {
-                    rec.recordJavaStartupTime(javaDuration);
-                }
-
                 // Kick off our background services. We do this by invoking the broadcast
                 // receiver, which uses the system alarm infrastructure to perform tasks at
                 // intervals.
                 GeckoPreferences.broadcastHealthReportUploadPref(GeckoApp.this);
             }
         }, 50);
 
         final int updateServiceDelay = 30 * 1000;
@@ -1970,42 +1944,23 @@ public abstract class GeckoApp
         }
 
         if (mAppStateListeners != null) {
             for (GeckoAppShell.AppStateListener listener : mAppStateListeners) {
                 listener.onResume();
             }
         }
 
-        // We use two times: a pseudo-unique wall-clock time to identify the
-        // current session across power cycles, and the elapsed realtime to
-        // track the duration of the session.
-        final long now = System.currentTimeMillis();
-        final long realTime = android.os.SystemClock.elapsedRealtime();
-
         ThreadUtils.postToBackgroundThread(new Runnable() {
             @Override
             public void run() {
-                // Now construct the new session on HealthRecorder's behalf. We do this here
-                // so it can benefit from a single near-startup prefs commit.
-                SessionInformation currentSession = new SessionInformation(now, realTime);
-
                 SharedPreferences prefs = GeckoApp.this.getSharedPreferences();
                 SharedPreferences.Editor editor = prefs.edit();
                 editor.putBoolean(GeckoApp.PREFS_WAS_STOPPED, false);
-                currentSession.recordBegin(editor);
                 editor.apply();
-
-                final HealthRecorder rec = mHealthRecorder;
-                if (rec != null) {
-                    rec.setCurrentSession(currentSession);
-                    rec.processDelayed();
-                } else {
-                    Log.w(LOGTAG, "Can't record session: rec is null.");
-                }
             }
         });
 
         Restrictions.update(this);
     }
 
     @Override
     public void onWindowFocusChanged(boolean hasFocus) {
@@ -2021,31 +1976,27 @@ public abstract class GeckoApp
             mLayerView.setFocusableInTouchMode(true);
             getWindow().setBackgroundDrawable(null);
         }
     }
 
     @Override
     public void onPause()
     {
-        final HealthRecorder rec = mHealthRecorder;
         final Context context = this;
 
         // In some way it's sad that Android will trigger StrictMode warnings
         // here as the whole point is to save to disk while the activity is not
         // interacting with the user.
         ThreadUtils.postToBackgroundThread(new Runnable() {
             @Override
             public void run() {
                 SharedPreferences prefs = GeckoApp.this.getSharedPreferences();
                 SharedPreferences.Editor editor = prefs.edit();
                 editor.putBoolean(GeckoApp.PREFS_WAS_STOPPED, true);
-                if (rec != null) {
-                    rec.recordSessionEnd("P", editor);
-                }
 
                 // If we haven't done it before, cleanup any old files in our old temp dir
                 if (prefs.getBoolean(GeckoApp.PREFS_CLEANUP_TEMP_FILES, true)) {
                     File tempDir = GeckoLoader.getGREDir(GeckoApp.this);
                     FileUtils.delTree(tempDir, new FileUtils.NameAndAgeFilter(null, ONE_DAY_MS), false);
 
                     editor.putBoolean(GeckoApp.PREFS_CLEANUP_TEMP_FILES, false);
                 }
@@ -2147,28 +2098,16 @@ public abstract class GeckoApp
 
         if (SmsManager.isEnabled()) {
             SmsManager.getInstance().stop();
             if (isFinishing()) {
                 SmsManager.getInstance().shutdown();
             }
         }
 
-        final HealthRecorder rec = mHealthRecorder;
-        mHealthRecorder = null;
-        if (rec != null && rec.isEnabled()) {
-            // Closing a HealthRecorder could incur a write.
-            ThreadUtils.postToBackgroundThread(new Runnable() {
-                @Override
-                public void run() {
-                    rec.close(GeckoApp.this);
-                }
-            });
-        }
-
         Favicons.close();
 
         super.onDestroy();
 
         Tabs.unregisterOnTabsChangedListener(this);
 
         if (!isFinishing()) {
             // GeckoApp was not intentionally destroyed, so keep our process alive.
@@ -2690,25 +2629,16 @@ public abstract class GeckoApp
      * in a nested activity, and then again when we get back up to GeckoApp.
      *
      * GeckoApp needs to do a bunch more stuff than, say, GeckoPreferences.
      */
     protected void onLocaleChanged(final String locale) {
         final boolean startNewSession = true;
         final boolean shouldRestart = false;
 
-        // If the HealthRecorder is not yet initialized (unlikely), the locale change won't
-        // trigger a session transition and subsequent events will be recorded in an environment
-        // with the wrong locale.
-        final HealthRecorder rec = mHealthRecorder;
-        if (rec != null) {
-            rec.onAppLocaleChanged(locale);
-            rec.onEnvironmentChanged(startNewSession, SESSION_END_LOCALE_CHANGED);
-        }
-
         if (!shouldRestart) {
             ThreadUtils.postToUiThread(new Runnable() {
                 @Override
                 public void run() {
                     GeckoApp.this.onLocaleReady(locale);
                 }
             });
             return;
@@ -2753,23 +2683,13 @@ public abstract class GeckoApp
                     mMainLayout.setSystemUiVisibility(View.SYSTEM_UI_FLAG_VISIBLE);
                 } else {
                     mMainLayout.setSystemUiVisibility(View.SYSTEM_UI_FLAG_LOW_PROFILE);
                 }
             }
         });
     }
 
-    protected HealthRecorder createHealthRecorder(final Context context,
-                                                  final String profilePath,
-                                                  final EventDispatcher dispatcher,
-                                                  final String osLocale,
-                                                  final String appLocale,
-                                                  final SessionInformation previousSession) {
-        // GeckoApp does not need to record any health information - return a stub.
-        return new StubbedHealthRecorder();
-    }
-
     protected StartupAction getStartupAction(final String passedURL) {
         // Default to NORMAL here. Subclasses can handle the other types.
         return StartupAction.NORMAL;
     }
 }
deleted file mode 100644
--- a/mobile/android/base/java/org/mozilla/gecko/health/HealthRecorder.java
+++ /dev/null
@@ -1,40 +0,0 @@
-/* -*- Mode: Java; c-basic-offset: 4; tab-width: 20; indent-tabs-mode: nil; -*-
- * This Source Code Form is subject to the terms of the Mozilla Public
- * License, v. 2.0. If a copy of the MPL was not distributed with this
- * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
-
-package org.mozilla.gecko.health;
-
-import android.content.Context;
-import android.content.SharedPreferences;
-
-import org.json.JSONObject;
-
-/**
- * HealthRecorder is an interface into the Firefox Health Report storage system.
- */
-public interface HealthRecorder {
-    /**
-     * Returns whether the Health Recorder is actively recording events.
-     */
-    public boolean isEnabled();
-
-    public void setCurrentSession(SessionInformation session);
-    public void checkForOrphanSessions();
-
-    public void recordGeckoStartupTime(long duration);
-    public void recordJavaStartupTime(long duration);
-    public void recordSearch(final String engineID, final String location);
-    public void recordSessionEnd(String reason, SharedPreferences.Editor editor);
-    public void recordSessionEnd(String reason, SharedPreferences.Editor editor, final int environment);
-
-    public void onAppLocaleChanged(String to);
-    public void onAddonChanged(String id, JSONObject json);
-    public void onAddonUninstalling(String id);
-    public void onEnvironmentChanged();
-    public void onEnvironmentChanged(final boolean startNewSession, final String sessionEndReason);
-
-    public void close(final Context context);
-
-    public void processDelayed();
-}
deleted file mode 100644
--- a/mobile/android/base/java/org/mozilla/gecko/health/SessionInformation.java
+++ /dev/null
@@ -1,137 +0,0 @@
-/* -*- Mode: Java; c-basic-offset: 4; tab-width: 20; indent-tabs-mode: nil; -*-
- * This Source Code Form is subject to the terms of the Mozilla Public
- * License, v. 2.0. If a copy of the MPL was not distributed with this
- * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
-
-package org.mozilla.gecko.health;
-
-import android.content.SharedPreferences;
-import android.util.Log;
-
-import org.mozilla.gecko.GeckoApp;
-
-import org.json.JSONException;
-import org.json.JSONObject;
-
-public class SessionInformation {
-    private static final String LOG_TAG = "GeckoSessInfo";
-
-    public static final String PREFS_SESSION_START = "sessionStart";
-
-    public final long wallStartTime;    // System wall clock.
-    public final long realStartTime;    // Realtime clock.
-
-    private final boolean wasOOM;
-    private final boolean wasStopped;
-
-    private volatile long timedGeckoStartup = -1;
-    private volatile long timedJavaStartup = -1;
-
-    // Current sessions don't (right now) care about wasOOM/wasStopped.
-    // Eventually we might want to lift that logic out of GeckoApp.
-    public SessionInformation(long wallTime, long realTime) {
-        this(wallTime, realTime, false, false);
-    }
-
-    // Previous sessions do...
-    public SessionInformation(long wallTime, long realTime, boolean wasOOM, boolean wasStopped) {
-        this.wallStartTime = wallTime;
-        this.realStartTime = realTime;
-        this.wasOOM = wasOOM;
-        this.wasStopped = wasStopped;
-    }
-
-    /**
-     * Initialize a new SessionInformation instance from the supplied prefs object.
-     *
-     * This includes retrieving OOM/crash data, as well as timings.
-     *
-     * If no wallStartTime was found, that implies that the previous
-     * session was correctly recorded, and an object with a zero
-     * wallStartTime is returned.
-     */
-    public static SessionInformation fromSharedPrefs(SharedPreferences prefs) {
-        boolean wasOOM = prefs.getBoolean(GeckoApp.PREFS_OOM_EXCEPTION, false);
-        boolean wasStopped = prefs.getBoolean(GeckoApp.PREFS_WAS_STOPPED, true);
-        long wallStartTime = prefs.getLong(PREFS_SESSION_START, 0L);
-        long realStartTime = 0L;
-        Log.d(LOG_TAG, "Building SessionInformation from prefs: " +
-                       wallStartTime + ", " + realStartTime + ", " +
-                       wasStopped + ", " + wasOOM);
-        return new SessionInformation(wallStartTime, realStartTime, wasOOM, wasStopped);
-    }
-
-    /**
-     * Initialize a new SessionInformation instance to 'split' the current
-     * session.
-     */
-    public static SessionInformation forRuntimeTransition() {
-        final boolean wasOOM = false;
-        final boolean wasStopped = true;
-        final long wallStartTime = System.currentTimeMillis();
-        final long realStartTime = android.os.SystemClock.elapsedRealtime();
-        Log.v(LOG_TAG, "Recording runtime session transition: " +
-                       wallStartTime + ", " + realStartTime);
-        return new SessionInformation(wallStartTime, realStartTime, wasOOM, wasStopped);
-    }
-
-    public boolean wasKilled() {
-        return wasOOM || !wasStopped;
-    }
-
-    /**
-     * Record the beginning of this session to SharedPreferences by
-     * recording our start time. If a session was already recorded, it is
-     * overwritten (there can only be one running session at a time). Does
-     * not commit the editor.
-     */
-    public void recordBegin(SharedPreferences.Editor editor) {
-        Log.d(LOG_TAG, "Recording start of session: " + this.wallStartTime);
-        editor.putLong(PREFS_SESSION_START, this.wallStartTime);
-    }
-
-    /**
-     * Record the completion of this session to SharedPreferences by
-     * deleting our start time. Does not commit the editor.
-     */
-    public void recordCompletion(SharedPreferences.Editor editor) {
-        Log.d(LOG_TAG, "Recording session done: " + this.wallStartTime);
-        editor.remove(PREFS_SESSION_START);
-    }
-
-    /**
-     * Return the JSON that we'll put in the DB for this session.
-     */
-    public JSONObject getCompletionJSON(String reason, long realEndTime) throws JSONException {
-        long durationSecs = (realEndTime - this.realStartTime) / 1000;
-        JSONObject out = new JSONObject();
-        out.put("r", reason);
-        out.put("d", durationSecs);
-        if (this.timedGeckoStartup > 0) {
-            out.put("sg", this.timedGeckoStartup);
-        }
-        if (this.timedJavaStartup > 0) {
-            out.put("sj", this.timedJavaStartup);
-        }
-        return out;
-    }
-
-    public JSONObject getCrashedJSON() throws JSONException {
-        JSONObject out = new JSONObject();
-        // We use ints here instead of booleans, because we're packing
-        // stuff into JSON, and saving bytes in the DB is a worthwhile
-        // goal.
-        out.put("oom", this.wasOOM ? 1 : 0);
-        out.put("stopped", this.wasStopped ? 1 : 0);
-        out.put("r", "A");
-        return out;
-    }
-
-    public void setTimedGeckoStartup(final long duration) {
-        timedGeckoStartup = duration;
-    }
-
-    public void setTimedJavaStartup(final long duration) {
-        timedJavaStartup = duration;
-    }
-}
deleted file mode 100644
--- a/mobile/android/base/java/org/mozilla/gecko/health/StubbedHealthRecorder.java
+++ /dev/null
@@ -1,53 +0,0 @@
-/* -*- Mode: Java; c-basic-offset: 4; tab-width: 20; indent-tabs-mode: nil; -*-
- * This Source Code Form is subject to the terms of the Mozilla Public
- * License, v. 2.0. If a copy of the MPL was not distributed with this
- * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
-
-package org.mozilla.gecko.health;
-
-import android.content.Context;
-import android.content.SharedPreferences;
-
-import org.json.JSONObject;
-
-/**
- * StubbedHealthRecorder is an implementation of HealthRecorder that does (you guessed it!)
- * nothing.
- */
-public class StubbedHealthRecorder implements HealthRecorder {
-    @Override
-    public boolean isEnabled() { return false; }
-
-    @Override
-    public void setCurrentSession(SessionInformation session) { }
-    @Override
-    public void checkForOrphanSessions() { }
-
-    @Override
-    public void recordGeckoStartupTime(long duration) { }
-    @Override
-    public void recordJavaStartupTime(long duration) { }
-    @Override
-    public void recordSearch(final String engineID, final String location) { }
-    @Override
-    public void recordSessionEnd(String reason, SharedPreferences.Editor editor) { }
-    @Override
-    public void recordSessionEnd(String reason, SharedPreferences.Editor editor, final int environment) { }
-
-    @Override
-    public void onAppLocaleChanged(String to) { }
-    @Override
-    public void onAddonChanged(String id, JSONObject json) { }
-    @Override
-    public void onAddonUninstalling(String id) { }
-    @Override
-    public void onEnvironmentChanged() { }
-    @Override
-    public void onEnvironmentChanged(final boolean startNewSession, final String sessionEndReason) { }
-
-    @Override
-    public void close(final Context context) { }
-
-    @Override
-    public void processDelayed() { }
-}
--- a/mobile/android/base/moz.build
+++ b/mobile/android/base/moz.build
@@ -347,19 +347,16 @@ gbjar.sources += ['java/org/mozilla/geck
     'gfx/SubdocumentScrollHelper.java',
     'gfx/TextureGenerator.java',
     'gfx/TextureReaper.java',
     'gfx/TouchEventHandler.java',
     'gfx/ViewTransform.java',
     'gfx/VirtualLayer.java',
     'GlobalHistory.java',
     'GuestSession.java',
-    'health/HealthRecorder.java',
-    'health/SessionInformation.java',
-    'health/StubbedHealthRecorder.java',
     'home/BookmarkFolderView.java',
     'home/BookmarksListAdapter.java',
     'home/BookmarksListView.java',
     'home/BookmarksPanel.java',
     'home/BrowserSearch.java',
     'home/DynamicPanel.java',
     'home/FramePanelLayout.java',
     'home/HistoryHeaderListCursorAdapter.java',