Bug 1124948 - search folders dont work on maildir, r=jcranmer, a=rkent
authorR Kent James <kent@caspia.com>
Thu, 26 Mar 2015 12:26:45 -0700
changeset 25826 bc07ed1b0df8bc8aeb4cf4fab28d4c730d9a939a
parent 25825 c91007a6f7b307114ab27cae89f93b5759577afb
child 25827 e600ae00a71887208dd93ffa9fe421da7e3aed15
push id1850
push userclokep@gmail.com
push dateWed, 08 Mar 2017 19:29:12 +0000
treeherdercomm-esr52@028df196b2d9 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjcranmer, rkent
bugs1124948
Bug 1124948 - search folders dont work on maildir, r=jcranmer, a=rkent
mail/base/test/unit/head_mailbase_maildir.js
mail/base/test/unit/test_viewWrapper_virtualFolder.js
mail/base/test/unit/xpcshell.ini
mail/base/test/unit/xpcshell_maildir.ini
mailnews/base/src/virtualFolderWrapper.js
mailnews/db/msgdb/src/nsMailDatabase.cpp
mailnews/db/msgdb/src/nsMsgDatabase.cpp
mailnews/local/src/nsLocalMailFolder.cpp
mailnews/local/src/nsMsgBrkMBoxStore.cpp
new file mode 100644
--- /dev/null
+++ b/mail/base/test/unit/head_mailbase_maildir.js
@@ -0,0 +1,5 @@
+// alternate head to set maildir as default
+load("head_mailbase.js");
+do_print("Running test with maildir");
+Services.prefs.setCharPref("mail.serverDefaultStoreContractID",
+                           "@mozilla.org/msgstore/maildirstore;1");
--- a/mail/base/test/unit/test_viewWrapper_virtualFolder.js
+++ b/mail/base/test/unit/test_viewWrapper_virtualFolder.js
@@ -35,16 +35,17 @@ function test_virtual_folder_single_load
 
   do_check_true(viewWrapper.isVirtual);
 
   assert_equals(gMockViewWrapperListener.allMessagesLoadedEventCount, 1,
                 "Should only have received a single all messages loaded" +
                 " notification!");
 
   verify_messages_in_view(setOne, viewWrapper);
+  virtFolder.parent.propagateDelete(virtFolder, true, null);
 }
 
 /**
  * Make sure we open a virtual folder backed by a single underlying folder
  *  correctly; one constraint.
  */
 function test_virtual_folder_single_load_simple_pred() {
   let viewWrapper = make_view_wrapper();
@@ -52,16 +53,17 @@ function test_virtual_folder_single_load
   let [folderOne, oneSubjFoo, oneNopers] = make_folder_with_sets([
     {subject: "foo"}, {}]);
 
   let virtFolder = make_virtual_folder([folderOne],
                                        {subject: "foo"});
   yield async_view_open(viewWrapper, virtFolder);
 
   verify_messages_in_view(oneSubjFoo, viewWrapper);
+  virtFolder.parent.propagateDelete(virtFolder, true, null);
 }
 
 /**
  * Make sure we open a virtual folder backed by a single underlying folder
  *  correctly; two constraints ANDed together.
  */
 function test_virtual_folder_single_load_complex_pred() {
   let viewWrapper = make_view_wrapper();
@@ -73,16 +75,17 @@ function test_virtual_folder_single_load
                            {subject: "foo", from: whoBar}, {}]);
 
   let virtFolder = make_virtual_folder([folderOne],
                                        {subject: "foo", from: "bar"},
                                        /* and? */ true);
   yield async_view_open(viewWrapper, virtFolder);
 
   verify_messages_in_view(oneBoth, viewWrapper);
