[css-grid] Implementation of Baseline Self-Alignment

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

 /*
  * Copyright (C) 2013 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 2472 matching lines @@
   for (const RootInlineBox* box = firstRootBox(); box;
        box = box->nextRootBox()) {
     count++;
     if (box == stopRootInlineBox)
       break;
   }
   return count;
 }
 
 int LayoutBlockFlow::firstLineBoxBaseline() const {
-  if (isWritingModeRoot() && !isRubyRun())
+  if (isWritingModeRoot() && !isRubyRun() && !isGridItem())
     return -1;
   if (!childrenInline())
     return LayoutBlock::firstLineBoxBaseline();
   if (firstLineBox()) {
     const SimpleFontData* fontData = style(true)->font().primaryFont();
     DCHECK(fontData);
     if (!fontData)
       return -1;
     return (firstLineBox()->logicalTop() +
-            fontData->getFontMetrics().ascent(firstRootBox()->baselineType()))
+            (firstLineBox()->isHorizontal()
+                 ? fontData->getFontMetrics().ascent(
+                       firstRootBox()->baselineType())
+                 : fontData->getFontMetrics().descent(
+                       firstRootBox()->baselineType())))

[kojii, 2016/11/15 04:21:15]

My intuition tells me that using descent is only correct for vertical-lr and not
for vertical-rl, but not really sure since flipping makes me hard to think
straightforward. Did tests pass?

[jfernandez, 2016/11/16 11:51:24]

Yes, you are totally right. using descent only makes sense for vertical-lr. And
now that you mention it, the tests are passing but incorrectly :) The problem is
that I'm using ref tests based on the flexbox behavior, but the code changed
affects to both layout models. Ideally, I should have used inline-blocks for
the reference tests. However, I can't do it because I think there are also bugs
in the baseline implementation for inline-blocks (see bug #664386)

Another point on this change is whether this is the right place to fix the
baseline implementation for vertical modes. Accepting that this case is not
correctly managed, we could perhaps find a common approach for the
different writing modes. 

In the PatchSet4 I applied the changes to make baseline implementation 
behave correctly for flex and grid in the case of vertical writing modes, except
n the case of opposite block-flow direction, which flex still does not support.
This is the cause why grid-self-baseline-02.html fails (the only test failing).

BTW, I filed bug  663355 to solve the flexbox's baseline issues with vertical
modes. However, I still have the problem of defining the proper reference test,
because of the current buggy implementation for inline-blocks.  

[kojii, 2016/11/16 17:18:06]

Thank you for doing the extensive research, and sorry about existing issues for
inline-blocks in vertical-lr writing-mode. I was aware of them but it's still
deep in the backlog. Not sure if I'd ever reach to them...

Ideally, there should be existing tests, and I feel sorry about that. If you
could verify the change doesn't break vertical-rl, with comments saying can't
test vertical-lr due to http://crbug.com/664386, that's still great. vertical-rl
has more than 1000 times usages than vertical-lr. But existing tests should
cover some of these cases, so I think this is optional.
Good point. I think the problem lies in that, we have
logical-top/bottom/left/right but we don't have logical-ascent/descent. If you
want to go that route (to add to FontMetrics,) I'd support that. If it's too
much for this CL (I guess so?), having here for now is fine with me, this should
be in our back log.
Thank you for your finding. I'm quite sure we have many more missing cases for
vertical-lr, and your previous point makes great sense.

[jfernandez, 2016/11/16 18:50:07]

I understand, I think we can still implement some basic support for Grid.
I think this could be a good approach, I'll think about it.
Yes, I agree that's definitively the root cause of the vertical-lr issues. If 
find the time, 
I'll try to implement some preliminary approach to solve the inline-block
issues.

         .toInt();
   }
   return -1;
 }
 
 int LayoutBlockFlow::inlineBlockBaseline(
     LineDirectionMode lineDirection) const {
   // CSS2.1 states that the baseline of an 'inline-block' is:
   // the baseline of the last line box in the normal flow, unless it has
   // either no in-flow line boxes or if its 'overflow' property has a computed
@@ skipping 2030 matching lines @@
   return LayoutBlock::invalidatePaintIfNeeded(paintInvalidationState);
 }
 
 void LayoutBlockFlow::invalidateDisplayItemClients(
     PaintInvalidationReason invalidationReason) const {
   BlockFlowPaintInvalidator(*this).invalidateDisplayItemClients(
       invalidationReason);
 }
 
 }  // namespace blink