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

Changeset 271245 in webkit


Ignore:
Timestamp:
Jan 7, 2021, 11:49:38 AM (6 years ago)
Author:
Chris Dumez
Message:

[GPUProcess] Implement GPUProcess crash recovery for MediaElementAudioSourceNode
​https://bugs.webkit.org/show_bug.cgi?id=220391

Reviewed by Geoffrey Garen.

Source/WebCore:

When audio is playing using WebAudio and a MediaElementAudioSourceNode, make sure that
audio resumes playing seemlessly after a GPUProcess crash.

  • html/HTMLMediaElement.cpp:

(WebCore::HTMLMediaElement::mediaEngineWasUpdated):
(WebCore::HTMLMediaElement::mediaPlayerEngineUpdated):
Reconstruct the audioSourceProvider (and add the audioSourceNode as its client) as soon
as possible after the media engine was (re-)initialized in mediaPlayerEngineUpdated(),
instead of doing it asynchronously in mediaEngineWasUpdated(). This does not run script
so it is safe to call synchronously. If the GPUProcess crashes while a
MediaElementAudioSourceNode is playing, MediaElementAudioSourceNode::process() needs the
audioSourceProvider to get the input audio. If we don't eagerly construct the
audioSourceProvider here, then it may happen on the audio rendering thread, when the
MediaElementAudioSourceNode actually needs it, which would not be safe.

(WebCore::HTMLMediaElement::mediaPlayerWillInitializeMediaEngine):
(WebCore::HTMLMediaElement::mediaPlayerDidInitializeMediaEngine):
As mentioned earlier, MediaElementAudioSourceNode::process() runs on the audio thread
and accesses HTMLMediaElement::audioSourceProvider(), which calls
MediaPlayer::audioSourceProvider(), which calls
MediaPlayerPrivate::audioSourceProvider(). To be thread-safe, we need to make sure we
hold the MediaElementAudioSourceNode's process lock while the MediaPlayerPrivate is
being reconstructed in MediaPlayer (which happens when the GPUProcess crashes).

  • html/HTMLMediaElement.h:
  • platform/graphics/MediaPlayer.cpp:

(WebCore::MediaPlayer::loadWithNextMediaEngine):

  • platform/graphics/MediaPlayer.h:

(WebCore::MediaPlayerClient::mediaPlayerWillInitializeMediaEngine):
(WebCore::MediaPlayerClient::mediaPlayerDidInitializeMediaEngine):
Add new MediaPlayerClient interface functions that are called before and after the
MediaPlayerPrivate gets (re-)constructed. This is needed by the HTMLMediaElement
to hold a lock while this is happening.

Source/WebKit:

When audio is playing using WebAudio and a MediaElementAudioSourceNode, make sure that
audio resumes playing seemlessly after a GPUProcess crash.

  • WebProcess/GPU/GPUProcessConnection.h:

Subclass CanMakeWeakPtr<> so that clients can hold a weak pointer to a GPUProcessConnection.

  • WebProcess/GPU/media/RemoteAudioSourceProvider.cpp:

(WebKit::RemoteAudioSourceProvider::create):
(WebKit::RemoteAudioSourceProvider::RemoteAudioSourceProvider):
(WebKit::RemoteAudioSourceProvider::close):
(WebKit::RemoteAudioSourceProvider::hasNewClient):

  • WebProcess/GPU/media/RemoteAudioSourceProvider.h:

Associate the RemoteAudioSourceProvider with a specific GPUProcessConnection instead of always
using the latest connection via WebProcess::ensureGPUProcessConnection(). In the event of a
GPUProcess crash, MediaPlayer will reconstruct its MediaPlayerPrivate which will reconstruct
a new RemoteAudioSourceProvider (since RemoteAudioSourceProvider is owned by
MediaPlayerPrivateRemote). As a result, RemoteAudioSourceProvider does not need to do anything
to deal with a GPUProcess crash. However, we need to make sure it only interacts with the
GPUProcessConnection that existed when it was constructed.

Tools:

Add API test coverage.

  • TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
  • TestWebKitAPI/Tests/WebKitCocoa/GPUProcess.mm:

(TEST):

  • TestWebKitAPI/Tests/WebKitCocoa/webaudio-createMediaElementSource.html: Added.
Location:
trunk
Files:
1 added
12 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r271239 r271245  
     12021-01-07  Chris Dumez  <cdumez@apple.com>
     2
     3        [GPUProcess] Implement GPUProcess crash recovery for MediaElementAudioSourceNode
     4        https://bugs.webkit.org/show_bug.cgi?id=220391
     5
     6        Reviewed by Geoffrey Garen.
     7
     8        When audio is playing using WebAudio and a MediaElementAudioSourceNode, make sure that
     9        audio resumes playing seemlessly after a GPUProcess crash.
     10
     11        * html/HTMLMediaElement.cpp:
     12        (WebCore::HTMLMediaElement::mediaEngineWasUpdated):
     13        (WebCore::HTMLMediaElement::mediaPlayerEngineUpdated):
     14        Reconstruct the audioSourceProvider (and add the audioSourceNode as its client) as soon
     15        as possible after the media engine was (re-)initialized in mediaPlayerEngineUpdated(),
     16        instead of doing it asynchronously in mediaEngineWasUpdated(). This does not run script
     17        so it is safe to call synchronously. If the GPUProcess crashes while a
     18        MediaElementAudioSourceNode is playing, MediaElementAudioSourceNode::process() needs the
     19        audioSourceProvider to get the input audio. If we don't eagerly construct the
     20        audioSourceProvider here, then it may happen on the audio rendering thread, when the
     21        MediaElementAudioSourceNode actually needs it, which would not be safe.
     22
     23        (WebCore::HTMLMediaElement::mediaPlayerWillInitializeMediaEngine):
     24        (WebCore::HTMLMediaElement::mediaPlayerDidInitializeMediaEngine):
     25        As mentioned earlier, MediaElementAudioSourceNode::process() runs on the audio thread
     26        and accesses HTMLMediaElement::audioSourceProvider(), which calls
     27        MediaPlayer::audioSourceProvider(), which calls
     28        MediaPlayerPrivate::audioSourceProvider(). To be thread-safe, we need to make sure we
     29        hold the MediaElementAudioSourceNode's process lock while the MediaPlayerPrivate is
     30        being reconstructed in MediaPlayer (which happens when the GPUProcess crashes).
     31
     32        * html/HTMLMediaElement.h:
     33        * platform/graphics/MediaPlayer.cpp:
     34        (WebCore::MediaPlayer::loadWithNextMediaEngine):
     35        * platform/graphics/MediaPlayer.h:
     36        (WebCore::MediaPlayerClient::mediaPlayerWillInitializeMediaEngine):
     37        (WebCore::MediaPlayerClient::mediaPlayerDidInitializeMediaEngine):
     38        Add new MediaPlayerClient interface functions that are called before and after the
     39        MediaPlayerPrivate gets (re-)constructed. This is needed by the HTMLMediaElement
     40        to hold a lock while this is happening.
     41
    1422021-01-07  Alex Christensen  <achristensen@webkit.org>
    243

  • trunk/Source/WebCore/html/HTMLMediaElement.cpp

    r271219 r271245  
    49674967    m_mediaSession->mediaEngineUpdated();
    49684968
    4969 #if ENABLE(WEB_AUDIO)
    4970     if (m_audioSourceNode && audioSourceProvider()) {
    4971         auto locker = holdLock(m_audioSourceNode->processLock());
    4972         audioSourceProvider()->setClient(m_audioSourceNode);
    4973     }
    4974 #endif
    4975 
    49764969#if ENABLE(ENCRYPTED_MEDIA)
    49774970    if (m_player && m_mediaKeys)
    … …  
    50125005#endif
    50135006
     5007#if ENABLE(WEB_AUDIO)
     5008    if (m_audioSourceNode) {
     5009        if (auto* provider = audioSourceProvider())
     5010            provider->setClient(m_audioSourceNode);
     5011    }
     5012#endif
     5013
    50145014    m_havePreparedToPlay = false;
    50155015
    50165016    scheduleMediaEngineWasUpdated();
     5017}
     5018
     5019void HTMLMediaElement::mediaPlayerWillInitializeMediaEngine()
     5020{
     5021    ASSERT(isMainThread());
     5022#if ENABLE(WEB_AUDIO)
     5023    // Make sure the MediaElementAudioSourceNode's process function does not try and access the media player while its engine is getting updated.
     5024    if (m_audioSourceNode)
     5025        m_audioSourceNode->processLock().lock();
     5026#endif
     5027}
     5028
     5029void HTMLMediaElement::mediaPlayerDidInitializeMediaEngine()
     5030{
     5031    ASSERT(isMainThread());
     5032#if ENABLE(WEB_AUDIO)
     5033    if (m_audioSourceNode)
     5034        m_audioSourceNode->processLock().unlock();
     5035#endif
    50175036}
    50185037
    … …  
    65616580}
    65626581
     6582// This may get called on the audio thread by MediaElementAudioSourceNode.
    65636583AudioSourceProvider* HTMLMediaElement::audioSourceProvider()
    65646584{

  • trunk/Source/WebCore/html/HTMLMediaElement.h

    r270171 r271245  
    659659    bool mediaPlayerAcceleratedCompositingEnabled() final;
    660660    void mediaPlayerEngineUpdated() final;
     661    void mediaPlayerWillInitializeMediaEngine() final;
     662    void mediaPlayerDidInitializeMediaEngine() final;
    661663
    662664    void scheduleMediaEngineWasUpdated();

  • trunk/Source/WebCore/platform/graphics/MediaPlayer.cpp

    r271219 r271245  
    562562    ASSERT(!m_initializingMediaEngine);
    563563    m_initializingMediaEngine = true;
     564    client().mediaPlayerWillInitializeMediaEngine();
    564565
    565566    const MediaPlayerFactory* engine = nullptr;
    … …  
    611612
    612613    m_initializingMediaEngine = false;
     614    client().mediaPlayerDidInitializeMediaEngine();
    613615}
    614616

  • trunk/Source/WebCore/platform/graphics/MediaPlayer.h

    r271219 r271245  
    212212#endif
    213213
     214    virtual void mediaPlayerWillInitializeMediaEngine() { }
     215    virtual void mediaPlayerDidInitializeMediaEngine() { }
     216
    214217    virtual String mediaPlayerReferrer() const { return String(); }
    215218    virtual String mediaPlayerUserAgent() const { return String(); }

  • trunk/Source/WebKit/ChangeLog

    r271243 r271245  
     12021-01-07  Chris Dumez  <cdumez@apple.com>
     2
     3        [GPUProcess] Implement GPUProcess crash recovery for MediaElementAudioSourceNode
     4        https://bugs.webkit.org/show_bug.cgi?id=220391
     5
     6        Reviewed by Geoffrey Garen.
     7
     8        When audio is playing using WebAudio and a MediaElementAudioSourceNode, make sure that
     9        audio resumes playing seemlessly after a GPUProcess crash.
     10
     11        * WebProcess/GPU/GPUProcessConnection.h:
     12        Subclass CanMakeWeakPtr<> so that clients can hold a weak pointer to a GPUProcessConnection.
     13
     14        * WebProcess/GPU/media/RemoteAudioSourceProvider.cpp:
     15        (WebKit::RemoteAudioSourceProvider::create):
     16        (WebKit::RemoteAudioSourceProvider::RemoteAudioSourceProvider):
     17        (WebKit::RemoteAudioSourceProvider::close):
     18        (WebKit::RemoteAudioSourceProvider::hasNewClient):
     19        * WebProcess/GPU/media/RemoteAudioSourceProvider.h:
     20        Associate the RemoteAudioSourceProvider with a specific GPUProcessConnection instead of always
     21        using the latest connection via WebProcess::ensureGPUProcessConnection(). In the event of a
     22        GPUProcess crash, MediaPlayer will reconstruct its MediaPlayerPrivate which will reconstruct
     23        a new RemoteAudioSourceProvider (since RemoteAudioSourceProvider is owned by
     24        MediaPlayerPrivateRemote). As a result, RemoteAudioSourceProvider does not need to do anything
     25        to deal with a GPUProcess crash. However, we need to make sure it only interacts with the
     26        GPUProcessConnection that existed when it was constructed.
     27
    1282021-01-07  Kimmo Kinnunen  <kkinnunen@apple.com>
    229

  • trunk/Source/WebKit/WebProcess/GPU/GPUProcessConnection.h

    r270720 r271245  
    3434#include <wtf/RefCounted.h>
    3535#include <wtf/WeakHashSet.h>
     36#include <wtf/WeakPtr.h>
    3637#include <wtf/text/WTFString.h>
    3738
    … …  
    4546struct WebPageCreationParameters;
    4647
    47 class GPUProcessConnection : public RefCounted<GPUProcessConnection>, IPC::Connection::Client {
     48class GPUProcessConnection : public RefCounted<GPUProcessConnection>, public CanMakeWeakPtr<GPUProcessConnection>, IPC::Connection::Client {
    4849public:
    4950    static Ref<GPUProcessConnection> create(IPC::Connection::Identifier connectionIdentifier)

  • trunk/Source/WebKit/WebProcess/GPU/media/RemoteAudioSourceProvider.cpp

    r270573 r271245  
    4545{
    4646    auto provider = adoptRef(*new RemoteAudioSourceProvider(identifier, helper));
    47 
    48     auto& gpuProcessConnection = WebProcess::singleton().ensureGPUProcessConnection();
    49     gpuProcessConnection.audioSourceProviderManager().addProvider(provider.copyRef());
    50 
    51 #if ENABLE(WEB_AUDIO)
    52     gpuProcessConnection.connection().send(Messages::RemoteMediaPlayerProxy::CreateAudioSourceProvider { }, provider->identifier());
    53 #endif
    54 
     47    provider->m_gpuProcessConnection->audioSourceProviderManager().addProvider(provider.copyRef());
    5548    return provider;
    5649}
    … …  
    5851RemoteAudioSourceProvider::RemoteAudioSourceProvider(MediaPlayerIdentifier identifier, WTF::LoggerHelper& helper)
    5952    : m_identifier(identifier)
     53    , m_gpuProcessConnection(makeWeakPtr(WebProcess::singleton().ensureGPUProcessConnection()))
    6054#if !RELEASE_LOG_DISABLED
    6155    , m_logger(helper.logger())
    … …  
    6559    ASSERT(isMainThread());
    6660    UNUSED_PARAM(helper);
     61
     62#if ENABLE(WEB_AUDIO)
     63    m_gpuProcessConnection->connection().send(Messages::RemoteMediaPlayerProxy::CreateAudioSourceProvider { }, identifier);
     64#endif
    6765}
    6866
    … …  
    7472{
    7573    ASSERT(isMainThread());
    76     WebProcess::singleton().ensureGPUProcessConnection().audioSourceProviderManager().removeProvider(m_identifier);
     74    if (m_gpuProcessConnection)
     75        m_gpuProcessConnection->audioSourceProviderManager().removeProvider(m_identifier);
    7776}
    7877
    7978void RemoteAudioSourceProvider::hasNewClient(AudioSourceProviderClient* client)
    8079{
    81     WebProcess::singleton().ensureGPUProcessConnection().connection().send(Messages::RemoteMediaPlayerProxy::SetShouldEnableAudioSourceProvider { !!client }, m_identifier);
     80    if (m_gpuProcessConnection)
     81        m_gpuProcessConnection->connection().send(Messages::RemoteMediaPlayerProxy::SetShouldEnableAudioSourceProvider { !!client }, m_identifier);
    8282}
    8383

  • trunk/Source/WebKit/WebProcess/GPU/media/RemoteAudioSourceProvider.h

    r268521 r271245  
    2828#if ENABLE(GPU_PROCESS) && ENABLE(WEB_AUDIO) && PLATFORM(COCOA)
    2929
     30#include "GPUProcessConnection.h"
    3031#include <WebCore/MediaPlayerIdentifier.h>
    3132#include <WebCore/WebAudioSourceProviderCocoa.h>
    … …  
    6667
    6768    WebCore::MediaPlayerIdentifier m_identifier;
     69    WeakPtr<GPUProcessConnection> m_gpuProcessConnection;
    6870#if !RELEASE_LOG_DISABLED
    6971    Ref<const Logger> m_logger;

  • trunk/Tools/ChangeLog

    r271244 r271245  
     12021-01-07  Chris Dumez  <cdumez@apple.com>
     2
     3        [GPUProcess] Implement GPUProcess crash recovery for MediaElementAudioSourceNode
     4        https://bugs.webkit.org/show_bug.cgi?id=220391
     5
     6        Reviewed by Geoffrey Garen.
     7
     8        Add API test coverage.
     9
     10        * TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
     11        * TestWebKitAPI/Tests/WebKitCocoa/GPUProcess.mm:
     12        (TEST):
     13        * TestWebKitAPI/Tests/WebKitCocoa/webaudio-createMediaElementSource.html: Added.
     14
    1152021-01-07  Sam Sneddon  <gsnedders@apple.com>
    216

  • trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj

    r271190 r271245  
    260260                468F2F942368DAF100F4B864 /* window-open-then-document-open.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = 468F2F932368DAA700F4B864 /* window-open-then-document-open.html */; };
    261261                46918EFC2237283C00468DFE /* DeviceOrientation.mm in Sources */ = {isa = PBXBuildFile; fileRef = 46918EFB2237283500468DFE /* DeviceOrientation.mm */; };
     262                46A44A5425A7830300F61E16 /* webaudio-createMediaElementSource.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = 46A44A5325A782DD00F61E16 /* webaudio-createMediaElementSource.html */; };
    262263                46A46A1A2575645600A1B118 /* SessionStorage.mm in Sources */ = {isa = PBXBuildFile; fileRef = 46A46A192575645600A1B118 /* SessionStorage.mm */; };
    263264                46A911592108E6780078D40D /* CustomUserAgent.mm in Sources */ = {isa = PBXBuildFile; fileRef = 46A911582108E66B0078D40D /* CustomUserAgent.mm */; };
    … …  
    16861687                                57EDFC5C245A1A3F00959521 /* web-authentication-make-credential-la-no-mock.html in Copy Resources */,
    16871688                                5742178E2400D2DF002B303D /* web-authentication-make-credential-la.html in Copy Resources */,
     1689                                46A44A5425A7830300F61E16 /* webaudio-createMediaElementSource.html in Copy Resources */,
    16881690                                1C2B81861C89259D00A5529F /* webfont.html in Copy Resources */,
    16891691                                31E9BDA3247F5729002E51A2 /* webgl.html in Copy Resources */,
    … …  
    20002002                468F2F932368DAA700F4B864 /* window-open-then-document-open.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = "window-open-then-document-open.html"; sourceTree = "<group>"; };
    20012003                46918EFB2237283500468DFE /* DeviceOrientation.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = DeviceOrientation.mm; sourceTree = "<group>"; };
     2004                46A44A5325A782DD00F61E16 /* webaudio-createMediaElementSource.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = "webaudio-createMediaElementSource.html"; sourceTree = "<group>"; };
    20022005                46A46A192575645600A1B118 /* SessionStorage.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = SessionStorage.mm; sourceTree = "<group>"; };
    20032006                46A911582108E66B0078D40D /* CustomUserAgent.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = CustomUserAgent.mm; sourceTree = "<group>"; };
    … …  
    39843987                                57EDFC5B245A18F500959521 /* web-authentication-make-credential-la-no-mock.html */,
    39853988                                5742178D2400D26C002B303D /* web-authentication-make-credential-la.html */,
     3989                                46A44A5325A782DD00F61E16 /* webaudio-createMediaElementSource.html */,
    39863990                                51714EB21CF8C761004723C4 /* WebProcessKillIDBCleanup-1.html */,
    39873991                                51714EB31CF8C761004723C4 /* WebProcessKillIDBCleanup-2.html */,

  • trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/GPUProcess.mm

    r270628 r271245  
    292292}
    293293
     294TEST(GPUProcess, CrashWhilePlayingAudioViaCreateMediaElementSource)
     295{
     296    auto configuration = adoptNS([[WKWebViewConfiguration alloc] init]);
     297    for (_WKInternalDebugFeature *feature in [WKPreferences _internalDebugFeatures]) {
     298        if ([feature.key isEqualToString:@"UseGPUProcessForMediaEnabled"]) {
     299            [[configuration preferences] _setEnabled:YES forInternalDebugFeature:feature];
     300            break;
     301        }
     302    }
     303
     304    auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 400, 400) configuration:configuration.get()]);
     305    [webView synchronouslyLoadTestPageNamed:@"webaudio-createMediaElementSource"];
     306
     307    __block bool done = false;
     308    [webView evaluateJavaScript:@"document.getElementById('testButton').click()" completionHandler:^(id result, NSError *error) {
     309        EXPECT_TRUE(!error);
     310        done = true;
     311    }];
     312    TestWebKitAPI::Util::run(&done);
     313
     314    auto webViewPID = [webView _webProcessIdentifier];
     315
     316    // The GPU process should get launched.
     317    auto* processPool = configuration.get().processPool;
     318    unsigned timeout = 0;
     319    while (![processPool _gpuProcessIdentifier] && timeout++ < 100)
     320        TestWebKitAPI::Util::sleep(0.1);
     321
     322    EXPECT_NE([processPool _gpuProcessIdentifier], 0);
     323    if (![processPool _gpuProcessIdentifier])
     324        return;
     325    auto gpuProcessPID = [processPool _gpuProcessIdentifier];
     326
     327    // Audio should be playing.
     328    timeout = 0;
     329    while (![webView _isPlayingAudio] && timeout++ < 100)
     330        TestWebKitAPI::Util::sleep(0.1);
     331    EXPECT_TRUE([webView _isPlayingAudio]);
     332
     333    // Kill the GPU Process.
     334    kill(gpuProcessPID, 9);
     335
     336    // GPU Process should get relaunched.
     337    timeout = 0;
     338    while ((![processPool _gpuProcessIdentifier] || [processPool _gpuProcessIdentifier] == gpuProcessPID) && timeout++ < 100)
     339        TestWebKitAPI::Util::sleep(0.1);
     340    EXPECT_NE([processPool _gpuProcessIdentifier], 0);
     341    EXPECT_NE([processPool _gpuProcessIdentifier], gpuProcessPID);
     342    gpuProcessPID = [processPool _gpuProcessIdentifier];
     343
     344    // Make sure the WebProcess did not crash.
     345    EXPECT_EQ(webViewPID, [webView _webProcessIdentifier]);
     346
     347    // FIXME: On iOS, video resumes after the GPU process crash but audio does not.
     348#if !PLATFORM(IOS)
     349    // Audio should resume playing.
     350    timeout = 0;
     351    while (![webView _isPlayingAudio] && timeout++ < 100)
     352        TestWebKitAPI::Util::sleep(0.1);
     353    EXPECT_TRUE([webView _isPlayingAudio]);
     354#endif
     355
     356    EXPECT_EQ(gpuProcessPID, [processPool _gpuProcessIdentifier]);
     357    EXPECT_EQ(webViewPID, [webView _webProcessIdentifier]);
     358}
     359
    294360static NSString *testCanvasPage = @"<body> \n"
    295361    "<canvas id='myCanvas' width='400px' height='400px'>\n"
Note: See TracChangeset for help on using the changeset viewer.