⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 267688 in webkit


Ignore:
Timestamp:
Sep 27, 2020, 5:23:07 PM (6 years ago)
Author:
Simon Fraser
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):

Location:
trunk/Source
Files:
10 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r267666 r267688  
     12020-09-24  Simon Fraser  <simon.fraser@apple.com>
     2
     3        WebKitLegacy should call Page::finalizeRenderingUpdate()
     4        https://bugs.webkit.org/show_bug.cgi?id=216958
     5
     6        Reviewed by Tim Horton.
     7
     8        Convert Page::m_inUpdateRendering to an enum which tracks the phase, which will be
     9        used in a later patch to prevent extra update scheduling (webkit.org/b/216726).
     10        Page has to track RenderingUpdatePhase as a stack to handle the re-entrancy when layout tests
     11        call notifyDone() inside of the JS callbacks; the stack only has one entry outside of testing.
     12
     13        Add isolatedUpdateRendering(), which is for callers who aren't going to call finalizeRenderingUpdate(),
     14        and use it for SVGImage updates.
     15
     16        * page/Page.cpp:
     17        (WebCore::Page::updateRendering):
     18        (WebCore::Page::isolatedUpdateRendering):
     19        (WebCore::Page::doAfterUpdateRendering):
     20        (WebCore::Page::finalizeRenderingUpdate):
     21        * page/Page.h:
     22
    1232020-09-27  Zalan Bujtas  <zalan@apple.com>
    224

  • trunk/Source/WebCore/page/Page.cpp

    r267611 r267688  
    14421442void Page::updateRendering()
    14431443{
    1444     // This function is not reentrant, e.g. a rAF callback may force repaint.
    1445     if (m_inUpdateRendering) {
     1444    m_updateRenderingPhaseStack.append(RenderingUpdatePhase::InUpdateRendering);
     1445
     1446    // This function is not reentrant, e.g. a rAF callback may trigger a forces repaint in testing.
     1447    // This is why we track updateRenderingPhase as a stack.
     1448    if (m_updateRenderingPhaseStack.size() > 1) {
    14461449        layoutIfNeeded();
    14471450        return;
    14481451    }
    14491452
    1450     SetForScope<bool> inUpdateRendering(m_inUpdateRendering, true);
    14511453    m_lastRenderingUpdateTimestamp = MonotonicTime::now();
    14521454
    … …  
    15101512        for (auto& image : document.cachedResourceLoader().allCachedSVGImages()) {
    15111513            if (auto* page = image->internalPage())
    1512                 page->updateRendering();
     1514                page->isolatedUpdateRendering();
    15131515        }
    15141516    });
     1517
     1518    ASSERT(m_updateRenderingPhaseStack.last() == RenderingUpdatePhase::InUpdateRendering);
    15151519
    15161520    for (auto& document : initialDocuments) {
    … …  
    15301534    if (!isSVGImagePage)
    15311535        tracePoint(RenderingUpdateEnd);
     1536
     1537    ASSERT(m_updateRenderingPhaseStack.last() == RenderingUpdatePhase::InUpdateRendering);
     1538}
     1539
     1540void Page::isolatedUpdateRendering()
     1541{
     1542    updateRendering();
     1543    m_updateRenderingPhaseStack.removeLast();
    15321544}
    15331545
    15341546void Page::doAfterUpdateRendering()
    15351547{
     1548    ASSERT(m_updateRenderingPhaseStack.last() == RenderingUpdatePhase::InUpdateRendering);
     1549
    15361550    // Code here should do once-per-frame work that needs to be done before painting, and requires
    15371551    // layout to be up-to-date. It should not run script, trigger layout, or dirty layout.
    … …  
    15871601    }
    15881602#endif
     1603
     1604    ASSERT(m_updateRenderingPhaseStack.last() == RenderingUpdatePhase::InUpdateRendering);
    15891605}
    15901606
    15911607void Page::finalizeRenderingUpdate(OptionSet<FinalizeRenderingUpdateFlags> flags)
    15921608{
     1609    ASSERT(m_updateRenderingPhaseStack.last() == RenderingUpdatePhase::InUpdateRendering);
     1610
    15931611    auto* view = mainFrame().view();
    15941612    if (!view)
    … …  
    15981616        view->invalidateImagesWithAsyncDecodes();
    15991617
     1618    m_updateRenderingPhaseStack.last() = RenderingUpdatePhase::LayerFlushing;
     1619
    16001620    view->flushCompositingStateIncludingSubframes();
     1621
     1622    m_updateRenderingPhaseStack.last() = RenderingUpdatePhase::PostLayerFlush;
    16011623
    16021624#if ENABLE(ASYNC_SCROLLING)
    … …  
    16091631    }
    16101632#endif
     1633
     1634    m_updateRenderingPhaseStack.removeLast();
    16111635}
    16121636

  • trunk/Source/WebCore/page/Page.h

    r267611 r267688  
    489489    WEBCORE_EXPORT void layoutIfNeeded();
    490490    WEBCORE_EXPORT void updateRendering();
    491    
     491    // A call to updateRendering() that is not followed by a call to finalizeRenderingUpdate().
     492    WEBCORE_EXPORT void isolatedUpdateRendering();
    492493    WEBCORE_EXPORT void finalizeRenderingUpdate(OptionSet<FinalizeRenderingUpdateFlags>);
    493494
    … …  
    781782
    782783private:
     784    enum class RenderingUpdatePhase : uint8_t {
     785        Outside,
     786        InUpdateRendering,
     787        LayerFlushing,
     788        PostLayerFlush
     789    };
     790
    783791    struct Navigation {
    784792        RegistrableDomain domain;
    … …  
    10181026    bool m_mediaPlaybackIsSuspended { false };
    10191027    bool m_mediaBufferingIsSuspended { false };
    1020     bool m_inUpdateRendering { false };
    10211028    bool m_hasResourceLoadClient { false };
    10221029    bool m_delegatesScaling { false };
    … …  
    10251032    bool m_isEditableRegionEnabled { false };
    10261033#endif
     1034
     1035    Vector<RenderingUpdatePhase, 2> m_updateRenderingPhaseStack;
    10271036
    10281037    UserInterfaceLayoutDirection m_userInterfaceLayoutDirection { UserInterfaceLayoutDirection::LTR };

  • trunk/Source/WebKit/ChangeLog

    r267655 r267688  
     12020-09-25  Simon Fraser  <simon.fraser@apple.com>
     2
     3        WebKitLegacy should call Page::finalizeRenderingUpdate()
     4        https://bugs.webkit.org/show_bug.cgi?id=216958
     5
     6        Reviewed by Tim Horton.
     7       
     8        dynamicViewportSizeUpdate() needs to call isolatedUpdateRendering() because it isn't followed
     9        by a finalizeRenderingUpdate().
     10
     11        * WebProcess/WebPage/ios/WebPageIOS.mm:
     12        (WebKit::WebPage::dynamicViewportSizeUpdate):
     13
    1142020-09-27  Carlos Garcia Campos  <cgarcia@igalia.com>
    215

  • trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm

    r267363 r267688  
    34423442    frameView.setScrollOffset(roundedUnobscuredContentRectPosition);
    34433443
    3444     m_page->updateRendering();
     3444    m_page->isolatedUpdateRendering();
    34453445
    34463446#if ENABLE(VIEWPORT_RESIZING)

  • trunk/Source/WebKitLegacy/mac/ChangeLog

    r267641 r267688  
     12020-09-24  Simon Fraser  <simon.fraser@apple.com>
     2
     3        WebKitLegacy should call Page::finalizeRenderingUpdate()
     4        https://bugs.webkit.org/show_bug.cgi?id=216958
     5
     6        Reviewed by Tim Horton.
     7
     8        This is a precursor to fixing webkit.org/b/216726. Page needs to track the phase
     9        of updateRendering that we are in, and to ease this tracking, WebKitLegacy needs to call
     10        Page::finalizeRenderingUpdate() so the state is tracked.
     11       
     12        Rename -_viewWillDrawInternal to -_updateRendering, and have it call updateRendering()
     13        and finalizeRenderingUpdate(). We can also move in the call to -_synchronizeCustomFixedPositionLayoutRect
     14        which both call sites do.
     15       
     16        Since updateRendering() is guaranteed to update layout, we know -_flushCompositingChanges would have
     17        never returned NO here, so we can remove the condition in LayerFlushController::flushLayers().
     18       
     19        -[WebHTMLView viewWillDraw] also does a similar -_web_updateLayoutAndStyleIfNeededRecursive
     20        then -_flushCompositingChanges but this is called from AppKit with a timing that we don't control;
     21        it may be redundant with -[WebView _updateRendering] but I leave that behavior unchanged.
     22
     23        * WebView/WebView.mm:
     24        (-[WebView _updateRendering]):
     25        (-[WebView _forceRepaintForTesting]):
     26        (LayerFlushController::flushLayers):
     27        (-[WebView _viewWillDrawInternal]): Deleted.
     28
    1292020-09-26  Sam Weinig  <weinig@apple.com>
    230

  • trunk/Source/WebKitLegacy/mac/WebView/WebView.mm

    r267641 r267688  
    17901790}
    17911791
    1792 - (void)_viewWillDrawInternal
    1793 {
    1794     if (_private->page)
     1792- (void)_updateRendering
     1793{
     1794#if PLATFORM(IOS_FAMILY)
     1795    // Ensure fixed positions layers are where they should be.
     1796    [self _synchronizeCustomFixedPositionLayoutRect];
     1797#endif
     1798
     1799    if (_private->page) {
    17951800        _private->page->updateRendering();
     1801        _private->page->finalizeRenderingUpdate({ });
     1802    }
    17961803}
    17971804
    … …  
    48144821- (void)_forceRepaintForTesting
    48154822{
    4816 #if PLATFORM(IOS_FAMILY)
    4817     // Ensure fixed positions layers are where they should be.
    4818     [self _synchronizeCustomFixedPositionLayoutRect];
    4819 #endif
    4820 
    4821     [self _viewWillDrawInternal];
    4822     [self _flushCompositingChanges];
     4823    [self _updateRendering];
    48234824    [CATransaction flush];
    48244825    [CATransaction synchronize];
    … …  
    92589259#endif // PLATFORM(MAC)
    92599260
    9260 #if PLATFORM(IOS_FAMILY)
    9261     // Ensure fixed positions layers are where they should be.
    9262     [m_webView _synchronizeCustomFixedPositionLayoutRect];
    9263 #endif
    9264 
    9265     [m_webView _viewWillDrawInternal];
    9266 
    9267     if ([m_webView _flushCompositingChanges]) {
     9261    [m_webView _updateRendering];
     9262
    92689263#if PLATFORM(MAC)
    9269         // AppKit may have disabled screen updates, thinking an upcoming window flush will re-enable them.
    9270         // In case setNeedsDisplayInRect() has prevented the window from needing to be flushed, re-enable screen
    9271         // updates here.
    9272         ALLOW_DEPRECATED_DECLARATIONS_BEGIN
    9273         if (![window isFlushWindowDisabled])
    9274             ALLOW_DEPRECATED_DECLARATIONS_END
    9275             [window _enableScreenUpdatesIfNeeded];
    9276 #endif
    9277 
    9278         return true;
    9279     }
    9280 
    9281     return false;
     9264    // AppKit may have disabled screen updates, thinking an upcoming window flush will re-enable them.
     9265    // In case setNeedsDisplayInRect() has prevented the window from needing to be flushed, re-enable screen
     9266    // updates here.
     9267    ALLOW_DEPRECATED_DECLARATIONS_BEGIN
     9268    if (![window isFlushWindowDisabled])
     9269        [window _enableScreenUpdatesIfNeeded];
     9270    ALLOW_DEPRECATED_DECLARATIONS_END
     9271#endif
     9272
     9273    return true;
    92829274}
    92839275

  • trunk/Source/WebKitLegacy/win/ChangeLog

    r267592 r267688  
     12020-09-25  Simon Fraser  <simon.fraser@apple.com>
     2
     3        WebKitLegacy should call Page::finalizeRenderingUpdate()
     4        https://bugs.webkit.org/show_bug.cgi?id=216958
     5
     6        Reviewed by Tim Horton.
     7
     8        Windows doesn't call finalizeRenderingUpdate() so needs to use isolatedUpdateRendering().
     9
     10        * WebCoreSupport/AcceleratedCompositingContext.cpp:
     11        (AcceleratedCompositingContext::flushAndRenderLayers):
     12        * WebView.cpp:
     13        (WebView::paint):
     14        (WebView::flushPendingGraphicsLayerChangesSoon):
     15        (WebView::flushPendingGraphicsLayerChanges):
     16
    1172020-09-25  Antoine Quint  <graouts@webkit.org>
    218

  • trunk/Source/WebKitLegacy/win/WebCoreSupport/AcceleratedCompositingContext.cpp

    r261577 r267688  
    298298        return;
    299299
    300     core(&m_webView)->updateRendering();
     300    core(&m_webView)->isolatedUpdateRendering();
    301301
    302302    if (!enabled())

  • trunk/Source/WebKitLegacy/win/WebView.cpp

    r267592 r267688  
    12861286    LOCAL_GDI_COUNTER(0, __FUNCTION__);
    12871287
    1288     m_page->updateRendering();
     1288    m_page->isolatedUpdateRendering();
    12891289
    12901290    if (paintCompositedContentToHDC(dc)) {
    … …  
    72127212#if USE(CA)
    72137213    if (!m_layerTreeHost) {
    7214         m_page->updateRendering();
     7214        m_page->isolatedUpdateRendering();
    72157215        return;
    72167216    }
    … …  
    72187218#elif USE(TEXTURE_MAPPER_GL)
    72197219    if (!isAcceleratedCompositing()) {
    7220         m_page->updateRendering();
     7220        m_page->isolatedUpdateRendering();
    72217221        return;
    72227222    }
    … …  
    74457445        return;
    74467446
    7447     m_page->updateRendering();
     7447    m_page->isolatedUpdateRendering();
    74487448
    74497449    // Updating layout might have taken us out of compositing mode.
Note: See TracChangeset for help on using the changeset viewer.