Bug 756316 - Cleanup redundant process in nsMsgLocalMailFolder::GetSubFolders and nsIMsgPluggableStore.discoverSubFolders. r=dbienvenu
authorHiroyuki Ikezoe <hiikezoe@mozilla-japan.org>
Thu, 14 Jun 2012 18:33:07 -0400
changeset 10437 9c0bd48911673eb51b4bf6245c42b42b1f85cdea
parent 10436 13b5b398738cffefe6c511db8fef272c991103ca
child 10438 72a1699e3ad5b636e408ff5cee7dd03c3f820b9b
push id7893
push userryanvm@gmail.com
push dateThu, 14 Jun 2012 22:35:28 +0000
treeherdercomm-central@9c0bd4891167 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdbienvenu
bugs756316
Bug 756316 - Cleanup redundant process in nsMsgLocalMailFolder::GetSubFolders and nsIMsgPluggableStore.discoverSubFolders. r=dbienvenu
mailnews/local/src/nsLocalMailFolder.cpp
mailnews/local/src/nsMsgBrkMBoxStore.cpp
mailnews/local/src/nsMsgMaildirStore.cpp
mailnews/local/test/unit/head_maillocal.js
mailnews/local/test/unit/test_localFolder.js
mailnews/local/test/unit/test_nsIMsgPluggableStore.js
mailnews/local/test/unit/xpcshell.ini
--- a/mailnews/local/src/nsLocalMailFolder.cpp
+++ b/mailnews/local/src/nsLocalMailFolder.cpp
@@ -207,68 +207,59 @@ NS_IMETHODIMP
 nsMsgLocalMailFolder::GetMsgDatabase(nsIMsgDatabase** aMsgDatabase)
 {
   return GetDatabaseWOReparse(aMsgDatabase);
 }
 
 NS_IMETHODIMP
 nsMsgLocalMailFolder::GetSubFolders(nsISimpleEnumerator **aResult)
 {
-  bool isServer;
-  nsresult rv = GetIsServer(&isServer);
-
   if (!mInitialized)
   {
     nsCOMPtr<nsIMsgIncomingServer> server;
-    rv = GetServer(getter_AddRefs(server));
+    nsresult rv = GetServer(getter_AddRefs(server));
     NS_ENSURE_SUCCESS(rv, NS_MSG_INVALID_OR_MISSING_SERVER);
     nsCOMPtr<nsIMsgPluggableStore> msgStore;
     // need to set this flag here to avoid infinite recursion
     mInitialized = true;
     rv = server->GetMsgStore(getter_AddRefs(msgStore));
     NS_ENSURE_SUCCESS(rv, rv);
     // This should add all existing folders as sub-folders of this folder.
     rv = msgStore->DiscoverSubFolders(this, true);
 
     nsCOMPtr<nsIFile> path;
     rv = GetFilePath(getter_AddRefs(path));
     if (NS_FAILED(rv))
       return rv;
 
-    bool exists, directory;
-    path->Exists(&exists);
-    if (!exists)
-      path->Create(nsIFile::DIRECTORY_TYPE, 0755);
-
+    bool directory;
     path->IsDirectory(&directory);
     if (directory)
     {
       SetFlag(nsMsgFolderFlags::Mail | nsMsgFolderFlags::Elided |
               nsMsgFolderFlags::Directory);
 
-      bool createdDefaultMailboxes = false;
-      nsCOMPtr<nsILocalMailIncomingServer> localMailServer;
-
+      bool isServer;
+      GetIsServer(&isServer);
       if (isServer)
       {
         nsCOMPtr<nsIMsgIncomingServer> server;
         rv = GetServer(getter_AddRefs(server));
         NS_ENSURE_SUCCESS(rv, NS_MSG_INVALID_OR_MISSING_SERVER);
+
+        nsCOMPtr<nsILocalMailIncomingServer> localMailServer;
         localMailServer = do_QueryInterface(server, &rv);
         NS_ENSURE_SUCCESS(rv, NS_MSG_INVALID_OR_MISSING_SERVER);
 
         // first create the folders on disk (as empty files)
         rv = localMailServer->CreateDefaultMailboxes(path);
-        NS_ENSURE_SUCCESS(rv, rv);
-        createdDefaultMailboxes = true;
-      }
-
-      // must happen after CreateSubFolders, or the folders won't exist.
-      if (createdDefaultMailboxes && isServer)
-      {
+        if (NS_FAILED(rv) && rv != NS_MSG_FOLDER_EXISTS)
+          return rv;
+
+        // must happen after CreateSubFolders, or the folders won't exist.
         rv = localMailServer->SetFlagsOnDefaultMailboxes();
         if (NS_FAILED(rv))
           return rv;
       }
     }
     UpdateSummaryTotals(false);
   }
 
--- a/mailnews/local/src/nsMsgBrkMBoxStore.cpp
+++ b/mailnews/local/src/nsMsgBrkMBoxStore.cpp
@@ -46,22 +46,19 @@ nsMsgBrkMBoxStore::~nsMsgBrkMBoxStore()
 }
 
 NS_IMPL_ISUPPORTS1(nsMsgBrkMBoxStore, nsIMsgPluggableStore)
 
 NS_IMETHODIMP nsMsgBrkMBoxStore::DiscoverSubFolders(nsIMsgFolder *aParentFolder,
                                                     bool aDeep)
 {
   NS_ENSURE_ARG_POINTER(aParentFolder);
-  bool isServer;
-  nsresult rv = aParentFolder->GetIsServer(&isServer);
-  NS_ENSURE_SUCCESS(rv, rv);
 
   nsCOMPtr<nsIFile> path;
-  rv = aParentFolder->GetFilePath(getter_AddRefs(path));
+  nsresult rv = aParentFolder->GetFilePath(getter_AddRefs(path));
   if (NS_FAILED(rv))
     return rv;
 
   bool exists, directory;
   path->Exists(&exists);
   if (!exists)
     path->Create(nsIFile::DIRECTORY_TYPE, 0755);
 
@@ -75,48 +72,17 @@ NS_IMETHODIMP nsMsgBrkMBoxStore::Discove
     dirFile->GetLeafName(leafName);
     leafName.AppendLiteral(".sbd");
     dirFile->SetLeafName(leafName);
     path = do_QueryInterface(dirFile);
     path->IsDirectory(&directory);
   }
 
   if (directory)
-  {
-    aParentFolder->SetFlag(nsMsgFolderFlags::Mail | nsMsgFolderFlags::Elided |
-                           nsMsgFolderFlags::Directory);
-
-    // now, discover those folders
     rv = AddSubFolders(aParentFolder, path, aDeep);
-    NS_ENSURE_SUCCESS(rv, rv);
-
-    nsCOMPtr<nsILocalMailIncomingServer> localMailServer;
-
-    if (isServer)
-    {
-      nsCOMPtr<nsIMsgIncomingServer> server;
-      rv = aParentFolder->GetServer(getter_AddRefs(server));
-      NS_ENSURE_SUCCESS(rv, NS_MSG_INVALID_OR_MISSING_SERVER);
-      localMailServer = do_QueryInterface(server, &rv);
-      NS_ENSURE_SUCCESS(rv, NS_MSG_INVALID_OR_MISSING_SERVER);
-
-      // first create the folders on disk (as empty files)
-      rv = localMailServer->CreateDefaultMailboxes(path);
-      if (NS_FAILED(rv) && rv != NS_MSG_FOLDER_EXISTS)
-        return rv;
-
-      // now, discover those folders
-      rv = AddSubFolders(aParentFolder, path, aDeep);
-      NS_ENSURE_SUCCESS(rv, rv);
-      
-      rv = localMailServer->SetFlagsOnDefaultMailboxes();
-      if (NS_FAILED(rv))
-        return rv;
-    }
-  }
   return rv;
 }
 
 NS_IMETHODIMP nsMsgBrkMBoxStore::CreateFolder(nsIMsgFolder *aParent,
                                               const nsAString &aFolderName,
                                               nsIMsgFolder **aResult)
 {
   nsCOMPtr<nsIFile> path;
--- a/mailnews/local/src/nsMsgMaildirStore.cpp
+++ b/mailnews/local/src/nsMsgMaildirStore.cpp
@@ -126,47 +126,19 @@ NS_IMETHODIMP nsMsgMaildirStore::Discove
   NS_ENSURE_SUCCESS(rv, rv);
 
   bool isServer, directory = false;
   aParentFolder->GetIsServer(&isServer);
   if (!isServer)
     GetDirectoryForFolder(path);
 
   path->IsDirectory(&directory);
-
   if (directory)
-  {
-    aParentFolder->SetFlag(nsMsgFolderFlags::Mail | nsMsgFolderFlags::Elided |
-                           nsMsgFolderFlags::Directory);
-
-    // discover existing folders
     rv = AddSubFolders(aParentFolder, path, aDeep);
-    NS_ENSURE_SUCCESS(rv, rv);
 
-    // if this is root folder - attempt to create default folders
-    if (isServer)
-    {
-      nsCOMPtr<nsIMsgIncomingServer> server;
-      rv = aParentFolder->GetServer(getter_AddRefs(server));
-      NS_ENSURE_SUCCESS(rv, NS_MSG_INVALID_OR_MISSING_SERVER);
-      nsCOMPtr<nsILocalMailIncomingServer> localMailServer;
-      localMailServer = do_QueryInterface(server, &rv);
-      NS_ENSURE_SUCCESS(rv, NS_MSG_INVALID_OR_MISSING_SERVER);
-
-      // first create the folders on disk
-      rv = localMailServer->CreateDefaultMailboxes(path);
-      NS_ENSURE_SUCCESS(rv, rv);
-      // now, discover those folders
-      rv = AddSubFolders(aParentFolder, path, aDeep);
-      NS_ENSURE_SUCCESS(rv, rv);
-      // and add flags on them
-      rv = localMailServer->SetFlagsOnDefaultMailboxes();
-      NS_ENSURE_SUCCESS(rv, rv);
-    }
-  }
   return (rv == NS_MSG_FOLDER_EXISTS) ? NS_OK : rv;
 }
 
 /**
  *Create a Maildir-style folder with "tmp", " and "cur" subfolders
  * but no "new" subfolder, because it's not sensical in the mail client context.
  ("new" directory is for messages on the server that haven't been seen by a
 *  mail client).
--- a/mailnews/local/test/unit/head_maillocal.js
+++ b/mailnews/local/test/unit/head_maillocal.js
@@ -103,8 +103,56 @@ function do_check_transaction(real, expe
   // closed after we have a chance to process it and not them. We therefore
   // excise this from the list
   if (real.them[real.them.length-1] == "QUIT")
     real.them.pop();
 
   do_check_eq(real.them.join(","), expected.join(","));
   dump("Passed test " + test + "\n");
 }
+
+function create_temporary_directory() {
+  let directory = Cc["@mozilla.org/file/directory_service;1"]
+    .getService(Ci.nsIProperties)
+    .get("TmpD", Ci.nsIFile);
+  directory.append("mailFolder");
+  directory.createUnique(Ci.nsIFile.DIRECTORY_TYPE, 0700);
+  return directory;
+}
+
+function create_sub_folders(parent, subFolders) {
+  parent.leafName = parent.leafName + ".sbd";
+  parent.create(Ci.nsIFile.DIRECTORY_TYPE, parseInt("0700", 8));
+
+  for (let folder in subFolders) {
+    let subFolder = parent.clone();
+    subFolder.append(subFolders[folder].name);
+    subFolder.create(Ci.nsIFile.NORMAL_FILE_TYPE, parseInt("0600", 8));
+    if (subFolders[folder].subFolders)
+      create_sub_folders(subFolder, subFolders[folder].subFolders);
+  }
+}
+
+function create_mail_directory(subFolders) {
+  let root = create_temporary_directory();
+
+  for (let folder in subFolders) {
+    if (!subFolders[folder].subFolders)
+      continue;
+    let directory = root.clone();
+    directory.append(subFolders[folder].name);
+    create_sub_folders(directory, subFolders[folder].subFolders);
+  }
+
+  return root;
+}
+
+function setup_mailbox(type, mailboxPath) {
+  let user = Cc["@mozilla.org/uuid-generator;1"]
+               .getService(Ci.nsIUUIDGenerator)
+               .generateUUID().toString();
+  let incomingServer =
+    MailServices.accounts.createIncomingServer(user, "Local Folder", type);
+  incomingServer.localPath = mailboxPath;
+
+  return incomingServer.rootFolder;
+}
+
new file mode 100644
--- /dev/null
+++ b/mailnews/local/test/unit/test_localFolder.js
@@ -0,0 +1,50 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this file,
+ * You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+/**
+ * nsIMsgFolder.subFolders tests
+ */
+
+function check_sub_folders(expected, actual) {
+  let i = 0;
+  while (actual.hasMoreElements()) {
+    let actualFolder = actual.getNext().QueryInterface(Ci.nsIMsgFolder);
+    do_check_eq(expected[i].name, actualFolder.name);
+    let pluggableStore = actualFolder.msgStore;
+    pluggableStore.discoverSubFolders(actualFolder, true);
+    do_check_eq(!!expected[i].subFolders, actualFolder.hasSubFolders);
+    if (actualFolder.hasSubFolders)
+      check_sub_folders(expected[i].subFolders, actualFolder.subFolders);
+    i++;
+  }
+}
+
+function test_default_mailbox(expected, type)
+{
+  let mailbox = setup_mailbox(type, create_temporary_directory());
+
+  check_sub_folders(expected, mailbox.subFolders);
+}
+
+function test_mailbox(expected, type) {
+  let mailbox = setup_mailbox(type, create_mail_directory(expected));
+
+  check_sub_folders(expected, mailbox.subFolders);
+}
+
+function run_test() {
+  test_default_mailbox([{ name: "Trash" }, { name: "Outbox" }], "none");
+  test_default_mailbox([{ name: "Inbox" }, { name: "Trash" }], "pop3");
+
+  test_mailbox([
+    {
+      name: "Inbox",
+      subFolders: [ { name: "sub4" } ]
+    },
+    {
+      name: "Trash"
+    }
+  ], "pop3");
+
+}
--- a/mailnews/local/test/unit/test_nsIMsgPluggableStore.js
+++ b/mailnews/local/test/unit/test_nsIMsgPluggableStore.js
@@ -6,63 +6,16 @@
  * nsIMsgPluggableStore interface tests
  */
 
 var gPluggableStores = [
   "@mozilla.org/msgstore/berkeleystore;1",
   "@mozilla.org/msgstore/maildirstore;1"
 ];
 
