Bug 142414 – Setting scroll-snap-desination to (100% 100%) locks up WebKit

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Brent Fulgham]

While working on Bug 136345, I noticed that defining "-webkit-scroll-snap-desination: 100% 100%" causes WebKit to hang while parsing the document.

[Radar WebKit Bug Importer]

<rdar://problem/20077275>

[Brent Fulgham]

This only happens in conjunction with setting 'scroll-snap-coordinate-x: repeat(100%)'.

We seem to be getting stuck in layout:

Thread 1Queue : com.apple.main-thread (serial)
#0	0x00000001060bed20 in signedAddOverflows(int, int, int&) at /Volumes/Data/Projects/WebKit/OpenSource/WebKitBuild/Debug/usr/local/include/wtf/SaturatedArithmetic.h:44
#1	0x00000001060becbd in saturatedAddition(int, int) at /Volumes/Data/Projects/WebKit/OpenSource/WebKitBuild/Debug/usr/local/include/wtf/SaturatedArithmetic.h:62
#2	0x00000001060bebfb in WebCore::operator+(WebCore::LayoutUnit const&, WebCore::LayoutUnit const&) at /Volumes/Data/Projects/WebKit/OpenSource/Source/WebCore/platform/LayoutUnit.h:667
#3	0x0000000106234c5b in WebCore::updateFromStyle(WTF::Vector<WebCore::LayoutUnit, 0ul, WTF::CrashOnOverflow>&, WebCore::RenderStyle const&, WebCore::ScrollEventAxis, WebCore::LayoutUnit, WebCore::LayoutUnit, WTF::Vector<WebCore::LayoutUnit, 0ul, WTF::CrashOnOverflow>&) at /Volumes/Data/Projects/WebKit/OpenSource/Source/WebCore/page/scrolling/AxisScrollSnapOffsets.cpp:89
#4	0x0000000106233ece in WebCore::updateSnapOffsetsForScrollableArea(WebCore::ScrollableArea&, WebCore::HTMLElement&, WebCore::RenderBox const&, WebCore::RenderStyle const&) at /Volumes/Data/Projects/WebKit/OpenSource/Source/WebCore/page/scrolling/AxisScrollSnapOffsets.cpp:154
#5	0x000000010791b627 in WebCore::RenderLayer::updateSnapOffsets() at /Volumes/Data/Projects/WebKit/OpenSource/Source/WebCore/rendering/RenderLayer.cpp:3193
#6	0x000000010791c420 in WebCore::RenderLayer::updateScrollInfoAfterLayout() at /Volumes/Data/Projects/WebKit/OpenSource/Source/WebCore/rendering/RenderLayer.cpp:3482
#7	0x00000001077d19b6 in WebCore::RenderBlock::updateScrollInfoAfterLayout() at /Volumes/Data/Projects/WebKit/OpenSource/Source/WebCore/rendering/RenderBlock.cpp:912
#8	0x00000001078062f3 in WebCore::RenderBlockFlow::layoutBlock(bool, WebCore::LayoutUnit) at /Volumes/Data/Projects/WebKit/OpenSource/Source/WebCore/rendering/RenderBlockFlow.cpp:536
#9	0x000000010791beae in WebCore::RenderLayer::updateScrollbarsAfterLayout() at /Volumes/Data/Projects/WebKit/OpenSource/Source/WebCore/rendering/RenderLayer.cpp:3416
#10	0x000000010791c377 in WebCore::RenderLayer::updateScrollInfoAfterLayout() at /Volumes/Data/Projects/WebKit/OpenSource/Source/WebCore/rendering/RenderLayer.cpp:3470
#11	0x00000001077d19b6 in WebCore::RenderBlock::updateScrollInfoAfterLayout() at /Volumes/Data/Projects/WebKit/OpenSource/Source/WebCore/rendering/RenderBlock.cpp:912
#12	0x00000001078062f3 in WebCore::RenderBlockFlow::layoutBlock(bool, WebCore::LayoutUnit) at /Volumes/Data/Projects/WebKit/OpenSource/Source/WebCore/rendering/RenderBlockFlow.cpp:536
#13	0x00000001077d1a3d in WebCore::RenderBlock::layout() at /Volumes/Data/Projects/WebKit/OpenSource/Source/WebCore/rendering/RenderBlock.cpp:926
#14	0x00000001078095e2 in WebCore::RenderBlockFlow::layoutBlockChild(WebCore::RenderBox&, WebCore::RenderBlockFlow::MarginInfo&, WebCore::LayoutUnit&, WebCore::LayoutUnit&) at /Volumes/Data/Projects/WebKit/OpenSource/Source/WebCore/rendering/RenderBlockFlow.cpp:705
#15	0x0000000107806f7a in WebCore::RenderBlockFlow::layoutBlockChildren(bool, WebCore::LayoutUnit&) at /Volumes/Data/Projects/WebKit/OpenSource/Source/WebCore/rendering/RenderBlockFlow.cpp:628

[Brent Fulgham]

Looks like I've managed to trigger an infinite loop in 'updateFromStyle' in AxisScrollSnapOffsets. The problem seems to happen when a repeating scroll snap point is used in conjunction with an equivalent scroll snap destination.

When this happens, we compute a 0 offset, and get stuck in the loop that looks for the next snap position.

I'm not sure why the original comparison was "potentialSnapPosition <= 0", because the potential snap position (when the snap destination equals the snap position) will always be 0.

Changing this comparison to be "potentialSnapPosition < 0" prevents the infinite loop, and provides correct behavior.

[Brent Fulgham]

Created attachment 248107 [details]
Patch

[Dean Jackson]

Comment on attachment 248107 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=248107&action=review

> Source/WebCore/page/scrolling/AxisScrollSnapOffsets.cpp:90
> -            if (potentialSnapPosition <= 0)
> +            if (potentialSnapPosition < 0)

Haha!!

[Brent Fulgham]

Committed r181194: <http://trac.webkit.org/changeset/181194>