From 3ffd336ad5a12f5ff479761eb8fd8b8a201161a3 Mon Sep 17 00:00:00 2001 From: Ojan Vafai Date: Tue, 9 Dec 2014 19:48:50 -0800 Subject: [PATCH] Remove relayout due to scrollbars taking up width We only have overlay scrollbars, so we never need to relayout due to the lack or presence of scrollbars. Also remove a few stracking has*Scrollbar methods. R=eseidel@chromium.org Review URL: https://codereview.chromium.org/783393006 --- .../rendering/RenderLayerScrollableArea.cpp | 73 ++++--------------- .../rendering/RenderLayerScrollableArea.h | 1 - engine/public/web/WebFrame.h | 3 - engine/web/WebLocalFrameImpl.cpp | 12 --- engine/web/WebLocalFrameImpl.h | 2 - engine/web/WebViewImpl.cpp | 12 --- engine/web/WebViewImpl.h | 4 - 7 files changed, 16 insertions(+), 91 deletions(-) diff --git a/engine/core/rendering/RenderLayerScrollableArea.cpp b/engine/core/rendering/RenderLayerScrollableArea.cpp index f076a41983c..16a6c2991ca 100644 --- a/engine/core/rendering/RenderLayerScrollableArea.cpp +++ b/engine/core/rendering/RenderLayerScrollableArea.cpp @@ -70,7 +70,6 @@ RenderLayerScrollableArea::RenderLayerScrollableArea(RenderLayer& layer) : m_layer(layer) , m_scrollsOverflow(false) , m_scrollDimensionsDirty(true) - , m_inOverflowRelayout(false) , m_nextTopmostScrollChild(0) , m_topmostScrollChild(0) , m_needsCompositedScrolling(false) @@ -408,67 +407,32 @@ void RenderLayerScrollableArea::updateAfterLayout() bool hasHorizontalOverflow = this->hasHorizontalOverflow(); bool hasVerticalOverflow = this->hasVerticalOverflow(); - { - // Hits in compositing/overflow/automatically-opt-into-composited-scrolling-after-style-change.html. - DisableCompositingQueryAsserts disabler; - - // overflow:scroll should just enable/disable. - if (box().style()->overflowX() == OSCROLL) - horizontalScrollbar()->setEnabled(hasHorizontalOverflow); - if (box().style()->overflowY() == OSCROLL) - verticalScrollbar()->setEnabled(hasVerticalOverflow); - } + // overflow:scroll should just enable/disable. + if (box().style()->overflowX() == OSCROLL) + horizontalScrollbar()->setEnabled(hasHorizontalOverflow); + if (box().style()->overflowY() == OSCROLL) + verticalScrollbar()->setEnabled(hasVerticalOverflow); // overflow:auto may need to lay out again if scrollbars got added/removed. - bool autoHorizontalScrollBarChanged = box().hasAutoHorizontalScrollbar() && (hasHorizontalScrollbar() != hasHorizontalOverflow); - bool autoVerticalScrollBarChanged = box().hasAutoVerticalScrollbar() && (hasVerticalScrollbar() != hasVerticalOverflow); + if (box().hasAutoHorizontalScrollbar()) + setHasHorizontalScrollbar(hasHorizontalOverflow); + if (box().hasAutoVerticalScrollbar()) + setHasVerticalScrollbar(hasVerticalOverflow); - if (autoHorizontalScrollBarChanged || autoVerticalScrollBarChanged) { - if (box().hasAutoHorizontalScrollbar()) - setHasHorizontalScrollbar(hasHorizontalOverflow); - if (box().hasAutoVerticalScrollbar()) - setHasVerticalScrollbar(hasVerticalOverflow); - - layer()->updateSelfPaintingLayer(); - - if (box().style()->overflowX() == OAUTO || box().style()->overflowY() == OAUTO) { - if (!m_inOverflowRelayout) { - // Our proprietary overflow: overlay value doesn't trigger a layout. - m_inOverflowRelayout = true; - SubtreeLayoutScope layoutScope(box()); - layoutScope.setNeedsLayout(&box()); - if (box().isRenderBlock()) { - RenderBlock& block = toRenderBlock(box()); - block.scrollbarsChanged(autoHorizontalScrollBarChanged, autoVerticalScrollBarChanged); - block.layoutBlock(true); - } else { - box().layout(); - } - m_inOverflowRelayout = false; - } - } + // Set up the range (and page step/line step). + if (Scrollbar* horizontalScrollbar = this->horizontalScrollbar()) { + int clientWidth = box().pixelSnappedClientWidth(); + horizontalScrollbar->setProportion(clientWidth, overflowRect().width()); } - - { - // Hits in compositing/overflow/automatically-opt-into-composited-scrolling-after-style-change.html. - DisableCompositingQueryAsserts disabler; - - // Set up the range (and page step/line step). - if (Scrollbar* horizontalScrollbar = this->horizontalScrollbar()) { - int clientWidth = box().pixelSnappedClientWidth(); - horizontalScrollbar->setProportion(clientWidth, overflowRect().width()); - } - if (Scrollbar* verticalScrollbar = this->verticalScrollbar()) { - int clientHeight = box().pixelSnappedClientHeight(); - verticalScrollbar->setProportion(clientHeight, overflowRect().height()); - } + if (Scrollbar* verticalScrollbar = this->verticalScrollbar()) { + int clientHeight = box().pixelSnappedClientHeight(); + verticalScrollbar->setProportion(clientHeight, overflowRect().height()); } bool hasOverflow = hasScrollableHorizontalOverflow() || hasScrollableVerticalOverflow(); updateScrollableAreaSet(hasOverflow); if (hasOverflow) { - DisableCompositingQueryAsserts disabler; positionOverflowControls(IntSize()); } } @@ -663,9 +627,6 @@ void RenderLayerScrollableArea::setHasHorizontalScrollbar(bool hasScrollbar) return; if (hasScrollbar) { - // This doesn't hit in any tests, but since the equivalent code in setHasVerticalScrollbar - // does, presumably this code does as well. - DisableCompositingQueryAsserts disabler; m_hBar = createScrollbar(HorizontalScrollbar); } else { destroyScrollbar(HorizontalScrollbar); @@ -678,8 +639,6 @@ void RenderLayerScrollableArea::setHasVerticalScrollbar(bool hasScrollbar) return; if (hasScrollbar) { - // Hits in compositing/overflow/automatically-opt-into-composited-scrolling-after-style-change.html - DisableCompositingQueryAsserts disabler; m_vBar = createScrollbar(VerticalScrollbar); } else { destroyScrollbar(VerticalScrollbar); diff --git a/engine/core/rendering/RenderLayerScrollableArea.h b/engine/core/rendering/RenderLayerScrollableArea.h index 1ea1c568a60..7474f869270 100644 --- a/engine/core/rendering/RenderLayerScrollableArea.h +++ b/engine/core/rendering/RenderLayerScrollableArea.h @@ -171,7 +171,6 @@ private: unsigned m_scrollsOverflow : 1; unsigned m_scrollDimensionsDirty : 1; - unsigned m_inOverflowRelayout : 1; RenderLayer* m_nextTopmostScrollChild; RenderLayer* m_topmostScrollChild; diff --git a/engine/public/web/WebFrame.h b/engine/public/web/WebFrame.h index 46e61a80918..8e9524ec0d0 100644 --- a/engine/public/web/WebFrame.h +++ b/engine/public/web/WebFrame.h @@ -110,9 +110,6 @@ public: // Returns the visible content rect (minus scrollbars, in absolute coordinate) virtual WebRect visibleContentRect() const = 0; - virtual bool hasHorizontalScrollbar() const = 0; - virtual bool hasVerticalScrollbar() const = 0; - // Hierarchy ---------------------------------------------------------- // Returns the containing view. diff --git a/engine/web/WebLocalFrameImpl.cpp b/engine/web/WebLocalFrameImpl.cpp index 2d10bc98107..be3bf891747 100644 --- a/engine/web/WebLocalFrameImpl.cpp +++ b/engine/web/WebLocalFrameImpl.cpp @@ -233,18 +233,6 @@ WebRect WebLocalFrameImpl::visibleContentRect() const return frame()->view()->frameRect(); } -bool WebLocalFrameImpl::hasHorizontalScrollbar() const -{ - // FIXME(sky): Remove - return false; -} - -bool WebLocalFrameImpl::hasVerticalScrollbar() const -{ - // FIXME(sky): Remove - return false; -} - WebView* WebLocalFrameImpl::view() const { return viewImpl(); diff --git a/engine/web/WebLocalFrameImpl.h b/engine/web/WebLocalFrameImpl.h index ccb786f4cb3..8ccfcb514af 100644 --- a/engine/web/WebLocalFrameImpl.h +++ b/engine/web/WebLocalFrameImpl.h @@ -65,8 +65,6 @@ public: virtual WebSize contentsSize() const override; virtual bool hasVisibleContent() const override; virtual WebRect visibleContentRect() const override; - virtual bool hasHorizontalScrollbar() const override; - virtual bool hasVerticalScrollbar() const override; virtual WebView* view() const override; virtual WebDocument document() const override; virtual void executeScript(const WebScriptSource&) override; diff --git a/engine/web/WebViewImpl.cpp b/engine/web/WebViewImpl.cpp index 9a905664827..1aeb003c773 100644 --- a/engine/web/WebViewImpl.cpp +++ b/engine/web/WebViewImpl.cpp @@ -697,18 +697,6 @@ bool WebViewImpl::isTrackingRepaints() const return view->isTrackingPaintInvalidations(); } -bool WebViewImpl::hasHorizontalScrollbar() -{ - // FIXME(sky): Remove - return false; -} - -bool WebViewImpl::hasVerticalScrollbar() -{ - // FIXME(sky): Remove - return false; -} - const WebInputEvent* WebViewImpl::m_currentInputEvent = 0; // FIXME: autogenerate this kind of code, and use it throughout Blink rather than diff --git a/engine/web/WebViewImpl.h b/engine/web/WebViewImpl.h index d7cdd0389a5..3c4964b30c1 100644 --- a/engine/web/WebViewImpl.h +++ b/engine/web/WebViewImpl.h @@ -225,10 +225,6 @@ public: // Exposed for the purpose of overriding device metrics. void sendResizeEventAndRepaint(); - // Exposed for testing purposes. - bool hasHorizontalScrollbar(); - bool hasVerticalScrollbar(); - WebSettingsImpl* settingsImpl(); IntPoint clampOffsetAtScale(const IntPoint& offset, float scale);