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

Changeset 280987 in webkit


Ignore:
Timestamp:
Aug 12, 2021, 3:24:55 PM (5 years ago)
Author:
Alan Coon
Message:

Cherry-pick r280981. rdar://problem/81870941

[GPU Process] REGRESSION: WebContent often crashes when using iCloud photos
​https://bugs.webkit.org/show_bug.cgi?id=228969
<rdar://81761078>

Reviewed by Simon Fraser.

Terminating the GPUProcess is very stressful situation which has to be
handled carefully. The side effect of each function which is called through
gpuProcessConnectionDidClose() has to be understood to get the right
sequence of calls. There are problems in releasing all kinds of resources.

  • Releasing NativeImage: Calling clearNativeImageMap() after clearing the backend of the ImageBuffers was causing a problem. When clearing the backend of an ImageBuffer, it will clear its DisplayList which may have the last reference to a NativeImage. The destructor of NativeImage calls releaseRemoteResource() before it is removed from the the NativeImageMap. This will send a message to the relaunched GPUP to release a NativeImage which is not in its cache.
  • Releasing Font: clearFontMap() was always calling releaseRemoteResource() even if it is called form remoteResourceCacheWasDestroyed(). This should not happen because the connection with GPUProcess has been closed.
  • Releasing ImageBuffer: This happen when a DisplayList of an ImageBuffer 'A' holds the last reference to another ImageBuffer 'B' and we call clearBackend() for 'A'. clearBackend() will clear the DisplayList of 'A' and causes the deletion of 'B'. In this case we should not call releaseImageBuffer() for 'B' because the GPUPProcess is closed.
  • WebProcess/GPU/graphics/RemoteImageBufferProxy.h: (WebKit::RemoteImageBufferProxy::~RemoteImageBufferProxy): If the ImageBuffer is being released because of the clean-up we do when the GPUProcess is terminated, we should not release the corresponding RemoteImageBuffer since it is already gone.
  • WebProcess/GPU/graphics/RemoteRenderingBackendProxy.h: (WebKit::RemoteRenderingBackendProxy::isGPUProcessConnectionClosed const): This will return true if we are deleting a RemoteImageBufferProxy through RemoteResourceCacheProxy::remoteResourceCacheWasDestroyed().
  • WebProcess/GPU/graphics/RemoteResourceCacheProxy.cpp: (WebKit::RemoteResourceCacheProxy::releaseAllRemoteFonts): This function will be used to release the remote fonts. It should be called from RemoteResourceCacheProxy::releaseMemory() where we sure the GPUP is alive and all the fonts are cached there.

(WebKit::RemoteResourceCacheProxy::clearFontMap):
The part of releasing the remote fonts was moved from this function to
releaseAllRemoteFonts().

(WebKit::RemoteResourceCacheProxy::remoteResourceCacheWasDestroyed):

  1. Clearing the NativeImages and the Fonts has to come before clearing the backends of the ImageBuffers. The reason is clearBackend() clears the DisplayList which may release the last reference of a NativeImage or Font. We want to detach the NativeImages and the Fonts from the cache before then.
  2. We should have two different loops: one for clearing the backends of the ImageBuffers and another one for recreating these backends. The reason for this is clearBackend() clears the DisplayList which may release the last reference of a another source RemoteImageBufferProxy used by a DrawImageBuffer item for example.

(WebKit::RemoteResourceCacheProxy::releaseMemory):

  • WebProcess/GPU/graphics/RemoteResourceCacheProxy.h:

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

Location:
branches/safari-612.1.27.0-branch/Source/WebKit
Files:
5 edited

Legend:

