Title: [124168] trunk/Source/WebCore
- Revision
- 124168
- Author
- [email protected]
- Date
- 2012-07-30 20:38:53 -0700 (Mon, 30 Jul 2012)
Log Message
Remove overflow: scroll handling in block flow layout methods
https://bugs.webkit.org/show_bug.cgi?id=92689
Reviewed by Simon Fraser.
The overflow: scroll scrollbars creation was done at layout time for all RenderBlocks and
descendants. This was not only wrong ('overflow' only changes at style change time) but it
was also introducing some code duplication.
The gist of this change is to share the code by moving it to RenderLayer::updateScrollbarsAfterStyleChange,
this includes the code from bug 69993 to special case list box part.
Covered by existing tests:
- All fast/overflow ones.
- For the list box change:
fast/forms/select-overflow-scroll-inherited.html
fast/forms/select-overflow-scroll.html
- For the flexbox:
css3/flexbox/preferred-widths-orthogonal.html
css3/flexbox/preferred-widths.html
* rendering/RenderBlock.cpp:
(WebCore::RenderBlock::layoutBlock):
* rendering/RenderDeprecatedFlexibleBox.cpp:
(WebCore::RenderDeprecatedFlexibleBox::layoutBlock):
* rendering/RenderGrid.cpp:
(WebCore::RenderGrid::layoutBlock):
* rendering/RenderFlexibleBox.cpp:
(WebCore::RenderFlexibleBox::layoutBlock):
Removed the common code here.
* rendering/RenderFlexibleBox.cpp:
(WebCore::RenderFlexibleBox::computePreferredLogicalWidths):
Changed to an ASSERT now that the right scrollbars are created. This is
fine as overflow-x/y are physical coordinates and our access was following that.
* rendering/RenderLayer.cpp:
(WebCore::RenderLayer::invalidateScrollbarRect):
Added an early return here if we are not attached yet as RenderLayer::styleChanged
is called at attachment time before we are inserted in the tree. This is fine as the
scrollbars are part of the object which will be painted after the first layout.
(WebCore::overflowRequiresAScrollbar):
(WebCore::overflowDefinesAutomaticScrollbar):
Split the logic in those 2 functions.
(WebCore::RenderLayer::updateScrollbarsAfterLayout):
Updated to use the require / can-have functions. Also added
an early return for list box parts as required by bug 69993.
(WebCore::RenderLayer::updateScrollbarsAfterStyleChange):
Added an early return for list box parts as required by bug 69993,
also removed some unneeded NULL-checks that were added for list box parts.
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (124167 => 124168)
--- trunk/Source/WebCore/ChangeLog 2012-07-31 03:36:44 UTC (rev 124167)
+++ trunk/Source/WebCore/ChangeLog 2012-07-31 03:38:53 UTC (rev 124168)
@@ -1,3 +1,59 @@
+2012-07-30 Julien Chaffraix <[email protected]>
+
+ Remove overflow: scroll handling in block flow layout methods
+ https://bugs.webkit.org/show_bug.cgi?id=92689
+
+ Reviewed by Simon Fraser.
+
+ The overflow: scroll scrollbars creation was done at layout time for all RenderBlocks and
+ descendants. This was not only wrong ('overflow' only changes at style change time) but it
+ was also introducing some code duplication.
+
+ The gist of this change is to share the code by moving it to RenderLayer::updateScrollbarsAfterStyleChange,
+ this includes the code from bug 69993 to special case list box part.
+
+ Covered by existing tests:
+ - All fast/overflow ones.
+ - For the list box change:
+ fast/forms/select-overflow-scroll-inherited.html
+ fast/forms/select-overflow-scroll.html
+ - For the flexbox:
+ css3/flexbox/preferred-widths-orthogonal.html
+ css3/flexbox/preferred-widths.html
+
+ * rendering/RenderBlock.cpp:
+ (WebCore::RenderBlock::layoutBlock):
+ * rendering/RenderDeprecatedFlexibleBox.cpp:
+ (WebCore::RenderDeprecatedFlexibleBox::layoutBlock):
+ * rendering/RenderGrid.cpp:
+ (WebCore::RenderGrid::layoutBlock):
+ * rendering/RenderFlexibleBox.cpp:
+ (WebCore::RenderFlexibleBox::layoutBlock):
+ Removed the common code here.
+
+ * rendering/RenderFlexibleBox.cpp:
+ (WebCore::RenderFlexibleBox::computePreferredLogicalWidths):
+ Changed to an ASSERT now that the right scrollbars are created. This is
+ fine as overflow-x/y are physical coordinates and our access was following that.
+
+ * rendering/RenderLayer.cpp:
+ (WebCore::RenderLayer::invalidateScrollbarRect):
+ Added an early return here if we are not attached yet as RenderLayer::styleChanged
+ is called at attachment time before we are inserted in the tree. This is fine as the
+ scrollbars are part of the object which will be painted after the first layout.
+
+ (WebCore::overflowRequiresAScrollbar):
+ (WebCore::overflowDefinesAutomaticScrollbar):
+ Split the logic in those 2 functions.
+
+ (WebCore::RenderLayer::updateScrollbarsAfterLayout):
+ Updated to use the require / can-have functions. Also added
+ an early return for list box parts as required by bug 69993.
+
+ (WebCore::RenderLayer::updateScrollbarsAfterStyleChange):
+ Added an early return for list box parts as required by bug 69993,
+ also removed some unneeded NULL-checks that were added for list box parts.
+
2012-07-30 Vivek Galatage <[email protected]>
fillWithEmptyClients method should also initialize chromeClient with EmptyChromeClient
Modified: trunk/Source/WebCore/rendering/RenderBlock.cpp (124167 => 124168)
--- trunk/Source/WebCore/rendering/RenderBlock.cpp 2012-07-31 03:36:44 UTC (rev 124167)
+++ trunk/Source/WebCore/rendering/RenderBlock.cpp 2012-07-31 03:38:53 UTC (rev 124168)
@@ -1472,14 +1472,6 @@
setPaginationStrut(0);
}
- // For overflow:scroll blocks, ensure we have both scrollbars in place always.
- if (scrollsOverflow() && style()->appearance() != ListboxPart) {
- if (styleToUse->overflowX() == OSCROLL)
- layer()->setHasHorizontalScrollbar(true);
- if (styleToUse->overflowY() == OSCROLL)
- layer()->setHasVerticalScrollbar(true);
- }
-
LayoutUnit repaintLogicalTop = ZERO_LAYOUT_UNIT;
LayoutUnit repaintLogicalBottom = ZERO_LAYOUT_UNIT;
LayoutUnit maxFloatLogicalBottom = ZERO_LAYOUT_UNIT;
Modified: trunk/Source/WebCore/rendering/RenderDeprecatedFlexibleBox.cpp (124167 => 124168)
--- trunk/Source/WebCore/rendering/RenderDeprecatedFlexibleBox.cpp 2012-07-31 03:36:44 UTC (rev 124167)
+++ trunk/Source/WebCore/rendering/RenderDeprecatedFlexibleBox.cpp 2012-07-31 03:38:53 UTC (rev 124168)
@@ -262,14 +262,6 @@
initMaxMarginValues();
- // For overflow:scroll blocks, ensure we have both scrollbars in place always.
- if (scrollsOverflow()) {
- if (style()->overflowX() == OSCROLL)
- layer()->setHasHorizontalScrollbar(true);
- if (style()->overflowY() == OSCROLL)
- layer()->setHasVerticalScrollbar(true);
- }
-
if (isHorizontal())
layoutHorizontalBox(relayoutChildren);
else
Modified: trunk/Source/WebCore/rendering/RenderFlexibleBox.cpp (124167 => 124168)
--- trunk/Source/WebCore/rendering/RenderFlexibleBox.cpp 2012-07-31 03:36:44 UTC (rev 124167)
+++ trunk/Source/WebCore/rendering/RenderFlexibleBox.cpp 2012-07-31 03:38:53 UTC (rev 124168)
@@ -194,10 +194,10 @@
LayoutUnit scrollbarWidth = 0;
if (hasOverflowClip()) {
if (isHorizontalWritingMode() && styleToUse->overflowY() == OSCROLL) {
- layer()->setHasVerticalScrollbar(true);
+ ASSERT(layer()->hasVerticalScrollbar());
scrollbarWidth = verticalScrollbarWidth();
} else if (!isHorizontalWritingMode() && styleToUse->overflowX() == OSCROLL) {
- layer()->setHasHorizontalScrollbar(true);
+ ASSERT(layer()->hasHorizontalScrollbar());
scrollbarWidth = horizontalScrollbarHeight();
}
}
@@ -248,14 +248,6 @@
m_overflow.clear();
- // For overflow:scroll blocks, ensure we have both scrollbars in place always.
- if (scrollsOverflow()) {
- if (style()->overflowX() == OSCROLL)
- layer()->setHasHorizontalScrollbar(true);
- if (style()->overflowY() == OSCROLL)
- layer()->setHasVerticalScrollbar(true);
- }
-
WTF::Vector<LineContext> lineContexts;
OrderHashSet orderValues;
computeMainAxisPreferredSizes(relayoutChildren, orderValues);
Modified: trunk/Source/WebCore/rendering/RenderGrid.cpp (124167 => 124168)
--- trunk/Source/WebCore/rendering/RenderGrid.cpp 2012-07-31 03:36:44 UTC (rev 124167)
+++ trunk/Source/WebCore/rendering/RenderGrid.cpp 2012-07-31 03:38:53 UTC (rev 124168)
@@ -80,13 +80,6 @@
m_overflow.clear();
- if (scrollsOverflow()) {
- if (style()->overflowX() == OSCROLL)
- layer()->setHasHorizontalScrollbar(true);
- if (style()->overflowY() == OSCROLL)
- layer()->setHasVerticalScrollbar(true);
- }
-
layoutGridItems();
LayoutUnit oldClientAfterEdge = clientLogicalBottom();
Modified: trunk/Source/WebCore/rendering/RenderLayer.cpp (124167 => 124168)
--- trunk/Source/WebCore/rendering/RenderLayer.cpp 2012-07-31 03:36:44 UTC (rev 124167)
+++ trunk/Source/WebCore/rendering/RenderLayer.cpp 2012-07-31 03:38:53 UTC (rev 124168)
@@ -2229,6 +2229,10 @@
IntRect scrollRect = rect;
RenderBox* box = renderBox();
ASSERT(box);
+ // If we are not yet inserted into the tree, there is no need to repaint.
+ if (!box->parent())
+ return;
+
if (scrollbar == m_vBar.get())
scrollRect.move(verticalScrollbarStart(0, box->width()), box->borderTop());
else
@@ -2522,13 +2526,17 @@
RenderBox* box = renderBox();
ASSERT(box);
+ // List box parts handle the scrollbars by themselves so we have nothing to do.
+ if (box->style()->appearance() == ListboxPart)
+ return;
+
bool hasHorizontalOverflow = this->hasHorizontalOverflow();
bool hasVerticalOverflow = this->hasVerticalOverflow();
// overflow:scroll should just enable/disable.
- if (m_hBar && renderer()->style()->overflowX() == OSCROLL)
+ if (renderer()->style()->overflowX() == OSCROLL)
m_hBar->setEnabled(hasHorizontalOverflow);
- if (m_vBar && renderer()->style()->overflowY() == OSCROLL)
+ if (renderer()->style()->overflowY() == OSCROLL)
m_vBar->setEnabled(hasVerticalOverflow);
// overflow:auto may need to lay out again if scrollbars got added/removed.
@@ -4818,33 +4826,45 @@
}
}
-static bool overflowCanHaveAScrollbar(EOverflow overflow)
+static bool overflowRequiresScrollbar(EOverflow overflow)
{
- return overflow == OAUTO || overflow == OSCROLL || overflow == OOVERLAY;
+ return overflow == OSCROLL;
}
+static bool overflowDefinesAutomaticScrollbar(EOverflow overflow)
+{
+ return overflow == OAUTO || overflow == OOVERLAY;
+}
+
void RenderLayer::updateScrollbarsAfterStyleChange(const RenderStyle* oldStyle)
{
// Overflow are a box concept.
- if (!renderBox())
+ RenderBox* box = renderBox();
+ if (!box)
return;
- EOverflow overflowX = renderBox()->style()->overflowX();
- EOverflow overflowY = renderBox()->style()->overflowY();
- if (hasHorizontalScrollbar() && !overflowCanHaveAScrollbar(overflowX))
- setHasHorizontalScrollbar(false);
- if (hasVerticalScrollbar() && !overflowCanHaveAScrollbar(overflowY))
- setHasVerticalScrollbar(false);
+ // List box parts handle the scrollbars by themselves so we have nothing to do.
+ if (box->style()->appearance() == ListboxPart)
+ return;
+ EOverflow overflowX = box->style()->overflowX();
+ EOverflow overflowY = box->style()->overflowY();
+
+ // To avoid doing a relayout in updateScrollbarsAfterLayout, we try to keep any automatic scrollbar that was already present.
+ bool needsHorizontalScrollbar = (hasHorizontalScrollbar() && overflowDefinesAutomaticScrollbar(overflowX)) || overflowRequiresScrollbar(overflowX);
+ bool needsVerticalScrollbar = (hasVerticalScrollbar() && overflowDefinesAutomaticScrollbar(overflowY)) || overflowRequiresScrollbar(overflowY);
+ setHasHorizontalScrollbar(needsHorizontalScrollbar);
+ setHasVerticalScrollbar(needsVerticalScrollbar);
+
// With overflow: scroll, scrollbars are always visible but may be disabled.
// When switching to another value, we need to re-enable them (see bug 11985).
- if (hasHorizontalScrollbar() && oldStyle->overflowX() == OSCROLL && overflowX != OSCROLL) {
- ASSERT(overflowCanHaveAScrollbar(overflowX));
+ if (needsHorizontalScrollbar && oldStyle && oldStyle->overflowX() == OSCROLL && overflowX != OSCROLL) {
+ ASSERT(hasHorizontalScrollbar());
m_hBar->setEnabled(true);
}
- if (hasVerticalScrollbar() && oldStyle->overflowY() == OSCROLL && overflowY != OSCROLL) {
- ASSERT(overflowCanHaveAScrollbar(overflowY));
+ if (needsVerticalScrollbar && oldStyle && oldStyle->overflowY() == OSCROLL && overflowY != OSCROLL) {
+ ASSERT(hasVerticalScrollbar());
m_vBar->setEnabled(true);
}
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo/webkit-changes
