[webkit-changes] [267688] trunk/Source

Sun, 27 Sep 2020 17:24:03 -0700

Title: [267688] trunk/Source
Revision
267688
Author
[email protected]
Date
2020-09-27 17:23:07 -0700 (Sun, 27 Sep 2020)

Log Message

WebKitLegacy should call Page::finalizeRenderingUpdate()
https://bugs.webkit.org/show_bug.cgi?id=216958

Reviewed by Tim Horton.
Source/WebCore:

Convert Page::m_inUpdateRendering to an enum which tracks the phase, which will be
used in a later patch to prevent extra update scheduling (webkit.org/b/216726).

Add isolatedUpdateRendering(), which is for callers who aren't going to call finalizeRenderingUpdate(),
and use it for SVGImage updates.

DRT/WTR can trigger Page::updateRendering() re-entrancy, so there's a bit of ugliness
that deals with that.

* page/Page.cpp:
(WebCore::Page::updateRendering):
(WebCore::Page::isolatedUpdateRendering):
(WebCore::Page::doAfterUpdateRendering):
(WebCore::Page::finalizeRenderingUpdate):
* page/Page.h:

Source/WebKit:

dynamicViewportSizeUpdate() needs to call isolatedUpdateRendering() because it isn't followed
by a finalizeRenderingUpdate().

* WebProcess/WebPage/ios/WebPageIOS.mm:
(WebKit::WebPage::dynamicViewportSizeUpdate):

Source/WebKitLegacy/mac:

This is a precursor to fixing webkit.org/b/216726. Page needs to track the phase
of updateRendering that we are in, and to ease this tracking, WebKitLegacy needs to call
Page::finalizeRenderingUpdate() so the state is tracked.

Rename -_viewWillDrawInternal to -_updateRendering, and have it call updateRendering()
and finalizeRenderingUpdate(). We can also move in the call to -_synchronizeCustomFixedPositionLayoutRect
which both call sites do.

Since updateRendering() is guaranteed to update layout, we know -_flushCompositingChanges would have
never returned NO here, so we can remove the condition in LayerFlushController::flushLayers().

-[WebHTMLView viewWillDraw] also does a similar -_web_updateLayoutAndStyleIfNeededRecursive
then -_flushCompositingChanges but this is called from AppKit with a timing that we don't control;
it may be redundant with -[WebView _updateRendering] but I leave that behavior unchanged.

* WebView/WebView.mm:
(-[WebView _updateRendering]):
(-[WebView _forceRepaintForTesting]):
(LayerFlushController::flushLayers):
(-[WebView _viewWillDrawInternal]): Deleted.

Source/WebKitLegacy/win:

Windows doesn't call finalizeRenderingUpdate() so needs to use isolatedUpdateRendering().

* WebCoreSupport/AcceleratedCompositingContext.cpp:
(AcceleratedCompositingContext::flushAndRenderLayers):
* WebView.cpp:
(WebView::paint):
(WebView::flushPendingGraphicsLayerChangesSoon):
(WebView::flushPendingGraphicsLayerChanges):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (267687 => 267688)


--- trunk/Source/WebCore/ChangeLog	2020-09-27 20:00:52 UTC (rev 267687)
+++ trunk/Source/WebCore/ChangeLog	2020-09-28 00:23:07 UTC (rev 267688)
@@ -1,3 +1,25 @@
+2020-09-24  Simon Fraser  <[email protected]>
+
+        WebKitLegacy should call Page::finalizeRenderingUpdate()
+        https://bugs.webkit.org/show_bug.cgi?id=216958
+
+        Reviewed by Tim Horton.
+
+        Convert Page::m_inUpdateRendering to an enum which tracks the phase, which will be
+        used in a later patch to prevent extra update scheduling (webkit.org/b/216726).
+        Page has to track RenderingUpdatePhase as a stack to handle the re-entrancy when layout tests
+        call notifyDone() inside of the JS callbacks; the stack only has one entry outside of testing.
+
+        Add isolatedUpdateRendering(), which is for callers who aren't going to call finalizeRenderingUpdate(),
+        and use it for SVGImage updates.
+
+        * page/Page.cpp:
+        (WebCore::Page::updateRendering):
+        (WebCore::Page::isolatedUpdateRendering):
+        (WebCore::Page::doAfterUpdateRendering):
+        (WebCore::Page::finalizeRenderingUpdate):
+        * page/Page.h:
+
 2020-09-27  Zalan Bujtas  <[email protected]>
 
         Unreviewed. Call showLayoutTree only when trees are mismatching.

