[css-grid] Move the track sizing algorithm to its own class

third_party/WebKit/Source/core/layout/GridTrackSizingAlgorithm.h

Index: third_party/WebKit/Source/core/layout/GridTrackSizingAlgorithm.h
diff --git a/third_party/WebKit/Source/core/layout/GridTrackSizingAlgorithm.h b/third_party/WebKit/Source/core/layout/GridTrackSizingAlgorithm.h
new file mode 100644
index 0000000000000000000000000000000000000000..a83e40b807cb74b3216591ad907f9b1494573458
--- /dev/null
+++ b/third_party/WebKit/Source/core/layout/GridTrackSizingAlgorithm.h
@@ -0,0 +1,280 @@
+#ifndef GridTrackSizingAlgorithm_h
+#define GridTrackSizingAlgorithm_h
+
+#include "core/style/GridPositionsResolver.h"
+#include "core/style/GridTrackSize.h"
+#include "platform/LayoutUnit.h"
+#include "wtf/HashSet.h"
+#include "wtf/Optional.h"
+#include <memory>
+
+namespace blink {
+
+static const int infinity = -1;
+
+class Grid;
+class GridTrackSizingAlgorithmStrategy;
+class LayoutGrid;
+
+enum SizingOperation { TrackSizing, IntrinsicSizeComputation };
+
+enum TrackSizeComputationPhase {
+  ResolveIntrinsicMinimums,
+  ResolveContentBasedMinimums,
+  ResolveMaxContentMinimums,
+  ResolveIntrinsicMaximums,
+  ResolveMaxContentMaximums,
+  MaximizeTracks,
+};
+
+class GridTrack {
+ public:
+  GridTrack() : m_infinitelyGrowable(false) {}
+
+  LayoutUnit baseSize() const;
+  void setBaseSize(LayoutUnit);
+
+  LayoutUnit growthLimit() const;
+  void setGrowthLimit(LayoutUnit);
+
+  bool infiniteGrowthPotential() const {
+    return growthLimitIsInfinite() || m_infinitelyGrowable;
+  }
+
+  LayoutUnit plannedSize() const { return m_plannedSize; }
+  void setPlannedSize(const LayoutUnit&);

[jfernandez, 2017/01/30 08:48:49]

Why we use a const LayoutUnit reference as input argument ? Does it really worth
to avoid that copy ? 

[svillar, 2017/01/30 09:35:45]

Yeah it does not really pay off, but I didn't change that code. I can use a
LayoutUnit directly as it's a small change.

+
+  LayoutUnit sizeDuringDistribution() const { return m_sizeDuringDistribution; }
+  void setSizeDuringDistribution(const LayoutUnit&);

[jfernandez, 2017/01/30 08:48:49]

ditto

+
+  void growSizeDuringDistribution(const LayoutUnit&);

[jfernandez, 2017/01/30 08:48:49]

ditto

+
+  bool infinitelyGrowable() const { return m_infinitelyGrowable; }
+  void setInfinitelyGrowable(bool infinitelyGrowable) {
+    m_infinitelyGrowable = infinitelyGrowable;
+  }
+
+  Optional<LayoutUnit> growthLimitCap() const { return m_growthLimitCap; }
+  void setGrowthLimitCap(Optional<LayoutUnit>);
+
+ private:
+  bool growthLimitIsInfinite() const { return m_growthLimit == infinity; }
+  bool isGrowthLimitBiggerThanBaseSize() const;
+  void ensureGrowthLimitIsBiggerThanBaseSize();
+
+  LayoutUnit m_baseSize;
+  LayoutUnit m_growthLimit;
+  LayoutUnit m_plannedSize;
+  LayoutUnit m_sizeDuringDistribution;
+  Optional<LayoutUnit> m_growthLimitCap;
+  bool m_infinitelyGrowable;
+};
+
+class GridTrackSizingAlgorithm final {
+ public:
+  GridTrackSizingAlgorithm(const LayoutGrid* layoutGrid, Grid& grid)

[jfernandez, 2017/01/30 08:48:49]

Wouldn't be better to use a reference ? I guess layoutGrid can't be null, right
?

[svillar, 2017/01/30 09:35:45]

Same reason here, we need to use the "this" pointer, which is a const pointer,
for the intrinsic size computation.

+      : m_grid(grid),
+        m_layoutGrid(layoutGrid),
+        m_sizingState(ColumnSizingFirstIteration) {}
+
+  // setup() must be run before calling run() as it configures the behaviour of
+  // the algorithm.
+  void setup(GridTrackSizingDirection,
+             size_t numTracks,
+             SizingOperation,
+             LayoutUnit availableSpace,
+             LayoutUnit freeSpace);
+  void run();
+  void reset();
+
+  // Required by LayoutGrid. Try to minimize the exposed surface.
+  const Grid& grid() const { return m_grid; }
+  GridTrackSize gridTrackSize(GridTrackSizingDirection,
+                              size_t translatedIndex,
+                              SizingOperation) const;
+  LayoutUnit minContentSize() const { return m_minContentSize; };
+  LayoutUnit maxContentSize() const { return m_maxContentSize; };
+
+  Vector<GridTrack>& tracks(GridTrackSizingDirection);
+  const Vector<GridTrack>& tracks(GridTrackSizingDirection) const;
+
+  LayoutUnit& freeSpace(GridTrackSizingDirection direction) {

[jfernandez, 2017/01/30 08:48:49]

Why we return a reference here ?

[svillar, 2017/01/30 09:35:45]

This hasn't changed. The reason is because some functions modify the free space.
Then you have 2 options, you return by value and then call setFreeSpace(), or
you return a reference and modify the value directly. The former was considered
more convenient because you don't need to add a setFreeSpace call for every
single early return in the code.

[Manuel Rego, 2017/01/30 11:00:46]

Just a suggestion, maybe we could rename the method so it's clear
we return a reference and avoid confusion with a "typo" or whatever.

[svillar, 2017/01/30 11:28:29]

I don't think the problem is the name. As mentioned if we want to change how we
use it, it deserves another separate patch.

+    return direction == ForRows ? m_freeSpaceRows : m_freeSpaceColumns;
+  }
+
+#if DCHECK_IS_ON()
+  bool tracksAreWiderThanMinTrackBreadth() const;
+#endif
+
+ private:
+  GridTrackSize gridTrackSize(GridTrackSizingDirection,
+                              size_t translatedIndex) const;
+  GridTrackSize rawGridTrackSize(GridTrackSizingDirection,
+                                 size_t translatedIndex) const;
+  LayoutUnit assumedRowsSizeForOrthogonalChild(const LayoutBox&) const;
+  LayoutUnit computeTrackBasedSize() const;
+
+  // Helper methods for step 1. initializeTrackSizes().
+  LayoutUnit initialBaseSize(const GridTrackSize&) const;
+  LayoutUnit initialGrowthLimit(const GridTrackSize&,
+                                LayoutUnit baseSize) const;
+
+  // Helper methods for step 2. resolveIntrinsicTrackSizes().
+  void sizeTrackToFitNonSpanningItem(const GridSpan&,
+                                     LayoutBox& gridItem,
+                                     GridTrack&);
+  bool spanningItemCrossesFlexibleSizedTracks(const GridSpan&) const;
+  typedef struct GridItemsSpanGroupRange GridItemsSpanGroupRange;
+  template <TrackSizeComputationPhase phase>
+  void increaseSizesToAccommodateSpanningItems(
+      const GridItemsSpanGroupRange& gridItemsWithSpan);
+  LayoutUnit itemSizeForTrackSizeComputationPhase(TrackSizeComputationPhase,
+                                                  LayoutBox&) const;
+  template <TrackSizeComputationPhase phase>
+  void distributeSpaceToTracks(Vector<GridTrack*>& tracks,
+                               Vector<GridTrack*>* growBeyondGrowthLimitsTracks,
+                               LayoutUnit& availableLogicalSpace) const;
+  LayoutUnit gridAreaBreadthForChild(const LayoutBox&,
+                                     GridTrackSizingDirection);
+
+  void computeGridContainerIntrinsicSizes();
+
+  // Helper methods for step 4.
+  typedef HashSet<size_t,
+                  DefaultHash<size_t>::Hash,
+                  WTF::UnsignedWithZeroKeyHashTraits<size_t>>
+      TrackIndexSet;
+  double computeFlexFactorUnitSize(
+      const Vector<GridTrack>& tracks,
+      double flexFactorSum,
+      LayoutUnit& leftOverSpace,
+      const Vector<size_t, 8>& flexibleTracksIndexes,
+      std::unique_ptr<TrackIndexSet> tracksToTreatAsInflexible = nullptr) const;
+  void computeFlexSizedTracksGrowth(double flexFraction,
+                                    Vector<LayoutUnit>& increments,
+                                    LayoutUnit& totalGrowth) const;
+  double findFrUnitSize(const GridSpan& tracksSpan,
+                        LayoutUnit leftOverSpace) const;
+
+  // Track sizing algorithm steps.

[Manuel Rego, 2017/01/30 11:00:46]

Here I see 3 steps.
But before I see helper methods for step 1, 2 and 4.
What happened with step 3?
Why we've 4 before and 3 now?

[svillar, 2017/01/30 11:28:29]

Because the maximizeTracks step is done entirely in the strategies. I could add
a oneliner in the algorithm which calls the strategy but I thought it was not
adding anything.

+  void initializeTrackSizes();
+  void resolveIntrinsicTrackSizes();
+  void stretchFlexibleTracks(LayoutUnit freeSpace);
+
+  // State machine.
+  void advanceNextState();
+  bool isValidTransition() const;
+
+  // Data.
+  bool m_needsSetup{true};
+  LayoutUnit m_availableSpace;
+
+  LayoutUnit m_freeSpaceColumns;
+  LayoutUnit m_freeSpaceRows;
+
+  // We need to keep both alive in order to properly size grids with orthogonal
+  // writing modes.
+  Vector<GridTrack> m_columns;
+  Vector<GridTrack> m_rows;
+  Vector<size_t> m_contentSizedTracksIndex;
+  Vector<size_t> m_flexibleSizedTracksIndex;
+
+  GridTrackSizingDirection m_direction;
+  SizingOperation m_sizingOperation;

[jfernandez, 2017/01/30 08:48:49]

Wouldn't make sense to initialize this field ?

[svillar, 2017/01/30 09:35:45]

I don't think so. You need to specify it in the setup() call as you don't know
in advance if you're going to do a layout or a intrinsic size computation.

+
+  Grid& m_grid;
+
+  const LayoutGrid* m_layoutGrid;

[jfernandez, 2017/01/30 08:48:49]

Why not using a reference here ?

[svillar, 2017/01/30 09:35:45]

See comments about the "this" pointer.

+  std::unique_ptr<GridTrackSizingAlgorithmStrategy> m_strategy;
+
+  // The track sizing algorithm is used for both layout and intrinsic size

[jfernandez, 2017/01/30 08:48:48]

I think these comments need some formatting.

[svillar, 2017/01/30 09:35:45]

Acknowledged.

+  // computation. We're
+  // normally just interested in intrinsic inline sizes (a.k.a widths in most of
+  // the cases) for the
+  // computeIntrinsicLogicalWidths() computations. That's why we don't need to
+  // keep around different
+  // values for rows/columns.
+  LayoutUnit m_minContentSize;
+  LayoutUnit m_maxContentSize;
+
+  enum SizingState {
+    ColumnSizingFirstIteration,
+    RowSizingFirstIteration,
+    ColumnSizingSecondIteration,
+    RowSizingSecondIteration
+  };
+  SizingState m_sizingState;
+
+  // This is a RAII class used to ensure that the track sizing algorithm is

[jfernandez, 2017/01/30 08:48:49]

need some formatting ? 

[svillar, 2017/01/30 09:35:45]

Acknowledged.

+  // executed as it is
+  // suppossed to be, i.e., first resolve columns and then rows. Only if
+  // required a second iteration
+  // is run following the same order, first columns and then rows.
+  class GridTrackSizingAlgorithmMachineState {
+   public:
+    GridTrackSizingAlgorithmMachineState(GridTrackSizingAlgorithm& algorithm)
+        : m_algorithm(algorithm) {
+      DCHECK(m_algorithm.isValidTransition());
+      DCHECK(!m_algorithm.m_needsSetup);
+    }
+
+    ~GridTrackSizingAlgorithmMachineState() {
+      m_algorithm.advanceNextState();

[jfernandez, 2017/01/30 08:48:48]

Why we need to call advanceNextState() when destroying the state machine
instance ?

[svillar, 2017/01/30 09:35:45]

Because that's how this class works. It's created inside the run() method. So
once created it checks the validity of the transition, and when the method
finishes then it automatically advances the state, that's why I mention it's a
RAII class, we use its lifetime to perform actions.

+      m_algorithm.m_needsSetup = true;
+    }
+
+   private:
+    GridTrackSizingAlgorithm& m_algorithm;
+  };
+
+  friend class GridTrackSizingAlgorithmStrategy;
+};
+
+class GridTrackSizingAlgorithmStrategy {
+  WTF_MAKE_NONCOPYABLE(GridTrackSizingAlgorithmStrategy);
+  USING_FAST_MALLOC(GridTrackSizingAlgorithmStrategy);
+
+ public:
+  virtual LayoutUnit minContentForChild(LayoutBox& child);

[jfernandez, 2017/01/30 08:48:49]

the argument name is not needed.

[svillar, 2017/01/30 09:35:45]

Acknowledged.

+  virtual LayoutUnit maxContentForChild(LayoutBox& child) const;
+  virtual LayoutUnit minSizeForChild(LayoutBox& child);
+
+  virtual void maximizeTracks(Vector<GridTrack>&, LayoutUnit& freeSpace);

[jfernandez, 2017/01/30 08:48:48]

Why freeSpace is a reference ?

[svillar, 2017/01/30 09:35:45]

Because the maximize operation reduces the free space. As the free space is an
attribute of the GridTrackSizingAlgorithm class we need to pass it here as
reference to get it updated.

+  virtual double findUsedFlexFraction(Vector<size_t>& flexibleSizedTracksIndex,
+                                      GridTrackSizingDirection,
+                                      LayoutUnit initialFreeSpace) const;
+  virtual bool recomputeUsedFlexFractionIfNeeded(
+      Vector<size_t>& flexibleSizedTracksIndex,
+      double& flexFraction,
+      Vector<LayoutUnit>& increments,
+      LayoutUnit& totalGrowth) const;
+
+ protected:
+  GridTrackSizingAlgorithmStrategy(GridTrackSizingAlgorithm& algorithm)
+      : m_algorithm(algorithm) {}
+
+  LayoutUnit logicalHeightForChild(LayoutBox&) const;
+
+  bool updateOverrideContainingBlockContentSizeForChild(
+      LayoutBox&,
+      GridTrackSizingDirection) const;
+  LayoutUnit computeTrackBasedSize() const {
+    return m_algorithm.computeTrackBasedSize();
+  }
+  GridTrackSizingDirection direction() const { return m_algorithm.m_direction; }
+  double findFrUnitSize(const GridSpan& tracksSpan,
+                        LayoutUnit leftOverSpace) const {
+    return m_algorithm.findFrUnitSize(tracksSpan, leftOverSpace);
+  }
+  void distributeSpaceToTracks(Vector<GridTrack*>& tracks,
+                               LayoutUnit& availableLogicalSpace) const {
+    m_algorithm.distributeSpaceToTracks<MaximizeTracks>(tracks, nullptr,
+                                                        availableLogicalSpace);
+  }
+  const LayoutGrid* layoutGrid() const { return m_algorithm.m_layoutGrid; }
+
+  GridTrackSizingAlgorithm& m_algorithm;
+};
+}
+
+#endif  // GridTrackSizingAlgorithm_h