Skip to content

GitLab

T
tangled
Commit ace3f1f7 authored by David Bokan's avatar David Bokan Committed by Commit Bot
Browse files
Options

[RootScroller] Allow main document horizontal overflow 

When implicitly promoting an element to be the root scroller, we first
check if the main document has any overflow. If it does, we err on the
side of not promoting since it is likely the main document's content is
important and meant to be scrollable.

This heuristic turns out to be too restrictive. For example, this makes
it hard to build a root scrolling carousel. This CL loosens the
restriction to allow horizontal overflow.

Bug: 838683
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I6007f6faa8e2006799a9dca6240b68ff0de7f09a
Reviewed-on: https://chromium-review.googlesource.com/1165859
Commit-Queue: David Bokan <bokan@chromium.org>
Reviewed-by: default avatarDave Tapuska <dtapuska@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581408}
parent 336adf22
Hide whitespace changes
Inline Side-by-side
Showing with 81 additions and 6 deletions
third_party/blink/renderer/core/page/scrolling/root_scroller_controller.cc
View file @ ace3f1f7
...@@ -92,6 +92,21 @@ PaintLayerScrollableArea* GetScrollableArea(const Element& element) { ...@@ -92,6 +92,21 @@ PaintLayerScrollableArea* GetScrollableArea(const Element& element) {
return ToLayoutBox(element.GetLayoutObject())->GetScrollableArea(); return ToLayoutBox(element.GetLayoutObject())->GetScrollableArea();
} }
bool ScrollsVerticalOverflow(LayoutView& layout_view) {
DCHECK(layout_view.GetScrollableArea());
if (layout_view.Size().IsZero() ||
!layout_view.GetScrollableArea()->HasVerticalOverflow() ||
!layout_view.ScrollsOverflowY())
return false;
ScrollbarMode h_mode;
ScrollbarMode v_mode;
layout_view.CalculateScrollbarModes(h_mode, v_mode);
return v_mode != kScrollbarAlwaysOff;
}
} // namespace } // namespace
// static // static
...@@ -374,9 +389,9 @@ void RootScrollerController::ProcessImplicitCandidates() { ...@@ -374,9 +389,9 @@ void RootScrollerController::ProcessImplicitCandidates() {
if (!document_->GetLayoutView()) if (!document_->GetLayoutView())
return; return;
// If the document has scrollable content, that's a good sign we shouldn't // If the main document has vertical scrolling, that's a good sign we
// implicitly promote anything. // shouldn't implicitly promote anything.
if (document_->GetLayoutView()->GetScrollableArea()->ScrollsOverflow()) if (ScrollsVerticalOverflow(*document_->GetLayoutView()))
return; return;
Element* highest_z_element = nullptr; Element* highest_z_element = nullptr;
......
third_party/blink/renderer/core/page/scrolling/root_scroller_test.cc
View file @ ace3f1f7
...@@ -2335,6 +2335,66 @@ TEST_F(ImplicitRootScrollerSimTest, PromotionChangesLayoutSize) { ...@@ -2335,6 +2335,66 @@ TEST_F(ImplicitRootScrollerSimTest, PromotionChangesLayoutSize) {
<< "Once loaded, the iframe should be promoted."; << "Once loaded, the iframe should be promoted.";
} }
// Test that we don't promote any elements implicitly if the main document have
// vertical overflow.
TEST_F(ImplicitRootScrollerSimTest, OverflowInMainDocumentRestrictsImplicit) {
WebView().ResizeWithBrowserControls(IntSize(800, 600), 50, 0, true);
SimRequest main_request("https://example.com/test.html", "text/html");
SimRequest child_request("https://example.com/child.html", "text/html");
LoadURL("https://example.com/test.html");
main_request.Complete(R"HTML(
<!DOCTYPE html>
<style>
::-webkit-scrollbar {
width: 0px;
height: 0px;
}
body, html {
width: 100%;
height: 100%;
margin: 0px;
}
iframe {
width: 100%;
height: 100%;
border: 0;
}
div {
position: absolute;
left: 0;
top: 0;
height: 150%;
width: 150%;
}
</style>
<iframe id="container" src="child.html">
</iframe>
<div id="spacer"></div>
)HTML");
child_request.Complete(R"HTML(
<!DOCTYPE html>
<style>
body {
height: 1000px;
}
</style>
)HTML");
Compositor().BeginFrame();
EXPECT_EQ(GetDocument(),
GetDocument().GetRootScrollerController().EffectiveRootScroller())
<< "iframe shouldn't be promoted due to overflow in the main document.";
Element* spacer = GetDocument().getElementById("spacer");
spacer->style()->setProperty(&GetDocument(), "height", "100%", String(),
ASSERT_NO_EXCEPTION);
Compositor().BeginFrame();
EXPECT_EQ(GetDocument().getElementById("container"),
GetDocument().GetRootScrollerController().EffectiveRootScroller())
<< "Once vertical overflow is removed, the iframe should be promoted.";
}
class RootScrollerHitTest : public RootScrollerTest { class RootScrollerHitTest : public RootScrollerTest {
public: public:
void CheckHitTestAtBottomOfScreen() { void CheckHitTestAtBottomOfScreen() {
......
third_party/blink/renderer/core/paint/paint_layer_scrollable_area.h
View file @ ace3f1f7
...@@ -545,14 +545,14 @@ class CORE_EXPORT PaintLayerScrollableArea final ...@@ -545,14 +545,14 @@ class CORE_EXPORT PaintLayerScrollableArea final
bool VisualViewportSuppliesScrollbars() const override; bool VisualViewportSuppliesScrollbars() const override;
bool HasHorizontalOverflow() const;
bool HasVerticalOverflow() const;
void Trace(blink::Visitor*) override; void Trace(blink::Visitor*) override;
private: private:
explicit PaintLayerScrollableArea(PaintLayer&); explicit PaintLayerScrollableArea(PaintLayer&);
bool HasHorizontalOverflow() const;
bool HasVerticalOverflow() const;
bool NeedsScrollbarReconstruction() const; bool NeedsScrollbarReconstruction() const;
void ResetScrollOriginChanged() { scroll_origin_changed_ = false; } void ResetScrollOriginChanged() { scroll_origin_changed_ = false; }
......
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