Modified: trunk/Source/WebCore/page/Page.cpp (267687 => 267688)


--- trunk/Source/WebCore/page/Page.cpp	2020-09-27 20:00:52 UTC (rev 267687)
+++ trunk/Source/WebCore/page/Page.cpp	2020-09-28 00:23:07 UTC (rev 267688)
@@ -1441,13 +1441,15 @@
 // https://html.spec.whatwg.org/multipage/webappapis.html#update-the-rendering
 void Page::updateRendering()
 {
-    // This function is not reentrant, e.g. a rAF callback may force repaint.
-    if (m_inUpdateRendering) {
+    m_updateRenderingPhaseStack.append(RenderingUpdatePhase::InUpdateRendering);
+
+    // This function is not reentrant, e.g. a rAF callback may trigger a forces repaint in testing.
+    // This is why we track updateRenderingPhase as a stack.
+    if (m_updateRenderingPhaseStack.size() > 1) {
         layoutIfNeeded();
         return;
     }
 
-    SetForScope<bool> inUpdateRendering(m_inUpdateRendering, true);
     m_lastRenderingUpdateTimestamp = MonotonicTime::now();
 
     bool isSVGImagePage = chrome().client().isSVGImageChromeClient();
@@ -1509,10 +1511,12 @@
     forEachDocument([] (Document& document) {
         for (auto& image : document.cachedResourceLoader().allCachedSVGImages()) {
             if (auto* page = image->internalPage())
-                page->updateRendering();
+                page->isolatedUpdateRendering();
         }
     });
 
+    ASSERT(m_updateRenderingPhaseStack.last() == RenderingUpdatePhase::InUpdateRendering);
+
     for (auto& document : initialDocuments) {
         if (document && document->domWindow())
             document->domWindow()->unfreezeNowTimestamp();
@@ -1529,10 +1533,20 @@
 
     if (!isSVGImagePage)
         tracePoint(RenderingUpdateEnd);
+
+    ASSERT(m_updateRenderingPhaseStack.last() == RenderingUpdatePhase::InUpdateRendering);
 }
 
+void Page::isolatedUpdateRendering()
+{
+    updateRendering();
+    m_updateRenderingPhaseStack.removeLast();
+}
+
 void Page::doAfterUpdateRendering()
 {
+    ASSERT(m_updateRenderingPhaseStack.last() == RenderingUpdatePhase::InUpdateRendering);
+
     // Code here should do once-per-frame work that needs to be done before painting, and requires
     // layout to be up-to-date. It should not run script, trigger layout, or dirty layout.
 
@@ -1586,10 +1600,14 @@
         ASSERT(!frameView || !frameView->needsLayout());
     }
 #endif
+
+    ASSERT(m_updateRenderingPhaseStack.last() == RenderingUpdatePhase::InUpdateRendering);
 }
 
 void Page::finalizeRenderingUpdate(OptionSet<FinalizeRenderingUpdateFlags> flags)
 {
+    ASSERT(m_updateRenderingPhaseStack.last() == RenderingUpdatePhase::InUpdateRendering);
+
     auto* view = mainFrame().view();
     if (!view)
         return;
@@ -1597,8 +1615,12 @@
     if (flags.contains(FinalizeRenderingUpdateFlags::InvalidateImagesWithAsyncDecodes))
         view->invalidateImagesWithAsyncDecodes();
 
+    m_updateRenderingPhaseStack.last() = RenderingUpdatePhase::LayerFlushing;
+
     view->flushCompositingStateIncludingSubframes();
 
+    m_updateRenderingPhaseStack.last() = RenderingUpdatePhase::PostLayerFlush;
+
 #if ENABLE(ASYNC_SCROLLING)
     if (auto* scrollingCoordinator = this->scrollingCoordinator()) {
         scrollingCoordinator->commitTreeStateIfNeeded();
@@ -1608,6 +1630,8 @@
         scrollingCoordinator->didCompleteRenderingUpdate();
     }
 #endif
+
+    m_updateRenderingPhaseStack.removeLast();
 }
 
 void Page::suspendScriptedAnimations()

Modified: trunk/Source/WebCore/page/Page.h (267687 => 267688)


--- trunk/Source/WebCore/page/Page.h	2020-09-27 20:00:52 UTC (rev 267687)
+++ trunk/Source/WebCore/page/Page.h	2020-09-28 00:23:07 UTC (rev 267688)
@@ -488,7 +488,8 @@
 
     WEBCORE_EXPORT void layoutIfNeeded();
     WEBCORE_EXPORT void updateRendering();
-    
+    // A call to updateRendering() that is not followed by a call to finalizeRenderingUpdate().
+    WEBCORE_EXPORT void isolatedUpdateRendering();
     WEBCORE_EXPORT void finalizeRenderingUpdate(OptionSet<FinalizeRenderingUpdateFlags>);
 
     // Do immediate or timed update as dictated by the ChromeClient.
@@ -780,6 +781,13 @@
     MonotonicTime lastRenderingUpdateTimestamp() const { return m_lastRenderingUpdateTimestamp; }
 
 private:
+    enum class RenderingUpdatePhase : uint8_t {
+        Outside,
+        InUpdateRendering,
+        LayerFlushing,
+        PostLayerFlush
+    };
+
     struct Navigation {
         RegistrableDomain domain;
         FrameLoadType type;
@@ -1017,7 +1025,6 @@
     bool m_shouldEnableICECandidateFilteringByDefault { true };
     bool m_mediaPlaybackIsSuspended { false };
     bool m_mediaBufferingIsSuspended { false };
-    bool m_inUpdateRendering { false };
     bool m_hasResourceLoadClient { false };
     bool m_delegatesScaling { false };
 
@@ -1025,6 +1032,8 @@
     bool m_isEditableRegionEnabled { false };
 #endif
 
+    Vector<RenderingUpdatePhase, 2> m_updateRenderingPhaseStack;
+
     UserInterfaceLayoutDirection m_userInterfaceLayoutDirection { UserInterfaceLayoutDirection::LTR };
     
     // For testing.

Modified: trunk/Source/WebKit/ChangeLog (267687 => 267688)


--- trunk/Source/WebKit/ChangeLog	2020-09-27 20:00:52 UTC (rev 267687)
+++ trunk/Source/WebKit/ChangeLog	2020-09-28 00:23:07 UTC (rev 267688)
@@ -1,3 +1,16 @@
+2020-09-25  Simon Fraser  <[email protected]>
+
+        WebKitLegacy should call Page::finalizeRenderingUpdate()
+        https://bugs.webkit.org/show_bug.cgi?id=216958
+
+        Reviewed by Tim Horton.
+        
+        dynamicViewportSizeUpdate() needs to call isolatedUpdateRendering() because it isn't followed
+        by a finalizeRenderingUpdate().
+
+        * WebProcess/WebPage/ios/WebPageIOS.mm:
+        (WebKit::WebPage::dynamicViewportSizeUpdate):
+
 2020-09-27  Carlos Garcia Campos  <[email protected]>
 
         [SOUP] WebSocket: cookies set in request don't appear in the inspector

Modified: trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm (267687 => 267688)


--- trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm	2020-09-27 20:00:52 UTC (rev 267687)
+++ trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm	2020-09-28 00:23:07 UTC (rev 267688)
@@ -3441,7 +3441,7 @@
     setDeviceOrientation(deviceOrientation);
     frameView.setScrollOffset(roundedUnobscuredContentRectPosition);
 
-    m_page->updateRendering();
+    m_page->isolatedUpdateRendering();
 
 #if ENABLE(VIEWPORT_RESIZING)
     shrinkToFitContent();

Modified: trunk/Source/WebKitLegacy/mac/ChangeLog (267687 => 267688)


--- trunk/Source/WebKitLegacy/mac/ChangeLog	2020-09-27 20:00:52 UTC (rev 267687)
+++ trunk/Source/WebKitLegacy/mac/ChangeLog	2020-09-28 00:23:07 UTC (rev 267688)
@@ -1,3 +1,31 @@
+2020-09-24  Simon Fraser  <[email protected]>
+
+        WebKitLegacy should call Page::finalizeRenderingUpdate()
+        https://bugs.webkit.org/show_bug.cgi?id=216958
+
+        Reviewed by Tim Horton.
+
+        This is a precursor to fixing webkit.org/b/216726. Page needs to track the phase
+        of updateRendering that we are in, and to ease this tracking, WebKitLegacy needs to call
+        Page::finalizeRenderingUpdate() so the state is tracked.
+        
+        Rename -_viewWillDrawInternal to -_updateRendering, and have it call updateRendering()
+        and finalizeRenderingUpdate(). We can also move in the call to -_synchronizeCustomFixedPositionLayoutRect
+        which both call sites do.
+        
+        Since updateRendering() is guaranteed to update layout, we know -_flushCompositingChanges would have
+        never returned NO here, so we can remove the condition in LayerFlushController::flushLayers().
+        
+        -[WebHTMLView viewWillDraw] also does a similar -_web_updateLayoutAndStyleIfNeededRecursive
+        then -_flushCompositingChanges but this is called from AppKit with a timing that we don't control;
+        it may be redundant with -[WebView _updateRendering] but I leave that behavior unchanged.
+
+        * WebView/WebView.mm:
+        (-[WebView _updateRendering]):
+        (-[WebView _forceRepaintForTesting]):
+        (LayerFlushController::flushLayers):
+        (-[WebView _viewWillDrawInternal]): Deleted.
+
 2020-09-26  Sam Weinig  <[email protected]>
 
         [Preferences] Generate Debug and Internal preferences for WebKitLegacy

Modified: trunk/Source/WebKitLegacy/mac/WebView/WebView.mm (267687 => 267688)


--- trunk/Source/WebKitLegacy/mac/WebView/WebView.mm	2020-09-27 20:00:52 UTC (rev 267687)
+++ trunk/Source/WebKitLegacy/mac/WebView/WebView.mm	2020-09-28 00:23:07 UTC (rev 267688)
@@ -1789,10 +1789,17 @@
     return self;
 }
 
-- (void)_viewWillDrawInternal
+- (void)_updateRendering
 {
-    if (_private->page)
+#if PLATFORM(IOS_FAMILY)
+    // Ensure fixed positions layers are where they should be.
+    [self _synchronizeCustomFixedPositionLayoutRect];
+#endif
+
+    if (_private->page) {
         _private->page->updateRendering();
+        _private->page->finalizeRenderingUpdate({ });
+    }
 }
 
 + (NSArray *)_supportedMIMETypes
@@ -4813,13 +4820,7 @@
 
 - (void)_forceRepaintForTesting
 {
-#if PLATFORM(IOS_FAMILY)
-    // Ensure fixed positions layers are where they should be.
-    [self _synchronizeCustomFixedPositionLayoutRect];
-#endif
-
-    [self _viewWillDrawInternal];
-    [self _flushCompositingChanges];
+    [self _updateRendering];
     [CATransaction flush];
     [CATransaction synchronize];
 }
@@ -9257,28 +9258,19 @@
     NSWindow *window = [m_webView window];
 #endif // PLATFORM(MAC)
 
-#if PLATFORM(IOS_FAMILY)
-    // Ensure fixed positions layers are where they should be.
-    [m_webView _synchronizeCustomFixedPositionLayoutRect];
-#endif
+    [m_webView _updateRendering];
 
-    [m_webView _viewWillDrawInternal];
-
-    if ([m_webView _flushCompositingChanges]) {
 #if PLATFORM(MAC)
-        // AppKit may have disabled screen updates, thinking an upcoming window flush will re-enable them.
-        // In case setNeedsDisplayInRect() has prevented the window from needing to be flushed, re-enable screen
-        // updates here.
-        ALLOW_DEPRECATED_DECLARATIONS_BEGIN
-        if (![window isFlushWindowDisabled])
-            ALLOW_DEPRECATED_DECLARATIONS_END
-            [window _enableScreenUpdatesIfNeeded];
+    // AppKit may have disabled screen updates, thinking an upcoming window flush will re-enable them.
+    // In case setNeedsDisplayInRect() has prevented the window from needing to be flushed, re-enable screen
+    // updates here.
+    ALLOW_DEPRECATED_DECLARATIONS_BEGIN
+    if (![window isFlushWindowDisabled])
+        [window _enableScreenUpdatesIfNeeded];
+    ALLOW_DEPRECATED_DECLARATIONS_END
 #endif
 
-        return true;
-    }
-
-    return false;
+    return true;
 }
 
 - (void)_scheduleUpdateRendering

