iOS] Scroll snap points trigger reentrant layout
authorbfulgham@apple.com <bfulgham@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 5 May 2015 23:35:41 +0000 (23:35 +0000)
committerbfulgham@apple.com <bfulgham@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 5 May 2015 23:35:41 +0000 (23:35 +0000)
https://bugs.webkit.org/show_bug.cgi?id=144644
<rdar://problem/20366547>

Reviewed by Simon Fraser.

Covered by scroll-snap-mandatory.html test.

We had an iOS code path in 'appendChildSnapOffsets' that used offsetLeft and offsetTop. This code
was sometimes called during layout, which triggered a reentrant layout call, resulting in a debug
assertion.

* page/scrolling/AxisScrollSnapOffsets.cpp:
(WebCore::appendChildSnapOffsets): Remove iOS codepath.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@183840 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/page/scrolling/AxisScrollSnapOffsets.cpp

index 4de9a01..6941d82 100644 (file)
@@ -1,3 +1,20 @@
+2015-05-05  Brent Fulgham  <bfulgham@apple.com>
+
+        iOS] Scroll snap points trigger reentrant layout
+        https://bugs.webkit.org/show_bug.cgi?id=144644
+        <rdar://problem/20366547>
+
+        Reviewed by Simon Fraser.
+
+        Covered by scroll-snap-mandatory.html test.
+
+        We had an iOS code path in 'appendChildSnapOffsets' that used offsetLeft and offsetTop. This code
+        was sometimes called during layout, which triggered a reentrant layout call, resulting in a debug
+        assertion.
+
+        * page/scrolling/AxisScrollSnapOffsets.cpp:
+        (WebCore::appendChildSnapOffsets): Remove iOS codepath.
+
 2015-05-05  Roger Fong  <roger_fong@apple.com>
 
         Blurry media control icons on non retina displays.
index 0b2bd4e..bcd1a8c 100644 (file)
@@ -45,23 +45,15 @@ static void appendChildSnapOffsets(HTMLElement& parent, bool shouldAddHorizontal
         if (RenderBox* box = child.renderBox()) {
             LayoutUnit viewWidth = box->width();
             LayoutUnit viewHeight = box->height();
-#if PLATFORM(IOS)
-            // FIXME: Dangerous to call offsetLeft and offsetTop because they call updateLayoutIgnorePendingStylesheets, which can invalidate the RenderBox pointer we are holding.
-            // FIXME: Investigate why using localToContainerPoint gives the wrong offsets for iOS main frame. Also, these offsets won't take transforms into account (make sure to test this!).
-            float left = child.offsetLeft();
-            float top = child.offsetTop();
-#else
-            // FIXME: Check that localToContainerPoint works with CSS rotations.
             FloatPoint position = box->localToContainerPoint(FloatPoint(), parent.renderBox());
-            float left = position.x();
-            float top = position.y();
-#endif
+            LayoutUnit left = position.x();
+            LayoutUnit top = position.y();
             for (auto& coordinate : box->style().scrollSnapCoordinates()) {
-                LayoutUnit lastPotentialSnapPositionX = LayoutUnit(left) + valueForLength(coordinate.width(), viewWidth);
+                LayoutUnit lastPotentialSnapPositionX = left + valueForLength(coordinate.width(), viewWidth);
                 if (shouldAddHorizontalChildOffsets && lastPotentialSnapPositionX > 0)
                     horizontalSnapOffsetSubsequence.append(lastPotentialSnapPositionX);
 
-                LayoutUnit lastPotentialSnapPositionY = LayoutUnit(top) + valueForLength(coordinate.height(), viewHeight);
+                LayoutUnit lastPotentialSnapPositionY = top + valueForLength(coordinate.height(), viewHeight);
                 if (shouldAddVerticalChildOffsets && lastPotentialSnapPositionY > 0)
                     verticalSnapOffsetSubsequence.append(lastPotentialSnapPositionY);
             }