Bug 1361229 - Add explanatory comments and assertion to nsTableFrame to ensure that the value of both smallerIter and aDeletedRowStoredIndex remains valid. r=dbaron
authorNeerja Pancholi <npancholi@mozilla.com>
Mon, 01 May 2017 18:46:05 -0700
changeset 391187 a208e1be391020ad135d5dc3add7fd08184cd341
parent 391186 2981a5649a3a14a6c6034efa3a79754e811bef09
child 391188 5f263d6fb16db08d0d7efc73aaccf53240d0b461
push id53
push userfmarier@mozilla.com
push dateMon, 15 May 2017 17:28:28 +0000
reviewersdbaron
bugs1361229
milestone55.0a1
Bug 1361229 - Add explanatory comments and assertion to nsTableFrame to ensure that the value of both smallerIter and aDeletedRowStoredIndex remains valid. r=dbaron MozReview-Commit-ID: A0dqwpqB2Q0
layout/tables/nsTableFrame.cpp
--- a/layout/tables/nsTableFrame.cpp
+++ b/layout/tables/nsTableFrame.cpp
@@ -987,19 +987,34 @@ nsTableFrame::AddDeletedRowIndex(int32_t
   //              If no such value exists, point to end of map.
   // smallerIter = will point to largest range in the map with higher value
   //              smaller than the aDeletedRowStoredIndex
   //              If no such value exists, point to beginning of map.
   // i.e. when both values exist below is true:
   // smallerIter->second < aDeletedRowStoredIndex < greaterIter->first
   auto greaterIter = mDeletedRowIndexRanges.upper_bound(aDeletedRowStoredIndex);
   auto smallerIter = greaterIter;
+
   if (smallerIter != mDeletedRowIndexRanges.begin()) {
     smallerIter--;
-  }
+    // While greaterIter might be out-of-bounds (by being equal to end()),
+    // smallerIter now cannot be, since we returned early above for a 0-size map.
+  }
+
+  // Note: smallerIter can only be equal to greaterIter when both
+  // of them point to the beginning of the map and in that case smallerIter
+  // does not "exist" but we clip smallerIter to point to beginning of map
+  // so that it doesn't point to something unknown or outside the map boundry.
+  // Note: When greaterIter is not the end (i.e. it "exists") upper_bound()
+  // ensures aDeletedRowStoredIndex < greaterIter->first so no need to
+  // assert that.
+  MOZ_ASSERT(smallerIter == greaterIter ||
+               aDeletedRowStoredIndex > smallerIter->second,
+             "aDeletedRowIndexRanges already contains aDeletedRowStoredIndex! "
+             "Trying to delete an already deleted row?");
 
   if (smallerIter->second == aDeletedRowStoredIndex - 1) {
     if (greaterIter != mDeletedRowIndexRanges.end() &&
         greaterIter->first == aDeletedRowStoredIndex + 1) {
       // merge current index with smaller and greater range as they are consecutive
       smallerIter->second = greaterIter->second;
       mDeletedRowIndexRanges.erase(greaterIter);
     }