Bug 1560014 - Make nsMsgAccountManager::RemoveAccount() return error when given an already removed account. r=mkmelin DONTBUILD

mail/components/extensions/test/browser/head.js

@@ -29,7 +29,6 @@ function createAccount() {
 
 function cleanUpAccount(account) {
   info(`Cleaning up account ${account.toString()}`);
-  MailServices.accounts.removeIncomingServer(account.incomingServer, true);
   MailServices.accounts.removeAccount(account, true);
 }
 

mail/components/extensions/test/xpcshell/head.js

@@ -25,7 +25,6 @@ function createAccount() {
 
 function cleanUpAccount(account) {
   info(`Cleaning up account ${account.toString()}`);
-  MailServices.accounts.removeIncomingServer(account.incomingServer, true);
   MailServices.accounts.removeAccount(account, true);
 }
 

mail/test/mozmill/account/test-account-deletion.js

@@ -16,7 +16,7 @@ var MODULE_NAME = "test-account-deletion";
 var RELATIVE_ROOT = "../shared-modules";
 var MODULE_REQUIRES = ["folder-display-helpers", "window-helpers", "account-manager-helpers"];
 
-var gPopAccount, gImapAccount, gNntpAccount, gOriginalAccountCount;
+var gPopAccount, gImapAccount, gOriginalAccountCount;
 
 function setupModule(module) {
   for (let lib of MODULE_REQUIRES) {
@@ -84,6 +84,7 @@ function subtest_account_data_deletion1(amc) {
   assert_true(inboxFile.isFile());
 
   remove_account(gPopAccount, amc, true, false);
+  gPopAccount = null;
   assert_true(accountDir.exists());
 }
 
@@ -103,5 +104,6 @@ function subtest_account_data_deletion2(amc) {
   assert_true(inboxFile.isFile());
 
   remove_account(gImapAccount, amc, true, true);
+  gImapAccount = null;
   assert_false(accountDir.exists());
 }

mail/test/mozmill/account/test-account-tree.js

@@ -43,8 +43,11 @@ function setupModule(module) {
 }
 
 function teardownModule(module) {
-  // Remove our test account to leave the profile clean.
-  MailServices.accounts.removeAccount(gPopAccount);
+  if (gPopAccount) {
+    // Remove our test account to leave the profile clean.
+    MailServices.accounts.removeAccount(gPopAccount);
+    gPopAccount = null;
+  }
   // There should be only the original accounts left.
   assert_equals(MailServices.accounts.allServers.length, gOriginalAccountCount);
 }
@@ -168,6 +171,7 @@ function subtest_check_selection_after_account_deletion(amc) {
 
   // Remove our account.
   remove_account(gPopAccount, amc);
+  gPopAccount = null;
   // Now there should be only the original accounts left.
   assert_equals(MailServices.accounts.allServers.length, gOriginalAccountCount);
 

mail/test/mozmill/account/test-mail-account-setup-wizard.js

@@ -45,13 +45,15 @@ function remove_account_internal(amc, aAccount, aOutgoing) {
     // Remove the account and incoming server
     let serverId = aAccount.incomingServer.serverURI;
     MailServices.accounts.removeAccount(aAccount);
+    aAccount = null;
     if (serverId in win.accountArray)
       delete win.accountArray[serverId];
     win.selectServer(null, null);
 
     // Remove the outgoing server
+    let smtpKey = aOutgoing.key;
     MailServices.smtp.deleteServer(aOutgoing);
-    win.replaceWithDefaultSmtpServer(aOutgoing.key);
+    win.replaceWithDefaultSmtpServer(smtpKey);
   } catch (ex) {
     throw new Error("failure to remove account: " + ex + "\n");
   }

mail/test/mozmill/composition/test-draft-identity.js

@@ -222,6 +222,7 @@ function teardownModule(module) {
   // which effectively destroys it.
   MailServices.accounts.getIdentity(gIdentities[0].key).clearAllValues();
   MailServices.accounts.removeAccount(gAccount);
+  gAccount = null;
 
   // Clear our drafts.
   be_in_folder(gDrafts);

mail/test/mozmill/instrumentation/test-instrument-setup.js

@@ -74,11 +74,10 @@ function tearDownModule(module) {
   let account = MailServices.accounts.FindAccountForServer(incomingServer);
 
   let identity = account.defaultIdentity;
-  MailServices.accounts.removeIncomingServer(incomingServer, true);
   let outgoingServer = MailServices.smtp.getServerByKey(identity.smtpServerKey);
   assert_equals(outgoingServer.hostname, user.outgoingHost);
   MailServices.smtp.deleteServer(outgoingServer);
-  MailServices.accounts.removeAccount(account);
+  MailServices.accounts.removeAccount(account, true);
 
   Services.prefs.clearUserPref("mailnews.auto_config_url");
 }

mail/test/mozmill/shared-modules/test-account-manager-helpers.js

@@ -170,6 +170,7 @@ function remove_account(aAccount, aController, aRemoveAccount = true, aRemoveDat
     cdc.window.document.documentElement.acceptDialog();
   });
 
+  aAccount = null;
   // Use the Remove item in the Account actions menu.
   aController.click(aController.eid("accountActionsButton"));
   aController.click_menus_in_sequence(aController.e("accountActionsDropdown"),

mailnews/base/prefs/content/removeAccount.js

@@ -107,6 +107,9 @@ function removeAccount() {
       } catch (e) { /* It is OK if this fails. */ }
       // Remove account
       MailServices.accounts.removeAccount(account, removeData);
+      account = null;
+      delete window.arguments[0].account;
+      gServer = null;
       window.arguments[0].result = true;
     } else if (removeData) {
       // Remove files only.

mailnews/base/src/nsMsgAccountManager.cpp

@@ -588,10 +588,11 @@ nsMsgAccountManager::RemoveAccount(nsIMsgAccount *aAccount,
   NS_ENSURE_SUCCESS(rv, rv);
 
   bool accountRemoved = m_accounts.RemoveElement(aAccount);
+  if (!accountRemoved) return NS_ERROR_INVALID_ARG;
 
   rv = OutputAccountsPref();
   // If we couldn't write out the pref, restore the account.
-  if (NS_FAILED(rv) && accountRemoved) {
+  if (NS_FAILED(rv)) {
     m_accounts.AppendElement(aAccount);
     return rv;
   }

mailnews/imap/test/unit/test_imapAuthMethods.js

@@ -120,8 +120,8 @@ function nextTest() {
       incomingServer.closeCachedConnections();
     } while (incomingServer.serverBusy);
     incomingServer.shutdown();
-    incomingServer.clearAllValues();
     deleteIMAPServer(incomingServer);
+    incomingServer = null;
     MailServices.accounts.closeCachedConnections();
     MailServices.accounts.shutdownServers();
     MailServices.accounts.UnloadAccounts();
@@ -138,9 +138,8 @@ function nextTest() {
 function deleteIMAPServer(incomingServer) {
   if (!incomingServer)
     return;
-  MailServices.accounts.removeIncomingServer(incomingServer, false); // TODO cleanup files = true fails
-  // incomingServer = null;
-  MailServices.accounts.removeAccount(MailServices.accounts.defaultAccount);
+  // MailServices.accounts.removeIncomingServer(incomingServer, false);  // TODO cleanup files = true fails
+  MailServices.accounts.removeAccount(MailServices.accounts.defaultAccount, true);
 }
 
 

mailnews/test/resources/IMAPpump.js

@@ -129,6 +129,7 @@ function teardownIMAPPump() {
     IMAPPump.server.resetTest();
     IMAPPump.server.stop();
     MailServices.accounts.removeIncomingServer(IMAPPump.incomingServer, false);
+    IMAPPump.incomingServer = null;
     localAccountUtils.clearAll();
   } catch (ex) { dump(ex); }
 }