Changeset 251048 in webkit


Ignore:
Timestamp:
Oct 12, 2019 9:17:52 AM (5 years ago)
Author:
Chris Dumez
Message:

Clearing Website data for a given session should not shut down cached processes for other sessions
​https://bugs.webkit.org/show_bug.cgi?id=202865
<rdar://problem/56202912>

Reviewed by Antti Koivisto.

When clearing Website data for a given data store, we now only clear cached processes
(either if BackForwardCache or WebProcessCache) if they are associated with this
particular data store. It is very wasteful otherwise.

  • UIProcess/ProvisionalPageProxy.cpp:

(WebKit::ProvisionalPageProxy::~ProvisionalPageProxy):

  • UIProcess/SuspendedPageProxy.cpp:

(WebKit::SuspendedPageProxy::~SuspendedPageProxy):
ProvisionalPageProxy & SuspendedPageProxy's destructors no longer call
WebProcessProxy::maybeShutdown() asynchronously. We now call maybeShutdown()
synchronously in WebProcessProxy::decrementSuspendedPageCount() and
WebProcessProxy::removeProvisionalPageProxy() instead. This makes things a
lot more predictable.

  • UIProcess/WebBackForwardCache.cpp:

(WebKit::WebBackForwardCache::removeEntriesForSession):
Add new removeEntriesForSession() method to clear only back / forward cache
entries associated with a particular session.

(WebKit::WebBackForwardCache::clear):
Stop taking a parameter indicating if we allow the processes to enter the
process cache. Now that we call maybeShutdown() synchronously when destroying
a SuspendedPageProxy, we can simply allow the processes to enter the process
cache unconditionally. If the caller does not want this processes in the page
cache, they can clear the process cache afterwards.

  • UIProcess/WebBackForwardCache.h:
  • UIProcess/WebPageProxy.cpp:

(WebKit::WebPageProxy::receivedNavigationPolicyDecision):
Now that destroying a SuspendedPageProxy or a ProvisionalPageProxy may shutdown
their process synchronously, add a scope here to prevent shutdown of the process
for the duration of this scope, since we're about to use it for a navigation and
we don't want it to get shutdown, even if there is no longer anything using it.

(WebKit::WebPageProxy::continueNavigationInNewProcess):
Add new assertion and rename parameter for consistency.

  • UIProcess/WebProcessPool.cpp:

(WebKit::WebProcessPool::handleMemoryPressureWarning):

  • UIProcess/WebProcessProxy.cpp:

(WebKit::WebProcessProxy::addProvisionalPageProxy):
Add new assertion.

(WebKit::WebProcessProxy::removeProvisionalPageProxy):
Call maybeShutDown() if this was the last provisional page.

(WebKit::WebProcessProxy::maybeShutDown):
Drop parameter indicating if we want to allow the process to enter the process
cache and allow caching unconditionally.

(WebKit::WebProcessProxy::canTerminateAuxiliaryProcess):
Prevent termination if there is a ScopePreventingShutdown.

(WebKit::WebProcessProxy::decrementSuspendedPageCount):
Call maybeShutDown() if this was the last suspended page.

  • UIProcess/WebProcessProxy.h:

(WebKit::WebProcessProxy::ScopePreventingShutdown::ScopePreventingShutdown):
(WebKit::WebProcessProxy::ScopePreventingShutdown::~ScopePreventingShutdown):
(WebKit::WebProcessProxy::makeScopePreventingShutdown):
Add new facility to prevent shutdown of a process for the duration of the scope.

  • UIProcess/WebsiteData/WebsiteDataStore.cpp:

(WebKit::WebsiteDataStore::removeData):
When removing website data, only clear cached processes if they are associated with
the current data store.

