[css-layout-api] IntrinsicSizes follow up

This change is a follow-up to the intrinsicSizes implementation based
on feedback in the following CL:
https://chromium-review.googlesource.com/c/chromium/src/+/1949536

The main changes are as follows:
1. Pass in the correct information to
CalculateChildPercentageBlockSizeForMinMax().
2. Subtract out border_scrollbar_padding_ instead of border_padding_
from the intrinsic sizes if in legacy layout.

Bug: 726125
Change-Id: I430a029ef76f7326a3224593c4c9bac18c0e94d0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1980895
Commit-Queue: Alison Maher <almaher@microsoft.com>
Reviewed-by: Christian Biesinger <cbiesinger@chromium.org>
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#728232}

third_party/blink/renderer/core/layout/ng/custom/css_layout_definition.cc

@@ -90,6 +90,7 @@ bool CSSLayoutDefinition::Instance::Layout(
     const NGBlockNode& node,
     const LogicalSize& border_box_size,
     const NGBoxStrut& border_scrollbar_padding,
+    const LayoutUnit child_percentage_resolution_block_size_for_min_max,
     CustomLayoutScope* custom_layout_scope,
     FragmentResultOptions* fragment_result_options,
     scoped_refptr<SerializedScriptValue>* fragment_result_data) {
@@ -144,8 +145,10 @@ bool CSSLayoutDefinition::Instance::Layout(
 
   // Run the work queue until exhaustion.
   while (!custom_layout_scope->Queue()->IsEmpty()) {
-    for (auto& task : *custom_layout_scope->Queue())
-      task.Run(node, space, node.Style(), border_scrollbar_padding);
+    for (auto& task : *custom_layout_scope->Queue()) {
+      task.Run(space, node.Style(),
+               child_percentage_resolution_block_size_for_min_max);
+    }
     custom_layout_scope->Queue()->clear();
     {
       v8::MicrotasksScope microtasks_scope(isolate, microtask_queue,
@@ -218,6 +221,7 @@ bool CSSLayoutDefinition::Instance::IntrinsicSizes(
     const NGBlockNode& node,
     const LogicalSize& border_box_size,
     const NGBoxStrut& border_scrollbar_padding,
+    const LayoutUnit child_percentage_resolution_block_size_for_min_max,
     CustomLayoutScope* custom_layout_scope,
     IntrinsicSizesResultOptions* intrinsic_sizes_result_options) {
   ScriptState* script_state = definition_->GetScriptState();
@@ -266,8 +270,10 @@ bool CSSLayoutDefinition::Instance::IntrinsicSizes(
 
   // Run the work queue until exhaustion.
   while (!custom_layout_scope->Queue()->IsEmpty()) {
-    for (auto& task : *custom_layout_scope->Queue())
-      task.Run(node, space, node.Style(), border_scrollbar_padding);
+    for (auto& task : *custom_layout_scope->Queue()) {
+      task.Run(space, node.Style(),
+               child_percentage_resolution_block_size_for_min_max);
+    }
     custom_layout_scope->Queue()->clear();
     {
       v8::MicrotasksScope microtasks_scope(isolate, microtask_queue,

third_party/blink/renderer/core/layout/ng/custom/css_layout_definition.h

@@ -18,6 +18,7 @@ namespace blink {
 class CustomLayoutScope;
 class FragmentResultOptions;
 class IntrinsicSizesResultOptions;
+class LayoutUnit;
 struct LogicalSize;
 class NGBlockNode;
 struct NGBoxStrut;
@@ -54,24 +55,28 @@ class CSSLayoutDefinition final : public GarbageCollected<CSSLayoutDefinition>,
     // Runs the web developer defined layout, returns true if everything
     // succeeded. It populates the FragmentResultOptions dictionary, and
     // fragment_result_data.
-    bool Layout(const NGConstraintSpace&,
-                const Document&,
-                const NGBlockNode&,
-                const LogicalSize& border_box_size,
-                const NGBoxStrut& border_scrollbar_padding,
-                CustomLayoutScope*,
-                FragmentResultOptions*,
-                scoped_refptr<SerializedScriptValue>* fragment_result_data);
+    bool Layout(
+        const NGConstraintSpace&,
+        const Document&,
+        const NGBlockNode&,
+        const LogicalSize& border_box_size,
+        const NGBoxStrut& border_scrollbar_padding,
+        const LayoutUnit child_percentage_resolution_block_size_for_min_max,
+        CustomLayoutScope*,
+        FragmentResultOptions*,
+        scoped_refptr<SerializedScriptValue>* fragment_result_data);
 
     // Runs the web developer defined intrinsicSizes, returns true if everything
     // succeeded. It populates the IntrinsicSizesResultOptions dictionary.
-    bool IntrinsicSizes(const NGConstraintSpace&,
-                        const Document&,
-                        const NGBlockNode&,
-                        const LogicalSize& border_box_size,
-                        const NGBoxStrut& border_scrollbar_padding,
-                        CustomLayoutScope*,
-                        IntrinsicSizesResultOptions*);
+    bool IntrinsicSizes(
+        const NGConstraintSpace&,
+        const Document&,
+        const NGBlockNode&,
+        const LogicalSize& border_box_size,
+        const NGBoxStrut& border_scrollbar_padding,
+        const LayoutUnit child_percentage_resolution_block_size_for_min_max,
+        CustomLayoutScope*,
+        IntrinsicSizesResultOptions*);
 
     void Trace(blink::Visitor*);
 

third_party/blink/renderer/core/layout/ng/custom/custom_intrinsic_sizes.h

@@ -15,7 +15,7 @@ class CustomLayoutChild;
 
 // This represents the result of intrinsicSizes (on a LayoutChild).
 //
-// This should eventually mirror the information in a MinMaxSize, it has the
+// This should mirror the information in a MinMaxSize, and it has the
 // additional capability that it is exposed to web developers.
 class CustomIntrinsicSizes : public ScriptWrappable {
   DEFINE_WRAPPERTYPEINFO();

third_party/blink/renderer/core/layout/ng/custom/custom_layout_work_task.cc

@@ -39,16 +39,17 @@ CustomLayoutWorkTask::CustomLayoutWorkTask(
 
 CustomLayoutWorkTask::~CustomLayoutWorkTask() = default;
 
-void CustomLayoutWorkTask::Run(const NGBlockNode& parent,
-                               const NGConstraintSpace& parent_space,
-                               const ComputedStyle& parent_style,
-                               const NGBoxStrut& border_scrollbar_padding) {
+void CustomLayoutWorkTask::Run(
+    const NGConstraintSpace& parent_space,
+    const ComputedStyle& parent_style,
+    const LayoutUnit child_percentage_resolution_block_size_for_min_max) {
   DCHECK(token_->IsValid());
   NGLayoutInputNode child = child_->GetLayoutNode();
 
   if (type_ == CustomLayoutWorkTask::TaskType::kIntrinsicSizes) {
-    RunIntrinsicSizesTask(parent, parent_space, parent_style,
-                          border_scrollbar_padding, child);
+    RunIntrinsicSizesTask(parent_style,
+                          child_percentage_resolution_block_size_for_min_max,
+                          child);
   } else {
     DCHECK_EQ(type_, CustomLayoutWorkTask::TaskType::kLayoutFragment);
     RunLayoutFragmentTask(parent_space, parent_style, child);
@@ -138,22 +139,13 @@ void CustomLayoutWorkTask::RunLayoutFragmentTask(
 }
 
 void CustomLayoutWorkTask::RunIntrinsicSizesTask(
-    const NGBlockNode& parent,
-    const NGConstraintSpace& parent_space,
     const ComputedStyle& parent_style,
-    const NGBoxStrut& border_scrollbar_padding,
+    const LayoutUnit child_percentage_resolution_block_size_for_min_max,
     NGLayoutInputNode child) {
   DCHECK_EQ(type_, CustomLayoutWorkTask::TaskType::kIntrinsicSizes);
   DCHECK(resolver_);
 
-  // TODO(almaher) should use border_padding instead of
-  // border_scrollbar_padding.
-  LayoutUnit child_percentage_resolution_block_size =
-      CalculateChildPercentageBlockSizeForMinMax(
-          parent_space, parent, border_scrollbar_padding,
-          parent_space.PercentageResolutionBlockSize());
-
-  MinMaxSizeInput input(child_percentage_resolution_block_size);
+  MinMaxSizeInput input(child_percentage_resolution_block_size_for_min_max);
   MinMaxSize sizes =
       ComputeMinAndMaxContentContribution(parent_style, child, input);
   resolver_->Resolve(MakeGarbageCollected<CustomIntrinsicSizes>(

third_party/blink/renderer/core/layout/ng/custom/custom_layout_work_task.h

@@ -14,12 +14,11 @@ namespace blink {
 class ComputedStyle;
 class CustomLayoutChild;
 class CustomLayoutToken;
-class NGBlockNode;
+class LayoutUnit;
 class NGConstraintSpace;
 class NGLayoutInputNode;
 class SerializedScriptValue;
 class ScriptPromiseResolver;
-struct NGBoxStrut;
 
 // Contains all the information needed to resolve a promise with a fragment or
 // intrinsic-sizes.
@@ -46,10 +45,9 @@ class CustomLayoutWorkTask {
   ~CustomLayoutWorkTask();
 
   // Runs this work task.
-  void Run(const NGBlockNode& parent,
-           const NGConstraintSpace& parent_space,
+  void Run(const NGConstraintSpace& parent_space,
            const ComputedStyle& parent_style,
-           const NGBoxStrut& border_scrollbar_padding);
+           const LayoutUnit child_percentage_resolution_block_size_for_min_max);
 
  private:
   Persistent<CustomLayoutChild> child_;
@@ -62,11 +60,10 @@ class CustomLayoutWorkTask {
   void RunLayoutFragmentTask(const NGConstraintSpace& parent_space,
                              const ComputedStyle& parent_style,
                              NGLayoutInputNode child);
-  void RunIntrinsicSizesTask(const NGBlockNode& parent,
-                             const NGConstraintSpace& parent_space,
-                             const ComputedStyle& parent_style,
-                             const NGBoxStrut& border_scrollbar_padding,
-                             NGLayoutInputNode child);
+  void RunIntrinsicSizesTask(
+      const ComputedStyle& parent_style,
+      const LayoutUnit child_percentage_resolution_block_size_for_min_max,
+      NGLayoutInputNode child);
 };
 
 }  // namespace blink

third_party/blink/renderer/core/layout/ng/custom/ng_custom_layout_algorithm.cc

@@ -31,6 +31,11 @@ NGCustomLayoutAlgorithm::NGCustomLayoutAlgorithm(
   container_builder_.SetIsNewFormattingContext(
       params.space.IsNewFormattingContext());
   container_builder_.SetInitialFragmentGeometry(params.fragment_geometry);
+  const NGConstraintSpace& space = ConstraintSpace();
+  child_percentage_resolution_block_size_for_min_max_ =
+      CalculateChildPercentageBlockSizeForMinMax(
+          space, Node(), border_padding_,
+          space.PercentageResolutionBlockSize());
 }
 
 base::Optional<MinMaxSize> NGCustomLayoutAlgorithm::ComputeMinMaxSize(
@@ -56,10 +61,11 @@ base::Optional<MinMaxSize> NGCustomLayoutAlgorithm::ComputeMinMaxSize(
 
   IntrinsicSizesResultOptions* intrinsic_sizes_result_options =
       IntrinsicSizesResultOptions::Create();
-  if (!instance->IntrinsicSizes(ConstraintSpace(), document, Node(),
-                                container_builder_.InitialBorderBoxSize(),
-                                border_scrollbar_padding_, &scope,
-                                intrinsic_sizes_result_options)) {
+  if (!instance->IntrinsicSizes(
+          ConstraintSpace(), document, Node(),
+          container_builder_.InitialBorderBoxSize(), border_scrollbar_padding_,
+          child_percentage_resolution_block_size_for_min_max_, &scope,
+          intrinsic_sizes_result_options)) {
     // TODO(ikilpatrick): Report this error to the developer.
     return FallbackMinMaxSize(input);
   }
@@ -72,7 +78,7 @@ base::Optional<MinMaxSize> NGCustomLayoutAlgorithm::ComputeMinMaxSize(
                           intrinsic_sizes_result_options->minContentSize()));
 
   if (input.size_type == NGMinMaxSizeType::kContentBoxSize)
-    sizes -= border_padding_.InlineSum();
+    sizes -= border_scrollbar_padding_.InlineSum();
 
   sizes.min_size.ClampNegativeToZero();
   sizes.max_size.ClampNegativeToZero();
@@ -105,10 +111,11 @@ scoped_refptr<const NGLayoutResult> NGCustomLayoutAlgorithm::Layout() {
   FragmentResultOptions* fragment_result_options =
       FragmentResultOptions::Create();
   scoped_refptr<SerializedScriptValue> fragment_result_data;
-  if (!instance->Layout(ConstraintSpace(), document, Node(),
-                        container_builder_.InitialBorderBoxSize(),
-                        border_scrollbar_padding_, &scope,
-                        fragment_result_options, &fragment_result_data)) {
+  if (!instance->Layout(
+          ConstraintSpace(), document, Node(),
+          container_builder_.InitialBorderBoxSize(), border_scrollbar_padding_,
+          child_percentage_resolution_block_size_for_min_max_, &scope,
+          fragment_result_options, &fragment_result_data)) {
     // TODO(ikilpatrick): Report this error to the developer.
     return FallbackLayout();
   }

third_party/blink/renderer/core/layout/ng/custom/ng_custom_layout_algorithm.h

@@ -32,6 +32,7 @@ class CORE_EXPORT NGCustomLayoutAlgorithm
   const NGLayoutAlgorithmParams& params_;
   const NGBoxStrut border_padding_;
   const NGBoxStrut border_scrollbar_padding_;
+  LayoutUnit child_percentage_resolution_block_size_for_min_max_;
 };
 
 }  // namespace blink