-function create_temporary_directory() {
-  let directory = Cc["@mozilla.org/file/directory_service;1"]
-    .getService(Ci.nsIProperties)
-    .get("TmpD", Ci.nsIFile);
-  directory.append("mailFolder");
-  directory.createUnique(Ci.nsIFile.DIRECTORY_TYPE, 0700);
-  return directory;
-}
-
-function create_sub_folders(parent, subFolders) {
-  parent.leafName = parent.leafName + ".sbd";
-  parent.create(Ci.nsIFile.DIRECTORY_TYPE, parseInt("0700", 8));
-
-  for (let folder in subFolders) {
-    let subFolder = parent.clone();
-    subFolder.append(subFolders[folder].name);
-    subFolder.create(Ci.nsIFile.NORMAL_FILE_TYPE, parseInt("0600", 8));
-    if (subFolders[folder].subFolders)
-      create_sub_folders(subFolder, subFolders[folder].subFolders);
-  }
-}
-
-function create_mail_directory(subFolders) {
-  let root = create_temporary_directory();
-
-  for (let folder in subFolders) {
-    if (!subFolders[folder].subFolders)
-      continue;
-    let directory = root.clone();
-    directory.append(subFolders[folder].name);
-    create_sub_folders(directory, subFolders[folder].subFolders);
-  }
-
-  return root;
-}
-
-function setup_mailbox(type, mailboxPath) {
-  let user = Cc["@mozilla.org/uuid-generator;1"]
-               .getService(Ci.nsIUUIDGenerator)
-               .generateUUID().toString();
-  let incomingServer =
-    MailServices.accounts.createIncomingServer(user, "Local Folder", type);
-  incomingServer.localPath = mailboxPath;
-
-  return incomingServer.rootFolder;
-}
-
 function test_discoverSubFolders() {
   let mailbox = setup_mailbox("none", create_temporary_directory());
 
   mailbox.msgStore.discoverSubFolders(mailbox, true);
 
 }
 
 function run_all_tests() {
--- a/mailnews/local/test/unit/xpcshell.ini
+++ b/mailnews/local/test/unit/xpcshell.ini
@@ -1,14 +1,15 @@
 [DEFAULT]
 head = head_maillocal.js
 tail = tail_local.js
 
 [test_bug457168.js]
 [test_fileName.js]
+[test_localFolder.js]
 [test_mailboxContentLength.js]
 [test_mailboxProtocol.js]
 [test_movemailDownload.js]
 [test_msgCopy.js]
 [test_msgIDParsing.js]
 [test_nsIMsgLocalMailFolder.js]
 [test_nsIMsgPluggableStore.js]
 [test_over2GBMailboxes.js]