Bug 447660 part 1 - Replace the #define DISABLE_FLOAT_BREAKING_IN_COLUMNS with a pref to enable fragmenting of floats inside columns. Set the pref enabled by default in non-RELEASE builds only. r=roc
authorMats Palmgren <mats@mozilla.com>
Fri, 19 Dec 2014 16:28:43 +0000
changeset 220552 3c8aa35169bfeb835ec925b0e9e0ae6df621c5df
parent 220551 8a89e46f8ff29b20632599a85d2a161409aa5d41
child 220553 b9c248025d37c5b71c8c59a257cdb044577dfb0f
push id53135
push usermpalmgren@mozilla.com
push dateFri, 19 Dec 2014 16:28:56 +0000
treeherdermozilla-inbound@4d886cc6c0b3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersroc
bugs447660
milestone37.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 447660 part 1 - Replace the #define DISABLE_FLOAT_BREAKING_IN_COLUMNS with a pref to enable fragmenting of floats inside columns. Set the pref enabled by default in non-RELEASE builds only. r=roc
layout/generic/crashtests/crashtests.list
layout/generic/nsBlockFrame.cpp
layout/generic/nsBlockReflowState.cpp
layout/generic/nsBlockReflowState.h
layout/reftests/bugs/563584-6-columns-ref-enabled.html
layout/reftests/bugs/reftest.list
layout/reftests/pagination/reftest.list
modules/libpref/init/all.js
--- a/layout/generic/crashtests/crashtests.list
+++ b/layout/generic/crashtests/crashtests.list
@@ -420,17 +420,18 @@ load 586806-3.html
 load 586973-1.html
 load 589002-1.html
 load 590404.html
 load 591141.html
 asserts(0-1) load 592118.html
 load 594808-1.html
 load 595435-1.xhtml
 load 595740-1.html
-load 600100.xhtml
+pref(layout.float-fragments-inside-column.enabled,true) asserts(1) load 600100.xhtml # bug 866955
+pref(layout.float-fragments-inside-column.enabled,false) load 600100.xhtml
 load 603490-1.html
 load 603510-1.html
 load 604314-1.html
 load 604843.html
 load 605340.html
 load 606642.xhtml
 load 619021.html
 load 621424-1.html
--- a/layout/generic/nsBlockFrame.cpp
+++ b/layout/generic/nsBlockFrame.cpp
@@ -53,18 +53,16 @@
 #include "nsISelection.h"
 
 #include "nsBidiPresUtils.h"
 
 static const int MIN_LINES_NEEDING_CURSOR = 20;
 
 static const char16_t kDiscCharacter = 0x2022;
 
