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

Changeset 276007 in webkit


Ignore:
Timestamp:
Apr 15, 2021, 12:37:06 AM (5 years ago)
Author:
Chris Dumez
Message:

REGRESSION(r275921-r275922): [ wk2 ] http/tests/security/webgl-remote-read-remote-image-allowed-with-credentials.html is flakey crashing
​https://bugs.webkit.org/show_bug.cgi?id=224566
<rdar://problem/76657007>

Reviewed by Tim Horton.

Source/WebKit:

r275921-r275922 changed the timing of when the WebProcess/RemoteRenderingBackendProxy sends the GPUConnectionToWebProcess::CreateRenderingBackend
IPC to the GPUProcess. This IPC used to get sent as soon as the RemoteRenderingBackendProxy was constructed. However, I delayed sending it until
the RemoteRenderingBackendProxy actually needs to create an ImageBuffer to avoid launching the GPUProcess prematurely.

Unfortunately, this exposed a pre-existing synchronization issue with regards to the RemoteRenderingBackend IPC. In particular, the
GPUConnectionToWebProcess::CreateRenderingBackend was getting sent asynchronously and processed by the GPUProcess on the main thread. This would
cause the creation of the RemoteRenderingBackend object on the GPUProcess side, which would then register itself as a WorkQueueMessageReceiver.
The issue is that the WebProcess was sending the RemoteRenderingBackend IPC so quickly after the CreateRenderingBackend IPC that when the IPC is
received (on the IPC thread), the RemoteRenderingBackend has not had a change to register itself as a WorkQueueMessageReceiver yet. As a result,
some of the early IPC was getting dispatched to the main thread and later IPC would get sent to the RemoteRenderingBackend WorkQueue. This was
causing RemoteRenderingBackend to get processed out of order.

To address the synchronization issue for now, I made the GPUConnectionToWebProcess::CreateRenderingBackend synchronous. This may not be the
best design long term but it is an easy and safe way to resolve this for now. It makes sure the WebProcess cannot send RemoteRenderingBackend
IPC until after the GPUProcess has created the RemoteRenderingBackend and the RemoteRenderingBackend object has registered itself as a
WorkQueueMessageReceiver. We probably want to follow-up when we come up with a better design. For now though, I have verified locally that this
fixes the out of order IPC messaging AND the flaky crashes on these canvas tests.

No new tests, unskipped existing tests that are no longer flaky.

  • GPUProcess/GPUConnectionToWebProcess.cpp:

(WebKit::GPUConnectionToWebProcess::createRenderingBackend):

  • GPUProcess/GPUConnectionToWebProcess.h:
  • GPUProcess/GPUConnectionToWebProcess.messages.in:
  • WebProcess/GPU/graphics/RemoteRenderingBackendProxy.cpp:

(WebKit::RemoteRenderingBackendProxy::ensureGPUProcessConnection):

LayoutTests:

Unskip layout tests that should no longer be flaky.

  • platform/ios-simulator-wk2/TestExpectations:
  • platform/mac-wk2/TestExpectations:
