Refactor scroll updates during flexbox layout.

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 c8a3f359e41ea58f55c8cc4f272d975c2f8e9123..33476440fa2034b430297027898ab8c7116d2016 100644
--- a/third_party/WebKit/Source/core/layout/LayoutBox.cpp
+++ b/third_party/WebKit/Source/core/layout/LayoutBox.cpp
@@ -793,12 +793,12 @@ int LayoutBox::intrinsicScrollbarLogicalWidth() const
 
     ASSERT(getScrollableArea());
 
-    if (isHorizontalWritingMode() && style()->overflowY() == OverflowScroll) {
+    if (isHorizontalWritingMode() && (style()->overflowY() == OverflowScroll || style()->overflowY() == OverflowAuto)) {

[cbiesinger, 2016/05/13 19:52:48]

Soo, I agree this will be better off in a different CL but I'm not sure I like
this change. With the change, what's the difference between this function and
scrollbarLogicalWidth()?

More broadly, the question of whether preferred widths should take o:auto into
account is tricky (what this change would do), in part because I think that adds
indeterminism to sizing of layout objects.

[szager1, 2016/05/13 20:52:05]

The thing that makes this work is the following change to
PaintLayerScrollableArea::updateAfterLayout in this CL:

    if ((verticalScrollbarChanged && box().isHorizontalWritingMode())
        || (horizontalScrollbarChanged && !box().isHorizontalWritingMode())) {
        box().setPreferredLogicalWidthsDirty();
        box().updateLogicalWidth();
    }

As for whether it's a good idea, I would point out that it does improve the
appearance of the tests that are updated in this patch, for example here is
scrollbars/scrollbars-on-positioned-content.html before and after this patch:

before: http://imgur.com/gTworqU
after: http://imgur.com/R9OTH7y

As you can see, in the 'before' rendering, the horizontal scrollbar is partially
covered by the auto vertical scrollbar; in the 'after' rendering, the block is
resized to avoid that.  The improvement in
flexbox-height-with-overflow-auto.html test is even more pronounced:

before: http://imgur.com/uWIDjMT
after: http://imgur.com/JwtYIEs

So, yes, this is a public-facing change, but I would argue it's a good one.

[cbiesinger, 2016/05/13 21:02:13]

Ah, thanks, I missed that part. Still, this now makes intrinsicScrollbarWidth()
identical to scrollbarWidth() right?

I do think this needs more careful consideration. For example, for the
non-flexbox shrinkwrapped case, I think your parent will be sized incorrectly
because when we compute the parent's preferred width, we use the non-scrollbar
child size, and when we update the child's preferred size, I don't think we
recalculate the parent.

Is this interoperable with other browsers?

Anyway, we should discuss this on a separate CL, as you suggested.

         // Even with OverflowScroll, the scrollbar may not exist (crbug.com/415031).
         return getScrollableArea()->hasVerticalScrollbar() ? verticalScrollbarWidth() : 0;
     }
 
-    if (!isHorizontalWritingMode() && style()->overflowX() == OverflowScroll) {
+    if (!isHorizontalWritingMode() && (style()->overflowX() == OverflowScroll || style()->overflowY() == OverflowAuto)) {
         // Even with OverflowScroll, the scrollbar may not exist (crbug.com/415031).
         return getScrollableArea()->hasHorizontalScrollbar() ? horizontalScrollbarHeight() : 0;
     }