Changeset 203684 in webkit

Timestamp:

Jul 25, 2016 9:03:31 AM (8 years ago)

Author:

Wenson Hsieh

Message:

The web process hangs when computing elements-based snap points for a container with large max scroll offset
​https://bugs.webkit.org/show_bug.cgi?id=152605
<rdar://problem/25353661>

Reviewed by Simon Fraser.

Source/WebCore:

Fixes a bug in the computation of axis snap points. The ScrollSnapPoints object, which tracks
snap points along a particular axis, has two flags, hasRepeat and usesElements. For elements-
based snapping, both flags would be turned on, since StyleBuilderConverter::convertScrollSnapPoints
short-circuits for elements-based snapping and does not default usesRepeat to false. To address this,
we make ScrollSnapPoints not repeat(100%) by default.

Test: css3/scroll-snap/scroll-snap-elements-container-larger-than-children.html

• css/StyleBuilderConverter.h:

(WebCore::StyleBuilderConverter::convertScrollSnapPoints): Deleted.

• rendering/style/StyleScrollSnapPoints.cpp:

(WebCore::ScrollSnapPoints::ScrollSnapPoints):

LayoutTests:

Adds a scroll snap offset computation test case that would have previously
caused the web process to hang before this patch.

• css3/scroll-snap/scroll-snap-elements-container-larger-than-children-expected.txt: Added.
• css3/scroll-snap/scroll-snap-elements-container-larger-than-children.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/css3/scroll-snap/scroll-snap-elements-container-larger-than-children-expected.txt (added)
• LayoutTests/css3/scroll-snap/scroll-snap-elements-container-larger-than-children.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/css/StyleBuilderConverter.h (modified) (1 diff)
• Source/WebCore/rendering/style/StyleScrollSnapPoints.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2016-07-24  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        The web process hangs when computing elements-based snap points for a container with large max scroll offset
+        https://bugs.webkit.org/show_bug.cgi?id=152605
+        <rdar://problem/25353661>
+
+        Reviewed by Simon Fraser.
+
+        Adds a scroll snap offset computation test case that would have previously
+        caused the web process to hang before this patch.
+
+        * css3/scroll-snap/scroll-snap-elements-container-larger-than-children-expected.txt: Added.
+        * css3/scroll-snap/scroll-snap-elements-container-larger-than-children.html: Added.
+
 2016-07-25  Sergio Villar Senin  <svillar@igalia.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2016-07-24  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        The web process hangs when computing elements-based snap points for a container with large max scroll offset
+        https://bugs.webkit.org/show_bug.cgi?id=152605
+        <rdar://problem/25353661>
+
+        Reviewed by Simon Fraser.
+
+        Fixes a bug in the computation of axis snap points. The ScrollSnapPoints object, which tracks
+        snap points along a particular axis, has two flags, hasRepeat and usesElements. For elements-
+        based snapping, both flags would be turned on, since StyleBuilderConverter::convertScrollSnapPoints
+        short-circuits for elements-based snapping and does not default usesRepeat to false. To address this,
+        we make ScrollSnapPoints not repeat(100%) by default.
+
+        Test: css3/scroll-snap/scroll-snap-elements-container-larger-than-children.html
+
+        * css/StyleBuilderConverter.h:
+        (WebCore::StyleBuilderConverter::convertScrollSnapPoints): Deleted.
+        * rendering/style/StyleScrollSnapPoints.cpp:
+        (WebCore::ScrollSnapPoints::ScrollSnapPoints):
+
 2016-07-25  Carlos Garcia Campos  <cgarcia@igalia.com>
 

trunk/Source/WebCore/css/StyleBuilderConverter.h

@@ -791 +791 @@
     }
 
-    points->hasRepeat = false;
     for (auto& currentValue : downcast<CSSValueList>(value)) {
         auto& itemValue = downcast<CSSPrimitiveValue>(currentValue.get());

trunk/Source/WebCore/rendering/style/StyleScrollSnapPoints.cpp

@@ -32 +32 @@
 
 ScrollSnapPoints::ScrollSnapPoints()
-    : repeatOffset(100, Percent)
-    , hasRepeat(true)
+    : repeatOffset(0, Fixed)
+    , hasRepeat(false)
     , usesElements(false)
 {