Location:
trunk/Source/WebKit
Files:
10 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebKit/ChangeLog

    r251045 r251048  
     12019-10-12  Chris Dumez  <cdumez@apple.com>
     2
     3        Clearing Website data for a given session should not shut down cached processes for other sessions
     4        https://bugs.webkit.org/show_bug.cgi?id=202865
     5        <rdar://problem/56202912>
     6
     7        Reviewed by Antti Koivisto.
     8
     9        When clearing Website data for a given data store, we now only clear cached processes
     10        (either if BackForwardCache or WebProcessCache) if they are associated with this
     11        particular data store. It is very wasteful otherwise.
     12
     13        * UIProcess/ProvisionalPageProxy.cpp:
     14        (WebKit::ProvisionalPageProxy::~ProvisionalPageProxy):
     15        * UIProcess/SuspendedPageProxy.cpp:
     16        (WebKit::SuspendedPageProxy::~SuspendedPageProxy):
     17        ProvisionalPageProxy & SuspendedPageProxy's destructors no longer call
     18        WebProcessProxy::maybeShutdown() asynchronously. We now call maybeShutdown()
     19        synchronously in WebProcessProxy::decrementSuspendedPageCount() and
     20        WebProcessProxy::removeProvisionalPageProxy() instead. This makes things a
     21        lot more predictable.
     22
     23        * UIProcess/WebBackForwardCache.cpp:
     24        (WebKit::WebBackForwardCache::removeEntriesForSession):
     25        Add new removeEntriesForSession() method to clear only back / forward cache
     26        entries associated with a particular session.
     27
     28        (WebKit::WebBackForwardCache::clear):
     29        Stop taking a parameter indicating if we allow the processes to enter the
     30        process cache. Now that we call maybeShutdown() synchronously when destroying
     31        a SuspendedPageProxy, we can simply allow the processes to enter the process
     32        cache unconditionally. If the caller does not want this processes in the page
     33        cache, they can clear the process cache afterwards.
     34
     35        * UIProcess/WebBackForwardCache.h:
     36        * UIProcess/WebPageProxy.cpp:
     37        (WebKit::WebPageProxy::receivedNavigationPolicyDecision):
     38        Now that destroying a SuspendedPageProxy or a ProvisionalPageProxy may shutdown
     39        their process synchronously, add a scope here to prevent shutdown of the process
     40        for the duration of this scope, since we're about to use it for a navigation and
     41        we don't want it to get shutdown, even if there is no longer anything using it.
     42
     43        (WebKit::WebPageProxy::continueNavigationInNewProcess):
     44        Add new assertion and rename parameter for consistency.
     45
     46        * UIProcess/WebProcessPool.cpp:
     47        (WebKit::WebProcessPool::handleMemoryPressureWarning):
     48
     49        * UIProcess/WebProcessProxy.cpp:
     50        (WebKit::WebProcessProxy::addProvisionalPageProxy):
     51        Add new assertion.
     52
     53        (WebKit::WebProcessProxy::removeProvisionalPageProxy):
     54        Call maybeShutDown() if this was the last provisional page.
     55
     56        (WebKit::WebProcessProxy::maybeShutDown):
     57        Drop parameter indicating if we want to allow the process to enter the process
     58        cache and allow caching unconditionally.
     59
     60        (WebKit::WebProcessProxy::canTerminateAuxiliaryProcess):
     61        Prevent termination if there is a ScopePreventingShutdown.
     62
     63        (WebKit::WebProcessProxy::decrementSuspendedPageCount):
     64        Call maybeShutDown() if this was the last suspended page.
     65
     66        * UIProcess/WebProcessProxy.h:
     67        (WebKit::WebProcessProxy::ScopePreventingShutdown::ScopePreventingShutdown):
     68        (WebKit::WebProcessProxy::ScopePreventingShutdown::~ScopePreventingShutdown):
     69        (WebKit::WebProcessProxy::makeScopePreventingShutdown):
     70        Add new facility to prevent shutdown of a process for the duration of the scope.
     71
     72        * UIProcess/WebsiteData/WebsiteDataStore.cpp:
     73        (WebKit::WebsiteDataStore::removeData):
     74        When removing website data, only clear cached processes if they are associated with
     75        the current data store.
     76
    1772019-10-12  Chris Fleizach  <cfleizach@apple.com>
    278

  • trunk/Source/WebKit/UIProcess/ProvisionalPageProxy.cpp

    r249329 r251048  
    8989ProvisionalPageProxy::~ProvisionalPageProxy()
    9090{
     91    if (!m_wasCommitted) {
     92        if (&m_process->websiteDataStore() != &m_page.websiteDataStore())
     93            m_process->processPool().pageEndUsingWebsiteDataStore(m_page.identifier(), m_process->websiteDataStore());
     94
     95        m_process->removeMessageReceiver(Messages::WebPageProxy::messageReceiverName(), m_webPageID);
     96        m_process->send(Messages::WebPage::Close(), m_webPageID);
     97        m_process->removeVisitedLinkStoreUser(m_page.visitedLinkStore(), m_page.identifier());
     98    }
     99
    91100    m_process->removeProvisionalPageProxy(*this);
    92 
    93     if (m_wasCommitted)
    94         return;
    95 
    96     if (&m_process->websiteDataStore() != &m_page.websiteDataStore())
    97         m_process->processPool().pageEndUsingWebsiteDataStore(m_page.identifier(), m_process->websiteDataStore());
    98 
    99     m_process->removeMessageReceiver(Messages::WebPageProxy::messageReceiverName(), m_webPageID);
    100     m_process->send(Messages::WebPage::Close(), m_webPageID);
    101     m_process->removeVisitedLinkStoreUser(m_page.visitedLinkStore(), m_page.identifier());
    102 
    103     RunLoop::main().dispatch([process = m_process.copyRef()] {
    104         process->maybeShutDown();
    105     });
    106101}
    107102

  • trunk/Source/WebKit/UIProcess/SuspendedPageProxy.cpp

    r251022 r251048  
    117117SuspendedPageProxy::~SuspendedPageProxy()
    118118{
    119     m_process->decrementSuspendedPageCount();
    120119    allSuspendedPages().remove(this);
    121120
    … …  
    126125    }
    127126
    128     if (m_suspensionState == SuspensionState::Resumed)
    129         return;
    130 
    131     // If the suspended page was not consumed before getting destroyed, then close the corresponding page
    132     // on the WebProcess side.
    133     close();
    134 
    135     if (m_suspensionState == SuspensionState::Suspending)
    136         m_process->removeMessageReceiver(Messages::WebPageProxy::messageReceiverName(), m_webPageID);
    137 
    138     // We call maybeShutDown() asynchronously since the SuspendedPage is currently being removed from the WebProcessPool
    139     // and we want to avoid re-entering WebProcessPool methods.
    140     RunLoop::main().dispatch([process = m_process.copyRef()] {
    141         process->maybeShutDown();
    142     });
     127    if (m_suspensionState != SuspensionState::Resumed) {
     128        // If the suspended page was not consumed before getting destroyed, then close the corresponding page
     129        // on the WebProcess side.
     130        close();
     131
     132        if (m_suspensionState == SuspensionState::Suspending)
     133            m_process->removeMessageReceiver(Messages::WebPageProxy::messageReceiverName(), m_webPageID);
     134    }
     135
     136    m_process->decrementSuspendedPageCount();
    143137}
    144138

  • trunk/Source/WebKit/UIProcess/WebBackForwardCache.cpp

    r251022 r251048  
    3131#include "WebPageProxy.h"
    3232#include "WebProcessProxy.h"
     33#include "WebsiteDataStore.h"
    3334
    3435namespace WebKit {
    … …  
    8485}
    8586
     87void WebBackForwardCache::removeEntriesForSession(PAL::SessionID sessionID)
     88{
     89    removeEntriesMatching([sessionID](auto& item) {
     90        return item.suspendedPage()->process().websiteDataStore().sessionID() == sessionID;
     91    });
     92}
     93
    8694void WebBackForwardCache::removeEntriesForPage(WebPageProxy& page)
    8795{
    … …  
    103111}
    104112
    105 void WebBackForwardCache::clear(AllowProcessCaching allowProcessCaching)
     113void WebBackForwardCache::clear()
    106114{
    107115    auto itemsWithCachedPage = WTFMove(m_itemsWithCachedPage);
    108     for (auto* item : itemsWithCachedPage) {
    109         auto process = makeRef(item->suspendedPage()->process());
     116    for (auto* item : itemsWithCachedPage)
    110117        item->setSuspendedPage(nullptr);
    111         process->maybeShutDown(allowProcessCaching);
    112     }
    113118}
    114119

  • trunk/Source/WebKit/UIProcess/WebBackForwardCache.h

    r251022 r251048  
    2626#pragma once
    2727
     28#include <pal/SessionID.h>
    2829#include <wtf/Forward.h>
    2930#include <wtf/ListHashSet.h>
    … …  
    3536class WebPageProxy;
    3637class WebProcessProxy;
    37 enum class AllowProcessCaching;
    3838
    3939class WebBackForwardCache {
    … …  
    4747    unsigned size() const { return m_itemsWithCachedPage.size(); }
    4848
    49     void clear(AllowProcessCaching);
     49    void clear();
    5050    void removeEntriesForProcess(WebProcessProxy&);
    5151    void removeEntriesForPage(WebPageProxy&);
     52    void removeEntriesForSession(PAL::SessionID);
    5253
    5354    void addEntry(WebBackForwardListItem&, std::unique_ptr<SuspendedPageProxy>&&);

  • trunk/Source/WebKit/UIProcess/WebPageProxy.cpp

    r251022 r251048  
    29552955
    29562956        if (shouldProcessSwap) {
     2957            // Make sure the process to be used for the navigation does not get shutDown now due to destroying SuspendedPageProxy or ProvisionalPageProxy objects.
     2958            auto preventNavigationProcessShutdown = processForNavigation->makeScopePreventingShutdown();
     2959
    29572960            auto suspendedPage = destinationSuspendedPage ? backForwardCache().takeEntry(*destinationSuspendedPage->backForwardListItem()) : nullptr;
    29582961            if (suspendedPage && suspendedPage->pageIsClosedOrClosing())
    … …  
    30363039}
    30373040
    3038 void WebPageProxy::continueNavigationInNewProcess(API::Navigation& navigation, std::unique_ptr<SuspendedPageProxy>&& suspendedPageProxy, Ref<WebProcessProxy>&& newProcess, ProcessSwapRequestedByClient processSwapRequestedByClient, Optional<WebsitePoliciesData>&& websitePolicies)
    3039 {
    3040     RELEASE_LOG_IF_ALLOWED(Loading, "continueNavigationInNewProcess: newProcessPID = %i, hasSuspendedPage = %i", newProcess->processIdentifier(), !!suspendedPageProxy);
     3041void WebPageProxy::continueNavigationInNewProcess(API::Navigation& navigation, std::unique_ptr<SuspendedPageProxy>&& suspendedPage, Ref<WebProcessProxy>&& newProcess, ProcessSwapRequestedByClient processSwapRequestedByClient, Optional<WebsitePoliciesData>&& websitePolicies)
     3042{
     3043    RELEASE_LOG_IF_ALLOWED(Loading, "continueNavigationInNewProcess: newProcessPID = %i, hasSuspendedPage = %i", newProcess->processIdentifier(), !!suspendedPage);
    30413044    LOG(Loading, "Continuing navigation %" PRIu64 " '%s' in a new web process", navigation.navigationID(), navigation.loggingString());
     3045    RELEASE_ASSERT(!newProcess->isInProcessCache());
    30423046
    30433047    if (m_provisionalPage) {
    … …  
    30483052    }
    30493053
    3050     m_provisionalPage = makeUnique<ProvisionalPageProxy>(*this, newProcess.copyRef(), WTFMove(suspendedPageProxy), navigation.navigationID(), navigation.currentRequestIsRedirect(), navigation.currentRequest(), processSwapRequestedByClient);
     3054    m_provisionalPage = makeUnique<ProvisionalPageProxy>(*this, newProcess.copyRef(), WTFMove(suspendedPage), navigation.navigationID(), navigation.currentRequestIsRedirect(), navigation.currentRequest(), processSwapRequestedByClient);
    30513055
    30523056    if (auto* item = navigation.targetItem()) {

  • trunk/Source/WebKit/UIProcess/WebProcessPool.cpp

    r251041 r251048  
    13871387    RELEASE_LOG(PerformanceLogging, "%p - WebProcessPool::handleMemoryPressureWarning", this);
    13881388
    1389 
    1390     m_backForwardCache->clear(AllowProcessCaching::No);
     1389    // Clear back/forward cache first as processes removed from the back/forward cache will likely
     1390    // be added to the WebProcess cache.
     1391    m_backForwardCache->clear();
    13911392    m_webProcessCache->clear();
    13921393

  • trunk/Source/WebKit/UIProcess/WebProcessProxy.cpp

    r250799 r251048  
    249249void WebProcessProxy::addProvisionalPageProxy(ProvisionalPageProxy& provisionalPage)
    250250{
     251    ASSERT(!m_isInProcessCache);
    251252    ASSERT(!m_provisionalPages.contains(&provisionalPage));
    252253    m_provisionalPages.add(&provisionalPage);
    … …  
    259260    m_provisionalPages.remove(&provisionalPage);
    260261    updateRegistrationWithDataStore();
     262    if (m_provisionalPages.isEmpty())
     263        maybeShutDown();
    261264}
    262265
    … …  
    938941}
    939942
    940 void WebProcessProxy::maybeShutDown(AllowProcessCaching allowProcessCaching)
     943void WebProcessProxy::maybeShutDown()
    941944{
    942945    if (processPool().dummyProcessProxy() == this && m_pageMap.isEmpty()) {
    … …  
    949952        return;
    950953
    951     if (allowProcessCaching == AllowProcessCaching::Yes && canBeAddedToWebProcessCache() && processPool().webProcessCache().addProcessIfPossible(*this))
     954    if (canBeAddedToWebProcessCache() && processPool().webProcessCache().addProcessIfPossible(*this))
    952955        return;
    953956
    … …  
    957960bool WebProcessProxy::canTerminateAuxiliaryProcess()
    958961{
    959     if (!m_pageMap.isEmpty() || m_suspendedPageCount || !m_provisionalPages.isEmpty() || m_isInProcessCache)
     962    if (!m_pageMap.isEmpty() || m_suspendedPageCount || !m_provisionalPages.isEmpty() || m_isInProcessCache || m_shutdownPreventingScopeCount)
    960963        return false;
    961964
    … …  
    14841487    ASSERT(m_suspendedPageCount);
    14851488    --m_suspendedPageCount;
    1486     if (!m_suspendedPageCount)
     1489    if (!m_suspendedPageCount) {
    14871490        send(Messages::WebProcess::SetHasSuspendedPageProxy(false), 0);
     1491        maybeShutDown();
     1492    }
    14881493}
    14891494

  • trunk/Source/WebKit/UIProcess/WebProcessProxy.h

    r250799 r251048  
    9595#endif
    9696
    97 enum class AllowProcessCaching { No, Yes };
    98 
    9997class WebProcessProxy : public AuxiliaryProcessProxy, public ResponsivenessTimer::Client, public ThreadSafeRefCounted<WebProcessProxy>, public CanMakeWeakPtr<WebProcessProxy>, private ProcessThrottlerClient {
    10098public:
    … …  
    273271    // Will potentially cause the WebProcessProxy object to be freed.
    274272    void shutDown();
    275     void maybeShutDown(AllowProcessCaching = AllowProcessCaching::Yes);
     273
     274    class ScopePreventingShutdown {
     275    public:
     276        explicit ScopePreventingShutdown(WebProcessProxy& process)
     277            : m_process(process)
     278        {
     279            ++(m_process->m_shutdownPreventingScopeCount);
     280        }
     281
     282        ~ScopePreventingShutdown()
     283        {
     284            ASSERT(m_process->m_shutdownPreventingScopeCount);
     285            if (!--(m_process->m_shutdownPreventingScopeCount))
     286                m_process->maybeShutDown();
     287        }
     288
     289    private:
     290        Ref<WebProcessProxy> m_process;
     291    };
     292
     293    ScopePreventingShutdown makeScopePreventingShutdown() { return ScopePreventingShutdown { *this }; }
    276294
    277295    void didStartProvisionalLoadForMainFrame(const URL&);
    … …  
    405423    void updateRegistrationWithDataStore();
    406424
     425    void maybeShutDown();
     426
    407427    enum class IsWeak { No, Yes };
    408428    template<typename T> class WeakOrStrongPtr {
    … …  
    483503
    484504    unsigned m_suspendedPageCount { 0 };
     505    unsigned m_shutdownPreventingScopeCount { 0 };
    485506    bool m_hasCommittedAnyProvisionalLoads { false };
    486507    bool m_isPrewarmed;

  • trunk/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp

    r251022 r251048  
    727727    if (webProcessAccessType != ProcessAccessType::None) {
    728728        for (auto& processPool : processPools()) {
    729             processPool->backForwardCache().clear(AllowProcessCaching::No);
    730             processPool->webProcessCache().clear();
     729            // Clear back/forward cache first as processes removed from the back/forward cache will likely
     730            // be added to the WebProcess cache.
     731            processPool->backForwardCache().removeEntriesForSession(sessionID());
     732            processPool->webProcessCache().clearAllProcessesForSession(sessionID());
    731733        }
    732734
Note: See TracChangeset for help on using the changeset viewer.