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

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

 /*
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  *           (C) 2005 Allan Sandfeld Jensen (kde@carewolf.com)
  *           (C) 2005, 2006 Samuel Weinig (sam.weinig@gmail.com)
  * Copyright (C) 2005, 2006, 2007, 2008, 2009, 2010 Apple Inc.
  *               All rights reserved.
  * Copyright (C) 2013 Adobe Systems Incorporated. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
@@ skipping 57 matching lines @@
 #include "platform/geometry/FloatRoundedRect.h"
 #include "wtf/PtrUtil.h"
 #include <algorithm>
 #include <math.h>
 
 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;
 static const unsigned backgroundObscurationTestMaxDepth = 4;
 
 struct SameSizeAsLayoutBox : public LayoutBoxModelObject {
   LayoutRect frameRect;
   LayoutUnit intrinsicContentLogicalHeight;
   LayoutRectOutsets marginBoxOutsets;
   LayoutUnit preferredLogicalWidth[2];
@@ skipping 25 matching lines @@
 
   if (hasOverflowClip())
     return OverflowClipPaintLayer;
 
   return NoPaintLayer;
 }
 
 void LayoutBox::willBeDestroyed() {
   clearOverrideSize();
   clearContainingBlockOverrideSize();
-  clearExtraInlineAndBlockOffests();
 
   if (isOutOfFlowPositioned())
     LayoutBlock::removePositionedObject(this);
   removeFromPercentHeightContainer();
   if (isOrthogonalWritingModeRoot() && !documentBeingDestroyed())
     unmarkOrthogonalWritingModeRoot();
 
   ShapeOutsideInfo::removeInfo(*this);
 
   BoxPaintInvalidator::boxWillBeDestroyed(*this);
@@ skipping 1166 matching lines @@
 }
 
 // TODO (lajava) Shouldn't we implement these functions based on physical
 // direction ?.
 void LayoutBox::clearOverrideContainingBlockContentLogicalHeight() {
   if (!m_rareData)
     return;
   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();
   LayoutUnit result(width);
   if (style()->boxSizing() == EBoxSizing::kContentBox)
     return result + bordersPlusPadding;
   return std::max(result, bordersPlusPadding);
 }
 
 LayoutUnit LayoutBox::adjustBorderBoxLogicalHeightForBoxSizing(
@@ skipping 2366 matching lines @@
 
 void LayoutBox::computeInlineStaticDistance(
     Length& logicalLeft,
     Length& logicalRight,
     const LayoutBox* child,
     const LayoutBoxModelObject* containerBlock,
     LayoutUnit containerLogicalWidth) {
   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();
 
   // FIXME: The static distance computation has not been patched for mixed
   // writing modes yet.
   if (child->parent()->style()->direction() == TextDirection::kLtr) {
     LayoutUnit staticPosition = child->layer()->staticInlinePosition() -
                                 containerBlock->borderLogicalLeft();
     for (LayoutObject* curr = child->parent(); curr && curr != containerBlock;
@@ skipping 160 matching lines @@
         logicalRightLength, marginLogicalLeft, marginLogicalRight, minValues);
 
     if (computedValues.m_extent < minValues.m_extent) {
       computedValues.m_extent = minValues.m_extent;
       computedValues.m_position = minValues.m_position;
       computedValues.m_margins.m_start = minValues.m_margins.m_start;
       computedValues.m_margins.m_end = minValues.m_margins.m_end;
     }
   }
 
-  if (!style()->hasStaticInlinePosition(isHorizontal))
-    computedValues.m_position += extraInlineOffset();
-
   computedValues.m_extent += bordersPlusPadding;
 }
 
 void LayoutBox::computeLogicalLeftPositionedOffset(
     LayoutUnit& logicalLeftPos,
     const LayoutBox* child,
     LayoutUnit logicalWidthValue,
     const LayoutBoxModelObject* containerBlock,
     LayoutUnit containerLogicalWidth) {
   // Deal with differing writing modes here. Our offset needs to be in the
@@ skipping 376 matching lines @@
                                         marginBefore, marginAfter, minValues);
 
     if (computedValues.m_extent < minValues.m_extent) {
       computedValues.m_extent = minValues.m_extent;
       computedValues.m_position = minValues.m_position;
       computedValues.m_margins.m_before = minValues.m_margins.m_before;
       computedValues.m_margins.m_after = minValues.m_margins.m_after;
     }
   }
 
-  if (!style()->hasStaticBlockPosition(isHorizontalWritingMode()))
-    computedValues.m_position += extraBlockOffset();
-
   // Set final height value.
   computedValues.m_extent += bordersPlusPadding;
 }
 
 void LayoutBox::computeLogicalTopPositionedOffset(
     LayoutUnit& logicalTopPos,
     const LayoutBox* child,
     LayoutUnit logicalHeightValue,
     const LayoutBoxModelObject* containerBlock,
     LayoutUnit containerLogicalHeight) {
@@ skipping 1375 matching lines @@
     block->adjustChildDebugRect(rect);
 
   return rect;
 }
 
 bool LayoutBox::shouldClipOverflow() const {
   return hasOverflowClip() || styleRef().containsPaint() || hasControlClip();
 }
 
 }  // namespace blink