Skip to content

GitLab

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

Reland "[RootScroller] Allow main document horizontal overflow" 

This is a reland of ace3f1f7

Original change's description:
> [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: Dave Tapuska <dtapuska@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#581408}

Bug: 838683
Change-Id: I886908a56db7a463c619135ed08712312ccacbfc
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
TBR: bokan, dtapuska
Reviewed-on: https://chromium-review.googlesource.com/1171717
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Reviewed-by: default avatarPeter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582458}
parent 42b4e9ec
Show 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 @ 801a32fc
...@@ -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 @ 801a32fc
...@@ -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 @ 801a32fc
...@@ -543,14 +543,14 @@ class CORE_EXPORT PaintLayerScrollableArea final ...@@ -543,14 +543,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