Bug 1265342 Part 4b: Add some logic and asserts to ensure distance field index values and image index values are in-bounds. r=dholbert
☠☠ backed out by 723a9f786923 ☠ ☠
authorBrad Werth <bwerth@mozilla.com>
Fri, 20 Apr 2018 14:04:15 -0700
changeset 471518 64fc631ef122fbb08ad6f0ebb7caff2f9290ebd2
parent 471517 332382c708e17e5a75d1a12a324987ca96e68093
child 471519 53eb7f14463d61084c148f147808a6c55ec7348b
push id1728
push userjlund@mozilla.com
push dateMon, 18 Jun 2018 21:12:27 +0000
treeherdermozilla-release@c296fde26f5f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdholbert
bugs1265342
milestone61.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1265342 Part 4b: Add some logic and asserts to ensure distance field index values and image index values are in-bounds. r=dholbert MozReview-Commit-ID: 1C9vB14Qhyj
layout/generic/nsFloatManager.cpp
--- a/layout/generic/nsFloatManager.cpp
+++ b/layout/generic/nsFloatManager.cpp
@@ -1155,31 +1155,36 @@ nsFloatManager::ImageShapeInfo::ImageSha
       LayoutDevicePixel::FromAppUnitsRounded(offsetPoint,
                                              aAppUnitsPerDevPixel);
 
     // Since our distance field is computed with a 5x5 neighborhood,
     // we need to expand our distance field by a further 4 pixels in
     // both axes, 2 on the leading edge and 2 on the trailing edge.
     // We call this edge area the "expanded region".
 
+    // Our expansion amounts need to be the same, and non-negative for our
+    // math to work, but we don't want to deal with casting them from
+    // unsigned ints.
+    static int32_t kExpansionPerSide = 2;
+
     // Since dfOffset will be used in comparisons against expanded region
-    // pixel values, it's convenient to add 2 to dfOffset in both axes, to
-    // simplify comparison math later.
-    dfOffset.x += 2;
-    dfOffset.y += 2;
+    // pixel values, it's convenient to add expansion amounts to dfOffset in
+    // both axes, to simplify comparison math later.
+    dfOffset.x += kExpansionPerSide;
+    dfOffset.y += kExpansionPerSide;
 
     // In all these calculations, we purposely ignore aStride, because
     // we don't have to replicate the packing that we received in
     // aAlphaPixels. When we need to convert from df coordinates to
     // alpha coordinates, we do that with math based on row and col.
     const LayoutDeviceIntSize marginRectDevPixels =
       LayoutDevicePixel::FromAppUnitsRounded(aMarginRect.Size(),
                                              aAppUnitsPerDevPixel);
-    const int32_t wEx = marginRectDevPixels.width + 4;
-    const int32_t hEx = marginRectDevPixels.height + 4;
+    const int32_t wEx = marginRectDevPixels.width + (kExpansionPerSide * 2);
+    const int32_t hEx = marginRectDevPixels.height + (kExpansionPerSide * 2);
 
     // Since the margin-box size is CSS controlled, and large values will
     // generate large wEx and hEx values, we do a falliable allocation for
     // the distance field. If allocation fails, we early exit and layout will
     // be wrong, but we'll avoid aborting from OOM.
     auto df = MakeUniqueFallible<dfType[]>(wEx * hEx);
     if (!df) {
       // Without a distance field, we can't reason about the float area.
@@ -1199,31 +1204,38 @@ nsFloatManager::ImageShapeInfo::ImageSha
     // this row by row, from top to bottom. For vertical writing modes, we do
     // column by column, from left to right. We define the two loops
     // generically, then figure out the rows and cols within the inner loop.
     for (int32_t b = 0; b < bSize; ++b) {
       for (int32_t i = 0; i < iSize; ++i) {
         const int32_t col = aWM.IsVertical() ? b : i;
         const int32_t row = aWM.IsVertical() ? i : b;
         const int32_t index = col + row * wEx;
+        MOZ_ASSERT(index >= 0 && index < (wEx * hEx),
+                   "Our distance field index should be in-bounds.");
 
         // Handle our three cases, in order.
-        if (col < 2 ||
-            col >= wEx - 2 ||
-            row < 2 ||
-            row >= hEx - 2) {
+        if (col < kExpansionPerSide ||
+            col >= wEx - kExpansionPerSide ||
+            row < kExpansionPerSide ||
+            row >= hEx - kExpansionPerSide) {
           // Case 1: Expanded pixel.
           df[index] = MAX_MARGIN_5X;
         } else if (col >= dfOffset.x &&
                    col < (dfOffset.x + w) &&
                    row >= dfOffset.y &&
                    row < (dfOffset.y + h) &&
                    aAlphaPixels[col - dfOffset.x +
                                 (row - dfOffset.y) * aStride] > threshold) {
           // Case 2: Image pixel that is opaque.
+          DebugOnly<int32_t> alphaIndex = col - dfOffset.x +
+                                          (row - dfOffset.y) * aStride;
+          MOZ_ASSERT(alphaIndex >= 0 && alphaIndex < (aStride * h),
+            "Our aAlphaPixels index should be in-bounds.");
+
           df[index] = 0;
         } else {
           // Case 3: Other pixel.
 
           // Backward-looking neighborhood distance from target pixel X
           // with chamfer 5-7-11 looks like:
           //
           // +--+--+--+--+--+
@@ -1232,16 +1244,21 @@ nsFloatManager::ImageShapeInfo::ImageSha
           // |11| 7| 5| 7|11|
           // +--+--+--+--+--+
           // |  | 5| X|  |  |
           // +--+--+--+--+--+
           //
           // X should be set to the minimum of MAX_MARGIN_5X and the
           // values of all of the numbered neighbors summed with the
           // value in that chamfer cell.
+          MOZ_ASSERT(index - (wEx * 2) - 1 >= 0 &&
+                     index - wEx - 2 >= 0,
+                     "Our distance field most extreme indices should be "
+                     "in-bounds.");
+
           df[index] = std::min<dfType>(MAX_MARGIN_5X,
                       std::min<dfType>(df[index - (wEx * 2) - 1] + 11,
                       std::min<dfType>(df[index - (wEx * 2) + 1] + 11,
                       std::min<dfType>(df[index - wEx - 2] + 11,
                       std::min<dfType>(df[index - wEx - 1] + 7,
                       std::min<dfType>(df[index - wEx] + 5,
                       std::min<dfType>(df[index - wEx + 1] + 7,
                       std::min<dfType>(df[index - wEx + 2] + 11,
@@ -1262,28 +1279,35 @@ nsFloatManager::ImageShapeInfo::ImageSha
     // expanded region pixels. For each pixel we iterate, we set the df value
     // to the minimum forward-looking neighborhood distance value, computed
     // with a 5-7-11 chamfer. We also check each df value against the
     // usedMargin5X threshold, and use that to set the iMin and iMax values
     // for the interval we'll create for that block axis value (b).
 
     // At the end of each row (or column in vertical writing modes),
     // if any of the other pixels had a value less than usedMargin5X,
-    // we create an interval.
-    for (int32_t b = bSize - 3; b >= 2; --b) {
+    // we create an interval. Note: "bSize - kExpansionPerSide - 1" is the
+    // index of the final row of pixels before the trailing expanded region.
+    for (int32_t b = bSize - kExpansionPerSide - 1;
+         b >= kExpansionPerSide; --b) {
       // iMin tracks the first df pixel and iMax the last df pixel whose
       // df[] value is less than usedMargin5X. Set iMin and iMax in
       // preparation for this row or column.
       int32_t iMin = iSize;
       int32_t iMax = -1;
 
-      for (int32_t i = iSize - 3; i >= 2; --i) {
+      // Note: "iSize - kExpansionPerSide - 1" is the index of the final row
+      // of pixels before the trailing expanded region.
+      for (int32_t i = iSize - kExpansionPerSide - 1;
+           i >= kExpansionPerSide; --i) {
         const int32_t col = aWM.IsVertical() ? b : i;
         const int32_t row = aWM.IsVertical() ? i : b;
         const int32_t index = col + row * wEx;
+        MOZ_ASSERT(index >= 0 && index < (wEx * hEx),
+                   "Our distance field index should be in-bounds.");
 
         // Only apply the chamfer calculation if the df value is not
         // already 0, since the chamfer can only reduce the value.
         if (df[index]) {
           // Forward-looking neighborhood distance from target pixel X
           // with chamfer 5-7-11 looks like:
           //
           // +--+--+--+--+--+
@@ -1292,16 +1316,21 @@ nsFloatManager::ImageShapeInfo::ImageSha
           // |11| 7| 5| 7|11|
           // +--+--+--+--+--+
           // |  |11|  |11|  |
           // +--+--+--+--+--+
           //
           // X should be set to the minimum of its current value and
           // the values of all of the numbered neighbors summed with
           // the value in that chamfer cell.
+          MOZ_ASSERT(index + (wEx * 2) + 1 < (wEx * hEx) &&
+                     index + wEx + 2 < (wEx * hEx),
+                     "Our distance field most extreme indices should be "
+                     "in-bounds.");
+
           df[index] = std::min<dfType>(df[index],
                       std::min<dfType>(df[index + (wEx * 2) + 1] + 11,
                       std::min<dfType>(df[index + (wEx * 2) - 1] + 11,
                       std::min<dfType>(df[index + wEx + 2] + 11,
                       std::min<dfType>(df[index + wEx + 1] + 7,
                       std::min<dfType>(df[index + wEx] + 5,
                       std::min<dfType>(df[index + wEx - 1] + 7,
                       std::min<dfType>(df[index + wEx - 2] + 11,
@@ -1317,22 +1346,23 @@ nsFloatManager::ImageShapeInfo::ImageSha
           MOZ_ASSERT(iMin > i);
           iMin = i;
         }
       }
 
       if (iMax != -1) {
         // Our interval values, iMin, iMax, and b are all calculated from
         // the expanded region, which is based on the margin rect. To create
-        // our interval, we have to subtract 2 from (iMin, iMax, and b) to
-        // account for the expanded region edges.  This produces coords that
-        // are relative to our margin-rect, so we pass in
-        // aMarginRect.TopLeft() to make CreateInterval convert to our
+        // our interval, we have to subtract kExpansionPerSide from (iMin,
+        // iMax, and b) to account for the expanded region edges. This
+        // produces coords that are relative to our margin-rect, so we pass
+        // in aMarginRect.TopLeft() to make CreateInterval convert to our
         // container's coordinate space.
-        CreateInterval(iMin - 2, iMax - 2, b - 2, aAppUnitsPerDevPixel,
+        CreateInterval(iMin - kExpansionPerSide, iMax - kExpansionPerSide,
+                       b - kExpansionPerSide, aAppUnitsPerDevPixel,
                        aMarginRect.TopLeft(), aWM, aContainerSize);
       }
     }
 
     if (!aWM.IsVerticalRL()) {
       // Anything other than vertical-rl or sideways-rl.
       // Because we assembled our intervals on the bottom-up pass,
       // they are reversed for most writing modes. Reverse them to