NGInlineLayoutAlgorithm should take margins of inline floats into account

third_party/WebKit/Source/core/layout/ng/ng_layout_algorithm.cc

+// Copyright 2017 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.
+
+#include "core/layout/ng/ng_layout_algorithm.h"
+
+#include "core/layout/ng/ng_block_break_token.h"
+#include "core/layout/ng/ng_block_node.h"
+#include "core/layout/ng/ng_inline_break_token.h"
+#include "core/layout/ng/ng_inline_node.h"
+#include "core/layout/ng/ng_length_utils.h"
+
+namespace blink {
+
+template <typename NGInputNodeType, typename NGBreakTokenType>

[kojii, 2017/04/06 13:44:52]

I'm probably missing something but can you explain why we use template, and
where NGBreakTokenType is used?

[Gleb Lanbin, 2017/04/06 16:35:36]

Acknowledged.

+NGBoxStrut
+NGLayoutAlgorithm<NGInputNodeType, NGBreakTokenType>::ComputeMarginsForChild(

[ikilpatrick, 2017/04/06 16:24:31]

So I don't think we want to pull this down into the base class - The only
special thing this does is applying auto margins, which is only applicable to
block-level block flow layout, (i.e. BlockLayoutAlgorithm).

If we pull this down here, I worry that as soon as someone implements grid using
this they'll place a:
 if (isGrid()) { 
  // do something else with margins
}

And we'll be back to the confusing legacy codebase :).

Better to rely on composition here for the moment.

[Gleb Lanbin, 2017/04/06 16:35:36]

Acknowledged.

+    const NGConstraintSpace& space,
+    NGLayoutInputNode* child) {
+  DCHECK(child);
+  const ComputedStyle& child_style = child->Style();

[kojii, 2017/04/06 13:44:52]

This doesn't look right to me...NGInlineNode::Style() is not a style for the
wrapper box it produces. I think you really want is:
  child->IsBlock() ? child->Style() : child->ContainerBlockNode()->Style();
(though we don't have ContainerBlockNode() today)

AFAIU, adding parent link in NGLayoutInputNode is fine, but I'd like to ask that
to Ian.

[Gleb Lanbin, 2017/04/06 16:35:36]

Acknowledged.

+
+  WTF::Optional<MinMaxContentSize> sizes;
+  if (NeedMinMaxContentSize(space, child_style))
+    sizes = child->ComputeMinMaxContentSize();
+
+  LayoutUnit child_inline_size =
+      ComputeInlineSizeForFragment(space, child_style, sizes);
+  NGBoxStrut margins = ComputeMargins(space, child_style, space.WritingMode(),
+                                      space.Direction());
+  ApplyAutoMargins(space, child_style, child_inline_size, &margins);
+  return margins;
+}
+template NGBoxStrut
+NGLayoutAlgorithm<NGBlockNode, NGBlockBreakToken>::ComputeMarginsForChild(
+    NGConstraintSpace const&,
+    NGLayoutInputNode*);
+template NGBoxStrut
+NGLayoutAlgorithm<NGInlineNode, NGInlineBreakToken>::ComputeMarginsForChild(
+    NGConstraintSpace const&,
+    NGLayoutInputNode*);
+
+}  // namespace blink