Bug 342632 - Allow defaultAccount to return success with nullptr result when there is no usable account. r=mkmelin
authoraceman <acelists@atlas.sk>
Mon, 19 Nov 2018 00:56:47 +0100
changeset 33730 38c2cc49dccc59a155c9060ef557dcadc2b978d3
parent 33729 c680423af71f9cc93a50876c0218967b85a8995b
child 33731 a6f1f63789af294ace5640de1e3d4cbeb9870877
push id388
push userclokep@gmail.com
push dateMon, 28 Jan 2019 20:54:56 +0000
reviewersmkmelin
bugs342632
Bug 342632 - Allow defaultAccount to return success with nullptr result when there is no usable account. r=mkmelin
mailnews/base/public/nsIMsgAccountManager.idl
mailnews/base/src/nsMsgAccountManager.cpp
mailnews/base/src/nsMsgAccountManager.h
--- a/mailnews/base/public/nsIMsgAccountManager.idl
+++ b/mailnews/base/public/nsIMsgAccountManager.idl
@@ -66,19 +66,23 @@ interface nsIMsgAccountManager : nsISupp
    * Gets the existing incoming server with the given key
    * if the server's type does not exist in the preference,
    * an error is returned/thrown
    */
   nsIMsgIncomingServer getIncomingServer(in ACString key);
 
   /* account list stuff */
 
-  /* defaultAccount should always be set if there are any accounts
-   * in the account manager. You can only set the defaultAccount to an
-   * account already in the account manager */
+  /**
+   * Returns the account that was marked as the default one.
+   * Only some server types can serve as default account.
+   * If there is no such account, null is returned.
+   * You can only set the defaultAccount to an
+   * account already in the account manager.
+   */
   attribute nsIMsgAccount defaultAccount;
 
   /**
    * Ordered list of all accounts, by the order they are in the prefs.
    * Accounts with hidden servers are not returned.
    * array of nsIMsgAccount
    */
   readonly attribute nsIArray accounts;
--- a/mailnews/base/src/nsMsgAccountManager.cpp
+++ b/mailnews/base/src/nsMsgAccountManager.cpp
@@ -644,19 +644,19 @@ nsMsgAccountManager::RemoveAccount(nsIMs
   rv  = OutputAccountsPref();
   // If we couldn't write out the pref, restore the account.
   if (NS_FAILED(rv) && accountRemoved)
   {
     m_accounts.AppendElement(aAccount);
     return rv;
   }
 
-  // if it's the default, clear the default account
+  // If it's the default, choose a new default account.
   if (m_defaultAccount.get() == aAccount)
-    SetDefaultAccount(nullptr);
+    AutosetDefaultAccount();
 
   // XXX - need to figure out if this is the last time this server is
   // being used, and only send notification then.
   // (and only remove from hashtable then too!)
   nsCOMPtr<nsIMsgIncomingServer> server;
   rv = aAccount->GetIncomingServer(getter_AddRefs(server));
   if (NS_SUCCEEDED(rv) && server)
     RemoveIncomingServer(server, aRemoveFiles);
@@ -717,94 +717,93 @@ nsMsgAccountManager::OutputAccountsPref(
     if (index)
       mAccountKeyList.Append(ACCOUNT_DELIMITER);
     mAccountKeyList.Append(accountKey);
   }
   return m_prefs->SetCharPref(PREF_MAIL_ACCOUNTMANAGER_ACCOUNTS,
                               mAccountKeyList);
 }
 
-/* get the default account. If no default account, pick the first account */
+/**
+ * Get the default account. If no default account, return null.
+ */
 NS_IMETHODIMP
 nsMsgAccountManager::GetDefaultAccount(nsIMsgAccount **aDefaultAccount)
 {
   NS_ENSURE_ARG_POINTER(aDefaultAccount);
 
   nsresult rv = LoadAccounts();
   NS_ENSURE_SUCCESS(rv, rv);
 
   if (!m_defaultAccount) {
-    uint32_t count = m_accounts.Length();
-    if (!count) {
-      *aDefaultAccount = nullptr;
-      return NS_ERROR_FAILURE;
-    }
-
     nsCString defaultKey;
     rv = m_prefs->GetCharPref(PREF_MAIL_ACCOUNTMANAGER_DEFAULTACCOUNT, defaultKey);
-
-    if (NS_SUCCEEDED(rv))
+    if (NS_SUCCEEDED(rv)) {
       rv = GetAccount(defaultKey, getter_AddRefs(m_defaultAccount));
-
-    if (NS_FAILED(rv) || !m_defaultAccount) {
-      nsCOMPtr<nsIMsgAccount> firstAccount;
-      uint32_t index;
-      bool foundValidDefaultAccount = false;
-      for (index = 0; index < count; index++) {
-        nsCOMPtr<nsIMsgAccount> account(m_accounts[index]);
-
-        // get incoming server
-        nsCOMPtr <nsIMsgIncomingServer> server;
-        // server could be null if created by an unloaded extension
-        (void) account->GetIncomingServer(getter_AddRefs(server));
-
-        bool canBeDefaultServer = false;
-        if (server)
-        {
-          server->GetCanBeDefaultServer(&canBeDefaultServer);
-          if (!firstAccount)
-            firstAccount = account;
-        }
-
-        // if this can serve as default server, set it as default and
-        // break out of the loop.
-        if (canBeDefaultServer) {
-          SetDefaultAccount(account);
-          foundValidDefaultAccount = true;
-          break;
-        }
-      }
-
-      if (!foundValidDefaultAccount) {
-        // Get the first account and use it.
-        // We need to fix this scenario, e.g. in bug 342632.
-        NS_WARNING("No valid default account found.");
-        if (firstAccount) {
-          NS_WARNING("Just using the first one (FIXME).");
-          SetDefaultAccount(firstAccount);
-        }
+      if (NS_SUCCEEDED(rv) && m_defaultAccount) {
+        bool canBeDefault = false;
+        rv = CheckDefaultAccount(m_defaultAccount, canBeDefault);
+        if (NS_FAILED(rv) || !canBeDefault)
+          m_defaultAccount = nullptr;
       }
     }
   }
 
-  if (!m_defaultAccount) {
-    // Absolutely no usable account found. Error out.
-    NS_ERROR("Default account is null, when not expected!");
-    *aDefaultAccount = nullptr;
-    return NS_ERROR_FAILURE;
+  NS_IF_ADDREF(*aDefaultAccount = m_defaultAccount);
+  return NS_OK;
+}
+
+/**
+ * Check if the given account can be default.
+ */
+nsresult
+nsMsgAccountManager::CheckDefaultAccount(nsIMsgAccount *aAccount, bool &aCanBeDefault)
+{
+  aCanBeDefault = false;
+  nsCOMPtr<nsIMsgIncomingServer> server;
+  // Server could be null if created by an unloaded extension.
+  nsresult rv = aAccount->GetIncomingServer(getter_AddRefs(server));
+  NS_ENSURE_SUCCESS(rv, rv);
+  if (server) {
+    // Check if server can be default.
+    rv = server->GetCanBeDefaultServer(&aCanBeDefault);
   }
-  NS_ADDREF(*aDefaultAccount = m_defaultAccount);
+  return rv;
+}
+
+/**
+ * Pick the first account that can be default and make it the default.
+ */
+nsresult
+nsMsgAccountManager::AutosetDefaultAccount()
+{
+  for (nsIMsgAccount* account : m_accounts) {
+    bool canBeDefault = false;
+    nsresult rv = CheckDefaultAccount(account, canBeDefault);
+    if (NS_SUCCEEDED(rv) && canBeDefault) {
+      return SetDefaultAccount(account);
+    }
+  }
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsMsgAccountManager::SetDefaultAccount(nsIMsgAccount *aDefaultAccount)
 {
+  if (!aDefaultAccount)
+    return NS_ERROR_INVALID_ARG;
+
   if (m_defaultAccount != aDefaultAccount)
   {
+    bool canBeDefault = false;
+    nsresult rv = CheckDefaultAccount(aDefaultAccount, canBeDefault);
+    if (NS_FAILED(rv) || !canBeDefault) {
+      // Report failure if we were explicitly asked to use an unusable server.
+      return NS_ERROR_INVALID_ARG;
+    }
     nsCOMPtr<nsIMsgAccount> oldAccount = m_defaultAccount;
     m_defaultAccount = aDefaultAccount;
     (void) setDefaultAccountPref(aDefaultAccount);
     (void) notifyDefaultServerChange(oldAccount, aDefaultAccount);
   }
   return NS_OK;
 }
 
--- a/mailnews/base/src/nsMsgAccountManager.h
+++ b/mailnews/base/src/nsMsgAccountManager.h
@@ -135,16 +135,26 @@ private:
                              const nsACString& type,
                              nsIMsgIncomingServer **_retval);
 
   nsresult createKeyedIdentity(const nsACString& key,
                                nsIMsgIdentity **_retval);
 
   nsresult GetLocalFoldersPrettyName(nsString &localFoldersName);
 
+  /**
+   * Check if the given account can be the set as the default account.
+   */
+  nsresult CheckDefaultAccount(nsIMsgAccount* aAccount, bool &aCanBeDefault);
+
+  /**
+   * Find a new account that can serve as default.
+   */
+  nsresult AutosetDefaultAccount();
+
   // sets the pref for the default server
   nsresult setDefaultAccountPref(nsIMsgAccount *aDefaultAccount);
 
   // Write out the accounts pref from the m_accounts list of accounts.
   nsresult OutputAccountsPref();
 
   // fires notifications to the appropriate root folders
   nsresult notifyDefaultServerChange(nsIMsgAccount *aOldAccount,