-#define DISABLE_FLOAT_BREAKING_IN_COLUMNS
-
 using namespace mozilla;
 using namespace mozilla::css;
 using namespace mozilla::layout;
 
 static void MarkAllDescendantLinesDirty(nsBlockFrame* aBlock)
 {
   nsLineList::iterator line = aBlock->begin_lines();
   nsLineList::iterator endLine = aBlock->end_lines();
@@ -5857,26 +5855,25 @@ nsBlockFrame::AdjustFloatAvailableSpace(
     // them in the next line
     availISize = aFloatAvailableSpace.ISize(wm);
   }
 
   nscoord availBSize = NS_UNCONSTRAINEDSIZE == aState.ContentBSize()
                        ? NS_UNCONSTRAINEDSIZE
                        : std::max(0, aState.ContentBEnd() - aState.mBCoord);
 
-#ifdef DISABLE_FLOAT_BREAKING_IN_COLUMNS
   if (availBSize != NS_UNCONSTRAINEDSIZE &&
+      !aState.GetFlag(BRS_FLOAT_FRAGMENTS_INSIDE_COLUMN_ENABLED) &&
       nsLayoutUtils::GetClosestFrameOfType(this, nsGkAtoms::columnSetFrame)) {
     // Tell the float it has unrestricted block-size, so it won't break.
     // If the float doesn't actually fit in the column it will fail to be
     // placed, and either move to the block-start of the next column or just
     // overflow.
     availBSize = NS_UNCONSTRAINEDSIZE;
   }
-#endif
 
   return LogicalRect(wm, aState.ContentIStart(), aState.ContentBStart(),
                      availISize, availBSize);
 }
 
 nscoord
 nsBlockFrame::ComputeFloatISize(nsBlockReflowState& aState,
                                 const LogicalRect&  aFloatAvailableSpace,
--- a/layout/generic/nsBlockReflowState.cpp
+++ b/layout/generic/nsBlockReflowState.cpp
@@ -10,25 +10,29 @@
 
 #include "mozilla/DebugOnly.h"
 
 #include "nsBlockFrame.h"
 #include "nsLineLayout.h"
 #include "nsPresContext.h"
 #include "nsIFrameInlines.h"
 #include "mozilla/AutoRestore.h"
+#include "mozilla/Preferences.h"
 #include <algorithm>
 
 #ifdef DEBUG
 #include "nsBlockDebugFlags.h"
 #endif
 
 using namespace mozilla;
 using namespace mozilla::layout;
 
+static bool sFloatFragmentsInsideColumnEnabled;
+static bool sFloatFragmentsInsideColumnPrefCached;
+
 nsBlockReflowState::nsBlockReflowState(const nsHTMLReflowState& aReflowState,
                                        nsPresContext* aPresContext,
                                        nsBlockFrame* aFrame,
                                        bool aBStartMarginRoot,
                                        bool aBEndMarginRoot,
                                        bool aBlockNeedsFloatManager,
                                        nscoord aConsumedBSize)
   : mBlock(aFrame),
@@ -41,16 +45,23 @@ nsBlockReflowState::nsBlockReflowState(c
     mOverflowTracker(nullptr),
     mBorderPadding(mReflowState.ComputedLogicalBorderPadding()),
     mPrevBEndMargin(),
     mLineNumber(0),
     mFlags(0),
     mFloatBreakType(NS_STYLE_CLEAR_NONE),
     mConsumedBSize(aConsumedBSize)
 {
+  if (!sFloatFragmentsInsideColumnPrefCached) {
+    sFloatFragmentsInsideColumnPrefCached = true;
+    Preferences::AddBoolVarCache(&sFloatFragmentsInsideColumnEnabled,
+                                 "layout.float-fragments-inside-column.enabled");
+  }
+  SetFlag(BRS_FLOAT_FRAGMENTS_INSIDE_COLUMN_ENABLED, sFloatFragmentsInsideColumnEnabled);
+  
   WritingMode wm = aReflowState.GetWritingMode();
   SetFlag(BRS_ISFIRSTINFLOW, aFrame->GetPrevInFlow() == nullptr);
   SetFlag(BRS_ISOVERFLOWCONTAINER, IS_TRUE_OVERFLOW_CONTAINER(aFrame));
 
   nsIFrame::LogicalSides logicalSkipSides =
     aFrame->GetLogicalSkipSides(&aReflowState);
   mBorderPadding.ApplySkipSides(logicalSkipSides);
 
@@ -854,22 +865,23 @@ nsBlockReflowState::FlowAndPlaceFloat(ns
   }
   if (aFloat->GetPrevInFlow())
     floatMargin.BStart(wm) = 0;
   if (NS_FRAME_IS_NOT_COMPLETE(reflowStatus))
     floatMargin.BEnd(wm) = 0;
 
   // In the case that we're in columns and not splitting floats, we need
   // to check here that the float's height fit, and if it didn't, bail.
-  // (This code is only for DISABLE_FLOAT_BREAKING_IN_COLUMNS .)
+  // (controlled by the pref "layout.float-fragments-inside-column.enabled")
   //
   // Likewise, if none of the float fit, and it needs to be pushed in
   // its entirety to the next page (NS_FRAME_IS_TRUNCATED or
   // NS_INLINE_IS_BREAK_BEFORE), we need to do the same.
   if ((ContentBSize() != NS_UNCONSTRAINEDSIZE &&
+       !GetFlag(BRS_FLOAT_FRAGMENTS_INSIDE_COLUMN_ENABLED) &&
        adjustedAvailableSpace.BSize(wm) == NS_UNCONSTRAINEDSIZE &&
        !mustPlaceFloat &&
        aFloat->BSize(wm) + floatMargin.BStartEnd(wm) >
        ContentBEnd() - floatPos.B(wm)) ||
       NS_FRAME_IS_TRUNCATED(reflowStatus) ||
       NS_INLINE_IS_BREAK_BEFORE(reflowStatus)) {
     PushFloatPastBreak(aFloat);
     return false;
--- a/layout/generic/nsBlockReflowState.h
+++ b/layout/generic/nsBlockReflowState.h
@@ -27,17 +27,19 @@ class nsOverflowContinuationTracker;
 // Set when the block has the equivalent of NS_BLOCK_FLOAT_MGR
 #define BRS_FLOAT_MGR             0x00000040
 // Set when nsLineLayout::LineIsEmpty was true at the end of reflowing
 // the current line
 #define BRS_LINE_LAYOUT_EMPTY     0x00000080
 #define BRS_ISOVERFLOWCONTAINER   0x00000100
 // Our mPushedFloats list is stored on the blocks' proptable
 #define BRS_PROPTABLE_FLOATCLIST  0x00000200
-#define BRS_LASTFLAG              BRS_PROPTABLE_FLOATCLIST
+// Set when the pref layout.float-fragments-inside-column.enabled is true.
+#define BRS_FLOAT_FRAGMENTS_INSIDE_COLUMN_ENABLED 0x00000400
+#define BRS_LASTFLAG              BRS_FLOAT_FRAGMENTS_INSIDE_COLUMN_ENABLED
 
 class nsBlockReflowState {
 public:
   nsBlockReflowState(const nsHTMLReflowState& aReflowState,
                      nsPresContext* aPresContext,
                      nsBlockFrame* aFrame,
                      bool aBStartMarginRoot, bool aBEndMarginRoot,
                      bool aBlockNeedsFloatManager,
new file mode 100644
--- /dev/null
+++ b/layout/reftests/bugs/563584-6-columns-ref-enabled.html
@@ -0,0 +1,8 @@
+<!DOCTYPE HTML>
+<title>Testcase for float breaking</title>
+<body style="margin: 0">
+  <div style="position: absolute; top: 0; left: 0; width: 150px; height: 100px; background: blue"></div>
+  <div style="position: absolute; top: 100px; left: 0; width: 75px; height: 125px; background: aqua"></div>
+  <div style="position: absolute; top: 100px; left: 75px; width: 100px; height: 125px; background: fuchsia"></div>
+  <div style="position: absolute; top: 0px; left: 200px; width: 75px; height: 75px; background: aqua"></div>
+  <div style="position: absolute; top: 0; left: 275px; width: 100px; height: 225px; background: fuchsia"></div>
--- a/layout/reftests/bugs/reftest.list
+++ b/layout/reftests/bugs/reftest.list
@@ -737,17 +737,18 @@ fails == 385823-2b.html 385823-2-ref.htm
 == 385823-2c.html 385823-2-ref.html
 == 385870-1.html 385870-1-ref.html
 == 385870-2.html 385870-2-ref.html
 == 386014-1a.html 386014-1-ref.html
 == 386014-1b.html 386014-1-ref.html
 == 386014-1c.html 386014-1-ref.html
 == 386065-1.html 386065-1-ref.html
 == 386065-2.html about:blank
-skip-if(B2G) fails == 386147-1.html 386147-1-ref.html # bug 447460
+test-pref(layout.float-fragments-inside-column.enabled,false) skip-if(B2G) fails == 386147-1.html 386147-1-ref.html
+test-pref(layout.float-fragments-inside-column.enabled,true) skip-if(B2G) == 386147-1.html 386147-1-ref.html
 == 386310-1a.html 386310-1-ref.html
 == 386310-1b.html 386310-1-ref.html
 == 386310-1c.html 386310-1-ref.html
 == 386310-1d.html 386310-1-ref.html
 == 386401-1.html 386401-1-ref.html
 == 386401-2.html 386401-2-ref.html
 == 386401-3.html 386401-3-ref.html
 == 386470-1a.html 386470-1-ref.html
@@ -1517,17 +1518,18 @@ skip-if(B2G) == 560455-1.xul 560455-1-re
 == 561981-8.html 561981-8-ref.html
 == 562835-1.html 562835-ref.html
 == 562835-2.html 562835-ref.html
 skip-if(B2G) == 563584-1.html 563584-1-ref.html
 skip-if(B2G) == 563584-2.html 563584-2-ref.html
 skip-if(B2G) == 563584-3.html 563584-3-ref.html
 skip-if(B2G) == 563584-4.html 563584-4-ref.html
 == 563584-5.html 563584-5-ref.html
-== 563584-6-columns.html 563584-6-columns-ref.html
+test-pref(layout.float-fragments-inside-column.enabled,false) == 563584-6-columns.html 563584-6-columns-ref.html
+test-pref(layout.float-fragments-inside-column.enabled,true) == 563584-6-columns.html 563584-6-columns-ref-enabled.html
 skip-if(B2G) == 563584-6-printing.html 563584-6-printing-ref.html
 skip-if(B2G) == 563584-7.html 563584-7-ref.html
 # FIXME: It would be nice to have variants of these -8 tests for the
 # table narrowing quirk causing a change to mIsTopOfPage (though I'm not
 # entirely sure our behavior is the right one, either).
 skip-if(B2G) == 563584-8a.html 563584-8a-ref.html
 skip-if(B2G) == 563584-8b.html 563584-8b-ref.html
 skip-if(B2G) == 563584-8c.html 563584-8c-ref.html
--- a/layout/reftests/pagination/reftest.list
+++ b/layout/reftests/pagination/reftest.list
@@ -26,19 +26,21 @@ fails == border-breaking-004-cols.xhtml 
 == content-inserted-003.xhtml content-inserted-002.ref.xhtml
 == content-inserted-004.xhtml content-inserted-002.ref.xhtml
 == content-inserted-005.xhtml content-inserted-002.ref.xhtml
 == content-inserted-006.xhtml content-inserted-002.ref.xhtml
 == content-inserted-007.xhtml content-inserted-002.ref.xhtml
 == content-inserted-008.xhtml content-inserted-001.ref.xhtml
 == content-inserted-009.xhtml content-inserted-002.ref.xhtml
 == dynamic-abspos-overflow-01-cols.xhtml dynamic-abspos-overflow-01-cols.ref.xhtml
-fails == float-clear-000.html float-clear-000.ref.html
+test-pref(layout.float-fragments-inside-column.enabled,false) fails == float-clear-000.html float-clear-000.ref.html
+test-pref(layout.float-fragments-inside-column.enabled,true) == float-clear-000.html float-clear-000.ref.html
 fails == float-clear-001.html float-clear-000.ref.html
-fails == float-clear-002.html float-clear-000.ref.html
+test-pref(layout.float-fragments-inside-column.enabled,false) fails == float-clear-002.html float-clear-000.ref.html
+test-pref(layout.float-fragments-inside-column.enabled,true) == float-clear-002.html float-clear-000.ref.html
 fails == float-clear-003.html float-clear-000.ref.html
 == float-clear-000-print.html float-clear-000-print.ref.html
 == float-clear-001-print.html float-clear-000-print.ref.html
 == float-clear-002-print.html float-clear-000-print.ref.html
 == float-clear-003-print.html float-clear-000-print.ref.html
 fails == float-continuations-000.html float-continuations-000.ref.html
 == resize-reflow-000.html resize-reflow-000.ref.html
 == resize-reflow-001.html resize-reflow-001.ref.html
--- a/modules/libpref/init/all.js
+++ b/modules/libpref/init/all.js
@@ -2242,16 +2242,23 @@ pref("layout.display-list.dump", false);
 // of the last notification. This can give less tight frame rates
 // but provides more time for other operations when the browser is
 // heavily loaded.
 pref("layout.frame_rate.precise", false);
 
 // pref to control whether layout warnings that are hit quite often are enabled
 pref("layout.spammy_warnings.enabled", true);
 
+// Should we fragment floats inside CSS column layout?
+#ifdef RELEASE_BUILD
+pref("layout.float-fragments-inside-column.enabled", false);
+#else
+pref("layout.float-fragments-inside-column.enabled", true);
+#endif
+
 // Is support for the Web Animations API enabled?
 #ifdef RELEASE_BUILD
 pref("dom.animations-api.core.enabled", false);
 #else
 pref("dom.animations-api.core.enabled", true);
 #endif
 
 // pref to permit users to make verified SOAP calls by default