Add left_offset to NGFloatingObject and set it on legacy FloatingObject

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

 // Copyright 2016 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_block_layout_algorithm.h"
 
 #include "core/layout/ng/ng_absolute_utils.h"
 #include "core/layout/ng/ng_block_break_token.h"
 #include "core/layout/ng/ng_box_fragment.h"
 #include "core/layout/ng/ng_column_mapper.h"
@@ skipping 154 matching lines @@
 
     opportunity_candidate = opportunity_iter.Next();
   }
   return opportunity;
 }
 
 // Calculates the logical offset for opportunity.
 NGLogicalOffset CalculateLogicalOffsetForOpportunity(
     const NGLayoutOpportunity& opportunity,
     const LayoutUnit float_offset,
-    const NGBoxStrut& margins,
-    const NGLogicalOffset& space_offset) {
+    const NGLogicalOffset& from_offset,
+    NGFloatingObject* floating_object) {
+  auto margins = floating_object->margins;
   // Adjust to child's margin.
   LayoutUnit inline_offset = margins.inline_start;
   LayoutUnit block_offset = margins.block_start;
 
   // Offset from the opportunity's block/inline start.
   inline_offset += opportunity.offset.inline_offset;
   block_offset += opportunity.offset.block_offset;
 
+  // Adjust to float: right offset if needed.
   inline_offset += float_offset;
 
-  block_offset -= space_offset.block_offset;
-  inline_offset -= space_offset.inline_offset;
+  block_offset -= from_offset.block_offset;
+  inline_offset -= from_offset.inline_offset;
 
   return NGLogicalOffset(inline_offset, block_offset);
 }
 
 // Calculates the relative position from {@code from_offset} of the
 // floating object that is requested to be positioned from {@code origin_point}.
 NGLogicalOffset PositionFloat(const NGLogicalOffset& origin_point,
                               const NGLogicalOffset& from_offset,
                               NGFloatingObject* floating_object) {
   NGConstraintSpace* float_space = floating_object->space;
@@ skipping 15 matching lines @@
   if (floating_object->exclusion_type == NGExclusion::kFloatRight) {
     float_offset = opportunity.size.inline_size - float_fragment.InlineSize();
   }
 
   // Add the float as an exclusion.
   const NGExclusion exclusion = CreateExclusion(
       float_fragment, opportunity, float_offset, floating_object->margins,
       floating_object->exclusion_type);
   float_space->AddExclusion(exclusion);
 
-  return CalculateLogicalOffsetForOpportunity(
-      opportunity, float_offset, floating_object->margins, from_offset);
+  return CalculateLogicalOffsetForOpportunity(opportunity, float_offset,
+                                              from_offset, floating_object);
+}
+
+// Updates the Floating Object's left offset from the provided parent_space and
+// {@code floating_object}'s space and margins.
+void UpdateFloatingObjectLeftOffset(

[cbiesinger, 2017/02/08 22:39:01]

Left -> Inline, no?

[Gleb Lanbin, 2017/02/09 18:39:25]

you're right. We should use physical coordinates here. I added TODO

At this moment writing modes do not work for me even with regular blocks.
Example: I see logical block_size calculated incorrectly, constantly hitting
ASSERT in ng_length_utils.cc with a simple example like
   <style>
        #container {
          height: 80px;
          width: 200px;
          writing-mode: vertical-lr;
          padding-left: 90px;
        }
        #div1 {
          margin-top: 5px;
          padding-top: 7px;
          height: 10px;
          width: 10px;
        }
      </style>
      <div id='container'>
        <div id='div1'>
        </div>
      </div>

So my plan was to land this patch that enables 
NGBlockLayoutAlgorithmTest::PositionFloatInsideEmptyBlocks test which is based
on the correct expectations, investigate some of TODOs, ASSERTs related to
writing modes and then fix UpdateFloatingObjectLeftOffset. Sounds good?

