Bug 1437382 - Part 3 - Flush session store tab data separately from application-background. r?jchen draft
authorJan Henning <jh+bugzilla@buttercookie.de>
Thu, 22 Feb 2018 19:55:13 +0100
changeset 760103 ec5f55a68fbdfe80b987f8b7027ffb1ad7210bfb
parent 759442 238e2d59be7746fe7ffc5a1556ac3f3befe84fbf
child 760104 405072feaec2b15346022ac858def51a5516faab
push id100541
push usermozilla@buttercookie.de
push dateMon, 26 Feb 2018 21:44:56 +0000
reviewersjchen
bugs1437382
milestone60.0a1
Bug 1437382 - Part 3 - Flush session store tab data separately from application-background. r?jchen We need to block onSaveInstanceState until we're sure that our private browsing data is up to date, however we can't block on the whole of our application- background handling (GeckoThread.onPause), as that will take too much time. In addition, we need to update the stored tab data not just when the application goes into the background, but every time we're leaving GeckoApp and onSave- InstanceState is called, e.g. when visiting our settings activity. MozReview-Commit-ID: DgtUvatD6h3
mobile/android/base/java/org/mozilla/gecko/GeckoApp.java
mobile/android/base/java/org/mozilla/gecko/GeckoApplication.java
mobile/android/components/SessionStore.js
--- a/mobile/android/base/java/org/mozilla/gecko/GeckoApp.java
+++ b/mobile/android/base/java/org/mozilla/gecko/GeckoApp.java
@@ -603,18 +603,26 @@ public abstract class GeckoApp extends G
             return true;
         }
 
         return super.onKeyDown(keyCode, event);
     }
 
     @Override
     protected void onSaveInstanceState(Bundle outState) {
+        // Through the GeckoActivityMonitor, this will flush tabs if the whole
+        // application is going into the background.
         super.onSaveInstanceState(outState);
 
+        // If on the other hand we're merely switching to a different activity
+        // within our app, we need to trigger a tabs flush ourselves.
+        if (!isApplicationInBackground()) {
+            EventDispatcher.getInstance().dispatch("Session:FlushTabs", null);
+        }
+
         outState.putBoolean(SAVED_STATE_IN_BACKGROUND, isApplicationInBackground());
         outState.putString(SAVED_STATE_PRIVATE_SESSION, mPrivateBrowsingSession);
     }
 
     public void addTab() { }
 
     public void addPrivateTab() { }
 
--- a/mobile/android/base/java/org/mozilla/gecko/GeckoApplication.java
+++ b/mobile/android/base/java/org/mozilla/gecko/GeckoApplication.java
@@ -174,16 +174,17 @@ public class GeckoApplication extends Ap
     public void onApplicationBackground() {
         mInBackground = 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.
+        EventDispatcher.getInstance().dispatch("Session:FlushTabs", null);
         GeckoThread.onPause();
         mPausedGecko = true;
 
         final BrowserDB db = BrowserDB.from(this);
         ThreadUtils.postToBackgroundThread(new Runnable() {
             @Override
             public void run() {
                 db.expireHistory(getContentResolver(), BrowserContract.ExpirePriority.NORMAL);
--- a/mobile/android/components/SessionStore.js
+++ b/mobile/android/components/SessionStore.js
@@ -197,16 +197,27 @@ SessionStore.prototype = {
         this._sendClosedTabsToJava(Services.wm.getMostRecentWindow("navigator:browser"));
         break;
 
       case "ClosedTabs:StopNotifications":
         this._notifyClosedTabs = false;
         log("ClosedTabs:StopNotifications");
         break;
 
+      case "Session:FlushTabs":
+        // We receive this notification when the activity or application is going into
+        // the background. If the application is backgrounded, it may be terminated at
+        // any point without notice; therefore, we must synchronously write out any
+        // pending save state to ensure that this data does not get lost.
+        log("Session:FlushTabs");
+        if (this._loadState == STATE_RUNNING) {
+          this.flushPendingState();
+        }
+        break;
+
       case "Session:Restore": {
         EventDispatcher.instance.unregisterListener(this, "Session:Restore");
         if (data) {
           // Be ready to handle any restore failures by making sure we have a valid tab opened
           let window = Services.wm.getMostRecentWindow("navigator:browser");
           let restoreCleanup = (aSubject, aTopic, aData) => {
               Services.obs.removeObserver(restoreCleanup, "sessionstore-windows-restored");
 
@@ -261,32 +272,32 @@ SessionStore.prototype = {
 
   observe: function ss_observe(aSubject, aTopic, aData) {
     let observerService = Services.obs;
     switch (aTopic) {
       case "app-startup":
         EventDispatcher.instance.registerListener(this, [
           "ClosedTabs:StartNotifications",
           "ClosedTabs:StopNotifications",
+          "Session:FlushTabs",
           "Session:Restore",
           "Session:RestoreRecentTabs",
           "Tab:KeepZombified",
           "Tabs:OpenMultiple",
         ]);
         observerService.addObserver(this, "final-ui-startup", true);
         observerService.addObserver(this, "domwindowopened", true);
         observerService.addObserver(this, "domwindowclosed", true);
         observerService.addObserver(this, "browser:purge-session-history", true);
         observerService.addObserver(this, "browser:purge-session-tabs", true);
         observerService.addObserver(this, "quit-application-requested", true);
         observerService.addObserver(this, "quit-application-proceeding", true);
         observerService.addObserver(this, "quit-application", true);
         observerService.addObserver(this, "Session:NotifyLocationChange", true);
         observerService.addObserver(this, "Content:HistoryChange", true);
-        observerService.addObserver(this, "application-background", true);
         observerService.addObserver(this, "application-foreground", true);
         observerService.addObserver(this, "last-pb-context-exited", true);
         break;
       case "final-ui-startup":
         observerService.removeObserver(this, "final-ui-startup");
         this.init();
         break;
       case "domwindowopened": {
@@ -370,26 +381,16 @@ SessionStore.prototype = {
           browser.__SS_historyChange =
             window.setTimeout(() => {
               delete browser.__SS_historyChange;
               this.onTabLoad(window, browser);
             }, 0);
         }
         break;
       }
-      case "application-background":
-        // We receive this notification when Android's onPause callback is
-        // executed. After onPause, the application may be terminated at any
-        // point without notice; therefore, we must synchronously write out any
-        // pending save state to ensure that this data does not get lost.
-        log("application-background");
-        if (this._loadState == STATE_RUNNING) {
-          this.flushPendingState();
-        }
-        break;
       case "application-foreground":
         log("application-foreground");
         // If we skipped restoring a zombified tab before backgrounding,
         // we might have to do it now instead.
         let window = Services.wm.getMostRecentWindow("navigator:browser");
         if (window && window.BrowserApp) { // Might not yet be ready during a cold startup.
           let tab = window.BrowserApp.selectedTab;
           if (tab) { // Can be null if closing a tab triggered an activity switch.