Unmodified
Added
Removed

  • branches/safari-612.1.27.0-branch/Source/WebKit/ChangeLog

    r280975 r280987  
     12021-08-12  Alan Coon  <alancoon@apple.com>
     2
     3        Cherry-pick r280981. rdar://problem/81870941
     4
     5    [GPU Process] REGRESSION: WebContent often crashes when using iCloud photos
     6    https://bugs.webkit.org/show_bug.cgi?id=228969
     7    <rdar://81761078>
     8   
     9    Reviewed by Simon Fraser.
     10   
     11    Terminating the GPUProcess is very stressful situation which has to be
     12    handled carefully. The side effect of each function which is called through
     13    gpuProcessConnectionDidClose() has to be understood to get the right
     14    sequence of calls. There are problems in releasing all kinds of resources.
     15   
     16    - Releasing NativeImage: Calling clearNativeImageMap() after clearing the
     17    backend of the ImageBuffers was causing a problem. When clearing the
     18    backend of an ImageBuffer, it will clear its DisplayList which may have
     19    the last reference to a NativeImage. The destructor of NativeImage calls
     20    releaseRemoteResource() before it is removed from the the NativeImageMap.
     21    This will send a message to the relaunched GPUP to release a NativeImage
     22    which is not in its cache.
     23   
     24    - Releasing Font: clearFontMap() was always calling releaseRemoteResource()
     25    even if it is called form  remoteResourceCacheWasDestroyed(). This should
     26    not happen because the connection with GPUProcess has been closed.
     27   
     28    - Releasing ImageBuffer: This happen when a DisplayList of an ImageBuffer
     29    'A' holds the last reference to another ImageBuffer 'B' and we call
     30    clearBackend() for 'A'. clearBackend() will clear the DisplayList of 'A'
     31    and causes the deletion of 'B'. In this case we should not call
     32    releaseImageBuffer() for 'B' because the GPUPProcess is closed.
     33   
     34    * WebProcess/GPU/graphics/RemoteImageBufferProxy.h:
     35    (WebKit::RemoteImageBufferProxy::~RemoteImageBufferProxy):
     36    If the ImageBuffer is being released because of the clean-up we do when
     37    the GPUProcess is terminated, we should not release the corresponding
     38    RemoteImageBuffer since it is already gone.
     39   
     40    * WebProcess/GPU/graphics/RemoteRenderingBackendProxy.h:
     41    (WebKit::RemoteRenderingBackendProxy::isGPUProcessConnectionClosed const):
     42    This will return true if we are deleting a RemoteImageBufferProxy through
     43    RemoteResourceCacheProxy::remoteResourceCacheWasDestroyed().
     44   
     45    * WebProcess/GPU/graphics/RemoteResourceCacheProxy.cpp:
     46    (WebKit::RemoteResourceCacheProxy::releaseAllRemoteFonts):
     47    This function will be used to release the remote fonts. It should be called
     48    from RemoteResourceCacheProxy::releaseMemory() where we sure the GPUP is
     49    alive and all the fonts are cached there.
     50   
     51    (WebKit::RemoteResourceCacheProxy::clearFontMap):
     52    The part of releasing the remote fonts was moved from this function to
     53    releaseAllRemoteFonts().
     54   
     55    (WebKit::RemoteResourceCacheProxy::remoteResourceCacheWasDestroyed):
     56    1. Clearing the NativeImages and the Fonts has to come before clearing
     57    the backends of the ImageBuffers. The reason is clearBackend() clears the
     58    DisplayList which may release the last reference of a NativeImage or Font.
     59    We want to detach the NativeImages and the Fonts from the cache before then.
     60    2. We should have two different loops: one for clearing the backends of
     61    the ImageBuffers and another one for recreating these backends. The reason
     62    for this is clearBackend() clears the DisplayList which may release the
     63    last reference of a another source RemoteImageBufferProxy used by a
     64    DrawImageBuffer item for example.
     65   
     66    (WebKit::RemoteResourceCacheProxy::releaseMemory):
     67    * WebProcess/GPU/graphics/RemoteResourceCacheProxy.h:
     68   
     69   
     70    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@280981 268f45cc-cd09-0410-ab3c-d52691b4dbfc
     71
     72    2021-08-12  Said Abou-Hallawa  <said@apple.com>
     73
     74            [GPU Process] REGRESSION: WebContent often crashes when using iCloud photos
     75            https://bugs.webkit.org/show_bug.cgi?id=228969
     76            <rdar://81761078>
     77
     78            Reviewed by Simon Fraser.
     79
     80            Terminating the GPUProcess is very stressful situation which has to be
     81            handled carefully. The side effect of each function which is called through
     82            gpuProcessConnectionDidClose() has to be understood to get the right
     83            sequence of calls. There are problems in releasing all kinds of resources.
     84
     85            - Releasing NativeImage: Calling clearNativeImageMap() after clearing the
     86            backend of the ImageBuffers was causing a problem. When clearing the
     87            backend of an ImageBuffer, it will clear its DisplayList which may have
     88            the last reference to a NativeImage. The destructor of NativeImage calls
     89            releaseRemoteResource() before it is removed from the the NativeImageMap.
     90            This will send a message to the relaunched GPUP to release a NativeImage
     91            which is not in its cache.
     92
     93            - Releasing Font: clearFontMap() was always calling releaseRemoteResource()
     94            even if it is called form  remoteResourceCacheWasDestroyed(). This should
     95            not happen because the connection with GPUProcess has been closed.
     96
     97            - Releasing ImageBuffer: This happen when a DisplayList of an ImageBuffer
     98            'A' holds the last reference to another ImageBuffer 'B' and we call
     99            clearBackend() for 'A'. clearBackend() will clear the DisplayList of 'A'
     100            and causes the deletion of 'B'. In this case we should not call
     101            releaseImageBuffer() for 'B' because the GPUPProcess is closed.
     102
     103            * WebProcess/GPU/graphics/RemoteImageBufferProxy.h:
     104            (WebKit::RemoteImageBufferProxy::~RemoteImageBufferProxy):
     105            If the ImageBuffer is being released because of the clean-up we do when
     106            the GPUProcess is terminated, we should not release the corresponding
     107            RemoteImageBuffer since it is already gone.
     108
     109            * WebProcess/GPU/graphics/RemoteRenderingBackendProxy.h:
     110            (WebKit::RemoteRenderingBackendProxy::isGPUProcessConnectionClosed const):
     111            This will return true if we are deleting a RemoteImageBufferProxy through
     112            RemoteResourceCacheProxy::remoteResourceCacheWasDestroyed().
     113
     114            * WebProcess/GPU/graphics/RemoteResourceCacheProxy.cpp:
     115            (WebKit::RemoteResourceCacheProxy::releaseAllRemoteFonts):
     116            This function will be used to release the remote fonts. It should be called
     117            from RemoteResourceCacheProxy::releaseMemory() where we sure the GPUP is
     118            alive and all the fonts are cached there.
     119
     120            (WebKit::RemoteResourceCacheProxy::clearFontMap):
     121            The part of releasing the remote fonts was moved from this function to
     122            releaseAllRemoteFonts().
     123
     124            (WebKit::RemoteResourceCacheProxy::remoteResourceCacheWasDestroyed):
     125            1. Clearing the NativeImages and the Fonts has to come before clearing
     126            the backends of the ImageBuffers. The reason is clearBackend() clears the
     127            DisplayList which may release the last reference of a NativeImage or Font.
     128            We want to detach the NativeImages and the Fonts from the cache before then.
     129            2. We should have two different loops: one for clearing the backends of
     130            the ImageBuffers and another one for recreating these backends. The reason
     131            for this is clearBackend() clears the DisplayList which may release the
     132            last reference of a another source RemoteImageBufferProxy used by a
     133            DrawImageBuffer item for example.
     134
     135            (WebKit::RemoteResourceCacheProxy::releaseMemory):
     136            * WebProcess/GPU/graphics/RemoteResourceCacheProxy.h:
     137
    11382021-08-12  Alan Coon  <alancoon@apple.com>
    2139

  • branches/safari-612.1.27.0-branch/Source/WebKit/WebProcess/GPU/graphics/RemoteImageBufferProxy.h

    r280696 r280987  
    6666    ~RemoteImageBufferProxy()
    6767    {
    68         if (!m_remoteRenderingBackendProxy) {
     68        if (!m_remoteRenderingBackendProxy || m_remoteRenderingBackendProxy->isGPUProcessConnectionClosed()) {
    6969            clearDisplayList();
    7070            return;

  • branches/safari-612.1.27.0-branch/Source/WebKit/WebProcess/GPU/graphics/RemoteRenderingBackendProxy.h

    r280337 r280987  
    123123    RenderingBackendIdentifier ensureBackendCreated();
    124124
     125    bool isGPUProcessConnectionClosed() const { return !m_gpuProcessConnection; }
     126
    125127private:
    126128    explicit RemoteRenderingBackendProxy(WebPage&);

  • branches/safari-612.1.27.0-branch/Source/WebKit/WebProcess/GPU/graphics/RemoteResourceCacheProxy.cpp

    r280694 r280987  
    165165}
    166166
    167 void RemoteResourceCacheProxy::clearFontMap()
     167void RemoteResourceCacheProxy::releaseAllRemoteFonts()
    168168{
    169169    for (auto& fontState : m_fonts)
    170170        m_remoteRenderingBackendProxy.releaseRemoteResource(fontState.key, fontState.value.useCount);
     171}
     172
     173void RemoteResourceCacheProxy::clearFontMap()
     174{
    171175    m_fonts.clear();
    172176    m_numberOfFontsUsedInCurrentRenderingUpdate = 0;
    … …  
    204208void RemoteResourceCacheProxy::remoteResourceCacheWasDestroyed()
    205209{
     210    clearNativeImageMap();
     211    clearFontMap();
     212
     213    // Get a copy of m_imageBuffers.values() because clearBackend()
     214    // may release some of the cached ImageBuffers.
     215    for (auto& item : copyToVector(m_imageBuffers.values())) {
     216        if (!item.imageBuffer)
     217            continue;
     218        item.useCount = 0;
     219        item.imageBuffer->clearBackend();
     220    }
     221
    206222    for (auto& item : m_imageBuffers.values()) {
    207223        if (!item.imageBuffer)
    208224            continue;
    209225        m_remoteRenderingBackendProxy.createRemoteImageBuffer(*item.imageBuffer);
    210         item.useCount = 0;
    211         item.imageBuffer->clearBackend();
    212     }
    213     clearNativeImageMap();
    214     clearFontMap();
     226    }
    215227}
    216228
    217229void RemoteResourceCacheProxy::releaseMemory()
    218230{
     231    releaseAllRemoteFonts();
    219232    clearFontMap();
    220233    m_remoteRenderingBackendProxy.deleteAllFonts();

  • branches/safari-612.1.27.0-branch/Source/WebKit/WebProcess/GPU/graphics/RemoteResourceCacheProxy.h

    r280694 r280987  
    5858
    5959    void remoteResourceCacheWasDestroyed();
     60    void releaseAllRemoteFonts();
    6061    void releaseMemory();
    6162
Note: See TracChangeset for help on using the changeset viewer.