Bug 1182270 - Avoid race conditions when deleting Firefox Accounts. r=vivek

mobile/android/base/fxa/authenticator/FxAccountAuthenticator.java

@@ -32,6 +32,7 @@ import org.mozilla.gecko.fxa.login.State.StateLabel;
 import org.mozilla.gecko.fxa.login.StateFactory;
 import org.mozilla.gecko.fxa.sync.FxAccountNotificationManager;
 import org.mozilla.gecko.fxa.sync.FxAccountSyncAdapter;
+import org.mozilla.gecko.util.ThreadUtils;
 
 import java.security.NoSuchAlgorithmException;
 import java.util.concurrent.Executor;
@@ -351,6 +352,17 @@ public class FxAccountAuthenticator extends AbstractAccountAuthenticator {
     // Broadcast intents protected with permissions are secure, so it's okay
     // to include private information such as a password.
     final AndroidFxAccount androidFxAccount = new AndroidFxAccount(context, account);
+
+    // Deleting the pickle file in a blocking manner will avoid race conditions that might happen when
+    // an account is unpickled while an FxAccount is being deleted.
+    // Also we have an assumption that this method is always called from a background thread, so we delete
+    // the pickle file directly without being afraid from a StrictMode violation.
+    ThreadUtils.assertNotOnUiThread();
+
+    Logger.info(LOG_TAG, "Firefox account named " + account.name + " being removed; " +
+            "deleting saved pickle file '" + FxAccountConstants.ACCOUNT_PICKLE_FILENAME + "'.");
+    deletePickle();
+
     final Intent intent = androidFxAccount.makeDeletedAccountIntent();
     Logger.info(LOG_TAG, "Account named " + account.name + " being removed; " +
         "broadcasting secure intent " + intent.getAction() + ".");
@@ -358,4 +370,13 @@ public class FxAccountAuthenticator extends AbstractAccountAuthenticator {
 
     return result;
   }
+
+  private void deletePickle() {
+    try {
+      AccountPickler.deletePickle(context, FxAccountConstants.ACCOUNT_PICKLE_FILENAME);
+    } catch (Exception e) {
+      // This should never happen, but we really don't want to die in a background thread.
+      Logger.warn(LOG_TAG, "Got exception deleting saved pickle file; ignoring.", e);
+    }
+  }
 }

mobile/android/base/fxa/receivers/FxAccountDeletedService.java

@@ -13,7 +13,6 @@ import org.mozilla.gecko.background.fxa.oauth.FxAccountOAuthClient10;
 import org.mozilla.gecko.fxa.FxAccountConstants;
 import org.mozilla.gecko.fxa.sync.FxAccountNotificationManager;
 import org.mozilla.gecko.fxa.sync.FxAccountSyncAdapter;
-import org.mozilla.gecko.sync.config.AccountPickler;
 import org.mozilla.gecko.sync.repositories.android.AndroidBrowserHistoryDataExtender;
 import org.mozilla.gecko.sync.repositories.android.ClientsDatabase;
 import org.mozilla.gecko.sync.repositories.android.FennecTabsRepository;
@@ -65,9 +64,6 @@ public class FxAccountDeletedService extends IntentService {
       return;
     }
 
-    Logger.info(LOG_TAG, "Firefox account named " + accountName + " being removed; " +
-        "deleting saved pickle file '" + FxAccountConstants.ACCOUNT_PICKLE_FILENAME + "'.");
-    deletePickle(context);
 
     // Delete client database and non-local tabs.
     Logger.info(LOG_TAG, "Deleting the entire Fennec clients database and non-local tabs");
@@ -155,13 +151,4 @@ public class FxAccountDeletedService extends IntentService {
       Logger.error(LOG_TAG, "Cached OAuth server URI is null or cached OAuth tokens are null; ignoring.");
     }
   }
-
-  public static void deletePickle(final Context context) {
-    try {
-      AccountPickler.deletePickle(context, FxAccountConstants.ACCOUNT_PICKLE_FILENAME);
-    } catch (Exception e) {
-      // This should never happen, but we really don't want to die in a background thread.
-      Logger.warn(LOG_TAG, "Got exception deleting saved pickle file; ignoring.", e);
-    }
-  }
 }