+    const NGConstraintSpace& parent_space,
+    const Persistent<NGFloatingObject>& floating_object) {
+  const auto& float_space = floating_object->space;
+  floating_object->left_offset = float_space->BfcOffset().inline_offset -
+                                 parent_space.BfcOffset().inline_offset +
+                                 floating_object->margins.inline_start;
 }
 
 // Positions pending floats stored on the fragment builder starting from
-// {@code origin_point}.
-void PositionPendingFloats(const NGLogicalOffset& origin_point,
+// {@code origin_point_block_offset}.
+void PositionPendingFloats(const LayoutUnit origin_point_block_offset,
+                           const NGConstraintSpace& parent_space,
                            NGFragmentBuilder* builder) {
   DCHECK(builder->BfcOffset()) << "Parent BFC offset should be known here";
-  NGLogicalOffset from_offset = builder->BfcOffset().value();
+  LayoutUnit from_offset_block_offset =

[cbiesinger, 2017/02/08 22:39:01]

I think "from_offset_block" reads better

[Gleb Lanbin, 2017/02/09 18:39:25]

Done.

+      builder->BfcOffset().value().block_offset;
 
   for (auto& floating_object : builder->UnpositionedFloats()) {
+    auto float_space = floating_object->space;

[cbiesinger, 2017/02/08 22:39:01]

auto*

[Gleb Lanbin, 2017/02/09 18:39:25]

done. it's Member<const NGConstraintSpace> which is a wrapper around our
pointer.

+    auto float_parent_space = floating_object->parent_space;
+
+    NGLogicalOffset origin_point = {float_space->BfcOffset().inline_offset,
+                                    origin_point_block_offset};
+    NGLogicalOffset from_offset = {
+        float_parent_space->BfcOffset().inline_offset,
+        from_offset_block_offset};
+
     NGLogicalOffset float_fragment_offset =
         PositionFloat(origin_point, from_offset, floating_object);
     builder->AddFloatingObject(floating_object, float_fragment_offset);
+    UpdateFloatingObjectLeftOffset(parent_space, floating_object);
   }
   builder->MutableUnpositionedFloats().clear();
 }
 
 // Whether an in-flow block-level child creates a new formatting context.
 //
 // This will *NOT* check the following cases:
 //  - The child is out-of-flow, e.g. floating or abs-pos.
 //  - The child is a inline-level, e.g. "display: inline-block".
 //  - The child establishes a new formatting context, but should be a child of
@@ skipping 202 matching lines @@
   builder_->SetBlockSize(block_size);
 
   // Layout our absolute and fixed positioned children.
   NGOutOfFlowLayoutPart(Style(), builder_.get()).Run();
 
   // Non empty blocks always know their position in space:
   if (block_size) {
     curr_bfc_offset_.block_offset += curr_margin_strut_.Sum();
     UpdateFragmentBfcOffset(curr_bfc_offset_, ConstraintSpace(),
                             builder_.get());
-    PositionPendingFloats(curr_bfc_offset_, builder_.get());
+    PositionPendingFloats(curr_bfc_offset_.block_offset, ConstraintSpace(),
+                          builder_.get());
   }
 
   // Margins collapsing:
   //   Do not collapse margins between the last in-flow child and bottom margin
   //   of its parent if the parent has height != auto()
   if (!Style().logicalHeight().isAuto()) {
     // TODO(glebl): handle minLogicalHeight, maxLogicalHeight.
     curr_margin_strut_ = NGMarginStrut();
   }
   builder_->SetEndMarginStrut(curr_margin_strut_);
@@ skipping 33 matching lines @@
   // Pull out unpositioned floats to the current fragment. This may needed if
   // for example the child fragment could not position its floats because it's
   // empty and therefore couldn't determine its position in space.
   builder_->MutableUnpositionedFloats().appendVector(
       physical_fragment->UnpositionedFloats());
 
   if (current_child_->Type() == NGLayoutInputNode::kLegacyBlock &&
       CurrentChildStyle().isFloating()) {
     NGFloatingObject* floating_object =
         new NGFloatingObject(physical_fragment.get(), space_for_current_child_,
-                             toNGBlockNode(current_child_), CurrentChildStyle(),
-                             curr_child_margins_);
+                             constraint_space_, toNGBlockNode(current_child_),
+                             CurrentChildStyle(), curr_child_margins_);
     builder_->AddUnpositionedFloat(floating_object);
     // No need to postpone the positioning if we know the correct offset.
     if (builder_->BfcOffset()) {
       NGLogicalOffset origin_point = curr_bfc_offset_;
       // Adjust origin point to the margins of the last child.
       // Example: <div style="margin-bottom: 20px"><float></div>
       //          <div style="margin-bottom: 30px"></div>
       origin_point.block_offset += curr_margin_strut_.Sum();
-      PositionPendingFloats(origin_point, builder_.get());
+      PositionPendingFloats(origin_point.block_offset, ConstraintSpace(),
+                            builder_.get());
     }
     return;
   }
 
   // Fragment that knows its offset can be used to set parent's BFC position.
   if (fragment.BfcOffset()) {
     curr_bfc_offset_.block_offset = fragment.BfcOffset().value().block_offset;
     UpdateFragmentBfcOffset(curr_bfc_offset_, ConstraintSpace(),
                             builder_.get());
-    PositionPendingFloats(curr_bfc_offset_, builder_.get());
+    PositionPendingFloats(curr_bfc_offset_.block_offset, ConstraintSpace(),
+                          builder_.get());
   }
   NGLogicalOffset fragment_offset = CalculateRelativeOffset(fragment);
 
   if (fragmentainer_mapper_)
     fragmentainer_mapper_->ToVisualOffset(fragment_offset);
   else
     fragment_offset.block_offset -= PreviousBreakOffset();
 
   // Update margin strut.
   curr_margin_strut_ = fragment.EndMarginStrut();
