Bug 1266594 - Part 1: Wait for Java-side sanitizing to be triggered before exiting the UI. r=jchen, a=jcristau
authorJan Henning <jh+bugzilla@buttercookie.de>
Tue, 10 Jan 2017 20:31:20 +0100
changeset 353515 7b1e59d764d526f2a50d434918cac0386d9dd8ff
parent 353514 9545084f13f897cce23ba1f0934753e2dbb83f47
child 353516 b2bd1a80baa5157ceb2a16eae07072f492ace26d
push id6795
push userjlund@mozilla.com
push dateMon, 23 Jan 2017 14:19:46 +0000
treeherdermozilla-esr52@76101b503191 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjchen, jcristau
bugs1266594
milestone52.0a2
Bug 1266594 - Part 1: Wait for Java-side sanitizing to be triggered before exiting the UI. r=jchen, a=jcristau When clearing private data on exit, Gecko sends a "Sanitize:ClearHistory" message back to the Java UI, so the latter can clear the history DB if necessary. For some users, this process is not working, presumably because there is a race condition depending on whether Gecko gets as far as sending that message before the background thread responsible for receiving that message has already shut down/had its listener unregistered, or not. Therefore, we now wait until the UI's sanitize handlers have been called before starting to shut it down as well.
mobile/android/base/java/org/mozilla/gecko/GeckoApp.java
mobile/android/chrome/content/browser.js
--- a/mobile/android/base/java/org/mozilla/gecko/GeckoApp.java
+++ b/mobile/android/base/java/org/mozilla/gecko/GeckoApp.java
@@ -549,17 +549,19 @@ public abstract class GeckoApp
                 try {
                     res.put("dontSaveSession", "quit".equals(sessionRestore));
                 } catch (JSONException ex) {
                     Log.e(LOGTAG, "Error adding session restore data", ex);
                 }
             }
 
             GeckoAppShell.notifyObservers("Browser:Quit", res.toString());
-            doShutdown();
+            // We don't call doShutdown() here because this creates a race condition which can
+            // cause the clearing of private data to fail. Instead, we shut down the UI only after
+            // we're done sanitizing.
             return true;
         }
 
         return super.onOptionsItemSelected(item);
     }
 
     @Override
     public void onOptionsMenuClosed(Menu menu) {
@@ -688,16 +690,23 @@ public abstract class GeckoApp
             showSiteSettingsDialog(permissions);
 
         } else if ("PrivateBrowsing:Data".equals(event)) {
             mPrivateBrowsingSession = message.optString("session", null);
 
         } else if ("Session:StatePurged".equals(event)) {
             onStatePurged();
 
+        } else if ("Sanitize:Finished".equals(event)) {
+            if (message.getBoolean("shutdown")) {
+                // Gecko is shutting down and has called our sanitize handlers,
+                // so we can start exiting, too.
+                doShutdown();
+            }
+
         } else if ("Share:Text".equals(event)) {
             final String text = message.getString("text");
             final Tab tab = Tabs.getInstance().getSelectedTab();
             String title = "";
             if (tab != null) {
                 title = tab.getDisplayTitle();
             }
             IntentHelper.openUriExternal(text, "text/plain", "", "", Intent.ACTION_SEND, title, false);
@@ -1209,16 +1218,17 @@ public abstract class GeckoApp
             "DevToolsAuth:Scan",
             "DOMFullScreen:Start",
             "DOMFullScreen:Stop",
             "Image:SetAs",
             "Locale:Set",
             "Permissions:Data",
             "PrivateBrowsing:Data",
             "RuntimePermissions:Prompt",
+            "Sanitize:Finished",
             "Session:StatePurged",
             "Share:Text",
             "Snackbar:Show",
             "SystemUI:Visibility",
             "ToggleChrome:Focus",
             "ToggleChrome:Hide",
             "ToggleChrome:Show",
             "Update:Check",
@@ -2242,16 +2252,17 @@ public abstract class GeckoApp
             "DevToolsAuth:Scan",
             "DOMFullScreen:Start",
             "DOMFullScreen:Stop",
             "Image:SetAs",
             "Locale:Set",
             "Permissions:Data",
             "PrivateBrowsing:Data",
             "RuntimePermissions:Prompt",
+            "Sanitize:Finished",
             "Session:StatePurged",
             "Share:Text",
             "Snackbar:Show",
             "SystemUI:Visibility",
             "ToggleChrome:Focus",
             "ToggleChrome:Hide",
             "ToggleChrome:Show",
             "Update:Check",
--- a/mobile/android/chrome/content/browser.js
+++ b/mobile/android/chrome/content/browser.js
@@ -1365,17 +1365,17 @@ var BrowserApp = {
     if (aClear.dontSaveSession) {
       let ss = Cc["@mozilla.org/browser/sessionstore;1"].getService(Ci.nsISessionStore);
       ss.removeWindow(window);
     }
 
     BrowserApp.sanitize(aClear.sanitize, function() {
       let appStartup = Cc["@mozilla.org/toolkit/app-startup;1"].getService(Ci.nsIAppStartup);
       appStartup.quit(Ci.nsIAppStartup.eForceQuit);
-    });
+    }, true);
   },
 
   saveAsPDF: function saveAsPDF(aBrowser) {
     RuntimePermissions.waitForPermissions(RuntimePermissions.WRITE_EXTERNAL_STORAGE).then(function(permissionGranted) {
       if (!permissionGranted) {
         return;
       }
 
@@ -1419,17 +1419,17 @@ var BrowserApp = {
       return this.PREF_TRACKING_PROTECTION_ENABLED;
     }
     if (Services.prefs.getBoolPref("privacy.trackingprotection.pbmode.enabled")) {
       return this.PREF_TRACKING_PROTECTION_ENABLED_PB;
     }
     return this.PREF_TRACKING_PROTECTION_DISABLED;
   },
 
-  sanitize: function (aItems, callback) {
+  sanitize: function (aItems, callback, aShutdown) {
     let success = true;
     var promises = [];
 
     for (let key in aItems) {
       if (!aItems[key])
         continue;
 
       key = key.replace("private.data.", "");
@@ -1442,27 +1442,29 @@ var BrowserApp = {
         default:
           promises.push(Sanitizer.clearItem(key));
       }
     }
 
     Promise.all(promises).then(function() {
       Messaging.sendRequest({
         type: "Sanitize:Finished",
-        success: true
+        success: true,
+        shutdown: aShutdown === true
       });
 
       if (callback) {
         callback();
       }
     }).catch(function(err) {
       Messaging.sendRequest({
         type: "Sanitize:Finished",
         error: err,
-        success: false
+        success: false,
+        shutdown: aShutdown === true
       });
 
       if (callback) {
         callback();
       }
     })
   },