[LayoutNG] Don't store size constraints in LayoutBox.

We don't need to store the sizes on the legacy object at all, since we
only need this during layout.

Reduces the need for allocating (fairly big) LayoutBoxRareData objects.
It also eliminates the need for storing OverrideAvailableInlineSize at
all in LayoutBoxRareData, since it's only set from LayoutNG.

Change-Id: I291713cac3c20d8fe81d656627eed7c8a830e9fd
Reviewed-on: https://chromium-review.googlesource.com/c/1472711
Commit-Queue: Morten Stenshorne <mstensho@chromium.org>
Reviewed-by: Emil A Eklund <eae@chromium.org>
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#632778}

third_party/blink/renderer/core/layout/BUILD.gn

@@ -31,6 +31,7 @@ blink_core_sources("layout") {
     "bidi_run.h",
     "bidi_run_for_line.cc",
     "bidi_run_for_line.h",
+    "box_layout_extra_input.h",
     "collapsed_border_value.cc",
     "collapsed_border_value.h",
     "column_balancer.cc",

third_party/blink/renderer/core/layout/box_layout_extra_input.h

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef THIRD_PARTY_BLINK_RENDERER_CORE_LAYOUT_BOX_LAYOUT_EXTRA_INPUT_H_
+#define THIRD_PARTY_BLINK_RENDERER_CORE_LAYOUT_BOX_LAYOUT_EXTRA_INPUT_H_
+
+#include "base/optional.h"
+#include "third_party/blink/renderer/platform/geometry/layout_unit.h"
+
+namespace blink {
+
+class LayoutBox;
+
+// Extra input data for laying out a LayoutBox. The object will automatically
+// associate itself with the specified LayoutBox upon creation, and dissociate
+// itself upon destruction.
+struct BoxLayoutExtraInput {
+  BoxLayoutExtraInput(LayoutBox&);
+  ~BoxLayoutExtraInput();
+
+  LayoutBox& box;
+
+  // When set, no attempt should be be made to resolve the inline size. Use this
+  // one instead.
+  base::Optional<LayoutUnit> override_inline_size;
+
+  // When set, no attempt should be be made to resolve the block size. Use this
+  // one instead.
+  base::Optional<LayoutUnit> override_block_size;
+
+  // Available inline size. https://drafts.csswg.org/css-sizing/#available
+  LayoutUnit available_inline_size;
+
+  // The content size of the containing block. These are somewhat vague legacy
+  // layout values, that typically either mean available size or percentage
+  // resolution size.
+  LayoutUnit containing_block_content_inline_size;
+  LayoutUnit containing_block_content_block_size;
+};
+
+}  // namespace blink
+
+#endif  // THIRD_PARTY_BLINK_RENDERER_CORE_LAYOUT_BOX_LAYOUT_EXTRA_INPUT_H_

third_party/blink/renderer/core/layout/layout_box.cc

@@ -43,6 +43,7 @@
 #include "third_party/blink/renderer/core/input/event_handler.h"
 #include "third_party/blink/renderer/core/layout/api/line_layout_block_flow.h"
 #include "third_party/blink/renderer/core/layout/api/line_layout_box.h"
+#include "third_party/blink/renderer/core/layout/box_layout_extra_input.h"
 #include "third_party/blink/renderer/core/layout/custom/layout_custom.h"
 #include "third_party/blink/renderer/core/layout/custom/layout_worklet.h"
 #include "third_party/blink/renderer/core/layout/custom/layout_worklet_global_scope_proxy.h"
@@ -96,12 +97,20 @@ struct SameSizeAsLayoutBox : public LayoutBoxModelObject {
   LayoutUnit intrinsic_content_logical_height;
   LayoutRectOutsets margin_box_outsets;
   LayoutUnit preferred_logical_width[2];
-  void* pointers[3];
+  void* pointers[4];
 };
 
 static_assert(sizeof(LayoutBox) == sizeof(SameSizeAsLayoutBox),
               "LayoutBox should stay small");
 
+BoxLayoutExtraInput::BoxLayoutExtraInput(LayoutBox& box) : box(box) {
+  box.SetBoxLayoutExtraInput(this);
+}
+
+BoxLayoutExtraInput::~BoxLayoutExtraInput() {
+  box.SetBoxLayoutExtraInput(nullptr);
+}
+
 LayoutBox::LayoutBox(ContainerNode* node)
     : LayoutBoxModelObject(node),
       intrinsic_content_logical_height_(-1),
@@ -1334,38 +1343,50 @@ LayoutUnit LayoutBox::MaxPreferredLogicalWidth() const {
 
 LayoutUnit LayoutBox::OverrideLogicalWidth() const {
   DCHECK(HasOverrideLogicalWidth());
+  if (extra_input_ && extra_input_->override_inline_size)
+    return *extra_input_->override_inline_size;
   return rare_data_->override_logical_width_;
 }
 
 LayoutUnit LayoutBox::OverrideLogicalHeight() const {
   DCHECK(HasOverrideLogicalHeight());
+  if (extra_input_ && extra_input_->override_block_size)
+    return *extra_input_->override_block_size;
   return rare_data_->override_logical_height_;
 }
 
 bool LayoutBox::HasOverrideLogicalHeight() const {
+  if (extra_input_ && extra_input_->override_block_size)
+    return true;
   return rare_data_ && rare_data_->override_logical_height_ != -1;
 }
 
 bool LayoutBox::HasOverrideLogicalWidth() const {
+  if (extra_input_ && extra_input_->override_inline_size)
+    return true;
   return rare_data_ && rare_data_->override_logical_width_ != -1;
 }
 
 void LayoutBox::SetOverrideLogicalHeight(LayoutUnit height) {
+  DCHECK(!extra_input_);
   DCHECK_GE(height, 0);
   EnsureRareData().override_logical_height_ = height;
 }
 
 void LayoutBox::SetOverrideLogicalWidth(LayoutUnit width) {
+  DCHECK(!extra_input_);
   DCHECK_GE(width, 0);
   EnsureRareData().override_logical_width_ = width;
 }
 
 void LayoutBox::ClearOverrideLogicalHeight() {
+  DCHECK(!extra_input_);
   if (rare_data_)
     rare_data_->override_logical_height_ = LayoutUnit(-1);
 }
 
 void LayoutBox::ClearOverrideLogicalWidth() {
+  DCHECK(!extra_input_);
   if (rare_data_)
     rare_data_->override_logical_width_ = LayoutUnit(-1);
 }
@@ -1390,40 +1411,41 @@ LayoutUnit LayoutBox::OverrideContentLogicalHeight() const {
 LayoutUnit LayoutBox::OverrideContainingBlockContentWidth() const {
   DCHECK(HasOverrideContainingBlockContentWidth());
   return ContainingBlock()->StyleRef().IsHorizontalWritingMode()
-             ? rare_data_->override_containing_block_content_logical_width_
-             : rare_data_->override_containing_block_content_logical_height_;
+             ? OverrideContainingBlockContentLogicalWidth()
+             : OverrideContainingBlockContentLogicalHeight();
 }
 
 LayoutUnit LayoutBox::OverrideContainingBlockContentHeight() const {
   DCHECK(HasOverrideContainingBlockContentHeight());
   return ContainingBlock()->StyleRef().IsHorizontalWritingMode()
-             ? rare_data_->override_containing_block_content_logical_height_
-             : rare_data_->override_containing_block_content_logical_width_;
+             ? OverrideContainingBlockContentLogicalHeight()
+             : OverrideContainingBlockContentLogicalWidth();
 }
 
 bool LayoutBox::HasOverrideContainingBlockContentWidth() const {
-  if (!rare_data_ || !ContainingBlock())
+  if (!ContainingBlock())
     return false;
 
   return ContainingBlock()->StyleRef().IsHorizontalWritingMode()
-             ? rare_data_->has_override_containing_block_content_logical_width_
-             : rare_data_
-                   ->has_override_containing_block_content_logical_height_;
+             ? HasOverrideContainingBlockContentLogicalWidth()
+             : HasOverrideContainingBlockContentLogicalHeight();
 }
 
 bool LayoutBox::HasOverrideContainingBlockContentHeight() const {
-  if (!rare_data_ || !ContainingBlock())
+  if (!ContainingBlock())
     return false;
 
   return ContainingBlock()->StyleRef().IsHorizontalWritingMode()
-             ? rare_data_->has_override_containing_block_content_logical_height_
-             : rare_data_->has_override_containing_block_content_logical_width_;
+             ? HasOverrideContainingBlockContentLogicalHeight()
+             : HasOverrideContainingBlockContentLogicalWidth();
 }
 
 // TODO (lajava) Shouldn't we implement these functions based on physical
 // direction ?.
 LayoutUnit LayoutBox::OverrideContainingBlockContentLogicalWidth() const {
   DCHECK(HasOverrideContainingBlockContentLogicalWidth());
+  if (extra_input_)
+    return extra_input_->containing_block_content_inline_size;
   return rare_data_->override_containing_block_content_logical_width_;
 }
 
@@ -1431,12 +1453,16 @@ LayoutUnit LayoutBox::OverrideContainingBlockContentLogicalWidth() const {
 // direction ?.
 LayoutUnit LayoutBox::OverrideContainingBlockContentLogicalHeight() const {
   DCHECK(HasOverrideContainingBlockContentLogicalHeight());
+  if (extra_input_)
+    return extra_input_->containing_block_content_block_size;
   return rare_data_->override_containing_block_content_logical_height_;
 }
 
 // TODO (lajava) Shouldn't we implement these functions based on physical
 // direction ?.
 bool LayoutBox::HasOverrideContainingBlockContentLogicalWidth() const {
+  if (extra_input_)
+    return true;
   return rare_data_ &&
          rare_data_->has_override_containing_block_content_logical_width_;
 }
@@ -1444,6 +1470,8 @@ bool LayoutBox::HasOverrideContainingBlockContentLogicalWidth() const {
 // TODO (lajava) Shouldn't we implement these functions based on physical
 // direction ?.
 bool LayoutBox::HasOverrideContainingBlockContentLogicalHeight() const {
+  if (extra_input_)
+    return true;
   return rare_data_ &&
          rare_data_->has_override_containing_block_content_logical_height_;
 }
@@ -1452,6 +1480,7 @@ bool LayoutBox::HasOverrideContainingBlockContentLogicalHeight() const {
 // direction ?.
 void LayoutBox::SetOverrideContainingBlockContentLogicalWidth(
     LayoutUnit logical_width) {
+  DCHECK(!extra_input_);
   DCHECK_GE(logical_width, LayoutUnit(-1));
   EnsureRareData().override_containing_block_content_logical_width_ =
       logical_width;
@@ -1462,6 +1491,7 @@ void LayoutBox::SetOverrideContainingBlockContentLogicalWidth(
 // direction ?.
 void LayoutBox::SetOverrideContainingBlockContentLogicalHeight(
     LayoutUnit logical_height) {
+  DCHECK(!extra_input_);
   DCHECK_GE(logical_height, LayoutUnit(-1));
   EnsureRareData().override_containing_block_content_logical_height_ =
       logical_height;
@@ -1471,6 +1501,7 @@ void LayoutBox::SetOverrideContainingBlockContentLogicalHeight(
 // TODO (lajava) Shouldn't we implement these functions based on physical
 // direction ?.
 void LayoutBox::ClearOverrideContainingBlockContentSize() {
+  DCHECK(!extra_input_);
   if (!rare_data_)
     return;
   EnsureRareData().has_override_containing_block_content_logical_width_ = false;
@@ -1492,11 +1523,6 @@ void LayoutBox::SetOverridePercentageResolutionBlockSize(
     LayoutUnit logical_height) {
   DCHECK_GE(logical_height, LayoutUnit(-1));
   auto& rare_data = EnsureRareData();
-
-  // The actual data field is shared with override available inline size. They
-  // cannot be in use at the same time.
-  DCHECK(!rare_data.has_override_available_inline_size_);
-
   rare_data.override_percentage_resolution_block_size_ = logical_height;
   rare_data.has_override_percentage_resolution_block_size_ = true;
 }
@@ -1509,29 +1535,9 @@ void LayoutBox::ClearOverridePercentageResolutionBlockSize() {
 
 LayoutUnit LayoutBox::OverrideAvailableInlineSize() const {
   DCHECK(HasOverrideAvailableInlineSize());
-  return rare_data_->override_percentage_resolution_block_size_;
-}
-
-bool LayoutBox::HasOverrideAvailableInlineSize() const {
-  return rare_data_ && rare_data_->has_override_available_inline_size_;
-}
-
-void LayoutBox::SetOverrideAvailableInlineSize(LayoutUnit inline_size) {
-  DCHECK_GE(inline_size, LayoutUnit());
-  auto& rare_data = EnsureRareData();
-
-  // The actual data field is shared with override block percentage resolution
-  // size. They cannot be in use at the same time.
-  DCHECK(!rare_data.has_override_percentage_resolution_block_size_);
-
-  rare_data.override_available_inline_size_ = inline_size;
-  rare_data.has_override_available_inline_size_ = true;
-}
-
-void LayoutBox::ClearOverrideAvailableInlineSize() {
-  if (!rare_data_)
-    return;
-  EnsureRareData().has_override_available_inline_size_ = false;
+  if (extra_input_)
+    return extra_input_->available_inline_size;
+  return LayoutUnit();
 }
 
 LayoutUnit LayoutBox::AdjustBorderBoxLogicalWidthForBoxSizing(

third_party/blink/renderer/core/layout/layout_box.h

@@ -39,6 +39,7 @@ namespace blink {
 class EventHandler;
 class LayoutBlockFlow;
 class LayoutMultiColumnSpannerPlaceholder;
+struct BoxLayoutExtraInput;
 struct NGPhysicalBoxStrut;
 class ShapeOutsideInfo;
 
@@ -72,7 +73,6 @@ struct LayoutBoxRareData {
         has_override_containing_block_content_logical_width_(false),
         has_override_containing_block_content_logical_height_(false),
         has_override_percentage_resolution_block_size_(false),
-        has_override_available_inline_size_(false),
         has_previous_content_box_rect_and_layout_overflow_rect_(false),
         percent_height_container_(nullptr),
         snap_container_(nullptr),
@@ -88,22 +88,11 @@ struct LayoutBoxRareData {
   bool has_override_containing_block_content_logical_width_ : 1;
   bool has_override_containing_block_content_logical_height_ : 1;
   bool has_override_percentage_resolution_block_size_ : 1;
-  bool has_override_available_inline_size_ : 1;
   bool has_previous_content_box_rect_and_layout_overflow_rect_ : 1;
 
   LayoutUnit override_containing_block_content_logical_width_;
   LayoutUnit override_containing_block_content_logical_height_;
-
-  // Put two members that aren't used at the same time inside a union, to save
-  // space. There are DCHECKs to make sure that they aren't actually used at the
-  // same time. Currently, the percentage resolution block size is only used by
-  // custom layout children, while the available inline size is only used by
-  // LayoutNG. So, since custom layout containers are laid out by legacy, this
-  // should be safe.
-  union {
-    LayoutUnit override_percentage_resolution_block_size_;
-    LayoutUnit override_available_inline_size_;
-  };
+  LayoutUnit override_percentage_resolution_block_size_;
 
   LayoutUnit offset_to_next_page_;
 
@@ -746,6 +735,10 @@ class CORE_EXPORT LayoutBox : public LayoutBoxModelObject {
                      MapCoordinatesFlags mode = 0) const override;
   FloatRect LocalBoundingBoxRectForAccessibility() const final;
 
+  void SetBoxLayoutExtraInput(const BoxLayoutExtraInput* input) {
+    extra_input_ = input;
+  }
+
   void UpdateLayout() override;
   void Paint(const PaintInfo&) const override;
 
@@ -802,9 +795,7 @@ class CORE_EXPORT LayoutBox : public LayoutBoxModelObject {
   // available inline size, rather than deducing it from the containing block
   // (and then subtract space taken up by adjacent floats).
   LayoutUnit OverrideAvailableInlineSize() const;
-  bool HasOverrideAvailableInlineSize() const;
-  void SetOverrideAvailableInlineSize(LayoutUnit);
-  void ClearOverrideAvailableInlineSize();
+  bool HasOverrideAvailableInlineSize() const { return extra_input_; }
 
   LayoutUnit AdjustBorderBoxLogicalWidthForBoxSizing(float width) const;
   LayoutUnit AdjustBorderBoxLogicalHeightForBoxSizing(float height) const;
@@ -1829,6 +1820,11 @@ class CORE_EXPORT LayoutBox : public LayoutBoxModelObject {
 
   std::unique_ptr<BoxOverflowModel> overflow_;
 
+  // Extra layout input data. This one may be set during layout, and cleared
+  // afterwards. Always nullptr when this object isn't in the process of being
+  // laid out.
+  const BoxLayoutExtraInput* extra_input_ = nullptr;
+
   union {
     // The inline box containing this LayoutBox, for atomic inline elements.
     // Valid only when !IsInLayoutNGInlineFormattingContext().

third_party/blink/renderer/core/layout/ng/ng_block_node.cc

@@ -8,6 +8,7 @@
 
 #include "third_party/blink/renderer/core/frame/local_frame_view.h"
 #include "third_party/blink/renderer/core/html/html_marquee_element.h"
+#include "third_party/blink/renderer/core/layout/box_layout_extra_input.h"
 #include "third_party/blink/renderer/core/layout/layout_block_flow.h"
 #include "third_party/blink/renderer/core/layout/layout_fieldset.h"
 #include "third_party/blink/renderer/core/layout/layout_inline.h"
@@ -858,42 +859,26 @@ scoped_refptr<NGLayoutResult> NGBlockNode::RunOldLayout(
   const NGConstraintSpace* old_space =
       block ? block->CachedConstraintSpace() : nullptr;
   if (!old_space || box_->NeedsLayout() || *old_space != constraint_space) {
-    LayoutUnit inline_size =
+    BoxLayoutExtraInput input(*box_);
+    input.containing_block_content_inline_size =
         CalculateAvailableInlineSizeForLegacy(*box_, constraint_space);
-    LayoutUnit block_size =
+    input.containing_block_content_block_size =
         CalculateAvailableBlockSizeForLegacy(*box_, constraint_space);
 
-    LayoutObject* containing_block = box_->ContainingBlock();
-    bool parallel_writing_mode;
-    if (!containing_block) {
-      parallel_writing_mode = true;
-    } else {
-      parallel_writing_mode = IsParallelWritingMode(
-          containing_block->StyleRef().GetWritingMode(), writing_mode);
-    }
-    if (parallel_writing_mode) {
-      box_->SetOverrideContainingBlockContentLogicalWidth(inline_size);
-      box_->SetOverrideContainingBlockContentLogicalHeight(block_size);
-    } else {
-      // OverrideContainingBlock should be in containing block writing mode.
-      box_->SetOverrideContainingBlockContentLogicalWidth(block_size);
-      box_->SetOverrideContainingBlockContentLogicalHeight(inline_size);
+    if (LayoutObject* containing_block = box_->ContainingBlock()) {
+      if (!IsParallelWritingMode(containing_block->StyleRef().GetWritingMode(),
+                                 writing_mode)) {
+        // The sizes should be in the containing block writing mode.
+        std::swap(input.containing_block_content_block_size,
+                  input.containing_block_content_inline_size);
+      }
     }
+    input.available_inline_size = constraint_space.AvailableSize().inline_size;
 
-    if (constraint_space.IsFixedSizeInline()) {
-      box_->SetOverrideLogicalWidth(
-          constraint_space.AvailableSize().inline_size);
-    } else {
-      box_->ClearOverrideLogicalWidth();
-    }
-    if (constraint_space.IsFixedSizeBlock()) {
-      box_->SetOverrideLogicalHeight(
-          constraint_space.AvailableSize().block_size);
-    } else {
-      box_->ClearOverrideLogicalHeight();
-    }
-    box_->SetOverrideAvailableInlineSize(
-        constraint_space.AvailableSize().inline_size);
+    if (constraint_space.IsFixedSizeInline())
+      input.override_inline_size = constraint_space.AvailableSize().inline_size;
+    if (constraint_space.IsFixedSizeBlock())
+      input.override_block_size = constraint_space.AvailableSize().block_size;
     box_->ComputeAndSetBlockDirectionMargins(box_->ContainingBlock());
 
     // Using |LayoutObject::LayoutIfNeeded| save us a little bit of overhead,
@@ -904,11 +889,6 @@ scoped_refptr<NGLayoutResult> NGBlockNode::RunOldLayout(
     else
       box_->ForceChildLayout();
 
-    // Reset the containing block size override size, now that we're done with
-    // subtree layout. Min/max calculation that depends on the block size of the
-    // container (e.g. objects with intrinsic ratio and percentage block size)
-    // in a subsequent layout pass might otherwise become wrong.
-    box_->ClearOverrideContainingBlockContentSize();
     if (block)
       block->SetCachedConstraintSpace(constraint_space);
   }