+  virtFolder.parent.propagateDelete(virtFolder, true, null);
 }
 
 /**
  * Open a single-backed virtual folder, verify, open another single-backed
  *  virtual folder, verify.  We are testing our ability to change folders
  *  without exploding.
  */
 function test_virtual_folder_single_load_after_load() {
@@ -97,32 +100,35 @@ function test_virtual_folder_single_load
 
   // use "bar" instead of "foo" to make sure constraints are properly changing
   let [folderTwo, twoSubjBar, twoNopers] = make_folder_with_sets([
     {subject: "bar"}, {}]);
   let virtTwo = make_virtual_folder([folderTwo],
                                     {subject: "bar"});
   yield async_view_open(viewWrapper, virtTwo);
   verify_messages_in_view([twoSubjBar], viewWrapper);
+  virtOne.parent.propagateDelete(virtOne, true, null);
+  virtTwo.parent.propagateDelete(virtTwo, true, null);
 }
 
 /**
  * Make sure we open a virtual folder backed by multiple underlying folders
  *  correctly; no constraints.
  */
 function test_virtual_folder_multi_load_no_pred() {
   let viewWrapper = make_view_wrapper();
 
   let [folderOne, setOne] = make_folder_with_sets(1);
   let [folderTwo, setTwo] = make_folder_with_sets(1);
 
   let virtFolder = make_virtual_folder([folderOne, folderTwo], {});
   yield async_view_open(viewWrapper, virtFolder);
 
   verify_messages_in_view([setOne, setTwo], viewWrapper);
+  virtFolder.parent.propagateDelete(virtFolder, true, null);
 }
 
 /**
  * Make sure the sort order of a virtual folder backed by multiple underlying
  * folders is persistent.
  */
 function test_virtual_folder_multi_sortorder_persistence() {
   let viewWrapper = make_view_wrapper();
@@ -139,17 +145,17 @@ function test_virtual_folder_multi_sorto
                    Ci.nsMsgViewSortOrder.ascending);
 
   viewWrapper.close();
   yield async_view_open(viewWrapper, virtFolder);
   assert_equals(viewWrapper.primarySortType, Ci.nsMsgViewSortType.bySubject,
                "should have remembered sort type.");
   assert_equals(viewWrapper.primarySortOrder, Ci.nsMsgViewSortOrder.ascending,
                "should have remembered sort order.");
-
+  virtFolder.parent.propagateDelete(virtFolder, true, null);
 }
 
 /**
  * Make sure we open a virtual folder backed by multiple underlying folders
  *  correctly; one constraint.
  */
 function test_virtual_folder_multi_load_simple_pred() {
   let viewWrapper = make_view_wrapper();
@@ -159,16 +165,17 @@ function test_virtual_folder_multi_load_
   let [folderTwo, twoSubjFoo, twoNopers] = make_folder_with_sets([
     {subject: "foo"}, {}]);
 
   let virtFolder = make_virtual_folder([folderOne, folderTwo],
                                        {subject: "foo"});
   yield async_view_open(viewWrapper, virtFolder);
 
   verify_messages_in_view([oneSubjFoo, twoSubjFoo], viewWrapper);
+  virtFolder.parent.propagateDelete(virtFolder, true, null);
 }
 
 /**
  * Make sure we open a virtual folder backed by multiple underlying folders
  *  correctly; two constraints ANDed together.
  */
 function test_virtual_folder_multi_load_complex_pred() {
   let viewWrapper = make_view_wrapper();
@@ -183,46 +190,49 @@ function test_virtual_folder_multi_load_
                            {subject: "foo", from: whoBar}, {}]);
 
   let virtFolder = make_virtual_folder([folderOne, folderTwo],
                                        {subject: "foo", from: "bar"},
                                        /* and? */ true);
   yield async_view_open(viewWrapper, virtFolder);
 
   verify_messages_in_view([oneBoth, twoBoth], viewWrapper);
+  virtFolder.parent.propagateDelete(virtFolder, true, null);
 }
 
 function test_virtual_folder_multi_load_alotta_folders_no_pred() {
   let viewWrapper = make_view_wrapper();
 
   const folderCount = 4;
   const messageCount = 64;
 
   let [folders, setOne] = make_folders_with_sets(folderCount,
                                                  [{count: messageCount}]);
 
   let virtFolder = make_virtual_folder(folders, {});
   yield async_view_open(viewWrapper, virtFolder);
 
   verify_messages_in_view([setOne], viewWrapper);
+  virtFolder.parent.propagateDelete(virtFolder, true, null);
 }
 
 function test_virtual_folder_multi_load_alotta_folders_simple_pred() {
   let viewWrapper = make_view_wrapper();
 
   const folderCount = 16;
   const messageCount = 256;
 
   let [folders, setOne] = make_folders_with_sets(folderCount,
     [{subject: "foo", count: messageCount}]);
 
   let virtFolder = make_virtual_folder(folders, {subject: "foo"});
   yield async_view_open(viewWrapper, virtFolder);
 
   verify_messages_in_view([setOne], viewWrapper);
+  virtFolder.parent.propagateDelete(virtFolder, true, null);
 }
 
 /**
  * Make sure that opening a virtual folder backed by multiple real folders, then
  *  opening another virtual folder of the same variety works without explosions.
  */
 function test_virtual_folder_multi_load_after_load() {
   let viewWrapper = make_view_wrapper();
@@ -238,16 +248,18 @@ function test_virtual_folder_multi_load_
     {subject: "bar"}, {}]);
   let virtTwo = make_virtual_folder(foldersTwo,
                                     {subject: "bar"});
   yield async_view_open(viewWrapper, virtTwo);
   verify_messages_in_view([twoSubjBar], viewWrapper);
 
   yield async_view_open(viewWrapper, virtOne);
   verify_messages_in_view([oneSubjFoo], viewWrapper);
+  virtOne.parent.propagateDelete(virtOne, true, null);
+  virtTwo.parent.propagateDelete(virtTwo, true, null);
 }
 
 /**
  * Make sure that opening a virtual folder backed by a single real folder, then
  *  a multi-backed one, then the single-backed one again doesn't explode.
  *
  * This is just test_virtual_folder_multi_load_after_load with foldersOne told
  *  to create just a single folder.
@@ -266,32 +278,35 @@ function test_virtual_folder_combo_load_
     {subject: "bar"}, {}]);
   let virtTwo = make_virtual_folder(foldersTwo,
                                     {subject: "bar"});
   yield async_view_open(viewWrapper, virtTwo);
   verify_messages_in_view([twoSubjBar], viewWrapper);
 
   yield async_view_open(viewWrapper, virtOne);
   verify_messages_in_view([oneSubjFoo], viewWrapper);
+  virtOne.parent.propagateDelete(virtOne, true, null);
+  virtTwo.parent.propagateDelete(virtTwo, true, null);
 }
 
 /**
  * Make sure that if a server is listed in a virtual folder's search Uris that
  *  it does not get into our list of _underlyingFolders.
  */
 function test_virtual_folder_filters_out_servers() {
   let viewWrapper = make_view_wrapper();
 
   let [folders] = make_folders_with_sets(2, []);
   folders.push(folders[0].rootFolder);
   let virtFolder = make_virtual_folder(folders, {});
   yield async_view_open(viewWrapper, virtFolder);
 
   assert_equals(viewWrapper._underlyingFolders.length, 2,
                 "Server folder should have been filtered out.");
+  virtFolder.parent.propagateDelete(virtFolder, true, null);
 }
 
 /**
  * Verify that if one of the folders backing our virtual folder is deleted that
  *  we do not explode.  Then verify that if we remove the rest of them that the
  *  view wrapper closes itself.
  */
 function test_virtual_folder_underlying_folder_deleted() {
@@ -313,16 +328,18 @@ function test_virtual_folder_underlying_
   verify_messages_in_view([twoSubjFoo], viewWrapper);
 
   // this one is not async though, because we are expecting to close the wrapper
   //  and ignore the view entirely, so do not yield.
   delete_folder(folderTwo);
 
   // now the view wrapper should have closed itself.
   do_check_eq(null, viewWrapper.displayedFolder);
+  // This fails because virtFolder.parent is null, not sure why
+  //virtFolder.parent.propagateDelete(virtFolder, true, null);
 }
 
 /* ===== Virtual Folder, Mail Views ===== */
 
 /*
  * We do not need to test all of the mail view permutations, realFolder
  *  already did that.  We just need to make sure it works at all.
  */
@@ -350,16 +367,17 @@ function test_virtual_folder_mail_views_
   verify_messages_in_view([fooOne, fooThree], viewWrapper);
 
   // make those things un-read again.
   fooTwo.setRead(false);
   // I thought this was a quick search limitation, but XFVF needs it to, at
   //  least for the unread case.
   yield async_view_refresh(viewWrapper);
   verify_messages_in_view([fooOne, fooTwo, fooThree], viewWrapper);
