Bug 1437382 - Part 4 - Make sure that flushing private tabs data reaches GeckoApp in time. r?jchen draft
authorJan Henning <jh+bugzilla@buttercookie.de>
Sun, 11 Feb 2018 17:54:58 +0100
changeset 760104 405072feaec2b15346022ac858def51a5516faab
parent 760103 ec5f55a68fbdfe80b987f8b7027ffb1ad7210bfb
child 760105 099a95091fc82a9861ed0f27b128b0aa62ebe6e4
child 761288 e146edabe9d179114059b52d6b7cc5ca5cae2c63
push id100541
push usermozilla@buttercookie.de
push dateMon, 26 Feb 2018 21:44:56 +0000
reviewersjchen
bugs1437382
milestone60.0a1
Bug 1437382 - Part 4 - Make sure that flushing private tabs data reaches GeckoApp in time. r?jchen There are two parts to this: 1. While the file writing itself by the session store can be done either synchronously or asynchronously, the session store's _writeFile function as a whole always behaves asynchronously. In addition, just writing the file (which will be done synchronously when flushing tabs) takes time which we don't have, as we should send the private tabs data as fast as possible to Java in order to avoid hanging the UI or missing a possible timeout. 2. Sending the data to Java needs to happen synchronously as well, so we need to listen for "PrivateBrowsing:Data" on the Gecko thread now. This in turn means that some sychronisation is now required between the UI thread handling onSaveInstanceState and the Gecko thread that is actually receiving the data. To avoid hanging the UI and causing ANRs, we only wait a limited amount of time for Gecko to respond with fresh private tabs data, though. As this still leaves a certain possibility of outdated private browsing data being saved and possibly restored after an OOM-kill, we're also going to speed up the processing of TabClosed events by the session store in the following parts. MozReview-Commit-ID: EkNFre5RhQW
mobile/android/base/java/org/mozilla/gecko/GeckoApp.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
@@ -313,16 +313,18 @@ public abstract class GeckoApp extends G
     };
 
     protected boolean mInitialized;
     protected boolean mWindowFocusInitialized;
     private Telemetry.Timer mJavaUiStartupTimer;
     private Telemetry.Timer mGeckoReadyStartupTimer;
 
     private String mPrivateBrowsingSession;
+    private boolean mPrivateBrowsingSessionOutdated;
+    private static final int MAX_PRIVATE_TABS_UPDATE_WAIT_MSEC = 500;
 
     private volatile HealthRecorder mHealthRecorder;
     private volatile Locale mLastLocale;
 
     private boolean mShutdownOnDestroy;
     private boolean mRestartOnShutdown;
 
     private boolean mWasFirstTabShownAfterActivityUnhidden;
@@ -603,28 +605,38 @@ public abstract class GeckoApp extends G
             return true;
         }
 
         return super.onKeyDown(keyCode, event);
     }
 
     @Override
     protected void onSaveInstanceState(Bundle outState) {
+        synchronized (this) {
+            mPrivateBrowsingSessionOutdated = true;
+        }
         // 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);
         }
+        synchronized (this) {
+            if (GeckoThread.isRunning() && mPrivateBrowsingSessionOutdated) {
+                try {
+                    wait(MAX_PRIVATE_TABS_UPDATE_WAIT_MSEC);
+                } catch (final InterruptedException e) { }
+            }
+            outState.putString(SAVED_STATE_PRIVATE_SESSION, mPrivateBrowsingSession);
+        }
 
         outState.putBoolean(SAVED_STATE_IN_BACKGROUND, isApplicationInBackground());
-        outState.putString(SAVED_STATE_PRIVATE_SESSION, mPrivateBrowsingSession);
     }
 
     public void addTab() { }
 
     public void addPrivateTab() { }
 
     public void showNormalTabs() { }
 