Location:
trunk
Files:
8 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r276004 r276007  
     12021-04-15  Chris Dumez  <cdumez@apple.com>
     2
     3        REGRESSION(r275921-r275922): [ wk2 ] http/tests/security/webgl-remote-read-remote-image-allowed-with-credentials.html is flakey crashing
     4        https://bugs.webkit.org/show_bug.cgi?id=224566
     5        <rdar://problem/76657007>
     6
     7        Reviewed by Tim Horton.
     8
     9        Unskip layout tests that should no longer be flaky.
     10
     11        * platform/ios-simulator-wk2/TestExpectations:
     12        * platform/mac-wk2/TestExpectations:
     13
    1142021-04-14  Diego Pino Garcia  <dpino@igalia.com>
    215

  • trunk/LayoutTests/platform/ios-simulator-wk2/TestExpectations

    r275987 r276007  
    170170http/tests/navigation/page-cache-mediastream.html [ Pass Crash ]
    171171
    172 # webkit.org/b/224566 The following three tests are flaky crashing causing issues with EWS/commit queue.
    173 http/tests/security/webgl-remote-read-remote-image-allowed-with-credentials.html [ Pass Crash ]
    174 imported/w3c/web-platform-tests/webmessaging/with-ports/011.html [ Pass Crash ]
    175 webgl/1.0.3/conformance/glsl/matrices/glsl-mat3-construction.html [ Pass Crash ]

  • trunk/LayoutTests/platform/mac-wk2/TestExpectations

    r275987 r276007  
    13781378http/tests/navigation/page-cache-mediastream.html [ Pass Crash ]
    13791379
    1380 # webkit.org/b/224566 The following three tests are flaky crashing causing issues with EWS/commit queue.
    1381 http/tests/security/webgl-remote-read-remote-image-allowed-with-credentials.html [ Pass Crash ]
    1382 imported/w3c/web-platform-tests/webmessaging/with-ports/011.html [ Pass Crash ]
    1383 webgl/1.0.3/conformance/glsl/matrices/glsl-mat3-construction.html [ Pass Crash ]
    1384 
    13851380webkit.org/b/221985 fast/mediastream/audio-track-enabled.html [ Pass Failure ]

  • trunk/Source/WebKit/ChangeLog

    r275992 r276007  
     12021-04-15  Chris Dumez  <cdumez@apple.com>
     2
     3        REGRESSION(r275921-r275922): [ wk2 ] http/tests/security/webgl-remote-read-remote-image-allowed-with-credentials.html is flakey crashing
     4        https://bugs.webkit.org/show_bug.cgi?id=224566
     5        <rdar://problem/76657007>
     6
     7        Reviewed by Tim Horton.
     8
     9        r275921-r275922 changed the timing of when the WebProcess/RemoteRenderingBackendProxy sends the GPUConnectionToWebProcess::CreateRenderingBackend
     10        IPC to the GPUProcess. This IPC used to get sent as soon as the RemoteRenderingBackendProxy was constructed. However, I delayed sending it until
     11        the RemoteRenderingBackendProxy actually needs to create an ImageBuffer to avoid launching the GPUProcess prematurely.
     12
     13        Unfortunately, this exposed a pre-existing synchronization issue with regards to the RemoteRenderingBackend IPC. In particular, the
     14        GPUConnectionToWebProcess::CreateRenderingBackend was getting sent asynchronously and processed by the GPUProcess on the main thread. This would
     15        cause the creation of the RemoteRenderingBackend object on the GPUProcess side, which would then register itself as a WorkQueueMessageReceiver.
     16        The issue is that the WebProcess was sending the RemoteRenderingBackend IPC so quickly after the CreateRenderingBackend IPC that when the IPC is
     17        received (on the IPC thread), the RemoteRenderingBackend has not had a change to register itself as a WorkQueueMessageReceiver yet. As a result,
     18        some of the early IPC was getting dispatched to the main thread and later IPC would get sent to the RemoteRenderingBackend WorkQueue. This was
     19        causing RemoteRenderingBackend to get processed out of order.
     20
     21        To address the synchronization issue for now, I made the GPUConnectionToWebProcess::CreateRenderingBackend synchronous. This may not be the
     22        best design long term but it is an easy and safe way to resolve this for now. It makes sure the WebProcess cannot send RemoteRenderingBackend
     23        IPC until after the GPUProcess has created the RemoteRenderingBackend and the RemoteRenderingBackend object has registered itself as a
     24        WorkQueueMessageReceiver. We probably want to follow-up when we come up with a better design. For now though, I have verified locally that this
     25        fixes the out of order IPC messaging AND the flaky crashes on these canvas tests.
     26
     27        No new tests, unskipped existing tests that are no longer flaky.
     28
     29        * GPUProcess/GPUConnectionToWebProcess.cpp:
     30        (WebKit::GPUConnectionToWebProcess::createRenderingBackend):
     31        * GPUProcess/GPUConnectionToWebProcess.h:
     32        * GPUProcess/GPUConnectionToWebProcess.messages.in:
     33        * WebProcess/GPU/graphics/RemoteRenderingBackendProxy.cpp:
     34        (WebKit::RemoteRenderingBackendProxy::ensureGPUProcessConnection):
     35
    1362021-04-14  Basuke Suzuki  <basuke.suzuki@sony.com>
    237

  • trunk/Source/WebKit/GPUProcess/GPUConnectionToWebProcess.cpp

    r275968 r276007  
    363363#endif
    364364
    365 void GPUConnectionToWebProcess::createRenderingBackend(RemoteRenderingBackendCreationParameters&& creationParameters)
     365void GPUConnectionToWebProcess::createRenderingBackend(RemoteRenderingBackendCreationParameters&& creationParameters, CompletionHandler<void()>&& completionHandler)
    366366{
    367367    auto addResult = m_remoteRenderingBackendMap.ensure(creationParameters.identifier, [&]() {
    … …  
    369369    });
    370370    ASSERT_UNUSED(addResult, addResult.isNewEntry);
     371    completionHandler();
    371372}
    372373

  • trunk/Source/WebKit/GPUProcess/GPUConnectionToWebProcess.h

    r275921 r276007  
    162162#endif
    163163
    164     void createRenderingBackend(RemoteRenderingBackendCreationParameters&&);
     164    void createRenderingBackend(RemoteRenderingBackendCreationParameters&&, CompletionHandler<void()>&&);
    165165    void releaseRenderingBackend(RenderingBackendIdentifier);
    166166

  • trunk/Source/WebKit/GPUProcess/GPUConnectionToWebProcess.messages.in

    r275921 r276007  
    2424
    2525messages -> GPUConnectionToWebProcess WantsDispatchMessage {
    26     void CreateRenderingBackend(struct WebKit::RemoteRenderingBackendCreationParameters creationParameters)
     26    void CreateRenderingBackend(struct WebKit::RemoteRenderingBackendCreationParameters creationParameters) -> () Synchronous
    2727    void ReleaseRenderingBackend(WebKit::RenderingBackendIdentifier renderingBackendIdentifier)
    2828#if ENABLE(WEBGL)

  • trunk/Source/WebKit/WebProcess/GPU/graphics/RemoteRenderingBackendProxy.cpp

    r275922 r276007  
    7676        gpuProcessConnection.addClient(*this);
    7777        gpuProcessConnection.messageReceiverMap().addMessageReceiver(Messages::RemoteRenderingBackendProxy::messageReceiverName(), renderingBackendIdentifier().toUInt64(), *this);
    78         gpuProcessConnection.connection().send(Messages::GPUConnectionToWebProcess::CreateRenderingBackend(m_parameters), 0, IPC::SendOption::DispatchMessageEvenWhenWaitingForSyncReply);
     78        // This message is synchronous to ensure that the RemoteRenderingBackend has been created and has registered itself as a WorkQueueMessageReceiver before we send it IPC.
     79        // Without this synchronization, some IPC messages may get received by the GPUProcess before the RemoteRenderingBackend has registered itself as a WorkQueueMessageReceiver
     80        // and IPC may get processed out of order.
     81        gpuProcessConnection.connection().sendSync(Messages::GPUConnectionToWebProcess::CreateRenderingBackend(m_parameters), Messages::GPUConnectionToWebProcess::CreateRenderingBackend::Reply(), 0);
    7982        m_gpuProcessConnection = makeWeakPtr(gpuProcessConnection);
    8083    }
Note: See TracChangeset for help on using the changeset viewer.