Bug 1265342 Part 4b: Add some logic and asserts to ensure distance field index values and image index values are in-bounds. r=dholbert
authorBrad Werth <bwerth@mozilla.com>
Fri, 20 Apr 2018 14:04:15 -0700
changeset 471639 546464551e6411c46b85203a43375ad205827bbb
parent 471638 e5c0631408b2159c4e7ccba4998b2dc35e00eacf
child 471640 c24818678398ed0d911a9d20958673e17a6b8899
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