+  virtFolder.parent.propagateDelete(virtFolder, true, null);
 }
 
 // This tests that clearing the new messages in a folder also clears the
 // new flag on saved search folders based on the real folder. This could be a
 // core view test, or a mozmill test, but I think the view wrapper stuff
 // is involved in some of the issues here, so this is a compromise.
 function test_virtual_folder_mail_new_handling() {
   let viewWrapper = make_view_wrapper();
@@ -380,16 +398,17 @@ function test_virtual_folder_mail_new_ha
   if (!folder.hasNewMessages)
     do_throw("folder should have new messages!");
 
   viewWrapper.close();
   folder.msgDatabase = null;
   folder.clearNewMessages();
   if (virtFolder.hasNewMessages)
     do_throw("saved search should not have new messages!");
+  virtFolder.parent.propagateDelete(virtFolder, true, null);
 }
 
 var tests = [
   // -- single-folder backed virtual folder
   test_virtual_folder_single_load_no_pred,
   test_virtual_folder_single_load_simple_pred,
   test_virtual_folder_single_load_complex_pred,
   test_virtual_folder_single_load_after_load,
--- a/mail/base/test/unit/xpcshell.ini
+++ b/mail/base/test/unit/xpcshell.ini
@@ -12,8 +12,10 @@ run-sequentially = Avoid bustage.
 [test_viewWrapper_logic.js]
 [test_viewWrapper_realFolder.js]
 [test_viewWrapper_virtualFolder.js]
 [test_viewWrapper_virtualFolderCustomTerm.js]
 run-sequentially = Avoid bustage.
 [test_windows_font_migration.js]
 skip-if = os != "win"
 [test_mailGlue_distribution.js]
+
+[include:xpcshell_maildir.ini]
new file mode 100644
--- /dev/null
+++ b/mail/base/test/unit/xpcshell_maildir.ini
@@ -0,0 +1,6 @@
+[DEFAULT]
+head = head_mailbase_maildir.js
+tail = tail_base.js
+support-files = distribution.ini resources/*
+
+[test_viewWrapper_virtualFolder.js]
--- a/mailnews/base/src/virtualFolderWrapper.js
+++ b/mailnews/base/src/virtualFolderWrapper.js
@@ -47,16 +47,17 @@ var VirtualFolderHelper = {
     msgFolder.setFlag(Ci.nsMsgFolderFlags.Virtual);
 
     let wrappedVirt = new VirtualFolderWrapper(msgFolder);
     wrappedVirt.searchTerms = aSearchTerms;
     wrappedVirt.searchFolders = aSearchFolders;
     wrappedVirt.onlineSearch = aOnlineSearch;
 
     let msgDatabase = msgFolder.msgDatabase;
+    msgDatabase.summaryValid = true;
     msgDatabase.Close(true);
 
     aParentFolder.NotifyItemAdded(msgFolder);
     MailServices.accounts.saveVirtualFolders();
 
     return wrappedVirt;
   },
 
--- a/mailnews/db/msgdb/src/nsMailDatabase.cpp
+++ b/mailnews/db/msgdb/src/nsMailDatabase.cpp
@@ -112,20 +112,27 @@ NS_IMETHODIMP nsMailDatabase::GetSummary
   nsresult rv = m_folder->GetMsgStore(getter_AddRefs(msgStore));
   NS_ENSURE_SUCCESS(rv, rv);
   return msgStore->IsSummaryFileValid(m_folder, this, aResult);
 }
 
 NS_IMETHODIMP nsMailDatabase::SetSummaryValid(bool aValid)
 {
   nsMsgDatabase::SetSummaryValid(aValid);
-  nsCOMPtr<nsIMsgPluggableStore> msgStore;
+
   if (!m_folder)
     return NS_ERROR_NULL_POINTER;
 
+  // If this is a virtual folder, there is no storage.
+  bool flag;
+  m_folder->GetFlag(nsMsgFolderFlags::Virtual, &flag);
+  if (flag)
+    return NS_OK;
+
+  nsCOMPtr<nsIMsgPluggableStore> msgStore;
   nsresult rv = m_folder->GetMsgStore(getter_AddRefs(msgStore));
   NS_ENSURE_SUCCESS(rv, rv);
   return msgStore->SetSummaryFileValid(m_folder, this, aValid);
 }
 
 NS_IMETHODIMP  nsMailDatabase::RemoveOfflineOp(nsIMsgOfflineImapOperation *op)
 {
   
--- a/mailnews/db/msgdb/src/nsMsgDatabase.cpp
+++ b/mailnews/db/msgdb/src/nsMsgDatabase.cpp
@@ -5716,19 +5716,18 @@ nsresult nsMsgDatabase::GetSearchResults
   return *table ? err : NS_ERROR_FAILURE;
 }
 
 NS_IMETHODIMP
 nsMsgDatabase::GetCachedHits(const char *aSearchFolderUri, nsISimpleEnumerator **aEnumerator)
 {
   nsCOMPtr <nsIMdbTable> table;
   nsresult err = GetSearchResultsTable(aSearchFolderUri, false, getter_AddRefs(table));
-  NS_ENSURE_SUCCESS(err, err);
   if (!table)
-    return NS_ERROR_FAILURE;
+    return NS_ERROR_FAILURE; // expected result for no cached hits
   nsMsgDBEnumerator* e = new nsMsgDBEnumerator(this, table, nullptr, nullptr);
   if (e == nullptr)
       return NS_ERROR_OUT_OF_MEMORY;
   NS_ADDREF(*aEnumerator = e);
   return NS_OK;
 }
 
 NS_IMETHODIMP nsMsgDatabase::RefreshCache(const char *aSearchFolderUri, uint32_t aNumKeys, nsMsgKey *aNewHits, uint32_t *aNumBadHits, nsMsgKey **aStaleHits)
--- a/mailnews/local/src/nsLocalMailFolder.cpp
+++ b/mailnews/local/src/nsLocalMailFolder.cpp
@@ -769,19 +769,24 @@ NS_IMETHODIMP nsMsgLocalMailFolder::Dele
   NS_ENSURE_SUCCESS(rv, rv);
 
   nsCOMPtr <nsIFile> summaryFile;
   rv = GetSummaryFile(getter_AddRefs(summaryFile));
   NS_ENSURE_SUCCESS(rv, rv);
 
   //Clean up .sbd folder if it exists.
   // Remove summary file.
-  summaryFile->Remove(false);
-
-  return msgStore->DeleteFolder(this);
+  rv = summaryFile->Remove(false);
+  NS_WARN_IF_FALSE(NS_SUCCEEDED(rv), "Could not delete msg summary file");
+
+  rv = msgStore->DeleteFolder(this);
+  if (rv == NS_ERROR_FILE_NOT_FOUND ||
+      rv == NS_ERROR_FILE_TARGET_DOES_NOT_EXIST)
+    rv = NS_OK; // virtual folders do not have a msgStore file
+  return rv;
 }
 
 NS_IMETHODIMP nsMsgLocalMailFolder::DeleteSubFolders(nsIArray *folders, nsIMsgWindow *msgWindow)
 {
   nsresult rv;
   bool isChildOfTrash;
   IsChildOfTrash(&isChildOfTrash);
 
--- a/mailnews/local/src/nsMsgBrkMBoxStore.cpp
+++ b/mailnews/local/src/nsMsgBrkMBoxStore.cpp
@@ -146,17 +146,18 @@ void nsMsgBrkMBoxStore::GetMailboxModPro
   // We'll simply return 0 on errors.
   *aDate = 0;
   *aSize = 0;
   nsCOMPtr<nsIFile> pathFile;
   nsresult rv = aFolder->GetFilePath(getter_AddRefs(pathFile));
   NS_ENSURE_SUCCESS_VOID(rv);
 
   rv = pathFile->GetFileSize(aSize);
-  NS_ENSURE_SUCCESS_VOID(rv);
+  if (NS_FAILED(rv))
+    return; // expected result for virtual folders
 
   PRTime lastModTime;
   rv = pathFile->GetLastModifiedTime(&lastModTime);
   NS_ENSURE_SUCCESS_VOID(rv);
 
   *aDate = (uint32_t) (lastModTime / PR_MSEC_PER_SEC);
 }