Skip to content

GitLab

T
tangled
Commit a687da48 authored by awoloszyn@chromium.org's avatar awoloszyn@chromium.org
Browse files
Options

Main-thread scroll if we have fixed-pos children and overflow:hidden. 

If we are overflow:hidden and have fixed-pos children we would transition
to impl-side scrolling. This was exposing another bug (304810)
where on impl-side where we do not correctly stop body elements from 
scrolling. This lead us to scroll elements on the impl-thread even 
if they need to be main-thread scrolled.

Here we do not short-circuit the main-thread requirements if
the frame is overflow:hidden. This is equivalent to our old
implementation, and lets gmail continue to be on impl-thread,
which is what abarth had originally changed that caused this issue.

BUG=411767
R=vollick@chromium.org

Review URL: https://codereview.chromium.org/699183004

git-svn-id: svn://svn.chromium.org/blink/trunk@184891 bbb929c8-8fbe-4397-9dbb-9b2b20218538
parent 0250e618
Hide whitespace changes
Inline Side-by-side
Showing with 74 additions and 5 deletions
third_party/WebKit/LayoutTests/compositing/layer-creation/main-thread-scrolling-for-non-composited-fixed-position-if-overflow-hidden-expected.txt 0 → 100644
View file @ a687da48
TEST
{
"bounds": [800, 5021],
"children": [
{
"bounds": [800, 5021],
"contentsOpaque": true,
"drawsContent": true
}
]
}
Has non-layer viewport-constrained objects
third_party/WebKit/LayoutTests/compositing/layer-creation/main-thread-scrolling-for-non-composited-fixed-position-if-overflow-hidden.html 0 → 100644
View file @ a687da48
<!DOCTYPE html>
<html>
<head>
<style>
.fixed {
position: fixed;
left: 10px;
top: 10px;
}
</style>
<script>
if (window.internals)
window.internals.settings.setPreferCompositingToLCDTextEnabled(false);
if (window.testRunner) {
testRunner.dumpAsText();
window.addEventListener("load", function() {
document.getElementById("layerTree").innerText = window.internals.layerTreeAsText(document);
document.getElementById("mainThreadScrollingReasons").innerText = window.internals.mainThreadScrollingReasons(document);
}, false);
}
</script>
</head>
<body style="height: 5000px; overflow:hidden">
<div class="fixed">TEST</div>
<pre id="layerTree"></pre>
<pre id="mainThreadScrollingReasons"></pre>
</body>
</html>
third_party/WebKit/Source/core/frame/FrameView.cpp
View file @ a687da48
...@@ -2269,7 +2269,13 @@ IntRect FrameView::scrollableAreaBoundingBox() const ...@@ -2269,7 +2269,13 @@ IntRect FrameView::scrollableAreaBoundingBox() const
return ownerRenderer->absoluteContentQuad().enclosingBoundingBox(); return ownerRenderer->absoluteContentQuad().enclosingBoundingBox();
} }
bool FrameView::isScrollable() bool FrameView::isScrollable()
{
return scrollingReasons() == Scrollable;
}
FrameView::ScrollingReasons FrameView::scrollingReasons()
{ {
// Check for: // Check for:
// 1) If there an actual overflow. // 1) If there an actual overflow.
...@@ -2281,22 +2287,22 @@ bool FrameView::isScrollable() ...@@ -2281,22 +2287,22 @@ bool FrameView::isScrollable()
IntSize contentsSize = this->contentsSize(); IntSize contentsSize = this->contentsSize();
IntSize visibleContentSize = visibleContentRect().size(); IntSize visibleContentSize = visibleContentRect().size();
if ((contentsSize.height() <= visibleContentSize.height() && contentsSize.width() <= visibleContentSize.width())) if ((contentsSize.height() <= visibleContentSize.height() && contentsSize.width() <= visibleContentSize.width()))
return false; return NotScrollableNoOverflow;
// Covers #2. // Covers #2.
// FIXME: Do we need to fix this for OOPI? // FIXME: Do we need to fix this for OOPI?
HTMLFrameOwnerElement* owner = m_frame->deprecatedLocalOwner(); HTMLFrameOwnerElement* owner = m_frame->deprecatedLocalOwner();
if (owner && (!owner->renderer() || !owner->renderer()->visibleToHitTesting())) if (owner && (!owner->renderer() || !owner->renderer()->visibleToHitTesting()))
return false; return NotScrollableNotVisible;
// Cover #3 and #4. // Cover #3 and #4.
ScrollbarMode horizontalMode; ScrollbarMode horizontalMode;
ScrollbarMode verticalMode; ScrollbarMode verticalMode;
calculateScrollbarModesForLayoutAndSetViewportRenderer(horizontalMode, verticalMode, RulesFromWebContentOnly); calculateScrollbarModesForLayoutAndSetViewportRenderer(horizontalMode, verticalMode, RulesFromWebContentOnly);
if (horizontalMode == ScrollbarAlwaysOff && verticalMode == ScrollbarAlwaysOff) if (horizontalMode == ScrollbarAlwaysOff && verticalMode == ScrollbarAlwaysOff)
return false; return NotScrollableExplicitlyDisabled;
return true; return Scrollable;
} }
void FrameView::updateScrollableAreaSet() void FrameView::updateScrollableAreaSet()
......
third_party/WebKit/Source/core/frame/FrameView.h
View file @ a687da48
...@@ -247,6 +247,14 @@ public: ...@@ -247,6 +247,14 @@ public:
bool isFrameViewScrollCorner(RenderScrollbarPart* scrollCorner) const { return m_scrollCorner == scrollCorner; } bool isFrameViewScrollCorner(RenderScrollbarPart* scrollCorner) const { return m_scrollCorner == scrollCorner; }
enum ScrollingReasons {
Scrollable,
NotScrollableNoOverflow,
NotScrollableNotVisible,
NotScrollableExplicitlyDisabled
};
ScrollingReasons scrollingReasons();
bool isScrollable(); bool isScrollable();
enum ScrollbarModesCalculationStrategy { RulesFromWebContentOnly, AnyRule }; enum ScrollbarModesCalculationStrategy { RulesFromWebContentOnly, AnyRule };
......
third_party/WebKit/Source/core/page/scrolling/ScrollingCoordinator.cpp
View file @ a687da48
...@@ -946,8 +946,18 @@ MainThreadScrollingReasons ScrollingCoordinator::mainThreadScrollingReasons() co ...@@ -946,8 +946,18 @@ MainThreadScrollingReasons ScrollingCoordinator::mainThreadScrollingReasons() co
if (frameView->hasSlowRepaintObjects()) if (frameView->hasSlowRepaintObjects())
reasons |= HasSlowRepaintObjects; reasons |= HasSlowRepaintObjects;
if (frameView->isScrollable() && hasVisibleSlowRepaintViewportConstrainedObjects(frameView)) FrameView::ScrollingReasons scrollingReasons = frameView->scrollingReasons();
const bool mayBeScrolledByInput = (scrollingReasons == FrameView::Scrollable);
const bool mayBeScrolledByScript = mayBeScrolledByInput || (scrollingReasons ==
FrameView::NotScrollableExplicitlyDisabled);
// TODO(awoloszyn) Currently crbug.com/304810 will let certain
// overflow:hidden elements scroll on the compositor thread, so we should
// not let this move there path as an optimization, when we have slow-repaint
// elements.
if (mayBeScrolledByScript && hasVisibleSlowRepaintViewportConstrainedObjects(frameView)) {
reasons |= HasNonLayerViewportConstrainedObjects; reasons |= HasNonLayerViewportConstrainedObjects;
}
return reasons; return reasons;
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment