Title: [203684] trunk
Revision
203684
Author
wenson_hs...@apple.com
Date
2016-07-25 09:03:31 -0700 (Mon, 25 Jul 2016)

Log 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.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (203683 => 203684)


--- trunk/LayoutTests/ChangeLog	2016-07-25 15:02:56 UTC (rev 203683)
+++ trunk/LayoutTests/ChangeLog	2016-07-25 16:03:31 UTC (rev 203684)
@@ -1,3 +1,17 @@
+2016-07-24  Wenson Hsieh  <wenson_hs...@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  <svil...@igalia.com>
 
         [css-grid] Implement repeat(auto-fit)

Added: trunk/LayoutTests/css3/scroll-snap/scroll-snap-elements-container-larger-than-children-expected.txt (0 => 203684)


--- trunk/LayoutTests/css3/scroll-snap/scroll-snap-elements-container-larger-than-children-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/css3/scroll-snap/scroll-snap-elements-container-larger-than-children-expected.txt	2016-07-25 16:03:31 UTC (rev 203684)
@@ -0,0 +1,8 @@
+PASS successfullyParsed is true
+
+TEST COMPLETE
+Scroll-snap offsets: horizontal = { 0, 200, 400, 600, 800, 1000, 1500 }
+PASS successfullyParsed is true
+
+TEST COMPLETE
+

Added: trunk/LayoutTests/css3/scroll-snap/scroll-snap-elements-container-larger-than-children.html (0 => 203684)


--- trunk/LayoutTests/css3/scroll-snap/scroll-snap-elements-container-larger-than-children.html	                        (rev 0)
+++ trunk/LayoutTests/css3/scroll-snap/scroll-snap-elements-container-larger-than-children.html	2016-07-25 16:03:31 UTC (rev 203684)
@@ -0,0 +1,61 @@
+<!DOCTYPE html>
+<html>
+    <head>
+        <style>
+            #outer-container {
+                width: 300px;
+                height: 300px;
+                overflow-y: hidden;
+                overflow-x: auto;
+                -webkit-scroll-snap-points-x: elements;
+                -webkit-scroll-snap-type: mandatory;
+            }
+            #inner-container {
+                width: 1800px;
+                height: 300px;
+            }
+            .snap-child {
+                height: 300px;
+                float: left;
+                min-width: 200px;
+                -webkit-scroll-snap-coordinate: 0 0;
+            }
+        </style>
+        <script src=""
+        <script>
+            function runTest()
+            {
+                var container = document.getElementById("outer-container");
+                debug("Scroll-snap offsets: " + window.internals.scrollSnapOffsets(container));
+
+                finishJSTest();
+                testRunner.notifyDone();
+            }
+
+            function onLoad()
+            {
+                if (window.testRunner) {
+                    window.jsTestIsAsync = true;
+                    testRunner.dumpAsText();
+                    testRunner.waitUntilDone();
+                    setTimeout(runTest, 0);
+                }
+            }
+        </script>
+    </head>
+    <body _onload_=onLoad()>
+        <div style="position: relative; height: 500px; width: 500px">
+            <div id="outer-container">
+                <div id="inner-container">
+                    <div id="item0" class="snap-child"></div>
+                    <div id="item1" class="snap-child"></div>
+                    <div id="item2" class="snap-child"></div>
+                    <div id="item3" class="snap-child"></div>
+                    <div id="item4" class="snap-child"></div>
+                    <div id="item5" class="snap-child"></div>
+                </div>
+            </div>
+        </div>
+        <script src=""
+    </body>
+</html>
\ No newline at end of file

Modified: trunk/Source/WebCore/ChangeLog (203683 => 203684)


--- trunk/Source/WebCore/ChangeLog	2016-07-25 15:02:56 UTC (rev 203683)
+++ trunk/Source/WebCore/ChangeLog	2016-07-25 16:03:31 UTC (rev 203684)
@@ -1,3 +1,24 @@
+2016-07-24  Wenson Hsieh  <wenson_hs...@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  <cgar...@igalia.com>
 
         REGRESSION(r200931): Invalid cast in highestAncestorToWrapMarkup()

Modified: trunk/Source/WebCore/css/StyleBuilderConverter.h (203683 => 203684)


--- trunk/Source/WebCore/css/StyleBuilderConverter.h	2016-07-25 15:02:56 UTC (rev 203683)
+++ trunk/Source/WebCore/css/StyleBuilderConverter.h	2016-07-25 16:03:31 UTC (rev 203684)
@@ -790,7 +790,6 @@
         return points;
     }
 
-    points->hasRepeat = false;
     for (auto& currentValue : downcast<CSSValueList>(value)) {
         auto& itemValue = downcast<CSSPrimitiveValue>(currentValue.get());
         if (auto* lengthRepeat = itemValue.getLengthRepeatValue()) {

Modified: trunk/Source/WebCore/rendering/style/StyleScrollSnapPoints.cpp (203683 => 203684)


--- trunk/Source/WebCore/rendering/style/StyleScrollSnapPoints.cpp	2016-07-25 15:02:56 UTC (rev 203683)
+++ trunk/Source/WebCore/rendering/style/StyleScrollSnapPoints.cpp	2016-07-25 16:03:31 UTC (rev 203684)
@@ -31,8 +31,8 @@
 namespace WebCore {
 
 ScrollSnapPoints::ScrollSnapPoints()
-    : repeatOffset(100, Percent)
-    , hasRepeat(true)
+    : repeatOffset(0, Fixed)
+    , hasRepeat(false)
     , usesElements(false)
 {
 }
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to