Modified: trunk/Source/WebKitLegacy/win/ChangeLog (267687 => 267688)


--- trunk/Source/WebKitLegacy/win/ChangeLog	2020-09-27 20:00:52 UTC (rev 267687)
+++ trunk/Source/WebKitLegacy/win/ChangeLog	2020-09-28 00:23:07 UTC (rev 267688)
@@ -1,3 +1,19 @@
+2020-09-25  Simon Fraser  <[email protected]>
+
+        WebKitLegacy should call Page::finalizeRenderingUpdate()
+        https://bugs.webkit.org/show_bug.cgi?id=216958
+
+        Reviewed by Tim Horton.
+
+        Windows doesn't call finalizeRenderingUpdate() so needs to use isolatedUpdateRendering().
+
+        * WebCoreSupport/AcceleratedCompositingContext.cpp:
+        (AcceleratedCompositingContext::flushAndRenderLayers):
+        * WebView.cpp:
+        (WebView::paint):
+        (WebView::flushPendingGraphicsLayerChangesSoon):
+        (WebView::flushPendingGraphicsLayerChanges):
+
 2020-09-25  Antoine Quint  <[email protected]>
 
         Add an experimental feature flag for CSS individual transform properties

Modified: trunk/Source/WebKitLegacy/win/WebCoreSupport/AcceleratedCompositingContext.cpp (267687 => 267688)


