Skip to content

Commit

Permalink
Bug 1500608: Don't skip the flex-item early-freeze for devtools after…
Browse files Browse the repository at this point in the history
… all. r=bradwerth

This patch basically reverts the functional part of changeset 52bd865d757c.
I'd optimistically hoped we could skip this early-freeze in order to compute &
report a bit more "potential flexing" information via devtools.  Bbut it turns
out that breaks assertions & produces bogus information for flex items whose
base size vs. min/max-clamped "hypothetical" sizes are very
different. (Specifically: it produces nonsense for flex items whose base sizes,
if unclamped, would reverse the directionality of flexing.)

Differential Revision: https://phabricator.services.mozilla.com/D9304

--HG--
extra : moz-landing-system : lando
  • Loading branch information
dholbert committed Oct 19, 2018
1 parent 8e9ffaa commit a597621
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 7 deletions.
11 changes: 11 additions & 0 deletions dom/flex/test/chrome/test_flex_items.html
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,13 @@
/* This just needs to be large enough so that no shrinking is required. */
width: 1600px;
}
.clamped-huge-item {
flex: 1 1 1650px; /* This needs to be bigger than #flex-sanity, to test
the scenario it's aiming to test (early clamping of a
flex item which, if left unclamped, would've reversed
the direction of flexing & made everything shrink). */
max-width: 10px;
}

.base { align-self: baseline; }
.lastbase { align-self: last baseline; }
Expand Down Expand Up @@ -144,6 +151,9 @@
mainDeltaSize: 0 },
{ mainBaseSize: 50,
mainMaxSize: 10 },
{ mainBaseSize: 1650,
mainMaxSize: 10,
mainDeltaSize: 0 },
{ mainDeltaSize: 0 },
{ /* final item is anonymous flex item */ },
];
Expand Down Expand Up @@ -241,6 +251,7 @@
<!-- Item that wants to grow but is trivially clamped to max-width
below base size: -->
<div style="flex: 1 1 50px; max-width: 10px"></div>
<div class="clamped-huge-item"></div>
<div style="display:contents">
<div class="white">replaced</div>
</div>
Expand Down
8 changes: 1 addition & 7 deletions layout/generic/nsFlexContainerFrame.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2534,13 +2534,7 @@ FlexLine::FreezeItemsEarly(bool aIsUsingFlexGrow,
if (!item->IsFrozen()) {
numUnfrozenItemsToBeSeen--;
bool shouldFreeze = (0.0f == item->GetFlexFactor(aIsUsingFlexGrow));
// NOTE: We skip the "could flex but base size out of range"
// early-freezing if flex devtools are active, so that we can let the
// first run of the main flex layout loop compute how much this item
// wants to flex. (This skipping shouldn't impact results, because
// any affected items will just immediately be caught & frozen as min/max
// violations in that first loop, and that'll trigger another loop.)
if (!shouldFreeze && !aLineInfo) {
if (!shouldFreeze) {
if (aIsUsingFlexGrow) {
if (item->GetFlexBaseSize() > item->GetMainSize()) {
shouldFreeze = true;
Expand Down

0 comments on commit a597621

Please sign in to comment.