[css-grid] Fix behavior of positioned items without specific dimensions

third_party/WebKit/Source/core/layout/LayoutBox.cpp

Index: third_party/WebKit/Source/core/layout/LayoutBox.cpp
diff --git a/third_party/WebKit/Source/core/layout/LayoutBox.cpp b/third_party/WebKit/Source/core/layout/LayoutBox.cpp
index 42db96d936b8e33d6e1432d45b4730df3e40e458..b69ead57aa2046f5ce7d7bc116f0405c4dc20421 100644
--- a/third_party/WebKit/Source/core/layout/LayoutBox.cpp
+++ b/third_party/WebKit/Source/core/layout/LayoutBox.cpp
@@ -75,9 +75,6 @@ namespace blink {
 // Used by flexible boxes when flexing this element and by table cells.
 typedef WTF::HashMap<const LayoutBox*, LayoutUnit> OverrideSizeMap;
 
-static OverrideSizeMap* gExtraInlineOffsetMap = nullptr;
-static OverrideSizeMap* gExtraBlockOffsetMap = nullptr;
-
 // Size of border belt for autoscroll. When mouse pointer in border belt,
 // autoscroll is started.
 static const int autoscrollBeltSize = 20;
@@ -123,7 +120,6 @@ PaintLayerType LayoutBox::layerTypeRequired() const {
 void LayoutBox::willBeDestroyed() {
   clearOverrideSize();
   clearContainingBlockOverrideSize();
-  clearExtraInlineAndBlockOffests();
 
   if (isOutOfFlowPositioned())
     LayoutBlock::removePositionedObject(this);
@@ -1310,34 +1306,6 @@ void LayoutBox::clearOverrideContainingBlockContentLogicalHeight() {
   ensureRareData().m_hasOverrideContainingBlockContentLogicalHeight = false;
 }
 
-LayoutUnit LayoutBox::extraInlineOffset() const {
-  return gExtraInlineOffsetMap ? gExtraInlineOffsetMap->get(this)
-                               : LayoutUnit();
-}
-
-LayoutUnit LayoutBox::extraBlockOffset() const {
-  return gExtraBlockOffsetMap ? gExtraBlockOffsetMap->get(this) : LayoutUnit();
-}
-
-void LayoutBox::setExtraInlineOffset(LayoutUnit inlineOffest) {
-  if (!gExtraInlineOffsetMap)
-    gExtraInlineOffsetMap = new OverrideSizeMap;
-  gExtraInlineOffsetMap->set(this, inlineOffest);
-}
-
-void LayoutBox::setExtraBlockOffset(LayoutUnit blockOffest) {
-  if (!gExtraBlockOffsetMap)
-    gExtraBlockOffsetMap = new OverrideSizeMap;
-  gExtraBlockOffsetMap->set(this, blockOffest);
-}
-
-void LayoutBox::clearExtraInlineAndBlockOffests() {
-  if (gExtraInlineOffsetMap)
-    gExtraInlineOffsetMap->remove(this);
-  if (gExtraBlockOffsetMap)
-    gExtraBlockOffsetMap->remove(this);
-}
-
 LayoutUnit LayoutBox::adjustBorderBoxLogicalWidthForBoxSizing(
     float width) const {
   LayoutUnit bordersPlusPadding = collapsedBorderAndCSSPaddingLogicalWidth();
@@ -3724,6 +3692,14 @@ void LayoutBox::computeInlineStaticDistance(
   if (!logicalLeft.isAuto() || !logicalRight.isAuto())
     return;
 
+  if (child->isGridItem() && child->containingBlock() == child->parent()) {
+    if (child->parent()->style()->direction() == TextDirection::kLtr)
+      logicalLeft.setValue(Fixed, 0);
+    else
+      logicalRight.setValue(Fixed, 0);
+    return;
+  }

[svillar, 2017/01/31 15:28:40]

Mind explaining this change?

[Manuel Rego, 2017/02/01 09:08:20]

So this method is used when in a positioned element both
left and right offsets are "auto".
In order to resolve the "auto" value for them, this method resolves
one of them (left in LTR and right in RTL).
Later in computePositionedLogicalWidthUsing() it's required that one
of them was resolved:
  // 'left' and 'right' cannot both be 'auto' because one would of been
  // converted to the static position already
  ASSERT(!(logicalLeft.isAuto() && logicalRight.isAuto()));

Regarding this method, it was using below things like
"enclosingBox->logicalWidth()", for absolutely positioned grid items
that's wrong, as we don't rely on the "enclosingBox" but on the grid area.

For grid items, if both properties are "auto" we can consider that "left"
(or "right" depending on the direction) are simply "0".
LayoutGrid will be in charge of place it properly depending
on the grid placement properties.

[svillar, 2017/02/01 09:49:07]

OK now I understand that we're just setting it to 0 because we need to resolve
one of them for another method. We should document this because it's far from
explicit in the code.

[Manuel Rego, 2017/02/01 11:08:08]

Actually I've realized we can do this in a different patch:
https://codereview.chromium.org/2665983004/

So that part could be probably easy to review.

[Manuel Rego, 2017/02/01 11:10:34]

Oops sorry this comment is wrong, it was about a different part of the patch.

+
   // For multicol we also need to keep track of the block position, since that
   // determines which column we're in and thus affects the inline position.
   LayoutUnit staticBlockPosition = child->layer()->staticBlockPosition();
@@ -3904,9 +3880,6 @@ void LayoutBox::computePositionedLogicalWidth(
     }
   }
 
-  if (!style()->hasStaticInlinePosition(isHorizontal))
-    computedValues.m_position += extraInlineOffset();
-
   computedValues.m_extent += bordersPlusPadding;
 }
 
@@ -4303,9 +4276,6 @@ void LayoutBox::computePositionedLogicalHeight(
     }
   }
 
-  if (!style()->hasStaticBlockPosition(isHorizontalWritingMode()))
-    computedValues.m_position += extraBlockOffset();
-
   // Set final height value.
   computedValues.m_extent += bordersPlusPadding;
 }