When using non-overlay scrollbars, ScrollView::updateScrollbars can reach its cMaxUpdateScrollbarsPass recursion limit, leaving layout in a state that's inconsistent wrt the presence of scrollbars. For example, starting with a page that's vertically scrollable, resize the window to reduce its width. Here's what happens in updateScrollbars: 1) The totalContentsSize() is wider than the visibleWidth(), so a horizontal scrollbar is added. This triggers another layout and a recursive call into updateScrollbars. 2) Layout changes the totalContentsSize() so that is no longer wider than the visible width. So the horizontal scrollbar needs to be removed. But updateScrollbars always removes both scrollbars when one needs to be removed, so the vertical scrollbar is removed as well. Another layout and recursive call into updateScrollbars happens. 3) Layout changes the width of totalContentsSize() under the assumption that there's no vertical scrollbar. But totalContentsSize() is still taller than visibleHeight(), so the vertical scrollbar is added back. We need to trigger another layout to account for this, but we can't since m_updateScrollbarsPass is already 2 (that is, cMaxUpdateScrollbarsPass). So we're left with the layout that was done under the assumption of no vertical scrollbar, even though we do have a vertical scrollbar. And that means the layout viewport is wider than it should be, by the width of the scrollbar. It does get the right size the next time something triggers layout. One way to fix this is to not remove the vertical scrollbar in step 2. Then there won't be any scrollbar change in step 3. Alternatively, we could increase cMaxUpdateScrollbarsPass.
Created attachment 317323 [details] Patch
Created attachment 318526 [details] Patch Rebased
I'm not really familiar with this code but that lgtm. I'm not sure what was the rationale for the "always disable the other bar when removing one bar", maybe someone involved in https://trac.webkit.org/changeset/42334 should confirm it's too strong.
This is "highly tuned" code and I suspect that the current logic is there for a good reason. Please talk to Dave Hyatt about this.
This should be ok.
Comment on attachment 318526 [details] Patch Rejecting attachment 318526 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-01', 'apply-attachment', '--no-update', '--non-interactive', 318526, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit Last 500 characters of output: 3. patching file LayoutTests/fast/dom/Window/window-resize-update-scrollbars-expected.txt patching file LayoutTests/fast/dom/Window/window-resize-update-scrollbars.html patching file LayoutTests/platform/ios/TestExpectations Hunk #1 FAILED at 1896. 1 out of 1 hunk FAILED -- saving rejects to file LayoutTests/platform/ios/TestExpectations.rej Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force', '--reviewer', u'David Hyatt']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit Full output: http://webkit-queues.webkit.org/results/5052531
Created attachment 325478 [details] Patch for landing Rebased.
Comment on attachment 325478 [details] Patch for landing Clearing flags on attachment: 325478 Committed r224241: <https://trac.webkit.org/changeset/224241>
All reviewed patches have been landed. Closing bug.
Comment on attachment 325478 [details] Patch for landing View in context: https://bugs.webkit.org/attachment.cgi?id=325478&action=review > Source/WebCore/platform/ScrollView.cpp:618 > + if (!newHasHorizontalScrollbar && hasHorizontalScrollbar && vScroll != ScrollbarAlwaysOn &&!hasVerticalScrollbar) { Formatting mistake, missing space after "&&".
Reopening to attach new patch.
Created attachment 325913 [details] Formatting fix Oops, here's a fix for the missing space.
Comment on attachment 325913 [details] Formatting fix Clearing flags on attachment: 325913 Committed r224409: <https://trac.webkit.org/changeset/224409>
<rdar://problem/35569029>