Bug 1318479 part 3. Remove uses of nsIDOMNode::AppendChild in HTMLOptionsCollection. r=ehsan
authorBoris Zbarsky <bzbarsky@mit.edu>
Fri, 18 Nov 2016 16:38:29 -0500
changeset 323524 8b6abb058e6aefd51cb0629fc4a9e896684f10c6
parent 323523 c9bd452a8b45f3ebb56eac66c818efe16fef365f
child 323525 333df199df73279ecb5b1243c9f6c4350a2f5250
push id21
push usermaklebus@msu.edu
push dateThu, 01 Dec 2016 06:22:08 +0000
reviewersehsan
bugs1318479
milestone53.0a1
Bug 1318479 part 3. Remove uses of nsIDOMNode::AppendChild in HTMLOptionsCollection. r=ehsan
dom/html/HTMLOptionsCollection.cpp
dom/html/HTMLOptionsCollection.h
dom/html/HTMLSelectElement.h
dom/interfaces/html/nsIDOMHTMLOptionsCollection.idl
--- a/dom/html/HTMLOptionsCollection.cpp
+++ b/dom/html/HTMLOptionsCollection.cpp
@@ -126,67 +126,66 @@ HTMLOptionsCollection::SetLength(uint32_
 {
   if (!mSelect) {
     return NS_ERROR_UNEXPECTED;
   }
 
   return mSelect->SetLength(aLength);
 }
 
-NS_IMETHODIMP
-HTMLOptionsCollection::SetOption(uint32_t aIndex,
-                                 nsIDOMHTMLOptionElement* aOption)
+void
+HTMLOptionsCollection::IndexedSetter(uint32_t aIndex,
+                                     HTMLOptionElement* aOption,
+                                     ErrorResult& aError)
 {
   if (!mSelect) {
-    return NS_OK;
+    return;
   }
 
   // if the new option is null, just remove this option.  Note that it's safe
   // to pass a too-large aIndex in here.
   if (!aOption) {
     mSelect->Remove(aIndex);
 
     // We're done.
-    return NS_OK;
-  }
-
-  nsresult rv = NS_OK;
-
-  uint32_t index = uint32_t(aIndex);
-
-  // Now we're going to be setting an option in our collection
-  if (index > mElements.Length()) {
-    // Fill our array with blank options up to (but not including, since we're
-    // about to change it) aIndex, for compat with other browsers.
-    rv = SetLength(index);
-    NS_ENSURE_SUCCESS(rv, rv);
+    return;
   }
 
-  NS_ASSERTION(index <= mElements.Length(), "SetLength lied");
-  
-  nsCOMPtr<nsIDOMNode> ret;
-  if (index == mElements.Length()) {
-    nsCOMPtr<nsIDOMNode> node = do_QueryInterface(aOption);
-    rv = mSelect->AppendChild(node, getter_AddRefs(ret));
-  } else {
-    // Find the option they're talking about and replace it
-    // hold a strong reference to follow COM rules.
-    RefPtr<HTMLOptionElement> refChild = ItemAsOption(index);
-    NS_ENSURE_TRUE(refChild, NS_ERROR_UNEXPECTED);
-
-    nsCOMPtr<nsINode> parent = refChild->GetParent();
-    if (parent) {
-      nsCOMPtr<nsINode> node = do_QueryInterface(aOption);
-      ErrorResult res;
-      parent->ReplaceChild(*node, *refChild, res);
-      rv = res.StealNSResult();
+  // Now we're going to be setting an option in our collection
+  if (aIndex > mElements.Length()) {
+    // Fill our array with blank options up to (but not including, since we're
+    // about to change it) aIndex, for compat with other browsers.
+    nsresult rv = SetLength(aIndex);
+    if (NS_WARN_IF(NS_FAILED(rv))) {
+      aError.Throw(rv);
+      return;
     }
   }
 
-  return rv;
+  NS_ASSERTION(aIndex <= mElements.Length(), "SetLength lied");
+  
+  if (aIndex == mElements.Length()) {
+    mSelect->AppendChild(*aOption, aError);
+    return;
+  }
+
+  // Find the option they're talking about and replace it
+  // hold a strong reference to follow COM rules.
+  RefPtr<HTMLOptionElement> refChild = ItemAsOption(aIndex);
+  if (!refChild) {
+    aError.Throw(NS_ERROR_UNEXPECTED);
+    return;
+  }
+
+  nsCOMPtr<nsINode> parent = refChild->GetParent();
+  if (!parent) {
+    return;
+  }
+
+  parent->ReplaceChild(*aOption, *refChild, aError);
 }
 
 int32_t
 HTMLOptionsCollection::GetSelectedIndex(ErrorResult& aError)
 {
   if (!mSelect) {
     aError.Throw(NS_ERROR_UNEXPECTED);
     return 0;
--- a/dom/html/HTMLOptionsCollection.h
+++ b/dom/html/HTMLOptionsCollection.h
@@ -144,21 +144,18 @@ public:
   }
 
   void Add(const HTMLOptionOrOptGroupElement& aElement,
            const Nullable<HTMLElementOrLong>& aBefore,
            ErrorResult& aError);
   void Remove(int32_t aIndex, ErrorResult& aError);
   int32_t GetSelectedIndex(ErrorResult& aError);
   void SetSelectedIndex(int32_t aSelectedIndex, ErrorResult& aError);
-  void IndexedSetter(uint32_t aIndex, nsIDOMHTMLOptionElement* aOption,
-                     ErrorResult& aError)
-  {
-    aError = SetOption(aIndex, aOption);
-  }
+  void IndexedSetter(uint32_t aIndex, HTMLOptionElement* aOption,
+                     ErrorResult& aError);
   virtual void GetSupportedNames(nsTArray<nsString>& aNames) override;
 
 private:
   /** The list of options (holds strong references).  This is infallible, so
    * various members such as InsertOptionAt are also infallible. */
   nsTArray<RefPtr<mozilla::dom::HTMLOptionElement> > mElements;
   /** The select element that contains this array */
   HTMLSelectElement* mSelect;
--- a/dom/html/HTMLSelectElement.h
+++ b/dom/html/HTMLSelectElement.h
@@ -267,16 +267,17 @@ public:
   // nsIConstraintValidation::Validity() is fine.
   // nsIConstraintValidation::GetValidationMessage() is fine.
   // nsIConstraintValidation::CheckValidity() is fine.
   using nsIConstraintValidation::CheckValidity;
   using nsIConstraintValidation::ReportValidity;
   // nsIConstraintValidation::SetCustomValidity() is fine.
 
   using nsINode::Remove;
+  using nsINode::AppendChild;
 
 
   // nsINode
   virtual JSObject* WrapNode(JSContext* aCx, JS::Handle<JSObject*> aGivenProto) override;
 
   // nsIContent
   virtual nsresult GetEventTargetParent(
                      EventChainPreVisitor& aVisitor) override;
--- a/dom/interfaces/html/nsIDOMHTMLOptionsCollection.idl
+++ b/dom/interfaces/html/nsIDOMHTMLOptionsCollection.idl
@@ -31,19 +31,16 @@ interface nsIDOMHTMLOptionsCollection : 
   nsIDOMNode         item(in unsigned long index);
 
   // FIXME namedItem (and getNamedItem) should return a NodeList if there are
   //       multiple matching items
   nsIDOMNode namedItem(in DOMString name);
 
            attribute long             selectedIndex;
 
-  [noscript] void    setOption(in unsigned long index,
-                               in nsIDOMHTMLOptionElement option);
-
   [noscript] readonly attribute nsIDOMHTMLSelectElement select;
 
   // This add method implementation means the following
   // since IDL doesn't support overloading.
   //   void add(in nsIDOMHTMLOptionElement,
   //            [optional] in nsIDOMHTMLOptionElement)
   //   void add(in nsIDOMHTMLOptionElement, in long)
   void                      add(in nsIDOMHTMLOptionElement option,