Changeset 265051 in webkit

Timestamp:

Jul 29, 2020 1:00:48 PM (4 years ago)

Author:

Simon Fraser

Message:

REGRESSION: ASSERTION FAILED: !needsLayout() on tiled-drawing/scrolling/fast-scroll-mainframe-zoom.html flakily
​https://bugs.webkit.org/show_bug.cgi?id=214651
Source/WebCore:

<rdar://problem/65952371>

Reviewed by Zalan Bujtas.

Page::doAfterUpdateRendering() is expected to only call functions that can't trigger layout. However,
it could cause the firing of the "monitorWheelEvent" callback which can run arbitrary JS, so move that
earlier in the rendering update, before we do the final layout.

Also assert that the main frame doesn't need layout at the end of Page::doAfterUpdateRendering();
we were only checking subframes.

Tested by lots of existing tests.

• page/Page.cpp:

(WebCore::Page::updateRendering):
(WebCore::Page::doAfterUpdateRendering):

LayoutTests:

Reviewed by Zalan Bujtas.

De-flake this test.

• fast/scrolling/mac/scrollbars/overflow-in-iframe-overlay-scrollbar-hovered-expected.txt:
• fast/scrolling/mac/scrollbars/overflow-in-iframe-overlay-scrollbar-hovered.html:

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/scrolling/mac/scrollbars/overflow-in-iframe-overlay-scrollbar-hovered-expected.txt (modified) (1 diff)
• LayoutTests/fast/scrolling/mac/scrollbars/overflow-in-iframe-overlay-scrollbar-hovered.html (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/page/Page.cpp (modified) (2 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2020-07-29  Simon Fraser  <simon.fraser@apple.com>
+
+        REGRESSION: ASSERTION FAILED: !needsLayout() on tiled-drawing/scrolling/fast-scroll-mainframe-zoom.html flakily
+        https://bugs.webkit.org/show_bug.cgi?id=214651
+
+        Reviewed by Zalan Bujtas.
+
+        De-flake this test.
+
+        * fast/scrolling/mac/scrollbars/overflow-in-iframe-overlay-scrollbar-hovered-expected.txt:
+        * fast/scrolling/mac/scrollbars/overflow-in-iframe-overlay-scrollbar-hovered.html:
+
 2020-07-29  Chris Dumez  <cdumez@apple.com>
 

trunk/LayoutTests/fast/scrolling/mac/scrollbars/overflow-in-iframe-overlay-scrollbar-hovered-expected.txt

@@ -8 +8 @@
 enabled
 Hovering vertical scrollbar should show expanded scrollbar
-PASS Scrollbar state: enabled,expanded,visible_track,visible_thumb
+PASS Scrollbar state contains: enabled and expanded
 Unhovering vertical scrollbar should hide it
 PASS Thumb and track hidden

trunk/LayoutTests/fast/scrolling/mac/scrollbars/overflow-in-iframe-overlay-scrollbar-hovered.html

@@ -49 +49 @@
             await UIHelper.waitForCondition(() => {
                 let state = iframeWindow.internals.verticalScrollbarState(scroller);
+                let enabled = state.indexOf('enabled') != -1;
                 let expanded = state.indexOf('expanded') != -1;
-                if (expanded)
-                    testPassed('Scrollbar state: ' + state);
+                if (enabled && expanded)
+                    testPassed('Scrollbar state contains: enabled and expanded');
                 return expanded;
             });

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2020-07-29  Simon Fraser  <simon.fraser@apple.com>
+
+        REGRESSION: ASSERTION FAILED: !needsLayout() on tiled-drawing/scrolling/fast-scroll-mainframe-zoom.html flakily
+        https://bugs.webkit.org/show_bug.cgi?id=214651
+        <rdar://problem/65952371>
+
+        Reviewed by Zalan Bujtas.
+
+        Page::doAfterUpdateRendering() is expected to only call functions that can't trigger layout. However,
+        it could cause the firing of the "monitorWheelEvent" callback which can run arbitrary JS, so move that
+        earlier in the rendering update, before we do the final layout.
+
+        Also assert that the main frame doesn't need layout at the end of Page::doAfterUpdateRendering();
+        we were only checking subframes.
+
+        Tested by lots of existing tests.
+
+        * page/Page.cpp:
+        (WebCore::Page::updateRendering):
+        (WebCore::Page::doAfterUpdateRendering):
+
 2020-07-29  Chris Dumez  <cdumez@apple.com>
 

trunk/Source/WebCore/page/Page.cpp

@@ -1505 +1505 @@
             document->domWindow()->unfreezeNowTimestamp();
     }
-    
+
+    if (UNLIKELY(isMonitoringWheelEvents()))
+        wheelEventTestMonitor()->checkShouldFireCallbacks();
+
     if (m_isTrackingRenderingUpdates)
         ++m_renderingUpdateCount;
@@ -1548 +1551 @@
     DebugPageOverlays::doAfterUpdateRendering(*this);
 
-    if (UNLIKELY(isMonitoringWheelEvents()))
-        wheelEventTestMonitor()->checkShouldFireCallbacks();
-
     forEachDocument([] (Document& document) {
         document.prepareCanvasesForDisplayIfNeeded();
     });
 
+    ASSERT(!mainFrame().view() || !mainFrame().view()->needsLayout());
 #if ASSERT_ENABLED
     for (Frame* child = mainFrame().tree().firstRenderedChild(); child; child = child->tree().traverseNextRendered()) {