Commit b9a57018 authored by Steve Kobes's avatar Steve Kobes Committed by Commit Bot

Fix feedback loop in extension popup autosizing.

FrameViewAutoSizeInfo should not add hypothetical vertical scrollbar
space when we already have a real vertical scrollbar.

The real scrollbar is included in the LayoutView's preferred width
(since crrev.com/397112).  If the redundant accomodation makes the
scrollbar detectably unnecessary, the next autosize reduces the width,
making it appear again.

The autosized FrameView now accomodates exactly one vertical scrollbar
in this scenario, instead of infinitely bouncing between two and zero.
The scrollbar could actually be removed even at this size, but the code
isn't smart enough to detect that.

Bug: 838150
Change-Id: I23dce5a37faed33de978e1d2182009a2fe0c6fd1
Reviewed-on: https://chromium-review.googlesource.com/1188767Reviewed-by: default avatarStefan Zager <szager@chromium.org>
Commit-Queue: Steve Kobes <skobes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#586062}
parent da1eb5e5
......@@ -90,7 +90,10 @@ void FrameViewAutoSizeInfo::AutoSizeIfNeeded() {
kHorizontalScrollbar));
// Don't bother checking for a vertical scrollbar because the width is at
// already greater the maximum.
} else if (new_size.Height() > max_auto_size_.Height()) {
} else if (new_size.Height() > max_auto_size_.Height() &&
// If we have a real vertical scrollbar, it's already included in
// MinPreferredLogicalWidth, so don't add a hypothetical one.
!layout_viewport->HasVerticalScrollbar()) {
new_size.Expand(
layout_viewport->HypotheticalScrollbarThickness(kVerticalScrollbar),
0);
......
......@@ -1870,6 +1870,41 @@ TEST_F(ScrollbarsTest, AutosizeTest) {
}
}
TEST_F(ScrollbarsTest, AutosizeAlmostRemovableScrollbar) {
ScopedOverlayScrollbarsForTest overlay_scrollbars(false);
WebView().EnableAutoResizeMode(WebSize(25, 25), WebSize(800, 600));
SimRequest resource("https://example.com/test.html", "text/html");
LoadURL("https://example.com/test.html");
resource.Complete(R"HTML(
<style>
body { margin: 0; padding: 15px }
#b1, #b2 { display: inline-block; width: 205px; height: 45px; }
#b1 { background: #888; }
#b2 { background: #bbb; }
#spacer { width: 400px; height: 490px; background: #eee; }
</style>
<div id="b1"></div><div id="b2"></div>
<div id="spacer"></div>
)HTML");
// Finish loading.
test::RunPendingTasks();
LocalFrameView* frame_view = WebView().MainFrameImpl()->GetFrameView();
ScrollableArea* layout_viewport = frame_view->LayoutViewport();
// Check three times to verify stability.
for (int i = 0; i < 3; i++) {
frame_view->SetNeedsLayout();
Compositor().BeginFrame();
EXPECT_TRUE(layout_viewport->VerticalScrollbar());
EXPECT_FALSE(layout_viewport->HorizontalScrollbar());
EXPECT_EQ(445, frame_view->Width());
EXPECT_EQ(600, frame_view->Height());
}
}
TEST_F(ScrollbarsTest,
HideTheOverlayScrollbarNotCrashAfterPLSADisposedPaintLayer) {
WebView().Resize(WebSize(200, 200));
......
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