Bug 1499542 part 2: Skip frozen flex items when recording grow-vs-shrink state and deltas. r=bradwerth
authorDaniel Holbert <dholbert@cs.stanford.edu>
Wed, 17 Oct 2018 18:52:33 +0000
changeset 490404 3ea646bde4de4772a439a31798406287d2be685e
parent 490403 52bd865d757c2787845e1bb79ed3936484d6ce3b
child 490405 2ce0ac92bee586c4e67a302929e1532ec8831505
push id247
push userfmarier@mozilla.com
push dateSat, 27 Oct 2018 01:06:44 +0000
reviewersbradwerth
bugs1499542
milestone64.0a1
Bug 1499542 part 2: Skip frozen flex items when recording grow-vs-shrink state and deltas. r=bradwerth Frozen flex items have already been clamped to their min/max sizes, so their size-change isn't a "how much we want to flex" measurement, and it's not useful for determining whether the rest of the line is shrinking. This patch is just adding a `if (!item->IsFrozen())` check around some code, and reindenting that code, and adjusting a few comments. This change is necessary to avoid failing the "GrowthState" assertions, because (for example) a flex item that's clamped to a small max-width could fool us into thinking we're in a "shrinking" state (since its final size is smaller than its base size), even though really we're in a "growth" state and the item was simply clamped. We avoid this problem by skipping (potentially-clamped) frozen items when determining the line's GrowthState. Depends on D8922 Differential Revision: https://phabricator.services.mozilla.com/D9018
layout/generic/nsFlexContainerFrame.cpp
--- a/layout/generic/nsFlexContainerFrame.cpp
+++ b/layout/generic/nsFlexContainerFrame.cpp
@@ -2855,47 +2855,54 @@ FlexLine::ResolveFlexibleLengths(nscoord
         // If we have an aLineInfo structure to fill out, capture any
         // size changes that may have occurred in the previous loop.
         // We don't do this inside the previous loop, because we don't
         // want to burden layout when aLineInfo is null.
         if (aLineInfo) {
           uint32_t itemIndex = 0;
           for (FlexItem* item = mItems.getFirst(); item; item = item->getNext(),
                                                          ++itemIndex) {
-            // Calculate a deltaSize that represents how much the
-            // flex sizing algorithm "wants" to stretch or shrink this
-            // item during this pass through the algorithm. Later
-            // passes through the algorithm may overwrite this value.
-            // Also, this value may not reflect how much the size of
-            // the item is actually changed, since the size of the
-            // item will be clamped to min and max values later in
-            // this pass. That's intentional, since we want to report
-            // the value that the sizing algorithm tried to stretch
-            // or shrink the item.
-            nscoord deltaSize = item->GetMainSize() -
-              aLineInfo->mItems[itemIndex].mMainBaseSize;
-
-            aLineInfo->mItems[itemIndex].mMainDeltaSize = deltaSize;
-            // If any item on the line is growing, mark the aLineInfo
-            // structure; likewise if any item is shrinking. Items in
-            // a line can't be both growing and shrinking.
-            if (deltaSize > 0) {
-              MOZ_ASSERT(item->IsFrozen() || isUsingFlexGrow,
-                "Unfrozen items shouldn't grow without isUsingFlexGrow.");
-              MOZ_ASSERT(aLineInfo->mGrowthState !=
-                         ComputedFlexLineInfo::GrowthState::SHRINKING);
-              aLineInfo->mGrowthState =
-                ComputedFlexLineInfo::GrowthState::GROWING;
-            } else if (deltaSize < 0) {
-              MOZ_ASSERT(item->IsFrozen() || !isUsingFlexGrow,
-               "Unfrozen items shouldn't shrink with isUsingFlexGrow.");
-              MOZ_ASSERT(aLineInfo->mGrowthState !=
-                         ComputedFlexLineInfo::GrowthState::GROWING);
-              aLineInfo->mGrowthState =
-                ComputedFlexLineInfo::GrowthState::SHRINKING;
+            if (!item->IsFrozen()) {
+              // Calculate a deltaSize that represents how much the flex sizing
+              // algorithm "wants" to stretch or shrink this item during this
+              // pass through the algorithm. Later passes through the algorithm
+              // may overwrite this, until this item is frozen. Note that this
+              // value may not reflect how much the size of the item is
+              // actually changed, since the size of the item will be clamped
+              // to min and max values later in this pass. That's intentional,
+              // since we want to report the value that the sizing algorithm
+              // tried to stretch or shrink the item.
+              nscoord deltaSize = item->GetMainSize() -
+                aLineInfo->mItems[itemIndex].mMainBaseSize;
+
+              aLineInfo->mItems[itemIndex].mMainDeltaSize = deltaSize;
+              // If any (unfrozen) item on the line is growing, we mark the
+              // aLineInfo structure; likewise if any item is shrinking.
+              // (Note: a line can't contain a mix of items that are growing
+              // and shrinking. Also, the sign of any delta should match the
+              // type of flex factor we're using [grow vs shrink].)
+              if (deltaSize > 0) {
+                MOZ_ASSERT(isUsingFlexGrow,
+                           "Unfrozen items can only grow if we're "
+                           "distributing (positive) space with flex-grow");
+                MOZ_ASSERT(aLineInfo->mGrowthState !=
+                           ComputedFlexLineInfo::GrowthState::SHRINKING,
+                           "shouldn't flip flop from shrinking to growing");
+                aLineInfo->mGrowthState =
+                  ComputedFlexLineInfo::GrowthState::GROWING;
+              } else if (deltaSize < 0) {
+                MOZ_ASSERT(!isUsingFlexGrow,
+                           "Unfrozen items can only shrink if we're "
+                           "distributing (negative) space with flex-shrink");
+                MOZ_ASSERT(aLineInfo->mGrowthState !=
+                           ComputedFlexLineInfo::GrowthState::GROWING,
+                           "shouldn't flip flop from growing to shrinking");
+                aLineInfo->mGrowthState =
+                  ComputedFlexLineInfo::GrowthState::SHRINKING;
+              }
             }
           }
         }
       }
     }
 
     // Fix min/max violations:
     nscoord totalViolation = 0; // keeps track of adjustments for min/max