Bug 903791 - Make HTMLOptionElement.index infallible; r=mounir
authorMs2ger <ms2ger@gmail.com>
Wed, 14 Aug 2013 08:57:04 +0200
changeset 150470 698c66920a411403dc07865237669a3e01493b98
parent 150469 c39238a4fbbebca28407465612f200b0eacc0ff4
child 150471 adb013097db236d2a9ce65c1d921e00b8034ab62
push id4254
push userakeybl@mozilla.com
push dateTue, 17 Sep 2013 14:18:33 +0000
treeherdermozilla-aurora@9edd56e694b0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmounir
bugs903791
milestone26.0a1
Bug 903791 - Make HTMLOptionElement.index infallible; r=mounir
content/html/content/src/HTMLOptionElement.cpp
content/html/content/src/HTMLOptionElement.h
dom/webidl/HTMLOptionElement.webidl
layout/forms/nsListControlFrame.cpp
--- a/content/html/content/src/HTMLOptionElement.cpp
+++ b/content/html/content/src/HTMLOptionElement.cpp
@@ -94,18 +94,17 @@ HTMLOptionElement::GetSelected(bool* aVa
 
 NS_IMETHODIMP
 HTMLOptionElement::SetSelected(bool aValue)
 {
   // Note: The select content obj maintains all the PresState
   // so defer to it to get the answer
   HTMLSelectElement* selectInt = GetSelect();
   if (selectInt) {
-    int32_t index;
-    GetIndex(&index);
+    int32_t index = Index();
     // This should end up calling SetSelectedInternal
     selectInt->SetOptionsSelectedByIndex(index, index, aValue,
                                          false, true, true);
   } else {
     SetSelectedInternal(aValue, true);
   }
 
   return NS_OK;
@@ -115,32 +114,40 @@ NS_IMPL_BOOL_ATTR(HTMLOptionElement, Def
 // GetText returns a whitespace compressed .textContent value.
 NS_IMPL_STRING_ATTR_WITH_FALLBACK(HTMLOptionElement, Label, label, GetText)
 NS_IMPL_STRING_ATTR_WITH_FALLBACK(HTMLOptionElement, Value, value, GetText)
 NS_IMPL_BOOL_ATTR(HTMLOptionElement, Disabled, disabled)
 
 NS_IMETHODIMP
 HTMLOptionElement::GetIndex(int32_t* aIndex)
 {
-  // When the element is not in a list of options, the index is 0.
-  *aIndex = 0;
+  *aIndex = Index();
+  return NS_OK;
+}
+
+int32_t
+HTMLOptionElement::Index()
+{
+  static int32_t defaultIndex = 0;
 
   // Only select elements can contain a list of options.
   HTMLSelectElement* selectElement = GetSelect();
   if (!selectElement) {
-    return NS_OK;
+    return defaultIndex;
   }
 
   HTMLOptionsCollection* options = selectElement->GetOptions();
   if (!options) {
-    return NS_OK;
+    return defaultIndex;
   }
 
-  // aIndex will not be set if GetOptionsIndex fails.
-  return options->GetOptionIndex(this, 0, true, aIndex);
+  int32_t index = defaultIndex;
+  MOZ_ALWAYS_TRUE(NS_SUCCEEDED(
+    options->GetOptionIndex(this, 0, true, &index)));
+  return index;
 }
 
 bool
 HTMLOptionElement::Selected() const
 {
   // If we haven't been explictly selected or deselected, use our default value
   if (!mSelectedChanged) {
     return DefaultSelected();
@@ -194,18 +201,17 @@ HTMLOptionElement::BeforeSetAttr(int32_t
   // Note that at this point mSelectedChanged is false and as long as that's
   // true it doesn't matter what value mIsSelected has.
   NS_ASSERTION(!mSelectedChanged, "Shouldn't be here");
 
   bool newSelected = (aValue != nullptr);
   bool inSetDefaultSelected = mIsInSetDefaultSelected;
   mIsInSetDefaultSelected = true;
 
-  int32_t index;
-  GetIndex(&index);
+  int32_t index = Index();
   // This should end up calling SetSelectedInternal, which we will allow to
   // take effect so that parts of SetOptionsSelectedByIndex that might depend
   // on it working don't get confused.
   selectInt->SetOptionsSelectedByIndex(index, index, newSelected,
                                        false, true, aNotify);
 
   // Now reset our members; when we finish the attr set we'll end up with the
   // rigt selected state.
--- a/content/html/content/src/HTMLOptionElement.h
+++ b/content/html/content/src/HTMLOptionElement.h
@@ -107,22 +107,17 @@ public:
   }
 
   // The XPCOM GetText is OK for us
   void SetText(const nsAString& aValue, ErrorResult& aRv)
   {
     aRv = SetText(aValue);
   }
 
-  int32_t GetIndex(ErrorResult& aRv)
-  {
-    int32_t id = 0;
-    aRv = GetIndex(&id);
-    return id;
-  }
+  int32_t Index();
 
 protected:
   virtual JSObject* WrapNode(JSContext* aCx,
                              JS::Handle<JSObject*> aScope) MOZ_OVERRIDE;
 
   /**
    * Get the select content element that contains this option, this
    * intentionally does not return nsresult, all we care about is if
--- a/dom/webidl/HTMLOptionElement.webidl
+++ b/dom/webidl/HTMLOptionElement.webidl
@@ -22,11 +22,10 @@ interface HTMLOptionElement : HTMLElemen
            attribute boolean defaultSelected;
            [SetterThrows]
            attribute boolean selected;
            [SetterThrows]
            attribute DOMString value;
 
            [SetterThrows]
            attribute DOMString text;
-           [GetterThrows]
   readonly attribute long index;
 };
--- a/layout/forms/nsListControlFrame.cpp
+++ b/layout/forms/nsListControlFrame.cpp
@@ -1722,17 +1722,17 @@ nsListControlFrame::GetIndexFromDOMEvent
   for (nsCOMPtr<nsIContent> content =
          PresContext()->EventStateManager()->GetEventTargetContent(nullptr);
        content && !option;
        content = content->GetParent()) {
     option = dom::HTMLOptionElement::FromContent(content);
   }
 
   if (option) {
-    option->GetIndex(&aCurIndex);
+    aCurIndex = option->Index();
     MOZ_ASSERT(aCurIndex >= 0);
     return NS_OK;
   }
 
   int32_t numOptions = GetNumberOfOptions();
   if (numOptions < 1)
     return NS_ERROR_FAILURE;