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 220625 3c8aa35169bfeb835ec925b0e9e0ae6df621c5df
parent 220624 8a89e46f8ff29b20632599a85d2a161409aa5d41
child 220626 b9c248025d37c5b71c8c59a257cdb044577dfb0f
push id10503
push userryanvm@gmail.com
push dateFri, 19 Dec 2014 20:13:42 +0000
treeherderfx-team@98ee95ac6be5 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersroc
bugs447660
milestone37.0a1
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