[Webkit-unassigned] [Bug 227277] New: REGRESSION(r279104): [GPU Process] Canvas layout tests became flaky

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=227277

            Bug ID: 227277
           Summary: REGRESSION(r279104): [GPU Process] Canvas layout tests
                    became flaky
           Product: WebKit
           Version: WebKit Nightly Build
          Hardware: Unspecified
                OS: Unspecified
            Status: NEW
          Severity: Normal
          Priority: P2
         Component: Canvas
          Assignee: webkit-unassigned at lists.webkit.org
          Reporter: sabouhallawa at apple.com
                CC: dino at apple.com

The change r279104 tried to fix some issues in RemoteResourceCacheProxy::didFinalizeRenderingUpdate().

It fixed the early return 

from:
    if (unusedFontCount < minimumFractionOfUnusedFontCountToTriggerRemoval * totalFontCount && unusedFontCount <= maximumUnusedFontCountToSkipRemoval)
        return;

    // which is equivalent to the following code because unusedFontCount and maximumUnusedFontCountToSkipRemoval are both unsigned and maximumUnusedFontCountToSkipRemoval = 0.
    // if (!unusedFontCount)
    //    return;

to:
    if (unusedFontCount <= minimumFractionOfUnusedFontCountToTriggerRemoval * totalFontCount) {
        prepareForNextRenderingUpdate();
        return;
    }

And it fixed the synchronization of the cached fonts between WebProcess and GPUP:

from:

    // This code removes the font object from GPUP but keeps the font identifier in WebProcess
    for (auto& item : m_fontIdentifierToLastRenderingUpdateVersionMap) {
        if (m_currentRenderingUpdateVersion - item.value >= minimumRenderingUpdateCountToKeepFontAlive)
            m_remoteRenderingBackendProxy.releaseRemoteResource(item.key);
    }

to:

    m_fontIdentifierToLastRenderingUpdateVersionMap.removeIf([&] (const auto& item) {
        if (m_currentRenderingUpdateVersion - item.value < minimumRenderingUpdateCountToKeepFontAlive)
            return false;
        m_remoteRenderingBackendProxy.releaseRemoteResource(item.key);
        return true;
    });


But when I looked closely at the original RemoteResourceCacheProxy::didFinalizeRenderingUpdate() before r279104, I found it was always making an early return and basically it was doing nothing. Here is the explanation.

1. When the fonts are cached for the first RenderingUpdate to a canvas, all the entries in m_fontIdentifierToLastRenderingUpdateVersionMap will have value = 1 since m_currentRenderingUpdateVersion is initialized with 1. 
2. m_numberOfFontsUsedInCurrentRenderingUpdate will be equal to the number of fonts cached during the first RenderingUpdate.
3. RemoteResourceCacheProxy::didFinalizeRenderingUpdate(), will find unusedFontCount == 0 because m_numberOfFontsUsedInCurrentRenderingUpdate == m_fontIdentifierToLastRenderingUpdateVersionMap.size(). So it will return without changing m_numberOfFontsUsedInCurrentRenderingUpdate or m_currentRenderingUpdateVersion.
4. If in the next RenderingUpdate a new font is cached, it will inherit m_currentRenderingUpdateVersion which is still 1. And m_numberOfFontsUsedInCurrentRenderingUpdate will be incremented by 1.
5. Caching more fonts will increment m_numberOfFontsUsedInCurrentRenderingUpdate as if all the cached fonts were used for m_currentRenderingUpdateVersion = 1.
6. RemoteResourceCacheProxy::didFinalizeRenderingUpdate() will make an early return again because unusedFontCount is always zero.

So the change r279104 exercises a new code path which was never exercised before. Removing the font from the cache in GPUP and recaching it causes RemoteRenderingBackend to wakeUpAndApplyDisplayList() more often if the Font was missing in the GPUP cache.

I think the solution is to remove RemoteResourceCacheProxy::didFinalizeRenderingUpdate() since it has been doing nothing since it was written.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20210623/630c4cda/attachment.htm>