Bug 1385324 - Part 2: Rewrite HTMLFormControlsCollection::GetSortedControls() to use RefPtr instead of raw pointers; r=qdot

dom/html/HTMLFormControlsCollection.cpp

@@ -261,7 +261,7 @@ HTMLFormControlsCollection::RemoveElementFromTable(
 
 nsresult
 HTMLFormControlsCollection::GetSortedControls(
-  nsTArray<nsGenericHTMLFormElement*>& aControls) const
+  nsTArray<RefPtr<nsGenericHTMLFormElement>>& aControls) const
 {
 #ifdef DEBUG
   HTMLFormElement::AssertDocumentOrder(mElements, mForm);

dom/html/HTMLFormControlsCollection.h

@@ -15,6 +15,8 @@
 
 class nsGenericHTMLFormElement;
 class nsIFormControl;
+template <class T>
+class RefPtr;
 
 namespace mozilla {
 namespace dom {
@@ -75,7 +77,7 @@ public:
    * @param aControls The list of sorted controls[out].
    * @return NS_OK or NS_ERROR_OUT_OF_MEMORY.
    */
-  nsresult GetSortedControls(nsTArray<nsGenericHTMLFormElement*>& aControls) const;
+  nsresult GetSortedControls(nsTArray<RefPtr<nsGenericHTMLFormElement>>& aControls) const;
 
   // nsWrapperCache
   using nsWrapperCache::GetWrapperPreserveColor;

dom/html/HTMLFormElement.cpp

@@ -1023,18 +1023,12 @@ HTMLFormElement::WalkFormElements(HTMLFormSubmission* aFormSubmission)
 {
   // This shouldn't be called recursively, so use a rather large value
   // for the preallocated buffer.
-  AutoTArray<nsGenericHTMLFormElement*, 100> sortedControls;
+  AutoTArray<RefPtr<nsGenericHTMLFormElement>, 100> sortedControls;
   nsresult rv = mControls->GetSortedControls(sortedControls);
   NS_ENSURE_SUCCESS(rv, rv);
 
   uint32_t len = sortedControls.Length();
 
-  // Hold a reference to the elements so they can't be deleted while
-  // calling SubmitNamesValues().
-  for (uint32_t i = 0; i < len; ++i) {
-    static_cast<nsGenericHTMLElement*>(sortedControls[i])->AddRef();
-  }
-
   //
   // Walk the list of nodes and call SubmitNamesValues() on the controls
   //
@@ -1043,11 +1037,6 @@ HTMLFormElement::WalkFormElements(HTMLFormSubmission* aFormSubmission)
     sortedControls[i]->SubmitNamesValues(aFormSubmission);
   }
 
-  // Release the references.
-  for (uint32_t i = 0; i < len; ++i) {
-    static_cast<nsGenericHTMLElement*>(sortedControls[i])->Release();
-  }
-
   return NS_OK;
 }
 
@@ -1141,6 +1130,32 @@ HTMLFormElement::AssertDocumentOrder(
     }
   }
 }
+
+/**
+ * Copy of the above function, but with RefPtrs.
+ *
+ * @param aControls List of form controls to check.
+ * @param aForm Parent form of the controls.
+ */
+/* static */ void
+HTMLFormElement::AssertDocumentOrder(
+  const nsTArray<RefPtr<nsGenericHTMLFormElement>>& aControls, nsIContent* aForm)
+{
+  // TODO: remove the return statement with bug 598468.
+  // This is done to prevent asserts in some edge cases.
+  return;
+
+  // Only iterate if aControls is not empty, since otherwise
+  // |aControls.Length() - 1| will be a very large unsigned number... not what
+  // we want here.
+  if (!aControls.IsEmpty()) {
+    for (uint32_t i = 0; i < aControls.Length() - 1; ++i) {
+      NS_ASSERTION(CompareFormControlPosition(aControls[i], aControls[i + 1],
+                                              aForm) < 0,
+                   "Form controls not ordered correctly");
+    }
+  }
+}
 #endif
 
 void
@@ -1872,19 +1887,13 @@ HTMLFormElement::CheckFormValidity(nsIMutableArray* aInvalidElements) const
 
   // This shouldn't be called recursively, so use a rather large value
   // for the preallocated buffer.
-  AutoTArray<nsGenericHTMLFormElement*, 100> sortedControls;
+  AutoTArray<RefPtr<nsGenericHTMLFormElement>, 100> sortedControls;
   if (NS_FAILED(mControls->GetSortedControls(sortedControls))) {
     return false;
   }
 
   uint32_t len = sortedControls.Length();
 
-  // Hold a reference to the elements so they can't be deleted while calling
-  // the invalid events.
-  for (uint32_t i = 0; i < len; ++i) {
-    sortedControls[i]->AddRef();
-  }
-
   for (uint32_t i = 0; i < len; ++i) {
     nsCOMPtr<nsIConstraintValidation> cvElmt = do_QueryObject(sortedControls[i]);
     if (cvElmt && cvElmt->IsCandidateForConstraintValidation() &&
@@ -1905,11 +1914,6 @@ HTMLFormElement::CheckFormValidity(nsIMutableArray* aInvalidElements) const
     }
   }
 
-  // Release the references.
-  for (uint32_t i = 0; i < len; ++i) {
-    static_cast<nsGenericHTMLElement*>(sortedControls[i])->Release();
-  }
-
   return ret;
 }
 

dom/html/HTMLFormElement.h

@@ -410,6 +410,9 @@ public:
   static void
   AssertDocumentOrder(const nsTArray<nsGenericHTMLFormElement*>& aControls,
                       nsIContent* aForm);
+  static void
+  AssertDocumentOrder(const nsTArray<RefPtr<nsGenericHTMLFormElement>>& aControls,
+                      nsIContent* aForm);
 #endif
 
   js::ExpandoAndGeneration mExpandoAndGeneration;