Fix Ruby visual rects in flipped blocks writing mode.

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
  * modify it under the terms of the GNU Library General Public
@@ skipping 2078 matching lines @@
 
     // We are now in our parent container's coordinate space.  Apply our transform to obtain a bounding box
     // in the parent's coordinate space that encloses us.
     if (hasLayer() && layer()->transform()) {
         // Use enclosingIntRect because we cannot properly compute pixel snapping for painted elements within
         // the transform since we don't know the desired subpixel accumulation at this point, and the transform may
         // include a scale.
         rect = LayoutRect(layer()->transform()->mapRect(enclosingIntRect(rect)));
     }
     LayoutPoint topLeft = rect.location();
-    if (container->isBox()) {
+    // LayoutRubyRun is the (anonymous) block container for LayoutRuby{Base,Text}, all of which are boxes.
+    // We need to only flip once, so we leave that to LayoutRubyRun.
+    if (container->isBox() && !container->isRubyRun()) {

[chrishtr, 2016/07/21 17:24:54]

!container->isInline() also makes this test pass, and makes more sense than
special-casing isRubyRun, especially w.r.t the logic in
PaintLayer::updateLayerPosition.

Could you try that?

[wkorman, 2016/07/26 00:38:14]

I looked into how PaintLayer::updateLayerPosition ends up working with Ruby, and
I believe it's due to this bit:

                localPoint.moveBy(toLayoutBox(curr)->topLeftLocation());

here:
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/paint/Pai...

Note no params are passed to topLeftLocation(), and so this is flipping within
containingBlock(), not container(), which I believe is necessary to get the
correct top-left location between LayoutRubyRun and its containing block, the
DIV.

I am essentially in the process of re-convincing myself that patchset 1 is not
unreasonable.

I spent more time fiddling with !container->isInline() today, I haven't yet
found solution that allows both the aforementioned
imported/csswg-test/css-writing-modes-3/* tests to pass along with the Ruby
tests, but perhaps we can find a way. I think we'll end up having to have some
conditional to identify the Ruby LayoutTree structure and do things differently,
at which point it's different but not IMO better than patchset 1.

We've rewritten mapToVisualRectInAncestorSpace() to use container, rather than
containingBlock(), in general. I think that's the delta between our logic and
PaintLayer logic, and it particularly affects Ruby due to the "LayoutRuby
(inline)" interspersed between the LayoutRubyRun and its containing DIV. We need
to reconcile that somehow, patchset 1 is one way.

[chrishtr, 2016/07/26 18:02:29]

Please add a TODO(wkorman) that it's not clear why specializing on isRubyRun
makes sense.

         topLeft.moveBy(topLeftLocation(toLayoutBox(container)));
         // If the row is the ancestor, however, add its offset back in. In effect, this passes from the joint <td> / <tr>
         // coordinate space to the parent space, then back to <tr> / <td>.
         if (tableRowContainer)
             topLeft.moveBy(-tableRowContainer->topLeftLocation(toLayoutBox(container)));
     } else {
         topLeft.moveBy(location());
     }
 
     const ComputedStyle& styleToUse = styleRef();
@@ skipping 2852 matching lines @@
         m_rareData->m_snapAreas->remove(&snapArea);
     }
 }
 
 SnapAreaSet* LayoutBox::snapAreas() const
 {
     return m_rareData ? m_rareData->m_snapAreas.get() : nullptr;
 }
 
 } // namespace blink