@@ -706,17 +718,23 @@ public abstract class GeckoApp extends G
         } else if ("Locale:Set".equals(event)) {
             setLocale(message.getString("locale"));
 
         } else if ("Permissions:Data".equals(event)) {
             final GeckoBundle[] permissions = message.getBundleArray("permissions");
             showSiteSettingsDialog(permissions);
 
         } else if ("PrivateBrowsing:Data".equals(event)) {
-            mPrivateBrowsingSession = message.getString("session");
+            synchronized (this) {
+                if (!message.getBoolean("noChange", false)) {
+                    mPrivateBrowsingSession = message.getString("session");
+                }
+                mPrivateBrowsingSessionOutdated = false;
+                notifyAll();
+            }
 
         } else if ("SystemUI:Visibility".equals(event)) {
             if (message.getBoolean("visible", true)) {
                 mMainLayout.setSystemUiVisibility(View.SYSTEM_UI_FLAG_VISIBLE);
             } else {
                 mMainLayout.setSystemUiVisibility(View.SYSTEM_UI_FLAG_LOW_PROFILE);
             }
 
@@ -1066,29 +1084,29 @@ public abstract class GeckoApp extends G
         session.getSettings().setString(GeckoSessionSettings.CHROME_URI,
                                         "chrome://browser/content/browser.xul");
         session.setContentListener(this);
 
         GeckoAccessibility.setDelegate(mLayerView);
 
         getAppEventDispatcher().registerGeckoThreadListener(this,
             "Locale:Set",
+            "PrivateBrowsing:Data",
             null);
 
         getAppEventDispatcher().registerUiThreadListener(this,
             "Accessibility:Event",
             "Contact:Add",
             "DevToolsAuth:Scan",
             "DOMFullScreen:Start",
             "DOMFullScreen:Stop",
             "Mma:reader_available",
             "Mma:web_save_image",
             "Mma:web_save_media",
             "Permissions:Data",
-            "PrivateBrowsing:Data",
             "SystemUI:Visibility",
             "ToggleChrome:Focus",
             "ToggleChrome:Hide",
             "ToggleChrome:Show",
             null);
 
         Tabs.getInstance().attachToContext(this, mLayerView, getAppEventDispatcher());
         Tabs.registerOnTabsChangedListener(this);
@@ -2073,29 +2091,29 @@ public abstract class GeckoApp extends G
             "Gecko:CorruptAPK",
             "Update:Check",
             "Update:Download",
             "Update:Install",
             null);
 
         getAppEventDispatcher().unregisterGeckoThreadListener(this,
             "Locale:Set",
+            "PrivateBrowsing:Data",
             null);
 
         getAppEventDispatcher().unregisterUiThreadListener(this,
             "Accessibility:Event",
             "Contact:Add",
             "DevToolsAuth:Scan",
             "DOMFullScreen:Start",
             "DOMFullScreen:Stop",
             "Mma:reader_available",
             "Mma:web_save_image",
             "Mma:web_save_media",
             "Permissions:Data",
-            "PrivateBrowsing:Data",
             "SystemUI:Visibility",
             "ToggleChrome:Focus",
             "ToggleChrome:Hide",
             "ToggleChrome:Show",
             null);
 
         if (mPromptService != null) {
             mPromptService.destroy();
--- a/mobile/android/components/SessionStore.js
+++ b/mobile/android/components/SessionStore.js
@@ -203,18 +203,24 @@ SessionStore.prototype = {
         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();
+        if (!this._loadState == STATE_RUNNING || !this.flushPendingState()) {
+          let window = Services.wm.getMostRecentWindow("navigator:browser");
+          if (window) { // can be null if we're restarting
+            window.WindowEventDispatcher.sendRequest({
+              type: "PrivateBrowsing:Data",
+              noChange: true
+            });
+          }
         }
         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");
@@ -987,22 +993,28 @@ SessionStore.prototype = {
   },
 
   saveState: function ss_saveState() {
     this._pendingWrite++;
     log("saveState(), incrementing _pendingWrite to " + this._pendingWrite);
     this._saveState(true);
   },
 
-  // Immediately and synchronously writes any pending state to disk.
+  /**
+   * Immediately and synchronously writes any pending state to disk.
+   *
+   * @return True if data was written, false if no pending file writes were present.
+   */
   flushPendingState: function ss_flushPendingState() {
     log("flushPendingState(), _pendingWrite = " + this._pendingWrite);
     if (this._pendingWrite) {
       this._saveState(false);
+      return true;
     }
+    return false;
   },
 
   _saveState: function ss_saveState(aAsync) {
     log("_saveState(aAsync = " + aAsync + ")");
     // Kill any queued timer and save immediately
     if (this._saveTimer) {
       this._saveTimer.cancel();
       this._saveTimer = null;
@@ -1051,35 +1063,35 @@ SessionStore.prototype = {
         let savedWin = tab.isPrivate ? privateData.windows[winIndex] : normalData.windows[winIndex];
         savedWin.tabs.push(tab);
         if (win.selectedTabId === tab.tabId) {
           savedWin.selected = savedWin.tabs.length; // 1-based index
         }
       }
     }
 
-    // Write only non-private data to disk
-    if (normalData.windows[0] && normalData.windows[0].tabs) {
-      log("_saveState() writing normal data, " +
-           normalData.windows[0].tabs.length + " tabs in window[0]");
-    } else {
-      log("_saveState() writing empty normal data");
-    }
-    this._writeFile(this._sessionFile, this._sessionFileTemp, normalData, aAsync);
-
     // If we have private data, send it to Java; otherwise, send null to
     // indicate that there is no private data
     let window = Services.wm.getMostRecentWindow("navigator:browser");
     if (window) { // can be null if we're restarting
       window.WindowEventDispatcher.sendRequest({
         type: "PrivateBrowsing:Data",
         session: (privateData.windows.length > 0 && privateData.windows[0].tabs.length > 0) ? JSON.stringify(privateData) : null
       });
     }
 
+    // Write only non-private data to disk
+    if (normalData.windows[0] && normalData.windows[0].tabs) {
+      log("_saveState() writing normal data, " +
+           normalData.windows[0].tabs.length + " tabs in window[0]");
+    } else {
+      log("_saveState() writing empty normal data");
+    }
+    this._writeFile(this._sessionFile, this._sessionFileTemp, normalData, aAsync);
+
     this._lastSaveTime = Date.now();
   },
 
   _getCurrentState: function ss_getCurrentState() {
     this._forEachBrowserWindow((aWindow) => {
       this._collectWindowData(aWindow);
     });