Patch that fixes the ugly refresh-all hack in nsXFormsModelElement.cpp. Should speed up things considerably, but is also a potential form-breaker. Bug 278368, r=aaronr+smaug, a=mkaply

extensions/xforms/nsIXFormsControl.idl

@@ -81,8 +81,6 @@ interface nsIXFormsControl : nsIXFormsContextControl
    * the control is bound to (mBoundNode). For example:
    * If a node has @ref="/share[@owner = /me]", it depends on all /share
    * nodes, all @owned attributes on /share nodes, and all /me nodes.
-   *
-   * @note The array MUST be sorted, pointer-order, ascending.
    */
   readonly attribute nsCOMArrayPtr dependencies;
 

extensions/xforms/nsXFormsMDGEngine.h

@@ -367,8 +367,6 @@ public:
    * Recalculate the MDG.
    * 
    * @param aChangedNodes    Returns the nodes that was changed during recalculation.
-   *
-   * @note aChangedNodes are unique and sorted in pointer-order, ascending.
    */
   nsresult Recalculate(nsCOMArray<nsIDOMNode> *aChangedNodes);
 

extensions/xforms/nsXFormsModelElement.cpp

@@ -544,16 +544,15 @@ nsXFormsModelElement::Revalidate()
   printf("nsXFormsModelElement::Revalidate()\n");
 #endif
 
-  /// @note Prerequisite: Both changed nodes and dependencies are sorted in
-  /// ascending order!
-
 #ifdef DEBUG_MODEL
-  printf("Changed nodes:\n");
+  printf("[%s] Changed nodes:\n", __TIME__);
   for (PRInt32 j = 0; j < mChangedNodes.Count(); ++j) {
-    nsCOMPtr<nsIDOMNode> node = mChangedNodes.GetNode(j);
+    nsCOMPtr<nsIDOMNode> node = mChangedNodes[j];
     nsAutoString name;
     node->GetNodeName(name);
-    printf("\t%s\n", NS_ConvertUCS2toUTF8(name).get());
+    printf("\t%s [%p]\n",
+           NS_ConvertUCS2toUTF8(name).get(),
+           (void*) node);
   }
 #endif
 
@@ -580,52 +579,63 @@ nsXFormsModelElement::Revalidate()
     // Get dependencies
     nsCOMArray<nsIDOMNode> *deps = nsnull;
     control->GetDependencies(&deps);    
-    PRUint32 depCount = deps ? deps->Count() : 0;
 
 #ifdef DEBUG_MODEL
+    PRUint32 depCount = deps ? deps->Count() : 0;
     nsCOMPtr<nsIDOMElement> controlElement;
     control->GetElement(getter_AddRefs(controlElement));
     if (controlElement) {
       printf("Checking control: ");      
-      //DBG_TAGINFO(controlElement);
+      // DBG_TAGINFO(controlElement);
       nsAutoString boundName;
       if (boundNode)
         boundNode->GetNodeName(boundName);
-      printf("\tBound to: '%s', dependencies: %d\n",
+      printf("\tDependencies: %d, Bound to: '%s' [%p]\n",
+             depCount,
              NS_ConvertUCS2toUTF8(boundName).get(),
-             depCount);
+             (void*) boundNode);
+
+      nsAutoString depNodeName;
+      for (PRUint32 t = 0; t < depCount; ++t) {
+        nsCOMPtr<nsIDOMNode> tmpdep = deps->ObjectAt(t);
+        if (tmpdep) {
+          tmpdep->GetNodeName(depNodeName);
+          printf("\t\t%s [%p]\n",
+                 NS_ConvertUCS2toUTF8(depNodeName).get(),
+                 (void*) tmpdep);
+        }
+      }
     }
 #endif
 
-    nsCOMPtr<nsIDOMNode> curDep, curChanged;
-    PRUint32 depPos = 0;
-    /// @bug This should be set to PR_FALSE! (XXX)
-    ///      Setting it to PR_TRUE rebinds all controls all the time
-    ///      @see https://bugzilla.mozilla.org/show_bug.cgi?id=278368
-    PRBool rebind = PR_TRUE;
+    nsCOMPtr<nsIDOM3Node> curChanged;
+    PRBool rebind = PR_FALSE;
     PRBool refresh = PR_FALSE;
 
     for (PRInt32 j = 0; j < mChangedNodes.Count(); ++j) {
-      curChanged = mChangedNodes[j];
+      curChanged = do_QueryInterface(mChangedNodes[j]);
 
-      if (curChanged == boundNode) {
-        refresh = PR_TRUE;
-        // We cannot break here, as we need to to check for any changed
-        // dependencies
+      // Check whether the bound node is dirty. If so, we need to refresh the
+      // control (get updated node value from the bound node)
+      if (!refresh && boundNode) {
+        curChanged->IsSameNode(boundNode, &refresh);
+        
+        if (refresh)
+          // We need to refresh the control. We cannot break out of the loop
+          // as we need to check dependencies
+          continue;
       }
 
-      if (depPos == depCount) {
-        continue;
-      }
-
-      while (depPos < depCount && (void*) curChanged > (void*) curDep) {
-        curDep = deps->ObjectAt(depPos);
-        ++depPos;
-      }
-
-      if (curDep == curChanged) {
-        rebind = PR_TRUE;
-        break;
+      // Check whether any dependencies are dirty. If so, we need to rebind
+      // the control (re-evaluate it's binding expression)
+      for (PRInt32 k = 0; k < deps->Count(); ++k) {
+        /// @note beaufour: I'm not to happy about this ...
+        /// O(mChangedNodes.Count() * deps->Count()), but using the pointers
+        /// for sorting and comparing does not work...
+        curChanged->IsSameNode(deps->ObjectAt(k), &rebind);
+        if (rebind)
+          // We need to rebind the control, no need to check any more
+          break;
       }
     }
 

extensions/xforms/nsXFormsOutputElement.cpp

@@ -250,8 +250,6 @@ nsXFormsOutputElement::Refresh()
       nsXFormsUtils::GetNodeValue(mBoundNode, text);
     }
   } else {
-    ///
-    /// @todo Update mBoundNode? (XXX)
     nsCOMPtr<nsIDOMXPathResult> result;
     rv = ProcessNodeBinding(NS_LITERAL_STRING("value"),
                             nsIDOMXPathResult::STRING_TYPE,

extensions/xforms/nsXFormsUtils.cpp

@@ -971,7 +971,7 @@ nsXFormsUtils::FocusControl(nsIDOMElement *aElement)
 int
 sortFunc(nsIDOMNode *aNode1, nsIDOMNode *aNode2, void *aArg) 
 {
-  return (void*) aNode1 < (void*) aNode2;
+  return (void*) aNode1 > (void*) aNode2;
 }
 
 /* static */ void

extensions/xforms/nsXFormsXPathAnalyzer.cpp

@@ -121,9 +121,7 @@ nsXFormsXPathAnalyzer::AnalyzeRecursively(nsIDOMNode              *aContextNode,
     printf("\tCon: %d, Predicate: %d, Literal: %d\n", aNode->mCon, aNode->mPredicate, aNode->mLiteral);
 #endif
 
-    if (   aNode->mEndIndex < 0
-        || aNode->mStartIndex >= aNode->mEndIndex
-        || ((PRUint32) aNode->mEndIndex == mCurExprString->Length() && aNode->mStartIndex == 0)) { 
+    if (aNode->mEndIndex < 0 || aNode->mStartIndex >= aNode->mEndIndex) {
       continue;
     }