Bug 1001623 - clean up error handling of index returning methods, r=tbsaunde
authorAlexander Surkov <surkov.alexander@gmail.com>
Fri, 16 May 2014 19:16:34 -0400
changeset 183646 885e99ddd324150b62001788cd7984fc7d709cec
parent 183645 28db7381c97909e92c0d0902d6f6bd67a2ff2dd6
child 183647 2882b91ad758b8d1ecc7c66246607315c75bb65b
push id6844
push userphilringnalda@gmail.com
push dateSun, 18 May 2014 01:12:08 +0000
treeherderfx-team@41a54c8add09 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstbsaunde
bugs1001623
milestone32.0a1
Bug 1001623 - clean up error handling of index returning methods, r=tbsaunde
accessible/src/generic/Accessible.h
accessible/src/generic/HyperTextAccessible-inl.h
accessible/src/generic/HyperTextAccessible.cpp
accessible/src/generic/HyperTextAccessible.h
--- a/accessible/src/generic/Accessible.h
+++ b/accessible/src/generic/Accessible.h
@@ -85,16 +85,36 @@ struct GroupPos
 {
   GroupPos() : level(0), posInSet(0), setSize(0) { }
 
   int32_t level;
   int32_t posInSet;
   int32_t setSize;
 };
 
+/**
+ * An index type. Assert if out of range value was attempted to be used.
+ */
+class index_t
+{
+public:
+  index_t(int32_t aVal) : mVal(aVal) {}
+
+  operator uint32_t() const
+  {
+    MOZ_ASSERT(mVal >= 0, "Attempt to use wrong index!");
+    return mVal;
+  }
+
+  bool IsValid() const { return mVal >= 0; }
+
+private:
+  int32_t mVal;
+};
+
 typedef nsRefPtrHashtable<nsPtrHashKey<const void>, Accessible>
   AccessibleHashtable;
 
 
 #define NS_ACCESSIBLE_IMPL_IID                          \
 {  /* 133c8bf4-4913-4355-bd50-426bd1d6e1ad */           \
   0x133c8bf4,                                           \
   0x4913,                                               \
--- a/accessible/src/generic/HyperTextAccessible-inl.h
+++ b/accessible/src/generic/HyperTextAccessible-inl.h
@@ -16,25 +16,27 @@
 #include "nsIPlaintextEditor.h"
 
 namespace mozilla {
 namespace a11y {
 
 inline bool
 HyperTextAccessible::IsValidOffset(int32_t aOffset)
 {
-  return ConvertMagicOffset(aOffset) <= CharacterCount();
+  index_t offset = ConvertMagicOffset(aOffset);
+  return offset.IsValid() && offset <= CharacterCount();
 }
 
 inline bool
 HyperTextAccessible::IsValidRange(int32_t aStartOffset, int32_t aEndOffset)
 {
-  uint32_t endOffset = ConvertMagicOffset(aEndOffset);
-  return ConvertMagicOffset(aStartOffset) <= endOffset &&
-    endOffset <= CharacterCount();
+  index_t startOffset = ConvertMagicOffset(aStartOffset);
+  index_t endOffset = ConvertMagicOffset(aEndOffset);
+  return startOffset.IsValid() && endOffset.IsValid() &&
+    startOffset <= endOffset && endOffset <= CharacterCount();
 }
 
 inline void
 HyperTextAccessible::SetCaretOffset(int32_t aOffset)
 {
   SetSelectionRange(aOffset, aOffset);
   // XXX: Force cache refresh until a good solution for AT emulation of user
   // input is implemented (AccessFu caret movement).
@@ -105,26 +107,26 @@ HyperTextAccessible::PasteText(int32_t a
 {
   nsCOMPtr<nsIEditor> editor = GetEditor();
   if (editor) {
     SetSelectionRange(aPosition, aPosition);
     editor->Paste(nsIClipboard::kGlobalClipboard);
   }
 }
 
-inline uint32_t
+inline index_t
 HyperTextAccessible::ConvertMagicOffset(int32_t aOffset) const
 {
   if (aOffset == nsIAccessibleText::TEXT_OFFSET_END_OF_TEXT)
     return CharacterCount();
 
   if (aOffset == nsIAccessibleText::TEXT_OFFSET_CARET)
     return CaretOffset();
 
-  return aOffset < 0 ? std::numeric_limits<uint32_t>::max() : aOffset;
+  return aOffset;
 }
 
 inline uint32_t
 HyperTextAccessible::AdjustCaretOffset(uint32_t aOffset) const
 {
   // It is the same character offset when the caret is visually at the very
   // end of a line or the start of a new line (soft line break). Getting text
   // at the line should provide the line with the visual caret, otherwise
--- a/accessible/src/generic/HyperTextAccessible.cpp
+++ b/accessible/src/generic/HyperTextAccessible.cpp
@@ -186,18 +186,23 @@ HyperTextAccessible::GetBoundsInFrame(ns
 }
 
 void
 HyperTextAccessible::TextSubstring(int32_t aStartOffset, int32_t aEndOffset,
                                    nsAString& aText)
 {
   aText.Truncate();
 
-  uint32_t startOffset = ConvertMagicOffset(aStartOffset);
-  uint32_t endOffset = ConvertMagicOffset(aEndOffset);
+  index_t startOffset = ConvertMagicOffset(aStartOffset);
+  index_t endOffset = ConvertMagicOffset(aEndOffset);
+  if (!startOffset.IsValid() || !endOffset.IsValid() ||
+      startOffset > endOffset || endOffset > CharacterCount()) {
+    NS_ERROR("Wrong in offset");
+    return;
+  }
 
   int32_t startChildIdx = GetChildIndexAtOffset(startOffset);
   if (startChildIdx == -1)
     return;
 
   int32_t endChildIdx = GetChildIndexAtOffset(endOffset);
   if (endChildIdx == -1)
     return;
@@ -603,19 +608,19 @@ void
 HyperTextAccessible::TextBeforeOffset(int32_t aOffset,
                                       AccessibleTextBoundary aBoundaryType,
                                       int32_t* aStartOffset, int32_t* aEndOffset,
                                       nsAString& aText)
 {
   *aStartOffset = *aEndOffset = 0;
   aText.Truncate();
 
-  uint32_t convertedOffset = ConvertMagicOffset(aOffset);
-  if (convertedOffset == std::numeric_limits<uint32_t>::max()) {
-    NS_ERROR("Wrong given offset!");
+  index_t convertedOffset = ConvertMagicOffset(aOffset);
+  if (!convertedOffset.IsValid() || convertedOffset > CharacterCount()) {
+    NS_ERROR("Wrong in offset!");
     return;
   }
 
   uint32_t adjustedOffset = convertedOffset;
   if (aOffset == nsIAccessibleText::TEXT_OFFSET_CARET)
     adjustedOffset = AdjustCaretOffset(adjustedOffset);
 
   switch (aBoundaryType) {
@@ -739,19 +744,19 @@ void
 HyperTextAccessible::TextAfterOffset(int32_t aOffset,
                                      AccessibleTextBoundary aBoundaryType,
                                      int32_t* aStartOffset, int32_t* aEndOffset,
                                      nsAString& aText)
 {
   *aStartOffset = *aEndOffset = 0;
   aText.Truncate();
 
-  uint32_t convertedOffset = ConvertMagicOffset(aOffset);
-  if (convertedOffset == std::numeric_limits<uint32_t>::max()) {
-    NS_ERROR("Wrong given offset!");
+  index_t convertedOffset = ConvertMagicOffset(aOffset);
+  if (!convertedOffset.IsValid() || convertedOffset > CharacterCount()) {
+    NS_ERROR("Wrong in offset!");
     return;
   }
 
   uint32_t adjustedOffset = convertedOffset;
   if (aOffset == nsIAccessibleText::TEXT_OFFSET_CARET)
     adjustedOffset = AdjustCaretOffset(adjustedOffset);
 
   switch (aBoundaryType) {
@@ -809,20 +814,25 @@ HyperTextAccessible::TextAttributes(bool
                                        int32_t* aEndOffset)
 {
   // 1. Get each attribute and its ranges one after another.
   // 2. As we get each new attribute, we pass the current start and end offsets
   //    as in/out parameters. In other words, as attributes are collected,
   //    the attribute range itself can only stay the same or get smaller.
 
   *aStartOffset = *aEndOffset = 0;
+  index_t offset = ConvertMagicOffset(aOffset);
+  if (!offset.IsValid() || offset > CharacterCount()) {
+    NS_ERROR("Wrong in offset!");
+    return nullptr;
+  }
+
   nsCOMPtr<nsIPersistentProperties> attributes =
     do_CreateInstance(NS_PERSISTENTPROPERTIES_CONTRACTID);
 
-  uint32_t offset = ConvertMagicOffset(aOffset);
   Accessible* accAtOffset = GetChildAtOffset(offset);
   if (!accAtOffset) {
     // Offset 0 is correct offset when accessible has empty text. Include
     // default attributes if they were requested, otherwise return empty set.
     if (offset == 0) {
       if (aIncludeDefAttrs) {
         TextAttrsMgr textAttrsMgr(this);
         textAttrsMgr.GetAttributes(attributes);
@@ -1024,21 +1034,24 @@ HyperTextAccessible::OffsetAtPoint(int32
 
   return -1; // Not found
 }
 
 nsIntRect
 HyperTextAccessible::TextBounds(int32_t aStartOffset, int32_t aEndOffset,
                                 uint32_t aCoordType)
 {
-  uint32_t startOffset = ConvertMagicOffset(aStartOffset);
-  uint32_t endOffset = ConvertMagicOffset(aEndOffset);
-  NS_ASSERTION(startOffset < endOffset &&
-               endOffset != std::numeric_limits<uint32_t>::max(),
-               "Wrong bad in!");
+  index_t startOffset = ConvertMagicOffset(aStartOffset);
+  index_t endOffset = ConvertMagicOffset(aEndOffset);
+  if (!startOffset.IsValid() || !endOffset.IsValid() ||
+      startOffset > endOffset || endOffset > CharacterCount()) {
+    NS_ERROR("Wrong in offset");
+    return nsIntRect();
+  }
+
 
   int32_t childIdx = GetChildIndexAtOffset(startOffset);
   if (childIdx == -1)
     return nsIntRect();
 
   nsIntRect bounds;
   int32_t prevOffset = GetChildOffset(childIdx);
   int32_t offset1 = startOffset - prevOffset;
@@ -1412,18 +1425,23 @@ HyperTextAccessible::SelectionBoundsAt(i
   return true;
 }
 
 bool
 HyperTextAccessible::SetSelectionBoundsAt(int32_t aSelectionNum,
                                           int32_t aStartOffset,
                                           int32_t aEndOffset)
 {
-  uint32_t startOffset = ConvertMagicOffset(aStartOffset);
-  uint32_t endOffset = ConvertMagicOffset(aEndOffset);
+  index_t startOffset = ConvertMagicOffset(aStartOffset);
+  index_t endOffset = ConvertMagicOffset(aEndOffset);
+  if (!startOffset.IsValid() || !endOffset.IsValid() ||
+      startOffset > endOffset || endOffset > CharacterCount()) {
+    NS_ERROR("Wrong in offset");
+    return false;
+  }
 
   dom::Selection* domSel = DOMSelection();
   if (!domSel)
     return false;
 
   nsRefPtr<nsRange> range;
   uint32_t rangeCount = domSel->GetRangeCount();
   if (aSelectionNum == static_cast<int32_t>(rangeCount))
@@ -1874,17 +1892,17 @@ HyperTextAccessible::GetSpellTextAttr(ns
   dom::Selection* domSel = fs->GetSelection(nsISelectionController::SELECTION_SPELLCHECK);
   if (!domSel)
     return;
 
   int32_t rangeCount = domSel->GetRangeCount();
   if (rangeCount <= 0)
     return;
 
-  int32_t startOffset = 0, endOffset = 0;
+  uint32_t startOffset = 0, endOffset = 0;
   for (int32_t idx = 0; idx < rangeCount; idx++) {
     nsRange* range = domSel->GetRangeAt(idx);
     if (range->Collapsed())
       continue;
 
     // See if the point comes after the range in which case we must continue in
     // case there is another range after this one.
     nsINode* endNode = range->GetEndParent();
--- a/accessible/src/generic/HyperTextAccessible.h
+++ b/accessible/src/generic/HyperTextAccessible.h
@@ -291,17 +291,21 @@ public:
   nsIntRect TextBounds(int32_t aStartOffset, int32_t aEndOffset,
                        uint32_t aCoordType = nsIAccessibleCoordinateType::COORDTYPE_SCREEN_RELATIVE);
 
   /**
    * Return a rect for character at given offset relative given coordinate
    * system.
    */
   nsIntRect CharBounds(int32_t aOffset, uint32_t aCoordType)
-    { return TextBounds(aOffset, aOffset + 1, aCoordType); }
+  {
+    int32_t endOffset = aOffset == static_cast<int32_t>(CharacterCount()) ?
+      aOffset : aOffset + 1;
+    return TextBounds(aOffset, endOffset, aCoordType);
+  }
 
   /**
    * Get/set caret offset, if no caret then -1.
    */
   int32_t CaretOffset() const;
   void SetCaretOffset(int32_t aOffset);
 
   /**
@@ -416,17 +420,17 @@ protected:
   virtual ENameValueFlag NativeName(nsString& aName) MOZ_OVERRIDE;
   virtual void CacheChildren() MOZ_OVERRIDE;
 
   // HyperTextAccessible
 
   /**
    * Transform magic offset into text offset.
    */
-  uint32_t ConvertMagicOffset(int32_t aOffset) const;
+  index_t ConvertMagicOffset(int32_t aOffset) const;
 
   /**
    * Adjust an offset the caret stays at to get a text by line boundary.
    */
   uint32_t AdjustCaretOffset(uint32_t aOffset) const;
 
   /**
    * Return true if caret is at end of line.