Bug 295088: Filter/Search term evaluator is extremely sub-optimal; p=Howard Chu <hyc@symas.com>, r=bienvenu, sr=dmose

mailnews/base/search/public/nsMsgSearchBoolExpression.h

@@ -20,6 +20,7 @@
  * the Initial Developer. All Rights Reserved.
  *
  * Contributor(s):
+ *   Howard Chu <hyc@symas.com>
  *
  * Alternatively, the contents of this file may be used under the terms of
  * either of the GNU General Public License Version 2 or later (the "GPL"),
@@ -51,13 +52,11 @@
    representing the search terms and their boolean operators as a binary
    expression tree. Each node in the tree consists of either
      (1) a boolean operator and two nsMsgSearchBoolExpressions or 
-	 (2) if the node is a leaf node then it contains a search term.
+     (2) if the node is a leaf node then it contains a search term.
-   With each search term that is part of the expression we also keep
+   With each search term that is part of the expression we may also keep
-   track of either an evaluation value (XP_BOOL) or a character string.
+   a character string. The character 
-   Evaluation values are used for offline searching. The character 
    string is used to store the IMAP/NNTP encoding of the search term. This
-   makes evaluating the expression (for offline) or generating a search
+   makes generating a search encoding (for online) easier.
-   encoding (for online) easier.
 
    For IMAP/NNTP: nsMsgSearchBoolExpression has/assumes knowledge about how
    AND and OR search terms are combined according to IMAP4 and NNTP protocol.
@@ -76,9 +75,7 @@ public:
 
 	// create a leaf node expression
 	nsMsgSearchBoolExpression(nsIMsgSearchTerm * aNewTerm,
-                              PRBool aEvaluationValue = PR_TRUE,
                               char * aEncodingString = NULL);         
-	nsMsgSearchBoolExpression(nsIMsgSearchTerm * aNewTerm, char * aEncodingString);
 
 	// create a non-leaf node expression containing 2 expressions
     // and a boolean operator
@@ -90,18 +87,19 @@ public:
 	~nsMsgSearchBoolExpression();  // recursively destroys all sub
                                    // expressions as well
 
-	// accesors
+	// accessors
     
     // Offline
-	static nsMsgSearchBoolExpression * AddSearchTerm (nsMsgSearchBoolExpression * aOrigExpr, nsIMsgSearchTerm * aNewTerm, PRBool aEvaluationValue);
+	static nsMsgSearchBoolExpression * AddSearchTerm (nsMsgSearchBoolExpression * aOrigExpr, nsIMsgSearchTerm * aNewTerm, char * aEncodingStr); // IMAP/NNTP
-	static nsMsgSearchBoolExpression * AddSearchTermWithEncoding (nsMsgSearchBoolExpression * aOrigExpr, nsIMsgSearchTerm * aNewTerm, char * aEncodingStr); // IMAP/NNTP
     static nsMsgSearchBoolExpression * AddExpressionTree(nsMsgSearchBoolExpression * aOrigExpr, nsMsgSearchBoolExpression * aExpression, PRBool aBoolOp);
 
     // parses the expression tree and all
-    // expressions underneath this node using
+    // expressions underneath this node to
-    // each EvaluationValue at each leaf to
     // determine if the end result is PR_TRUE or PR_FALSE.
-	PRBool OfflineEvaluate();
+	PRBool OfflineEvaluate(nsIMsgDBHdr *msgToMatch,
+          const char *defaultCharset, nsIMsgSearchScopeTerm *scope,
+          nsIMsgDatabase *db, const char *headers, PRUint32 headerSize,
+          PRBool Filtering);
     
     // assuming the expression is for online
     // searches, determine the length of the
@@ -112,15 +110,6 @@ public:
     // memory in buffer with 
     // the IMAP/NNTP encoding for the expression
 	void GenerateEncodeStr(nsCString * buffer); 
-protected:
-	// if we are a leaf node, all we have is a search term
-    // and a Evaluation value for that search term
-    
-    nsIMsgSearchTerm * m_term;
-	PRBool m_evalValue;
-    
-    // store IMAP/NNTP encoding for the search term if applicable
-	nsCString m_encodingStr;     
 
 	// if we are not a leaf node, then we have two other expressions
     // and a boolean operator
