Changeset 260373 in webkit


Ignore:
Timestamp:
Apr 20, 2020 10:48:42 AM (4 years ago)
Author:
Chris Dumez
Message:

[iOS] Refactor WebKit media playback process assertion logic to minimize chances of leaking them
https://bugs.webkit.org/show_bug.cgi?id=210670

Reviewed by Geoffrey Garen.

Refactor WebKit media playback process assertion logic to minimize chances of leaking them. In particular,
the following changes were made:

  1. Instead of the WebProcessPool having a HashMap of media playback process assertions for the WebProcess, we now store the assertion on the WebProcessProxy object itself. This is less likely to get out of sync and leak.
  2. Add a RefCounter to the WebProcessPool to count WebProcesses that have audible media. Whenever a WebProcess starts or stops playing audible media, it merely grabs a RefCounter token or releases it. The WebProcessPool relies on this counter to decide whether or not to take a media playback assertion on behalf of the UIProcess. Since this is token-based and the token is stored on the WebProcessProxy object, it makes it less likely to leak the assertion.
  3. The WebProcessProxy object now has a AudibleMediaActivity data structure wrapping both its media playback assertion and its WebProcessWithAudibleMediaToken that it got from the WebProcessPool. When the WebProcess shuts down (normally or due to crash/termination), we make sure to clear this data structure.
  4. Make sure that the WebProcessProxy updates its AudibleMediaActivity whenever a page is removed from the WebProcess.
  • UIProcess/WebPageProxy.cpp:
  • UIProcess/WebProcessPool.cpp:

(WebKit::m_webProcessWithAudibleMediaCounter):
(WebKit::WebProcessPool::disconnectProcess):
(WebKit::WebProcessPool::webProcessWithAudibleMediaToken const):
(WebKit::WebProcessPool::updateAudibleMediaAssertions):

  • UIProcess/WebProcessPool.h:
  • UIProcess/WebProcessProxy.cpp:

(WebKit::WebProcessProxy::shutDown):
(WebKit::WebProcessProxy::removeWebPage):
(WebKit::WebProcessProxy::updateAudibleMediaAssertions):

  • UIProcess/WebProcessProxy.h:
Location:
trunk/Source/WebKit
Files:
6 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/WebKit/ChangeLog

    r260372 r260373  
     12020-04-20  Chris Dumez  <cdumez@apple.com>
     2
     3        [iOS] Refactor WebKit media playback process assertion logic to minimize chances of leaking them
     4        https://bugs.webkit.org/show_bug.cgi?id=210670
     5
     6        Reviewed by Geoffrey Garen.
     7
     8        Refactor WebKit media playback process assertion logic to minimize chances of leaking them. In particular,
     9        the following changes were made:
     10        1. Instead of the WebProcessPool having a HashMap of media playback process assertions for
     11           the WebProcess, we now store the assertion on the WebProcessProxy object itself. This is
     12           less likely to get out of sync and leak.
     13        2. Add a RefCounter to the WebProcessPool to count WebProcesses that have audible media.
     14           Whenever a WebProcess starts or stops playing audible media, it merely grabs a RefCounter
     15           token or releases it. The WebProcessPool relies on this counter to decide whether or not
     16           to take a media playback assertion on behalf of the UIProcess. Since this is token-based
     17           and the token is stored on the WebProcessProxy object, it makes it less likely to leak
     18           the assertion.
     19        3. The WebProcessProxy object now has a AudibleMediaActivity data structure wrapping both
     20           its media playback assertion and its WebProcessWithAudibleMediaToken that it got from
     21           the WebProcessPool. When the WebProcess shuts down (normally or due to crash/termination),
     22           we make sure to clear this data structure.
     23        4. Make sure that the WebProcessProxy updates its AudibleMediaActivity whenever a page is
     24           removed from the WebProcess.
     25
     26        * UIProcess/WebPageProxy.cpp:
     27        * UIProcess/WebProcessPool.cpp:
     28        (WebKit::m_webProcessWithAudibleMediaCounter):
     29        (WebKit::WebProcessPool::disconnectProcess):
     30        (WebKit::WebProcessPool::webProcessWithAudibleMediaToken const):
     31        (WebKit::WebProcessPool::updateAudibleMediaAssertions):
     32        * UIProcess/WebProcessPool.h:
     33        * UIProcess/WebProcessProxy.cpp:
     34        (WebKit::WebProcessProxy::shutDown):
     35        (WebKit::WebProcessProxy::removeWebPage):
     36        (WebKit::WebProcessProxy::updateAudibleMediaAssertions):
     37        * UIProcess/WebProcessProxy.h:
     38
    1392020-04-20  Kate Cheney  <katherine_cheney@apple.com>
    240
  • trunk/Source/WebKit/UIProcess/WebPageProxy.cpp

    r260303 r260373  
    89308930        videoControlsManagerDidChange();
    89318931
    8932     m_process->webPageMediaStateDidChange(*this);
     8932    m_process->updateAudibleMediaAssertions();
    89338933}
    89348934
  • trunk/Source/WebKit/UIProcess/WebProcessPool.cpp

    r260283 r260373  
    261261    , m_backForwardCache(makeUniqueRef<WebBackForwardCache>(*this))
    262262    , m_webProcessCache(makeUniqueRef<WebProcessCache>(*this))
     263    , m_webProcessWithAudibleMediaCounter([this](RefCounterEvent) { updateAudibleMediaAssertions(); })
    263264{
    264265    static std::once_flag onceFlag;
     
    11831184{
    11841185    ASSERT(m_processes.contains(process));
    1185     ASSERT(!m_processesPlayingAudibleMedia.contains(process->coreProcessIdentifier()));
    11861186
    11871187    if (m_prewarmedProcess == process) {
     
    23512351#endif
    23522352
    2353 void WebProcessPool::setWebProcessIsPlayingAudibleMedia(WebCore::ProcessIdentifier processID)
    2354 {
    2355     auto* process = WebProcessProxy::processForIdentifier(processID);
    2356     ASSERT(process);
    2357 
    2358     WEBPROCESSPOOL_RELEASE_LOG(ProcessSuspension, "setWebProcessIsPlayingAudibleMedia: Web process is now playing audible media (process=%p, PID=%i)", process, process->processIdentifier());
    2359 
    2360     if (m_processesPlayingAudibleMedia.isEmpty()) {
    2361         WEBPROCESSPOOL_RELEASE_LOG(ProcessSuspension, "setWebProcessIsPlayingAudibleMedia: The number of processes playing audible media is now one. Taking UI process assertion.");
    2362 
    2363         ASSERT(!m_uiProcessMediaPlaybackAssertion);
    2364         m_uiProcessMediaPlaybackAssertion = makeUnique<ProcessAssertion>(getCurrentProcessID(), "WebKit Media Playback"_s, ProcessAssertionType::MediaPlayback);
     2353WebProcessWithAudibleMediaToken WebProcessPool::webProcessWithAudibleMediaToken() const
     2354{
     2355    return m_webProcessWithAudibleMediaCounter.count();
     2356}
     2357
     2358void WebProcessPool::updateAudibleMediaAssertions()
     2359{
     2360    if (!m_webProcessWithAudibleMediaCounter.value()) {
     2361        WEBPROCESSPOOL_RELEASE_LOG(ProcessSuspension, "updateAudibleMediaAssertions: The number of processes playing audible media now zero. Releasing UI process assertion.");
     2362        m_audibleMediaActivity = WTF::nullopt;
     2363        return;
     2364    }
     2365
     2366    if (m_audibleMediaActivity)
     2367        return;
     2368
     2369    WEBPROCESSPOOL_RELEASE_LOG(ProcessSuspension, "updateAudibleMediaAssertions: The number of processes playing audible media is now greater than zero. Taking UI process assertion.");
     2370    m_audibleMediaActivity = AudibleMediaActivity {
     2371        makeUniqueRef<ProcessAssertion>(getCurrentProcessID(), "WebKit Media Playback"_s, ProcessAssertionType::MediaPlayback)
    23652372#if ENABLE(GPU_PROCESS)
    2366         if (GPUProcessProxy::singletonIfCreated())
    2367             m_gpuProcessMediaPlaybackAssertion = makeUnique<ProcessAssertion>(GPUProcessProxy::singleton().processIdentifier(), "WebKit Media Playback"_s, ProcessAssertionType::MediaPlayback);
    2368 #endif
    2369     }
    2370 
    2371     auto result = m_processesPlayingAudibleMedia.add(processID, nullptr);
    2372     ASSERT(result.isNewEntry);
    2373     result.iterator->value = makeUnique<ProcessAssertion>(process->processIdentifier(), "WebKit Media Playback"_s, ProcessAssertionType::MediaPlayback);
    2374 }
    2375 
    2376 void WebProcessPool::clearWebProcessIsPlayingAudibleMedia(WebCore::ProcessIdentifier processID)
    2377 {
    2378     auto result = m_processesPlayingAudibleMedia.take(processID);
    2379     ASSERT_UNUSED(result, result);
    2380 
    2381     auto* process = WebProcessProxy::processForIdentifier(processID);
    2382     ASSERT_UNUSED(process, process);
    2383 
    2384     WEBPROCESSPOOL_RELEASE_LOG(ProcessSuspension, "clearWebProcessIsPlayingAudibleMedia: Web process is no longer playing audible media (process=%p, PID=%i)", process, process->processIdentifier());
    2385 
    2386     if (m_processesPlayingAudibleMedia.isEmpty()) {
    2387         WEBPROCESSPOOL_RELEASE_LOG(ProcessSuspension, "clearWebProcessIsPlayingAudibleMedia: The number of processes playing audible media now zero. Releasing UI process assertion.");
    2388 
    2389         ASSERT(m_uiProcessMediaPlaybackAssertion);
    2390         m_uiProcessMediaPlaybackAssertion = nullptr;
    2391         m_gpuProcessMediaPlaybackAssertion = nullptr;
    2392     }
     2373        , GPUProcessProxy::singletonIfCreated() ? makeUnique<ProcessAssertion>(GPUProcessProxy::singleton().processIdentifier(), "WebKit Media Playback"_s, ProcessAssertionType::MediaPlayback) : nullptr
     2374#endif
     2375    };
    23932376}
    23942377
  • trunk/Source/WebKit/UIProcess/WebProcessPool.h

    r260283 r260373  
    518518#endif
    519519
    520     void setWebProcessIsPlayingAudibleMedia(WebCore::ProcessIdentifier);
    521     void clearWebProcessIsPlayingAudibleMedia(WebCore::ProcessIdentifier);
     520    WebProcessWithAudibleMediaToken webProcessWithAudibleMediaToken() const;
    522521
    523522    void disableDelayedWebProcessLaunch() { m_isDelayedWebProcessLaunchDisabled = true; }
     
    563562
    564563    void updateProcessAssertions();
     564    void updateAudibleMediaAssertions();
    565565
    566566    // IPC::MessageReceiver.
     
    788788#endif
    789789
    790     HashMap<WebCore::ProcessIdentifier, std::unique_ptr<ProcessAssertion>> m_processesPlayingAudibleMedia;
    791     std::unique_ptr<ProcessAssertion> m_uiProcessMediaPlaybackAssertion;
    792     std::unique_ptr<ProcessAssertion> m_gpuProcessMediaPlaybackAssertion;
     790    WebProcessWithAudibleMediaCounter m_webProcessWithAudibleMediaCounter;
     791
     792    struct AudibleMediaActivity {
     793        UniqueRef<ProcessAssertion> uiProcessMediaPlaybackAssertion;
     794#if ENABLE(GPU_PROCESS)
     795        std::unique_ptr<ProcessAssertion> gpuProcessMediaPlaybackAssertion;
     796#endif
     797    };
     798    Optional<AudibleMediaActivity> m_audibleMediaActivity;
    793799
    794800#if PLATFORM(IOS)
  • trunk/Source/WebKit/UIProcess/WebProcessProxy.cpp

    r260283 r260373  
    415415    m_backgroundResponsivenessTimer.invalidate();
    416416    m_activityForHoldingLockedFiles = nullptr;
     417    m_audibleMediaActivity = WTF::nullopt;
    417418
    418419    for (auto& frame : copyToVector(m_frameMap.values()))
     
    514515    removeVisitedLinkStoreUser(webPage.visitedLinkStore(), webPage.identifier());
    515516    updateRegistrationWithDataStore();
    516 
     517    updateAudibleMediaAssertions();
    517518    updateBackgroundResponsivenessTimer();
    518519
     
    13831384}
    13841385
    1385 void WebProcessProxy::webPageMediaStateDidChange(WebPageProxy&)
     1386void WebProcessProxy::updateAudibleMediaAssertions()
    13861387{
    13871388    bool newHasAudibleWebPage = WTF::anyOf(m_pageMap.values(), [] (auto& page) { return page->isPlayingAudio(); });
    1388     if (m_hasAudibleWebPage == newHasAudibleWebPage)
    1389         return;
    1390     m_hasAudibleWebPage = newHasAudibleWebPage;
    1391 
    1392     if (m_hasAudibleWebPage)
    1393         processPool().setWebProcessIsPlayingAudibleMedia(coreProcessIdentifier());
    1394     else
    1395         processPool().clearWebProcessIsPlayingAudibleMedia(coreProcessIdentifier());
     1389
     1390    bool hasAudibleMediaActivity = !!m_audibleMediaActivity;
     1391    if (hasAudibleMediaActivity == newHasAudibleWebPage)
     1392        return;
     1393
     1394    if (newHasAudibleWebPage) {
     1395        RELEASE_LOG(ProcessSuspension, "%p - Taking MediaPlayback assertion for WebProcess with PID %d", this, processIdentifier());
     1396        m_audibleMediaActivity = AudibleMediaActivity {
     1397            makeUniqueRef<ProcessAssertion>(processIdentifier(), "WebKit Media Playback"_s, ProcessAssertionType::MediaPlayback),
     1398            processPool().webProcessWithAudibleMediaToken()
     1399        };
     1400    } else {
     1401        RELEASE_LOG(ProcessSuspension, "%p - Releasing MediaPlayback assertion for WebProcess with PID %d", this, processIdentifier());
     1402        m_audibleMediaActivity = WTF::nullopt;
     1403    }
    13961404}
    13971405
  • trunk/Source/WebKit/UIProcess/WebProcessProxy.h

    r260283 r260373  
    107107typedef RefCounter<BackgroundWebProcessCounterType> BackgroundWebProcessCounter;
    108108typedef BackgroundWebProcessCounter::Token BackgroundWebProcessToken;
     109enum WebProcessWithAudibleMediaCounterType { };
     110using WebProcessWithAudibleMediaCounter = RefCounter<WebProcessWithAudibleMediaCounterType>;
     111using WebProcessWithAudibleMediaToken = WebProcessWithAudibleMediaCounter::Token;
    109112
    110113class WebProcessProxy : public AuxiliaryProcessProxy, public ResponsivenessTimer::Client, public ThreadSafeRefCounted<WebProcessProxy>, public CanMakeWeakPtr<WebProcessProxy>, private ProcessThrottlerClient {
     
    346349#endif
    347350
    348     void webPageMediaStateDidChange(WebPageProxy&);
     351    void updateAudibleMediaAssertions();
    349352
    350353    void ref() final { ThreadSafeRefCounted::ref(); }
     
    569572    bool m_hasCommittedAnyProvisionalLoads { false };
    570573    bool m_isPrewarmed;
    571     bool m_hasAudibleWebPage { false };
    572574#if ENABLE(ATTACHMENT_ELEMENT) && PLATFORM(IOS_FAMILY)
    573575    bool m_hasIssuedAttachmentElementRelatedSandboxExtensions { false };
     
    594596
    595597    HashMap<WebCore::SleepDisablerIdentifier, std::unique_ptr<WebCore::SleepDisabler>> m_sleepDisablers;
     598
     599    struct AudibleMediaActivity {
     600        UniqueRef<ProcessAssertion> assertion;
     601        WebProcessWithAudibleMediaToken token;
     602    };
     603    Optional<AudibleMediaActivity> m_audibleMediaActivity;
    596604};
    597605
Note: See TracChangeset for help on using the changeset viewer.