[CSS Grid Layout] Don't resolve align-self and justify-self properties

Source/core/css/CSSComputedStyleDeclaration.cpp

Index: Source/core/css/CSSComputedStyleDeclaration.cpp
diff --git a/Source/core/css/CSSComputedStyleDeclaration.cpp b/Source/core/css/CSSComputedStyleDeclaration.cpp
index edf275fb242cb567cfbe21a909e9f6092b17b53d..9bb6e34483f110bb5323ccb6072782789822207e 100644
--- a/Source/core/css/CSSComputedStyleDeclaration.cpp
+++ b/Source/core/css/CSSComputedStyleDeclaration.cpp
@@ -1537,17 +1537,6 @@ Node* CSSComputedStyleDeclaration::styledNode() const
     return m_node.get();
 }
 
-static ItemPosition resolveAlignmentAuto(ItemPosition position, Node* element)
-{
-    if (position != ItemPositionAuto)
-        return position;
-
-    bool isFlexOrGrid = element && element->computedStyle()
-        && element->computedStyle()->isDisplayFlexibleOrGridBox();
-
-    return isFlexOrGrid ? ItemPositionStretch : ItemPositionStart;
-}
-
 static PassRefPtrWillBeRawPtr<CSSValueList> valueForItemPositionWithOverflowAlignment(ItemPosition itemPosition, OverflowAlignment overflowAlignment, ItemPositionType positionType)
 {
     RefPtrWillBeRawPtr<CSSValueList> result = CSSValueList::createSpaceSeparated();
@@ -1830,9 +1819,19 @@ PassRefPtrWillBeRawPtr<CSSValue> CSSComputedStyleDeclaration::getPropertyCSSValu
         case CSSPropertyAlignContent:
             return cssValuePool().createValue(style->alignContent());
         case CSSPropertyAlignItems:
-            return valueForItemPositionWithOverflowAlignment(resolveAlignmentAuto(style->alignItems(), styledNode), style->alignItemsOverflowAlignment(), NonLegacyPosition);
-        case CSSPropertyAlignSelf:
-            return valueForItemPositionWithOverflowAlignment(resolveAlignmentAuto(style->alignSelf(), styledNode->parentNode()), style->alignSelfOverflowAlignment(), NonLegacyPosition);
+            return valueForItemPositionWithOverflowAlignment(style->alignItems(), style->alignItemsOverflowAlignment(), NonLegacyPosition);
+        case CSSPropertyAlignSelf: {
+            ItemPosition alignSelf = style->alignSelf();
+            OverflowAlignment overflow = style->alignSelfOverflowAlignment();
+            if (alignSelf == ItemPositionAuto) {
+                Node* parent = styledNode->parentNode();
+                if (parent && parent->computedStyle()) {
+                    alignSelf = parent->computedStyle()->alignItems();
+                    overflow = parent->computedStyle()->alignItemsOverflowAlignment();

[esprehn, 2014/08/19 00:56:33]

This seems like code duplication, also same issues as below.

[jfernandez, 2014/08/19 21:00:12]

Do you mean code duplication with the logic managing the justify-self property ?

[esprehn, 2014/09/03 21:45:28]

Yes.

[jfernandez, 2014/09/03 22:27:49]

Done.

+                }
+            }
+            return valueForItemPositionWithOverflowAlignment(alignSelf, overflow, NonLegacyPosition);
+        }
         case CSSPropertyFlex:
             return valuesForShorthandProperty(flexShorthand());
         case CSSPropertyFlexBasis:
@@ -2000,9 +1999,19 @@ PassRefPtrWillBeRawPtr<CSSValue> CSSComputedStyleDeclaration::getPropertyCSSValu
         case CSSPropertyIsolation:
             return cssValuePool().createValue(style->isolation());
         case CSSPropertyJustifyItems:
-            return valueForItemPositionWithOverflowAlignment(resolveAlignmentAuto(style->justifyItems(), styledNode), style->justifyItemsOverflowAlignment(), style->justifyItemsPositionType());
-        case CSSPropertyJustifySelf:
-            return valueForItemPositionWithOverflowAlignment(resolveAlignmentAuto(style->justifySelf(), styledNode->parentNode()), style->justifySelfOverflowAlignment(), NonLegacyPosition);
+            return valueForItemPositionWithOverflowAlignment(style->justifyItems(), style->justifyItemsOverflowAlignment(), style->justifyItemsPositionType());
+        case CSSPropertyJustifySelf: {
+            ItemPosition justifySelf = style->justifySelf();
+            OverflowAlignment overflow = style->justifySelfOverflowAlignment();
+            if (justifySelf == ItemPositionAuto) {
+                Node* parent = styledNode->parentNode();
+                if (parent && parent->computedStyle()) {

[esprehn, 2014/08/19 00:56:33]

You need to use NodeRenderingTraversal::parent(), parentNode() is not where your
parent style comes from (or where you're going to be inserted into) with Shadow
DOM or pseudo elements. You should also not call computedStyle() repeatedly like
this either.

What are you trying to do here?

[jfernandez, 2014/08/19 21:00:12]

I used parentNode() because it was what it was used in the original code, so
that where the confusion comes from. Also, I had the idea that this
computedStyle thing was a DOM matter, independently of how the render tree is
finally built. 

Anyway, I think I've probably misunderstood the concept, so I'll apply your
suggestions. Of course, I totally agree with avoiding repeatedly calling to
computedStyle. 
The idea is that 'auto' values on aling-self are resolved to the aling-items's
computed value on its parent element, as the Box Alignment Specification
describes. 

[esprehn, 2014/09/03 21:45:27]

I don't understand what you mean, style is resolved by using the distributed
parent per how Shadow DOM works. You can't use parentNode() to check your parent
style.
NodeRenderingTraversal::parent() is where your parent style comes from, not from
parentNode(). The traversal version is what deals with shadow dom distributions.

[jfernandez, 2014/09/03 22:27:49]

Acknowledged.

+                    justifySelf = parent->computedStyle()->justifyItems();
+                    overflow = parent->computedStyle()->justifyItemsOverflowAlignment();
+                }
+            }
+            return valueForItemPositionWithOverflowAlignment(justifySelf, overflow, NonLegacyPosition);
+        }
         case CSSPropertyLeft:
             return valueForPositionOffset(*style, CSSPropertyLeft, renderer);
         case CSSPropertyLetterSpacing: