Changeset 260856 in webkit


Ignore:
Timestamp:
Apr 28, 2020 5:03:23 PM (4 years ago)
Author:
Chris Dumez
Message:

[iOS][WK2] Drop process assertion logic for holding locked files
​https://bugs.webkit.org/show_bug.cgi?id=206910

Reviewed by Alex Christensen.

Drop process assertion logic for holding locked files as it is causing issues and
should no longer be needed.

The intention of this code was that the Network Process would send a
SetIsHoldingLockedFiles(bool) IPC to the UIProcess whenever there are pending
SQLite transactions and the UIProcess would then take a process assertion on behalf
of the WebProcess (or release it) to prevent / allow suspension.

This approach has some issues:

  1. It was noisy in terms of IPC
  2. Because it relies on an IPC to the UIProcess to keep running, it was inherently racy
  3. We already have logic to deal with suspension in the network / web processes. The UIProcess sends a PrepareForSuspension IPC to its child processes. In turn, those processes will make sure to suspend their database threads to prevent any further database work. The child processes would then respond to the PrepareForSuspension IPC to let the UIProcess know they are ready to suspend. The 2 approaches were conflicting, because after the PrepareForSuspension IPC is sent, the child process could process a transaction and send the SetIsHoldingLockedFiles() IPC to the UIProcess which would incorrectly cancel the suspension.
  • NetworkProcess/NetworkProcess.cpp:

(WebKit::NetworkProcess::NetworkProcess):
(WebKit::m_messagePortChannelRegistry): Deleted.

  • NetworkProcess/NetworkProcess.h:
  • Shared/WebSQLiteDatabaseTracker.cpp: Removed.
  • Shared/WebSQLiteDatabaseTracker.h: Removed.
  • UIProcess/Network/NetworkProcessProxy.cpp:

(WebKit::NetworkProcessProxy::didClose):
(WebKit::NetworkProcessProxy::didSetAssertionState):
(WebKit::NetworkProcessProxy::setIsHoldingLockedFiles): Deleted.

  • UIProcess/Network/NetworkProcessProxy.h:
  • UIProcess/Network/NetworkProcessProxy.messages.in:
  • UIProcess/WebProcessProxy.cpp:

(WebKit::WebProcessProxy::shutDown):
(WebKit::WebProcessProxy::setIsHoldingLockedFiles): Deleted.

  • UIProcess/WebProcessProxy.h:
  • UIProcess/WebProcessProxy.messages.in:
  • WebKit.xcodeproj/project.pbxproj:
  • WebProcess/WebProcess.cpp:

(WebKit::m_nonVisibleProcessCleanupTimer):
(WebKit::m_webSQLiteDatabaseTracker): Deleted.

  • WebProcess/WebProcess.h:
Location:
trunk/Source/WebKit
Files:
2 deleted
13 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebKit/ChangeLog

    r260851 r260856  
     12020-04-28  Chris Dumez  <cdumez@apple.com>
     2
     3        [iOS][WK2] Drop process assertion logic for holding locked files
     4        https://bugs.webkit.org/show_bug.cgi?id=206910
     5
     6        Reviewed by Alex Christensen.
     7
     8        Drop process assertion logic for holding locked files as it is causing issues and
     9        should no longer be needed.
     10
     11        The intention of this code was that the Network Process would send a
     12        SetIsHoldingLockedFiles(bool) IPC to the UIProcess whenever there are pending
     13        SQLite transactions and the UIProcess would then take a process assertion on behalf
     14        of the WebProcess (or release it) to prevent / allow suspension.
     15
     16        This approach has some issues:
     17        1. It was noisy in terms of IPC
     18        2. Because it relies on an IPC to the UIProcess to keep running, it was inherently
     19           racy
     20        3. We already have logic to deal with suspension in the network / web processes.
     21           The UIProcess sends a PrepareForSuspension IPC to its child processes. In turn,
     22           those processes will make sure to suspend their database threads to prevent any
     23           further database work. The child processes would then respond to the
     24           PrepareForSuspension IPC to let the UIProcess know they are ready to suspend.
     25           The 2 approaches were conflicting, because after the PrepareForSuspension IPC is
     26           sent, the child process could process a transaction and send the
     27           SetIsHoldingLockedFiles() IPC to the UIProcess which would incorrectly cancel the
     28           suspension.
     29
     30        * NetworkProcess/NetworkProcess.cpp:
     31        (WebKit::NetworkProcess::NetworkProcess):
     32        (WebKit::m_messagePortChannelRegistry): Deleted.
     33        * NetworkProcess/NetworkProcess.h:
     34        * Shared/WebSQLiteDatabaseTracker.cpp: Removed.
     35        * Shared/WebSQLiteDatabaseTracker.h: Removed.
     36        * UIProcess/Network/NetworkProcessProxy.cpp:
     37        (WebKit::NetworkProcessProxy::didClose):
     38        (WebKit::NetworkProcessProxy::didSetAssertionState):
     39        (WebKit::NetworkProcessProxy::setIsHoldingLockedFiles): Deleted.
     40        * UIProcess/Network/NetworkProcessProxy.h:
     41        * UIProcess/Network/NetworkProcessProxy.messages.in:
     42        * UIProcess/WebProcessProxy.cpp:
     43        (WebKit::WebProcessProxy::shutDown):
     44        (WebKit::WebProcessProxy::setIsHoldingLockedFiles): Deleted.
     45        * UIProcess/WebProcessProxy.h:
     46        * UIProcess/WebProcessProxy.messages.in:
     47        * WebKit.xcodeproj/project.pbxproj:
     48        * WebProcess/WebProcess.cpp:
     49        (WebKit::m_nonVisibleProcessCleanupTimer):
     50        (WebKit::m_webSQLiteDatabaseTracker): Deleted.
     51        * WebProcess/WebProcess.h:
     52
    1532020-04-28  Noam Rosenthal  <noam@webkit.org>
    254

  • trunk/Source/WebKit/NetworkProcess/NetworkProcess.cpp

    r260841 r260856  
    152152    , m_networkContentRuleListManager(*this)
    153153#endif
    154 #if PLATFORM(IOS_FAMILY)
    155     , m_webSQLiteDatabaseTracker([this](bool isHoldingLockedFiles) { parentProcessConnection()->send(Messages::NetworkProcessProxy::SetIsHoldingLockedFiles(isHoldingLockedFiles), 0); })
    156 #endif
    157154    , m_messagePortChannelRegistry(createMessagePortChannelRegistry(*this))
    158155{
    … …  
    22112208#if PLATFORM(IOS_FAMILY) && ENABLE(INDEXED_DATABASE)
    22122209    for (auto& server : m_webIDBServers.values())
    2213         server->suspend(isSuspensionImminent ? WebIDBServer::ShouldForceStop::Yes : WebIDBServer::ShouldForceStop::No);
    2214 #endif
    2215 
    2216 #if PLATFORM(IOS_FAMILY)
    2217     m_webSQLiteDatabaseTracker.setIsSuspended(true);
     2210        server->suspend(WebIDBServer::ShouldForceStop::Yes);
    22182211#endif
    22192212
    … …  
    22682261void NetworkProcess::resume()
    22692262{
    2270 #if PLATFORM(IOS_FAMILY)
    2271     m_webSQLiteDatabaseTracker.setIsSuspended(false);
    2272 #endif
    2273 
    22742263    platformProcessDidResume();
    22752264    for (auto& connection : m_webProcessConnections.values())

  • trunk/Source/WebKit/NetworkProcess/NetworkProcess.h

    r260841 r260856  
    5858#include <wtf/RetainPtr.h>
    5959#include <wtf/WeakPtr.h>
    60 
    61 #if PLATFORM(IOS_FAMILY)
    62 #include "WebSQLiteDatabaseTracker.h"
    63 #endif
    6460
    6561#if PLATFORM(COCOA)
    … …  
    554550#endif
    555551
    556 #if PLATFORM(IOS_FAMILY)
    557     WebSQLiteDatabaseTracker m_webSQLiteDatabaseTracker;
    558 #endif
    559 
    560552    Ref<WorkQueue> m_storageTaskQueue { WorkQueue::create("com.apple.WebKit.StorageTask") };
    561553

  • trunk/Source/WebKit/SourcesCocoa.txt

    r260739 r260856  
    135135Shared/FocusedElementInformation.cpp
    136136Shared/VisibleContentRectUpdateInfo.cpp
    137 Shared/WebSQLiteDatabaseTracker.cpp
    138137
    139138Shared/cf/ArgumentCodersCF.cpp @no-unify

  • trunk/Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp

    r260841 r260856  
    266266    m_customProtocolManagerProxy.invalidate();
    267267#endif
    268 
    269     m_activityForHoldingLockedFiles = nullptr;
    270268   
    271269    m_syncAllCookiesActivity = nullptr;
    … …  
    11811179        send(Messages::NetworkProcess::ProcessDidResume(), 0);
    11821180}
    1183    
    1184 void NetworkProcessProxy::setIsHoldingLockedFiles(bool isHoldingLockedFiles)
    1185 {
    1186     if (!isHoldingLockedFiles) {
    1187         RELEASE_LOG(ProcessSuspension, "UIProcess is releasing a background assertion because the Network process is no longer holding locked files");
    1188         m_activityForHoldingLockedFiles = nullptr;
    1189         return;
    1190     }
    1191     if (!m_activityForHoldingLockedFiles) {
    1192         RELEASE_LOG(ProcessSuspension, "UIProcess is taking a background assertion because the Network process is holding locked files");
    1193         m_activityForHoldingLockedFiles = m_throttler.backgroundActivity("Holding locked files"_s).moveToUniquePtr();
    1194     }
    1195 }
    11961181
    11971182void NetworkProcessProxy::syncAllCookies()

  • trunk/Source/WebKit/UIProcess/Network/NetworkProcessProxy.h

    r260841 r260856  
    184184    void synthesizeAppIsBackground(bool background);
    185185
    186     void setIsHoldingLockedFiles(bool);
    187 
    188186    void syncAllCookies();
    189187    void didSyncAllCookies();
    … …  
    304302#endif
    305303    ProcessThrottler m_throttler;
    306     std::unique_ptr<ProcessThrottler::BackgroundActivity> m_activityForHoldingLockedFiles;
    307304    std::unique_ptr<ProcessThrottler::BackgroundActivity> m_syncAllCookiesActivity;
    308305    ProcessThrottler::ActivityVariant m_activityFromWebProcesses;

  • trunk/Source/WebKit/UIProcess/Network/NetworkProcessProxy.messages.in

    r260658 r260856  
    3434    TestProcessIncomingSyncMessagesWhenWaitingForSyncReply(WebKit::WebPageProxyIdentifier pageID) -> (bool handled) Synchronous
    3535    TerminateUnresponsiveServiceWorkerProcesses(WebCore::ProcessIdentifier processIdentifier)
    36 
    37     SetIsHoldingLockedFiles(bool isHoldingLockedFiles)
    3836
    3937    # Diagnostic messages logging

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

    r260707 r260856  
    414414    m_responsivenessTimer.invalidate();
    415415    m_backgroundResponsivenessTimer.invalidate();
    416     m_activityForHoldingLockedFiles = nullptr;
    417416    m_audibleMediaActivity = WTF::nullopt;
    418417
    … …  
    14031402}
    14041403
    1405 void WebProcessProxy::setIsHoldingLockedFiles(bool isHoldingLockedFiles)
    1406 {
    1407     if (!isHoldingLockedFiles) {
    1408         RELEASE_LOG(ProcessSuspension, "UIProcess is releasing a background assertion because the WebContent process is no longer holding locked files");
    1409         m_activityForHoldingLockedFiles = nullptr;
    1410         return;
    1411     }
    1412     if (!m_activityForHoldingLockedFiles) {
    1413         RELEASE_LOG(ProcessSuspension, "UIProcess is taking a background assertion because the WebContent process is holding locked files");
    1414         m_activityForHoldingLockedFiles = m_throttler.backgroundActivity("Holding locked files"_s).moveToUniquePtr();
    1415     }
    1416 }
    1417 
    14181404void WebProcessProxy::isResponsive(CompletionHandler<void(bool isWebProcessResponsive)>&& callback)
    14191405{

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

    r260453 r260856  
    243243    void windowServerConnectionStateChanged();
    244244
    245     void setIsHoldingLockedFiles(bool);
    246 
    247245    ProcessThrottler& throttler() { return m_throttler; }
    248246
    … …  
    538536    int m_numberOfTimesSuddenTerminationWasDisabled;
    539537    ProcessThrottler m_throttler;
    540     std::unique_ptr<ProcessThrottler::BackgroundActivity> m_activityForHoldingLockedFiles;
    541538    ForegroundWebProcessToken m_foregroundToken;
    542539    BackgroundWebProcessToken m_backgroundToken;

  • trunk/Source/WebKit/UIProcess/WebProcessProxy.messages.in

    r259200 r260856  
    4343#endif
    4444
    45     SetIsHoldingLockedFiles(bool isHoldingLockedFiles)
    46 
    4745    DidExceedActiveMemoryLimit()
    4846    DidExceedInactiveMemoryLimit()

  • trunk/Source/WebKit/WebKit.xcodeproj/project.pbxproj

    r260842 r260856  
    13251325                832ED18C1E2FE157006BA64A /* PerActivityStateCPUUsageSampler.h in Headers */ = {isa = PBXBuildFile; fileRef = 832ED18A1E2FE13B006BA64A /* PerActivityStateCPUUsageSampler.h */; };
    13261326                834B250F1A831A8D00CFB150 /* NetworkCacheFileSystem.h in Headers */ = {isa = PBXBuildFile; fileRef = 834B250E1A831A8D00CFB150 /* NetworkCacheFileSystem.h */; };
    1327                 836034A01ACB34D600626549 /* WebSQLiteDatabaseTracker.h in Headers */ = {isa = PBXBuildFile; fileRef = 8360349E1ACB34D600626549 /* WebSQLiteDatabaseTracker.h */; };
    13281327                8372DB251A674C8F00C697C5 /* WKPageDiagnosticLoggingClient.h in Headers */ = {isa = PBXBuildFile; fileRef = 8372DB241A674C8F00C697C5 /* WKPageDiagnosticLoggingClient.h */; settings = {ATTRIBUTES = (Private, ); }; };
    13291328                8372DB291A67562800C697C5 /* WebPageDiagnosticLoggingClient.h in Headers */ = {isa = PBXBuildFile; fileRef = 8372DB271A67562800C697C5 /* WebPageDiagnosticLoggingClient.h */; };
    … …  
    43254324                83397C6722124BD100B62388 /* WebProcessCache.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = WebProcessCache.h; sourceTree = "<group>"; };
    43264325                834B250E1A831A8D00CFB150 /* NetworkCacheFileSystem.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = NetworkCacheFileSystem.h; sourceTree = "<group>"; };
    4327                 8360349D1ACB34D600626549 /* WebSQLiteDatabaseTracker.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = WebSQLiteDatabaseTracker.cpp; sourceTree = "<group>"; };
    4328                 8360349E1ACB34D600626549 /* WebSQLiteDatabaseTracker.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = WebSQLiteDatabaseTracker.h; sourceTree = "<group>"; };
    43294326                8372DB241A674C8F00C697C5 /* WKPageDiagnosticLoggingClient.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = WKPageDiagnosticLoggingClient.h; sourceTree = "<group>"; };
    43304327                8372DB261A67562800C697C5 /* WebPageDiagnosticLoggingClient.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = WebPageDiagnosticLoggingClient.cpp; sourceTree = "<group>"; };
    … …  
    61756172                                0EDE85022004E74900030560 /* WebsitePopUpPolicy.h */,
    61766173                                71FB810A2260627A00323677 /* WebsiteSimulatedMouseEventsDispatchPolicy.h */,
    6177                                 8360349D1ACB34D600626549 /* WebSQLiteDatabaseTracker.cpp */,
    6178                                 8360349E1ACB34D600626549 /* WebSQLiteDatabaseTracker.h */,
    61796174                                C0337DD7127A51B6008FF4F4 /* WebTouchEvent.cpp */,
    61806175                                7C065F291C8CD95F00C2D950 /* WebUserContentControllerDataTypes.cpp */,
    … …  
    1137111366                                417915B12256C0D600D6F97E /* WebSocketChannelManager.h in Headers */,
    1137211367                                C149380922347104000CD707 /* WebSpeechSynthesisClient.h in Headers */,
    11373                                 836034A01ACB34D600626549 /* WebSQLiteDatabaseTracker.h in Headers */,
    1137411368                                1A52C0F81A38CDC70016160A /* WebStorageNamespaceProvider.h in Headers */,
    1137511369                                9356F2DC2152B6B500E6D5DF /* WebSWClientConnection.h in Headers */,

  • trunk/Source/WebKit/WebProcess/WebProcess.cpp

    r260102 r260856  
    147147#endif
    148148
    149 #if PLATFORM(IOS_FAMILY)
    150 #include "WebSQLiteDatabaseTracker.h"
    151 #endif
    152 
    153149#if ENABLE(SEC_ITEM_SHIM)
    154150#include "SecItemShim.h"
    … …  
    228224#endif
    229225    , m_nonVisibleProcessCleanupTimer(*this, &WebProcess::nonVisibleProcessCleanupTimerFired)
    230 #if PLATFORM(IOS_FAMILY)
    231     , m_webSQLiteDatabaseTracker([this](bool isHoldingLockedFiles) { parentProcessConnection()->send(Messages::WebProcessProxy::SetIsHoldingLockedFiles(isHoldingLockedFiles), 0); })
    232 #endif
    233226{
    234227    // Initialize our platform strategies.
    … …  
    14401433
    14411434#if PLATFORM(IOS_FAMILY)
    1442     m_webSQLiteDatabaseTracker.setIsSuspended(true);
    14431435    SQLiteDatabase::setIsDatabaseOpeningForbidden(true);
    14441436    if (DatabaseTracker::isInitialized())
    … …  
    15191511   
    15201512#if PLATFORM(IOS_FAMILY)
    1521     m_webSQLiteDatabaseTracker.setIsSuspended(false);
    15221513    SQLiteDatabase::setIsDatabaseOpeningForbidden(false);
    15231514    accessibilityProcessSuspendedNotification(false);

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

    r260798 r260856  
    4040#include "WebPageProxyIdentifier.h"
    4141#include "WebProcessCreationParameters.h"
    42 #include "WebSQLiteDatabaseTracker.h"
    4342#include "WebSocketChannelManager.h"
    4443#include <WebCore/ActivityState.h>
    … …  
    580579
    581580#if PLATFORM(IOS_FAMILY)
    582     WebSQLiteDatabaseTracker m_webSQLiteDatabaseTracker;
    583581    std::unique_ptr<ProcessAssertion> m_uiProcessDependencyProcessAssertion;
    584582#endif
Note: See TracChangeset for help on using the changeset viewer.