Refactor scroll updates during flexbox layout.

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 775 matching lines @@
     return getScrollableArea()->horizontalScrollbarHeight();
 }
 
 int LayoutBox::intrinsicScrollbarLogicalWidth() const
 {
     if (!hasOverflowClip())
         return 0;
 
     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;
     }
 
     return 0;
 }
 
 ScrollResult LayoutBox::scroll(ScrollGranularity granularity, const FloatSize& delta)
 {
     // Presumably the same issue as in setScrollTop. See crbug.com/343132.
@@ skipping 4139 matching lines @@
         m_rareData->m_snapAreas->remove(&snapArea);
     }
 }
 
 SnapAreaSet* LayoutBox::snapAreas() const
 {
     return m_rareData ? m_rareData->m_snapAreas.get() : nullptr;
 }
 
 } // namespace blink