@@ -128,6 +117,13 @@ protected:
 	nsMsgSearchBoolExpression * m_rightChild;
 	nsMsgSearchBooleanOperator m_boolOp;
 
+protected:
+	// if we are a leaf node, all we have is a search term
+    
+    nsIMsgSearchTerm * m_term;
+    
+    // store IMAP/NNTP encoding for the search term if applicable
+	nsCString m_encodingStr;     
 
 	// internal methods
 
@@ -138,7 +134,6 @@ protected:
     // that by calling leftToRightAddTerm. If future forms of evaluation
     // need to be supported, add new methods here for proper tree construction.
 	nsMsgSearchBoolExpression * leftToRightAddTerm(nsIMsgSearchTerm * newTerm,
-                                                   PRBool EvaluationValue,
                                                    char * encodingStr); 
 };
 

mailnews/base/search/src/nsMsgFilter.cpp

@@ -22,6 +22,7 @@
  * Contributor(s):
  *   Pierre Phaneuf <pp@ludusdesign.com>
  *   Seth Spitzer <sspitzer@netscape.com>
+ *   Howard Chu <hyc@symas.com>
  *
  * Alternatively, the contents of this file may be used under the terms of
  * either of the GNU General Public License Version 2 or later (the "GPL"),
@@ -170,7 +171,8 @@ nsMsgRuleAction::GetStrValue(char **aStrValue)
 nsMsgFilter::nsMsgFilter():
     m_temporary(PR_FALSE),
     m_unparseable(PR_FALSE),