--- trunk/Source/WebKitLegacy/win/WebCoreSupport/AcceleratedCompositingContext.cpp	2020-09-27 20:00:52 UTC (rev 267687)
+++ trunk/Source/WebKitLegacy/win/WebCoreSupport/AcceleratedCompositingContext.cpp	2020-09-28 00:23:07 UTC (rev 267688)
@@ -297,7 +297,7 @@
     if (!enabled())
         return;
 
-    core(&m_webView)->updateRendering();
+    core(&m_webView)->isolatedUpdateRendering();
 
     if (!enabled())
         return;

Modified: trunk/Source/WebKitLegacy/win/WebView.cpp (267687 => 267688)


--- trunk/Source/WebKitLegacy/win/WebView.cpp	2020-09-27 20:00:52 UTC (rev 267687)
+++ trunk/Source/WebKitLegacy/win/WebView.cpp	2020-09-28 00:23:07 UTC (rev 267688)
@@ -1285,7 +1285,7 @@
 {
     LOCAL_GDI_COUNTER(0, __FUNCTION__);
 
-    m_page->updateRendering();
+    m_page->isolatedUpdateRendering();
 
     if (paintCompositedContentToHDC(dc)) {
         ::ValidateRect(m_viewWindow, nullptr);
@@ -7211,13 +7211,13 @@
 {
 #if USE(CA)
     if (!m_layerTreeHost) {
-        m_page->updateRendering();
+        m_page->isolatedUpdateRendering();
         return;
     }
     m_layerTreeHost->flushPendingGraphicsLayerChangesSoon();
 #elif USE(TEXTURE_MAPPER_GL)
     if (!isAcceleratedCompositing()) {
-        m_page->updateRendering();
+        m_page->isolatedUpdateRendering();
         return;
     }
     if (!m_acceleratedCompositingContext)
@@ -7444,7 +7444,7 @@
     if (!isAcceleratedCompositing())
         return;
 
-    m_page->updateRendering();
+    m_page->isolatedUpdateRendering();
 
     // Updating layout might have taken us out of compositing mode.
     if (m_backingLayer)

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to