@@ skipping 203 matching lines @@
   // https://software.hixie.ch/utilities/js/live-dom-viewer/saved/4847
   if (current_child_style.clear() != EClear::kNone) {
     curr_bfc_offset_.block_offset += curr_margin_strut_.Sum();
     UpdateFragmentBfcOffset(curr_bfc_offset_, ConstraintSpace(),
                             builder_.get());
     // Only collapse margins if it's an adjoining block with clearance.
     if (!content_size_) {
       curr_margin_strut_ = NGMarginStrut();
       curr_child_margins_.block_start = LayoutUnit();
     }
-    PositionPendingFloats(curr_bfc_offset_, builder_.get());
+    PositionPendingFloats(curr_bfc_offset_.block_offset, ConstraintSpace(),
+                          builder_.get());
     AdjustToClearance(constraint_space_->Exclusions(), current_child_style,
                       builder_->BfcOffset().value(), &content_size_);
   }
 
   // Append the current margin strut with child's block start margin.
   // Non empty border/padding use cases are handled inside of the child's
   // layout.
   curr_margin_strut_.Append(curr_child_margins_.block_start);
   space_builder_->SetMarginStrut(curr_margin_strut_);
 
   // Set estimated BFC offset to the next child's constraint space.
   curr_bfc_offset_ = builder_->BfcOffset() ? builder_->BfcOffset().value()
                                            : ConstraintSpace().BfcOffset();
   curr_bfc_offset_.block_offset += content_size_;
   curr_bfc_offset_.inline_offset += border_and_padding_.inline_start;
-  if (ConstraintSpace().IsNewFormattingContext()) {
+
+  if (!CurrentChildStyle().isFloating()) {
     curr_bfc_offset_.inline_offset += curr_child_margins_.inline_start;
   }
+
   space_builder_->SetBfcOffset(curr_bfc_offset_);
 
   return space_builder_->ToConstraintSpace();
 }
 
 }  // namespace blink