-    m_filterList(nsnull)
+    m_filterList(nsnull),
+    m_expressionTree(nsnull)
 {
   NS_NewISupportsArray(getter_AddRefs(m_termList));
   NS_NewISupportsArray(getter_AddRefs(m_actionList));
@@ -180,6 +182,7 @@ nsMsgFilter::nsMsgFilter():
 
 nsMsgFilter::~nsMsgFilter()
 {
+  delete m_expressionTree;
 }
 
 NS_IMPL_ISUPPORTS1(nsMsgFilter, nsIMsgFilter)
@@ -243,7 +246,8 @@ NS_IMETHODIMP nsMsgFilter::AddTerm(
 NS_IMETHODIMP nsMsgFilter::AppendTerm(nsIMsgSearchTerm * aTerm)
 {
     NS_ENSURE_TRUE(aTerm, NS_ERROR_NULL_POINTER);
-    
+    // invalidate expression tree if we're changing the terms
+    delete m_expressionTree;
     return m_termList->AppendElement(NS_STATIC_CAST(nsISupports*,aTerm));
 }
 
@@ -391,6 +395,7 @@ NS_IMETHODIMP nsMsgFilter::GetSearchTerms(nsISupportsArray **aResult)
 
 NS_IMETHODIMP nsMsgFilter::SetSearchTerms(nsISupportsArray *aSearchList)
 {
+    delete m_expressionTree;
     m_termList = aSearchList;
     return NS_OK;
 }
@@ -511,7 +516,7 @@ NS_IMETHODIMP nsMsgFilter::MatchHdr(nsIMsgDBHdr	*msgHdr, nsIMsgFolder *folder, n
   nsXPIDLCString folderCharset;
   folder->GetCharset(getter_Copies(folderCharset));
   nsresult rv = nsMsgSearchOfflineMail::MatchTermsForFilter(msgHdr, m_termList,
-                  folderCharset.get(),  scope,  db,  headers,  headersSize, pResult);
+                  folderCharset.get(),  scope,  db,  headers,  headersSize, &m_expressionTree, pResult);
   delete scope;
   return rv;
 }

mailnews/base/search/src/nsMsgFilter.h

@@ -21,6 +21,7 @@
  *
  * Contributor(s):
  *   David Bienvenu (bienvenu@nventure.com)
+ *   Howard Chu <hyc@symas.com>
  *
  * Alternatively, the contents of this file may be used under the terms of
  * either of the GNU General Public License Version 2 or later (the "GPL"),
@@ -44,6 +45,7 @@
 #include "nsIMsgFilter.h"
 #include "nsMsgSearchArray.h"
 #include "nsIMsgSearchScopeTerm.h"
+#include "nsMsgSearchBoolExpression.h"
 
 class nsMsgRuleAction : public nsIMsgRuleAction
 {
@@ -120,6 +122,7 @@ protected:
   nsCOMPtr<nsIMsgSearchScopeTerm> m_scope;         /* default for mail rules is inbox, but news rules could
                                                   have a newsgroup - LDAP would be invalid */
   nsCOMPtr<nsISupportsArray> m_actionList;
+  nsMsgSearchBoolExpression *m_expressionTree;
 };
 
 #endif

mailnews/base/search/src/nsMsgLocalSearch.cpp

@@ -20,6 +20,7 @@
  * the Initial Developer. All Rights Reserved.
  *
  * Contributor(s):
+ *   Howard Chu <hyc@symas.com>
  *
  * Alternatively, the contents of this file may be used under the terms of
  * either of the GNU General Public License Version 2 or later (the "GPL"),
@@ -67,19 +68,11 @@ extern "C"
 //----------------------------------------------------------------------------
 
 
-nsMsgSearchBoolExpression * nsMsgSearchBoolExpression::AddSearchTermWithEncoding(nsMsgSearchBoolExpression * aOrigExpr, nsIMsgSearchTerm * aNewTerm, char * aEncodingStr)
+nsMsgSearchBoolExpression * nsMsgSearchBoolExpression::AddSearchTerm(nsMsgSearchBoolExpression * aOrigExpr, nsIMsgSearchTerm * aNewTerm, char * aEncodingStr)
 // appropriately add the search term to the current expression and return a pointer to the
 // new expression. The encodingStr is the IMAP/NNTP encoding string for newTerm.
 {
-    return aOrigExpr->leftToRightAddTerm(aNewTerm, PR_FALSE, aEncodingStr);
+    return aOrigExpr->leftToRightAddTerm(aNewTerm, aEncodingStr);
-}
-
-nsMsgSearchBoolExpression * nsMsgSearchBoolExpression::AddSearchTerm(nsMsgSearchBoolExpression * aOrigExpr, nsIMsgSearchTerm * aNewTerm, PRBool aEvalValue)
-// appropriately add the search term to the current expression
-// Returns: a pointer to the new expression which includes this new search term
-{
-    return aOrigExpr->leftToRightAddTerm(aNewTerm, aEvalValue, nsnull);   // currently we build our expressions to
-                                                          // evaluate left to right.
 }
 
 nsMsgSearchBoolExpression * nsMsgSearchBoolExpression::AddExpressionTree(nsMsgSearchBoolExpression * aOrigExpr, nsMsgSearchBoolExpression * aExpression, PRBool aBoolOp)
@@ -100,19 +93,17 @@ nsMsgSearchBoolExpression::nsMsgSearchBoolExpression()
 {
     m_term = nsnull;
     m_boolOp = nsMsgSearchBooleanOp::BooleanAND;
-    m_evalValue = PR_FALSE;
     m_leftChild = nsnull;
     m_rightChild = nsnull;
 }
 
-nsMsgSearchBoolExpression::nsMsgSearchBoolExpression (nsIMsgSearchTerm * newTerm, PRBool evalValue, char * encodingStr) 
+nsMsgSearchBoolExpression::nsMsgSearchBoolExpression (nsIMsgSearchTerm * newTerm, char * encodingStr) 
 // we are creating an expression which contains a single search term (newTerm) 
 // and the search term's IMAP or NNTP encoding value for online search expressions AND
 // a boolean evaluation value which is used for offline search expressions.
 {
     m_term = newTerm;
     m_encodingStr = encodingStr;
-    m_evalValue = evalValue;
     m_boolOp = nsMsgSearchBooleanOp::BooleanAND;
 
     // this expression does not contain sub expressions
@@ -130,7 +121,6 @@ nsMsgSearchBoolExpression::nsMsgSearchBoolExpression (nsMsgSearchBoolExpression
     m_boolOp = boolOp;
 
     m_term = nsnull;
-    m_evalValue = PR_FALSE;
 }
 
 nsMsgSearchBoolExpression::~nsMsgSearchBoolExpression()
@@ -143,18 +133,17 @@ nsMsgSearchBoolExpression::~nsMsgSearchBoolExpression()
 }
 
 nsMsgSearchBoolExpression *
-nsMsgSearchBoolExpression::leftToRightAddTerm(nsIMsgSearchTerm * newTerm, PRBool evalValue, char * encodingStr)
+nsMsgSearchBoolExpression::leftToRightAddTerm(nsIMsgSearchTerm * newTerm, char * encodingStr)
 {
     // we have a base case where this is the first term being added to the expression:
     if (!m_term && !m_leftChild && !m_rightChild)
     {
         m_term = newTerm;
-        m_evalValue = evalValue;
         m_encodingStr = encodingStr;
         return this;
     }
 
-    nsMsgSearchBoolExpression * tempExpr = new nsMsgSearchBoolExpression (newTerm,evalValue,encodingStr);
+    nsMsgSearchBoolExpression * tempExpr = new nsMsgSearchBoolExpression (newTerm,encodingStr);
     if (tempExpr)  // make sure creation succeeded
     {
       PRBool booleanAnd;
@@ -169,36 +158,42 @@ nsMsgSearchBoolExpression::leftToRightAddTerm(nsIMsgSearchTerm * newTerm, PRBool
 }
 
 
-PRBool nsMsgSearchBoolExpression::OfflineEvaluate()
+// returns PR_TRUE or PR_FALSE depending on what the current expression evaluates to. 
-// returns PR_TRUE or PR_FALSE depending on what the current expression evaluates to. Since this is
+PRBool nsMsgSearchBoolExpression::OfflineEvaluate(nsIMsgDBHdr *msgToMatch, const char *defaultCharset,
-// offline, when we created the expression we stored an evaluation value for each search term in 
+  nsIMsgSearchScopeTerm *scope, nsIMsgDatabase *db, const char *headers,
-// the expression. These are the values we use to determine if the expression is PR_TRUE or PR_FALSE.
+  PRUint32 headerSize, PRBool Filtering)
 {
+    PRBool result = PR_TRUE;    // always default to false positives
+    PRBool isAnd;
+
     if (m_term) // do we contain just a search term?
-        return m_evalValue;
+    {
+      nsMsgSearchOfflineMail::ProcessSearchTerm(msgToMatch, m_term,
+        defaultCharset, scope, db, headers, headerSize, Filtering, &result);
+      return result;
+    }
     
     // otherwise we must recursively determine the value of our sub expressions
-    PRBool result1 = PR_TRUE;    // always default to false positives
+
-    PRBool result2 = PR_TRUE;
+    isAnd = (m_boolOp == nsMsgSearchBooleanOp::BooleanAND);
 
     if (m_leftChild)
     {
-        result1 = m_leftChild->OfflineEvaluate();
+        result = m_leftChild->OfflineEvaluate(msgToMatch, defaultCharset,
-        if (m_boolOp == nsMsgSearchBooleanOp::BooleanOR && result1 ||
+          scope, db, headers, headerSize, Filtering);
-           (m_boolOp == nsMsgSearchBooleanOp::BooleanAND && !result1))
+        // If (TRUE and OR) or (FALSE and AND) return result
-            return result1;
+        if (result ^ isAnd)
+          return result;
     }
 
+    // If we got this far, either there was no leftChild (which is impossible)
+    // or we got (FALSE and OR) or (TRUE and AND) from the first result. That
+    // means the outcome depends entirely on the rightChild.
     if (m_rightChild)
-        result2 = m_rightChild->OfflineEvaluate();
+        result = m_rightChild->OfflineEvaluate(msgToMatch, defaultCharset,
+          scope, db, headers, headerSize, Filtering);
 
-    if (m_boolOp == nsMsgSearchBooleanOp::BooleanOR)
+    return result;
-        return (result1 || result2) ? PR_TRUE : PR_FALSE;
-
-    if (m_boolOp == nsMsgSearchBooleanOp::BooleanAND && result1 && result2) 
-        return PR_TRUE;
-
-    return PR_FALSE;
 }
 
 // ### Maybe we can get rid of these because of our use of nsString???
@@ -350,9 +345,10 @@ nsMsgSearchOfflineMail::MatchTermsForFilter(nsIMsgDBHdr *msgToMatch,
                                             nsIMsgDatabase * db, 
                                             const char * headers,
                                             PRUint32 headerSize,
+                                            nsMsgSearchBoolExpression ** aExpressionTree,
                                             PRBool *pResult)
 {
-    return MatchTerms(msgToMatch, termList, defaultCharset, scope, db, headers, headerSize, PR_TRUE, pResult);
+    return MatchTerms(msgToMatch, termList, defaultCharset, scope, db, headers, headerSize, PR_TRUE, aExpressionTree, pResult);
 }
 
 // static method which matches a header against a list of search terms.
@@ -362,46 +358,28 @@ nsMsgSearchOfflineMail::MatchTermsForSearch(nsIMsgDBHdr *msgToMatch,
                                             const char *defaultCharset,
                                             nsIMsgSearchScopeTerm *scope,
                                             nsIMsgDatabase *db,
+                                            nsMsgSearchBoolExpression ** aExpressionTree,
                                             PRBool *pResult)
 {
 
-    return MatchTerms(msgToMatch, termList, defaultCharset, scope, db, nsnull, 0, PR_FALSE, pResult);
+    return MatchTerms(msgToMatch, termList, defaultCharset, scope, db, nsnull, 0, PR_FALSE, aExpressionTree, pResult);
 }
 
-nsresult nsMsgSearchOfflineMail::ConstructExpressionTree(nsIMsgDBHdr *msgToMatch,
+nsresult nsMsgSearchOfflineMail::ConstructExpressionTree(nsISupportsArray * termList,
-                                            nsISupportsArray * termList,
+                                            PRUint32 termCount,
                                             PRUint32 &aStartPosInList,
-                                            const char *defaultCharset,
+                                            nsMsgSearchBoolExpression ** aExpressionTree)
-                                            nsIMsgSearchScopeTerm * scope,
-                                            nsIMsgDatabase * db, 
-                                            const char * headers,
-                                            PRUint32 headerSize,
-                                            PRBool Filtering,
-                                            nsMsgSearchBoolExpression ** aExpressionTree,
-											PRBool *pResult)
 {
-  PRBool result;
+  nsMsgSearchBoolExpression * finalExpression = *aExpressionTree;
 
-  NS_ENSURE_ARG_POINTER(pResult);
+  if (!finalExpression)
+    finalExpression = new nsMsgSearchBoolExpression(); 
 
-  *pResult = PR_FALSE;
+  while (aStartPosInList < termCount)
-
-  // Don't even bother to look at expunged messages awaiting compression
-  PRUint32 msgFlags;
-  msgToMatch->GetFlags(&msgFlags);
-  if (msgFlags & MSG_FLAG_EXPUNGED)
-    result = PR_FALSE;
-    
-  PRUint32 count;
-  termList->Count(&count);
-
-  nsMsgSearchBoolExpression * finalExpression = new nsMsgSearchBoolExpression(); 
-
-  while (aStartPosInList < count)
   {
       nsCOMPtr<nsIMsgSearchTerm> pTerm;
       termList->QueryElementAt(aStartPosInList, NS_GET_IID(nsIMsgSearchTerm), (void **)getter_AddRefs(pTerm));
-      NS_ASSERTION (msgToMatch, "couldn't get term to match");
+      NS_ASSERTION (pTerm, "couldn't get term to match");
 
       PRBool beginsGrouping;
       PRBool endsGrouping;
@@ -412,12 +390,8 @@ nsresult nsMsgSearchOfflineMail::ConstructExpressionTree(nsIMsgDBHdr *msgToMatch
       {
           //temporarily turn off the grouping for our recursive call
           pTerm->SetBeginsGrouping(PR_FALSE); 
-          // recursively process this inner expression
           nsMsgSearchBoolExpression * innerExpression = new nsMsgSearchBoolExpression(); 
 
-          ConstructExpressionTree(msgToMatch, termList, aStartPosInList, defaultCharset, scope, db, headers, 
-              headerSize, Filtering, &innerExpression, &result);
-
           // the first search term in the grouping is the one that holds the operator for how this search term
           // should be joined with the expressions to it's left. 
           PRBool booleanAnd;
@@ -426,29 +400,25 @@ nsresult nsMsgSearchOfflineMail::ConstructExpressionTree(nsIMsgDBHdr *msgToMatch
           // now add this expression tree to our overall expression tree...
           finalExpression = nsMsgSearchBoolExpression::AddExpressionTree(finalExpression, innerExpression, booleanAnd);
 
+          // recursively process this inner expression
+          ConstructExpressionTree(termList, termCount, aStartPosInList, 
+            &finalExpression->m_rightChild);
+
           // undo our damage
           pTerm->SetBeginsGrouping(PR_TRUE); 
 
       }
       else
       {
-        ProcessSearchTerm(msgToMatch, pTerm, defaultCharset, scope, db, headers, headerSize, Filtering, &result);
+        finalExpression = nsMsgSearchBoolExpression::AddSearchTerm(finalExpression, pTerm, nsnull);    // add the term to the expression tree
-        finalExpression = nsMsgSearchBoolExpression::AddSearchTerm(finalExpression, pTerm, result);    // added the term and its value to the expression tree
 
         if (endsGrouping)
-        {
+          break;
-          // okay, this term marks the end of a grouping...kick out of this function.
-          *pResult = result; 
-          *aExpressionTree = finalExpression;
-          return NS_OK;
-
-        }
       }
 
       aStartPosInList++;
   } // while we still have terms to process in this group
 
-  *pResult = PR_TRUE;
   *aExpressionTree = finalExpression;
 
   return NS_OK; 
@@ -619,17 +589,35 @@ nsresult nsMsgSearchOfflineMail::MatchTerms(nsIMsgDBHdr *msgToMatch,
                                             const char * headers,
                                             PRUint32 headerSize,
                                             PRBool Filtering,
+                                            nsMsgSearchBoolExpression ** aExpressionTree,
 											PRBool *pResult) 
 {
-  nsMsgSearchBoolExpression * expressionTree = nsnull; 
+  NS_ENSURE_ARG(aExpressionTree);
-  PRUint32 initialPos = 0; 
+  nsresult err;
-  nsresult err = ConstructExpressionTree(msgToMatch, termList, initialPos, defaultCharset, scope, db, headers, headerSize, 
+
-                          Filtering, &expressionTree, pResult);
+  // Don't even bother to look at expunged messages awaiting compression
+  PRUint32 msgFlags;
+  msgToMatch->GetFlags(&msgFlags);
+  if (msgFlags & MSG_FLAG_EXPUNGED)
+  {
+    *pResult = PR_FALSE;
+    return NS_OK;
+  }
+
+  if (!*aExpressionTree)
+  {
+    PRUint32 initialPos = 0; 
+    PRUint32 count;
+    termList->Count(&count);
+    err = ConstructExpressionTree(termList, count, initialPos, aExpressionTree);
+    if (NS_FAILED(err))
+      return err;
+  }
 
   // evaluate the expression tree and return the result
-  if (NS_SUCCEEDED(err) && expressionTree)
+  if (NS_SUCCEEDED(err) && *aExpressionTree)
-    *pResult = expressionTree->OfflineEvaluate();
+    *pResult = (*aExpressionTree)->OfflineEvaluate(msgToMatch,
-  delete expressionTree;
+                 defaultCharset, scope, db, headers, headerSize, Filtering);
 
   return err;
 }
@@ -642,6 +630,7 @@ nsresult nsMsgSearchOfflineMail::Search (PRBool *aDone)
   NS_ENSURE_ARG(aDone);
   nsresult dbErr = NS_OK;
   nsCOMPtr<nsIMsgDBHdr> msgDBHdr;
+  nsMsgSearchBoolExpression *expressionTree = nsnull;
   
   const PRUint32 kTimeSliceInMS = 200;
 
@@ -678,7 +667,7 @@ nsresult nsMsgSearchOfflineMail::Search (PRBool *aDone)
           GetSearchCharsets(getter_Copies(nullCharset), getter_Copies(folderCharset));
           NS_ConvertUCS2toUTF8 charset(folderCharset);
           // Is this message a hit?
-          err = MatchTermsForSearch (msgDBHdr, m_searchTerms, charset.get(), m_scope, m_db, &match);
+          err = MatchTermsForSearch (msgDBHdr, m_searchTerms, charset.get(), m_scope, m_db, &expressionTree, &match);
           // Add search hits to the results list
           if (NS_SUCCEEDED(err) && match)
           {
@@ -696,6 +685,8 @@ nsresult nsMsgSearchOfflineMail::Search (PRBool *aDone)
   else
     *aDone = PR_TRUE; // we couldn't open up the DB. This is an unrecoverable error so mark the scope as done.
   
+  delete expressionTree;
+
   // in the past an error here would cause an "infinite" search because the url would continue to run...
   // i.e. if we couldn't open the database, it returns an error code but the caller of this function says, oh,
   // we did not finish so continue...what we really want is to treat this current scope as done

mailnews/base/search/src/nsMsgLocalSearch.h

@@ -73,39 +73,19 @@ public:
                                          nsIMsgDatabase * db, 
                                          const char * headers,
                                          PRUint32 headerSize,
+                                         nsMsgSearchBoolExpression ** aExpressionTree,
 										 PRBool *pResult);
 
   static nsresult MatchTermsForSearch(nsIMsgDBHdr * msgTomatch,
                                       nsISupportsArray * termList,
                                       const char *defaultCharset,
                                       nsIMsgSearchScopeTerm *scope,
-                                      nsIMsgDatabase *db, PRBool *pResult);
+                                      nsIMsgDatabase *db,
+                                      nsMsgSearchBoolExpression ** aExpressionTree,
+ PRBool *pResult);
 
 	virtual nsresult OpenSummaryFile ();
 
-protected:
-	static nsresult MatchTerms(nsIMsgDBHdr *msgToMatch,
-                                nsISupportsArray *termList,
-                                const char *defaultCharset,
-                                nsIMsgSearchScopeTerm *scope, 
-                                nsIMsgDatabase * db, 
-                                const char * headers,
-                                PRUint32 headerSize,
-                                PRBool ForFilters,
-								PRBool *pResult);
-
-    static nsresult ConstructExpressionTree(nsIMsgDBHdr *msgToMatch,
-                                      nsISupportsArray * termList,
-                                      PRUint32 &aStartPosInList,
-                                      const char *defaultCharset,
-                                      nsIMsgSearchScopeTerm * scope,
-                                      nsIMsgDatabase * db, 
-                                      const char * headers,
-                                      PRUint32 headerSize,
-                                      PRBool Filtering,
-                                      nsMsgSearchBoolExpression ** aExpressionTree,
-				        			  PRBool *pResult);
-    
      static nsresult ProcessSearchTerm(nsIMsgDBHdr *msgToMatch,
                                nsIMsgSearchTerm * aTerm,
                                const char *defaultCharset,
@@ -115,6 +95,23 @@ protected:
                                PRUint32 headerSize,
                                PRBool Filtering,
 							   PRBool *pResult); 
+protected:
+	static nsresult MatchTerms(nsIMsgDBHdr *msgToMatch,
+                                nsISupportsArray *termList,
+                                const char *defaultCharset,
+                                nsIMsgSearchScopeTerm *scope, 
+                                nsIMsgDatabase * db, 
+                                const char * headers,
+                                PRUint32 headerSize,
+                                PRBool ForFilters,
+                                nsMsgSearchBoolExpression ** aExpressionTree,
+								PRBool *pResult);
+
+    static nsresult ConstructExpressionTree(nsISupportsArray * termList,
+                                      PRUint32 termCount,
+                                      PRUint32 &aStartPosInList,
+                                      nsMsgSearchBoolExpression ** aExpressionTree);
+    
 	nsCOMPtr <nsIMsgDatabase> m_db;
 	nsCOMPtr<nsISimpleEnumerator> m_listContext;
 	void CleanUpScope();

mailnews/base/search/src/nsMsgSearchAdapter.cpp

@@ -20,6 +20,7 @@
  * the Initial Developer. All Rights Reserved.
  *
  * Contributor(s):
+ *   Howard Chu <hyc@symas.com>
  *
  * Alternatively, the contents of this file may be used under the terms of
  * either of the GNU General Public License Version 2 or later (the "GPL"),
@@ -786,7 +787,7 @@ nsresult nsMsgSearchAdapter::EncodeImap (char **ppOutEncoding, nsISupportsArray
     err = EncodeImapTerm (pTerm, reallyDredd, srcCharset, destCharset, &termEncoding);
     if (NS_SUCCEEDED(err) && nsnull != termEncoding)
     {
-      expression = nsMsgSearchBoolExpression::AddSearchTermWithEncoding(expression, pTerm, termEncoding);
+      expression = nsMsgSearchBoolExpression::AddSearchTerm(expression, pTerm, termEncoding);
       delete [] termEncoding;
     }
   }

mailnews/base/search/src/nsMsgSearchSession.cpp

@@ -20,6 +20,7 @@
  * the Initial Developer. All Rights Reserved.
  *
  * Contributor(s):
+ *   Howard Chu <hyc@symas.com>
  *
  * Alternatively, the contents of this file may be used under the terms of
  * either of the GNU General Public License Version 2 or later (the "GPL"),
@@ -59,6 +60,7 @@ nsMsgSearchSession::nsMsgSearchSession()
   m_idxRunningScope = 0; 
   m_urlQueueIndex = 0;
   m_handlingError = PR_FALSE;
+  m_expressionTree = nsnull;
   m_searchPaused = PR_FALSE;
   NS_NewISupportsArray(getter_AddRefs(m_termList));
 }
@@ -66,6 +68,7 @@ nsMsgSearchSession::nsMsgSearchSession()
 nsMsgSearchSession::~nsMsgSearchSession()
 {
   InterruptSearch();
+  delete m_expressionTree;
   DestroyResultList ();
   DestroyScopeList ();
   DestroyTermList ();
@@ -90,6 +93,8 @@ nsMsgSearchSession::AddSearchTerm(nsMsgSearchAttribValue attrib,
 	if (nsnull == pTerm)
 		return NS_ERROR_OUT_OF_MEMORY;
 	m_termList->AppendElement (pTerm);
+        // force the expression tree to rebuild whenever we change the terms
+        delete m_expressionTree;
 	return NS_OK;
 }
 
@@ -98,6 +103,7 @@ nsMsgSearchSession::AppendTerm(nsIMsgSearchTerm *aTerm)
 {
     NS_ENSURE_ARG_POINTER(aTerm);
     NS_ENSURE_TRUE(m_termList, NS_ERROR_NOT_INITIALIZED);
+    delete m_expressionTree;
     return m_termList->AppendElement(aTerm);
 }
 
@@ -641,6 +647,7 @@ void nsMsgSearchSession::DestroyScopeList()
 
 void nsMsgSearchSession::DestroyTermList ()
 {
+    delete m_expressionTree;
     m_termList->Clear();
 }
 
@@ -746,7 +753,7 @@ nsMsgSearchSession::MatchHdr(nsIMsgDBHdr *aMsgHdr, nsIMsgDatabase *aDatabase, PR
       nsXPIDLString nullCharset, folderCharset;
       scope->m_adapter->GetSearchCharsets(getter_Copies(nullCharset), getter_Copies(folderCharset));
       NS_ConvertUCS2toUTF8 charset(folderCharset.get());
-      nsMsgSearchOfflineMail::MatchTermsForSearch(aMsgHdr, m_termList, charset.get(), scope, aDatabase, aResult);
+      nsMsgSearchOfflineMail::MatchTermsForSearch(aMsgHdr, m_termList, charset.get(), scope, aDatabase, &m_expressionTree, aResult);
     }
   }
   return NS_OK;

mailnews/base/search/src/nsMsgSearchSession.h

@@ -50,6 +50,7 @@
 #include "nsWeakReference.h"
 
 class nsMsgSearchAdapter;
+class nsMsgSearchBoolExpression;
 
 class nsMsgSearchSession : public nsIMsgSearchSession, public nsIUrlListener, public nsSupportsWeakReference
 {
@@ -103,6 +104,7 @@ protected:
   nsCStringArray m_urlQueue;
   nsCOMPtr <nsITimer> m_backgroundTimer;
   PRBool m_searchPaused;
+  nsMsgSearchBoolExpression *m_expressionTree;
 };
 
 #endif