Bug 1346413 - Part 1 - GeckoActivityMonitor/onStop-based application-background/foreground tracking. r?jchen draft
authorJan Henning <jh+bugzilla@buttercookie.de>
Tue, 23 May 2017 21:10:55 +0200
changeset 583890 ec1d1ba955c3441a168c681c1f18b7dc6d9c94da
parent 583178 2495970fce11bbad7ba7e9f962d2341f7341878f
child 583891 b2923ef23e8d3c39a18a7e11181062f34e2fbe38
push id60590
push usermozilla@buttercookie.de
push dateWed, 24 May 2017 19:23:05 +0000
reviewersjchen
bugs1346413
milestone55.0a1
Bug 1346413 - Part 1 - GeckoActivityMonitor/onStop-based application-background/foreground tracking. r?jchen We want to do certain things on the Gecko side when our app goes into the background (being prepared for being killed without further ado [1], setting tab visibility status, ...) or comes back into the foreground. At the moment, this works by having participating activities manually call into GeckoApplication from onPause()/onResume(), from where we then forward this to Gecko as appropriate. There also is some additional logic because we want to avoid triggering a background-/foreground-notification combo each time we switch activities *within* Firefox, e.g. when going from BrowserApp into our settings, or navigating within the settings themselves. The problem with the current implementation is that we basically need to second guess whether we were opening one of our own activities or not when an onPause() call comes in and that we have to keep remembering the assumptions made there when implementing new activities. E.g. currently we assume that if an activity is finishing during onPause(), we're not leaving the app and therefore never trigger an application-background notification. Until recently, this was correct because except for quitting the app [2], we only finished activities when going backwards through our settings menu, from which we would only end up back in our main activity. With the advent of custom tabs, this is no longer correct, as we have to finish those as well when going back - and in that case we're returning to the app that originally created the custom tab, so we ought to send an application-background notification in fact. With this patch, we therefore switch our approach and base our background-/foreground tracking on watching onStop() instead. While unlike onPause(), onStop() has the slight drawback of not being absolutely guaranteed to be called before Android possibly kills us, the big advantage is that because of the normal Android lifecycle event order [3], by the time onStop for the previous activity is called we know for sure whether another activity of our own has been launched or whether we're being sent into the background. Additionally, using an ActivityLifecycleCallbacks instance makes it easy to monitor *all* onStop/onStart calls in our app without requiring any special support from our activities. [1] E.g. cleanly shutting down the cache service, flushing pending session store writes... [2] In which case Gecko would be shutting down on its own anyway, so a missing application-background notification wouldn't matter. [3] Old activity onPause, new activity onCreate, onStart, onResume, old activity onStop, onDestroy. MozReview-Commit-ID: 4QEUMz6NLfV
mobile/android/base/java/org/mozilla/gecko/GeckoActivityMonitor.java
mobile/android/base/java/org/mozilla/gecko/GeckoApplication.java
--- a/mobile/android/base/java/org/mozilla/gecko/GeckoActivityMonitor.java
+++ b/mobile/android/base/java/org/mozilla/gecko/GeckoActivityMonitor.java
@@ -1,81 +1,85 @@
 /* -*- Mode: Java; c-basic-offset: 4; tab-width: 4; 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;
 
-import android.annotation.SuppressLint;
 import android.app.Activity;
 import android.app.Application;
+import android.content.Context;
 import android.os.Bundle;
 
+import java.lang.ref.WeakReference;
+
 public class GeckoActivityMonitor implements Application.ActivityLifecycleCallbacks {
     private static final String LOGTAG = "GeckoActivityMonitor";
 
-    // We only hold a reference to the currently running activity - when this activity pauses,
-    // the reference is released or else overwritten by the next activity.
-    @SuppressLint("StaticFieldLeak")
     private static final GeckoActivityMonitor instance = new GeckoActivityMonitor();
 
-    private Activity currentActivity;
+    private GeckoApplication appContext;
+    private WeakReference<Activity> currentActivity = new WeakReference<>(null);
+    private boolean mInitialized;
 
     public static GeckoActivityMonitor getInstance() {
         return instance;
     }
 
     private GeckoActivityMonitor() { }
 
     public Activity getCurrentActivity() {
-        return currentActivity;
+        return currentActivity.get();
+    }
+
+    public synchronized void initialize(final Context context) {
+        if (mInitialized) {
+            return;
+        }
+
+        appContext = (GeckoApplication) context.getApplicationContext();
+
+        appContext.registerActivityLifecycleCallbacks(this);
+        mInitialized = true;
     }
 
     @Override
-    public void onActivityCreated(Activity activity, Bundle savedInstanceState) {
-        currentActivity = activity;
-    }
+    public void onActivityCreated(Activity activity, Bundle savedInstanceState) { }
 
     // onNewIntent happens in-between a pause/resume cycle, which means that we wouldn't have
     // a current activity to report if we were using only the official ActivityLifecycleCallbacks.
     // For code that wants to know the current activity even at this point we therefore have to
     // handle this ourselves.
-    public void onActivityNewIntent(Activity activity) {
-        currentActivity = activity;
-    }
+    public void onActivityNewIntent(Activity activity) { }
 
     @Override
     public void onActivityStarted(Activity activity) {
-        currentActivity = activity;
+        if (currentActivity.get() == null) {
+            appContext.onApplicationForeground();
+        }
+        currentActivity = new WeakReference<>(activity);
     }
 
     @Override
-    public void onActivityResumed(Activity activity) {
-        currentActivity = activity;
-    }
+    public void onActivityResumed(Activity activity) { }
 
     @Override
-    public void onActivityPaused(Activity activity) {
-        releaseIfCurrentActivity(activity);
-    }
+    public void onActivityPaused(Activity activity) { }
 
     @Override
     public void onActivityStopped(Activity activity) {
-        releaseIfCurrentActivity(activity);
+        // onStop for the previous activity is called after onStart for the new activity, so if
+        // we're switching activities within our app, currentActivity should already refer to the
+        // next activity at this point.
+        // If it doesn't, it means we've been backgrounded.
+        if (currentActivity.get() == activity) {
+            currentActivity.clear();
+            appContext.onApplicationBackground();
+        }
     }
 
     @Override
     public void onActivitySaveInstanceState(Activity activity, Bundle outState) { }
 
     @Override
-    public void onActivityDestroyed(Activity activity) {
-        releaseIfCurrentActivity(activity);
-    }
-
-    private void releaseIfCurrentActivity(Activity activity) {
-        // If the next activity has already started by the time the previous activity is being
-        // stopped/destroyed, we no longer need to clear the previous activity.
-        if (currentActivity == activity) {
-            currentActivity = null;
-        }
-    }
+    public void onActivityDestroyed(Activity activity) { }
 }
--- a/mobile/android/base/java/org/mozilla/gecko/GeckoApplication.java
+++ b/mobile/android/base/java/org/mozilla/gecko/GeckoApplication.java
@@ -168,42 +168,43 @@ public class GeckoApplication extends Ap
         } catch (IllegalStateException ex) {
             // GeckoApp hasn't started, so we have no ContextGetter in BrowserLocaleManager.
             Log.w(LOG_TAG, "Couldn't correct locale.", ex);
         }
 
         super.onConfigurationChanged(config);
     }
 
-    public void onActivityPause(GeckoActivityStatus activity) {
+    public void onActivityPause(GeckoActivityStatus activity) { }
+
+    public void onApplicationBackground() {
         mInBackground = true;
 
-        if ((activity.isFinishing() == false) &&
-            (activity.isGeckoActivityOpened() == false)) {
-            // Notify Gecko that we are pausing; the cache service will be
-            // shutdown, closing the disk cache cleanly. If the android
-            // low memory killer subsequently kills us, the disk cache will
-            // be left in a consistent state, avoiding costly cleanup and
-            // re-creation.
-            GeckoThread.onPause();
-            mPausedGecko = true;
+        // Notify Gecko that we are pausing; the cache service will be
+        // shutdown, closing the disk cache cleanly. If the android
+        // low memory killer subsequently kills us, the disk cache will
+        // be left in a consistent state, avoiding costly cleanup and
+        // re-creation.
+        GeckoThread.onPause();
+        mPausedGecko = true;
 
-            final BrowserDB db = BrowserDB.from(this);
-            ThreadUtils.postToBackgroundThread(new Runnable() {
-                @Override
-                public void run() {
-                    db.expireHistory(getContentResolver(), BrowserContract.ExpirePriority.NORMAL);
-                }
-            });
+        final BrowserDB db = BrowserDB.from(this);
+        ThreadUtils.postToBackgroundThread(new Runnable() {
+            @Override
+            public void run() {
+                db.expireHistory(getContentResolver(), BrowserContract.ExpirePriority.NORMAL);
+            }
+        });
 
-            GeckoNetworkManager.getInstance().stop();
-        }
+        GeckoNetworkManager.getInstance().stop();
     }
 
-    public void onActivityResume(GeckoActivityStatus activity) {
+    public void onActivityResume(GeckoActivityStatus activity) { }
+
+    public void onApplicationForeground() {
         if (mIsInitialResume) {
             GeckoBatteryManager.getInstance().start(this);
             GeckoFontScaleListener.getInstance().initialize(this);
             GeckoNetworkManager.getInstance().start(this);
             mIsInitialResume = false;
         } else if (mPausedGecko) {
             GeckoThread.onResume();
             mPausedGecko = false;
@@ -231,17 +232,17 @@ public class GeckoApplication extends Ap
         }
 
         mIsInitialResume = true;
 
         mRefWatcher = LeakCanary.install(this);
 
         sSessionUUID = UUID.randomUUID().toString();
 
-        registerActivityLifecycleCallbacks(GeckoActivityMonitor.getInstance());
+        GeckoActivityMonitor.getInstance().initialize(this);
 
         final Context context = getApplicationContext();
         GeckoAppShell.setApplicationContext(context);
         HardwareUtils.init(context);
         FilePicker.init(context);
         DownloadsIntegration.init();
         HomePanelsManager.getInstance().init(context);