Attachment 370976 Details for Bug 198201 – Patch for landing

[patch] Patch for landing

bug-198201-20190530134304.patch (text/plain), 15.81 KB, created by Sihui Liu on 2019-05-30 13:43:05 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Sihui Liu

Created: 2019-05-30 13:43:05 PDT

Size: 15.81 KB

patch

obsolete

>Subversion Revision: 245901
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index a475d662af977a52f6297efc25c990e7be77622b..e8bb30b8af12d198588205aba77e81308d4ac0e6 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,24 @@
>+2019-05-30  Sihui Liu  <sihui_liu@apple.com>
>+
>+        Stop StorageManager when network process is ready to suspend
>+        https://bugs.webkit.org/show_bug.cgi?id=198201
>+        <rdar://problem/49683172>
>+
>+        Reviewed by Youenn Fablet.
>+
>+        To avoid local storage database operations that can hold lock to database files, suspend thread of
>+        StorageManager when network process is about to suspend.
>+
>+        * NetworkProcess/NetworkProcess.cpp:
>+        (WebKit::NetworkProcess::actualPrepareToSuspend):
>+        (WebKit::NetworkProcess::resume):
>+        * NetworkProcess/NetworkSession.cpp:
>+        (WebKit::NetworkSession::~NetworkSession):
>+        * NetworkProcess/WebStorage/StorageManager.cpp:
>+        (WebKit::StorageManager::suspend):
>+        (WebKit::StorageManager::resume):
>+        * NetworkProcess/WebStorage/StorageManager.h:
>+
> 2019-05-30  David Quesada  <david_quesada@apple.com>
> 
>         REGRESSION (r245756) [Mac] 2 TestWebKitAPI.DownloadProgress* and TestWebKitAPI._WKDownload.DownloadMonitorCancel are flaky timeouts
>diff --git a/Source/WebKit/NetworkProcess/NetworkProcess.cpp b/Source/WebKit/NetworkProcess/NetworkProcess.cpp
>index e5c2beae58c8da7ba99a230c392493aae73296f6..dd9a1e3a8c5d76d2dd435303b7b8365ccae092e2 100644
>--- a/Source/WebKit/NetworkProcess/NetworkProcess.cpp
>+++ b/Source/WebKit/NetworkProcess/NetworkProcess.cpp
>@@ -2038,6 +2038,9 @@ void NetworkProcess::actualPrepareToSuspend(ShouldAcknowledgeWhenReadyToSuspend
>     for (auto& server : m_swServers.values())
>         server->startSuspension([delayedTaskCounter] { });
> #endif
>+
>+    for (auto& session : m_networkSessions)
>+        session.value->storageManager().suspend([delayedTaskCounter] { });
> }
> 
> void NetworkProcess::processWillSuspendImminently()
>@@ -2106,6 +2109,9 @@ void NetworkProcess::resume()
>     for (auto& server : m_idbServers.values())
>         server->resume();
> #endif
>+
>+    for (auto& session : m_networkSessions)
>+        session.value->storageManager().resume();
> }
> 
> void NetworkProcess::prefetchDNS(const String& hostname)
>diff --git a/Source/WebKit/NetworkProcess/NetworkSession.cpp b/Source/WebKit/NetworkProcess/NetworkSession.cpp
>index 94a123700e5f1709995439e29cb5f7c42b8ddd88..1d0a8ab9ba750b371749a51bc2b45b26d101c5b6 100644
>--- a/Source/WebKit/NetworkProcess/NetworkSession.cpp
>+++ b/Source/WebKit/NetworkProcess/NetworkSession.cpp
>@@ -90,6 +90,7 @@ NetworkSession::NetworkSession(NetworkProcess& networkProcess, PAL::SessionID se
> 
> NetworkSession::~NetworkSession()
> {
>+    m_storageManager->resume();
>     m_storageManager->waitUntilWritesFinished();
> }
> 
>diff --git a/Source/WebKit/NetworkProcess/WebStorage/StorageManager.cpp b/Source/WebKit/NetworkProcess/WebStorage/StorageManager.cpp
>index 003945053f55034c23087e90798bf9b7dfbe8623..434f3e36f5d789bf9501680d8f99a49a0644166f 100644
>--- a/Source/WebKit/NetworkProcess/WebStorage/StorageManager.cpp
>+++ b/Source/WebKit/NetworkProcess/WebStorage/StorageManager.cpp
>@@ -944,6 +944,43 @@ void StorageManager::waitUntilWritesFinished()
>     semaphore.wait();
> }
> 
>+void StorageManager::suspend(CompletionHandler<void()>&& completionHandler)
>+{
>+    if (m_isEphemeral)
>+        return;
>+
>+    Locker<Lock> stateLocker(m_stateLock);
>+    if (m_state != State::Running)
>+        return;
>+    m_state = State::WillSuspend;
>+
>+    m_queue->dispatch([this, protectedThis = makeRef(*this), completionHandler = WTFMove(completionHandler)] () mutable {
>+        Locker<Lock> stateLocker(m_stateLock);
>+        ASSERT(m_state != State::Suspended);
>+
>+        completionHandler();
>+
>+        if (m_state != State::WillSuspend)
>+            return;
>+        m_state = State::Suspended;
>+        while (m_state == State::Suspended)
>+            m_stateChangeCondition.wait(m_stateLock);
>+        ASSERT(m_state == State::Running);
>+    });
>+}
>+
>+void StorageManager::resume()
>+{
>+    if (m_isEphemeral)
>+        return;
>+
>+    Locker<Lock> stateLocker(m_stateLock);
>+    auto previousState = m_state;
>+    m_state = State::Running;
>+    if (previousState == State::Suspended)
>+        m_stateChangeCondition.notifyOne();
>+}
>+
> StorageManager::StorageArea* StorageManager::findStorageArea(IPC::Connection& connection, uint64_t storageMapID) const
> {
>     std::pair<IPC::Connection::UniqueID, uint64_t> connectionAndStorageMapIDPair(connection.uniqueID(), storageMapID);
>diff --git a/Source/WebKit/NetworkProcess/WebStorage/StorageManager.h b/Source/WebKit/NetworkProcess/WebStorage/StorageManager.h
>index a888332efcfb594130b2608846af4a182182b790..f6c032210185f051c92608104663362470092499 100644
>--- a/Source/WebKit/NetworkProcess/WebStorage/StorageManager.h
>+++ b/Source/WebKit/NetworkProcess/WebStorage/StorageManager.h
>@@ -58,6 +58,8 @@ public:
> 
>     void processDidCloseConnection(IPC::Connection&);
>     void waitUntilWritesFinished();
>+    void suspend(CompletionHandler<void()>&&);
>+    void resume();
> 
>     void getSessionStorageOrigins(Function<void(HashSet<WebCore::SecurityOriginData>&&)>&& completionHandler);
>     void deleteSessionStorageOrigins(Function<void()>&& completionHandler);
>@@ -113,6 +115,15 @@ private:
> 
>     HashMap<WebCore::SecurityOriginData, Ref<WebCore::StorageMap>> m_ephemeralStorage;
>     bool m_isEphemeral { false };
>+
>+    enum class State {
>+        Running,
>+        WillSuspend,
>+        Suspended
>+    };
>+    State m_state;
>+    Lock m_stateLock;
>+    Condition m_stateChangeCondition;
> };
> 
> } // namespace WebKit
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index 1c612b97f065452cc129aa7b4419a73f2b38c2a1..9346a14f7ed22871e875033bc37cdb61dd861e0a 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,17 @@
>+2019-05-30  Sihui Liu  <sihui_liu@apple.com>
>+
>+        Stop StorageManager when network process is ready to suspend
>+        https://bugs.webkit.org/show_bug.cgi?id=198201
>+        <rdar://problem/49683172>
>+
>+        Reviewed by Youenn Fablet.
>+
>+        * TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
>+        * TestWebKitAPI/Tests/WebKitCocoa/LocalStoragePersistence.mm:
>+        (TEST):
>+        * TestWebKitAPI/Tests/WebKitCocoa/local-storage-process-suspends-1.html: Added.
>+        * TestWebKitAPI/Tests/WebKitCocoa/local-storage-process-suspends-2.html: Added.
>+
> 2019-05-30  David Quesada  <david_quesada@apple.com>
> 
>         REGRESSION (r245756) [Mac] 2 TestWebKitAPI.DownloadProgress* and TestWebKitAPI._WKDownload.DownloadMonitorCancel are flaky timeouts
>diff --git a/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj b/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj
>index 57aa7d182e8fcf005390e22b654f306045e9c37f..60801c0a3a6f5ae841141625b4262c4f890c10ac 100644
>--- a/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj
>+++ b/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj
>@@ -650,6 +650,8 @@
> 		935786CE20F6A2A10000CDFC /* IndexedDB.sqlite3-shm in Copy Resources */ = {isa = PBXBuildFile; fileRef = 934FA5C620F69FED0040DC1B /* IndexedDB.sqlite3-shm */; };
> 		9361002914DC95A70061379D /* lots-of-iframes.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = 9361002814DC957B0061379D /* lots-of-iframes.html */; };
> 		93625D271CD9741C006DC1F1 /* large-video-without-audio.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = 93625D261CD973AF006DC1F1 /* large-video-without-audio.html */; };
>+		9368A25E229EFB4700A829CA /* local-storage-process-suspends-1.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = 9368A25D229EFB3A00A829CA /* local-storage-process-suspends-1.html */; };
>+		9368A25F229EFB4700A829CA /* local-storage-process-suspends-2.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = 9368A25C229EFB3A00A829CA /* local-storage-process-suspends-2.html */; };
> 		936F72801CD7D9EC0068A0FB /* large-video-with-audio.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = 936F727E1CD7D9D00068A0FB /* large-video-with-audio.html */; };
> 		936F72811CD7D9EC0068A0FB /* large-video-with-audio.mp4 in Copy Resources */ = {isa = PBXBuildFile; fileRef = 936F727F1CD7D9D00068A0FB /* large-video-with-audio.mp4 */; };
> 		93AF4ECE1506F064007FD57E /* NewFirstVisuallyNonEmptyLayoutForImages_Bundle.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 93AF4ECD1506F064007FD57E /* NewFirstVisuallyNonEmptyLayoutForImages_Bundle.cpp */; };
>@@ -1231,6 +1233,8 @@
> 				573255A722139BC700396AE8 /* load-web-archive-2.html in Copy Resources */,
> 				57901FB11CAF142D00ED64F9 /* LoadInvalidURLRequest.html in Copy Resources */,
> 				CA7787FF228CEFDB00E50463 /* local-storage-process-crashes.html in Copy Resources */,
>+				9368A25E229EFB4700A829CA /* local-storage-process-suspends-1.html in Copy Resources */,
>+				9368A25F229EFB4700A829CA /* local-storage-process-suspends-2.html in Copy Resources */,
> 				8C10AF98206467920018FD90 /* localstorage-empty-string-value.html in Copy Resources */,
> 				51E6A8961D2F1CA700C004B6 /* LocalStorageClear.html in Copy Resources */,
> 				46C519E61D3563FD00DAA51A /* LocalStorageNullEntries.html in Copy Resources */,
>@@ -1911,6 +1915,8 @@
> 		93575C551D30366E000D604D /* focus-inputs.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = "focus-inputs.html"; sourceTree = "<group>"; };
> 		9361002814DC957B0061379D /* lots-of-iframes.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = "lots-of-iframes.html"; sourceTree = "<group>"; };
> 		93625D261CD973AF006DC1F1 /* large-video-without-audio.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = "large-video-without-audio.html"; sourceTree = "<group>"; };
>+		9368A25C229EFB3A00A829CA /* local-storage-process-suspends-2.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = "local-storage-process-suspends-2.html"; sourceTree = "<group>"; };
>+		9368A25D229EFB3A00A829CA /* local-storage-process-suspends-1.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = "local-storage-process-suspends-1.html"; sourceTree = "<group>"; };
> 		936F727E1CD7D9D00068A0FB /* large-video-with-audio.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = "large-video-with-audio.html"; sourceTree = "<group>"; };
> 		936F727F1CD7D9D00068A0FB /* large-video-with-audio.mp4 */ = {isa = PBXFileReference; lastKnownFileType = file; path = "large-video-with-audio.mp4"; sourceTree = "<group>"; };
> 		939BA91614103412001A01BD /* DeviceScaleFactorOnBack.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = DeviceScaleFactorOnBack.mm; sourceTree = "<group>"; };
>@@ -3058,6 +3064,8 @@
> 				F41AB99D1EF4692C0083FA08 /* link-and-target-div.html */,
> 				F46128D1211E2D2500D9FADB /* link-in-iframe-and-input.html */,
> 				CA7787FE228CEFC700E50463 /* local-storage-process-crashes.html */,
>+				9368A25D229EFB3A00A829CA /* local-storage-process-suspends-1.html */,
>+				9368A25C229EFB3A00A829CA /* local-storage-process-suspends-2.html */,
> 				8C10AF97206467830018FD90 /* localstorage-empty-string-value.html */,
> 				51E6A8951D2F1C7700C004B6 /* LocalStorageClear.html */,
> 				46C519E21D35629600DAA51A /* LocalStorageNullEntries.html */,
>diff --git a/Tools/TestWebKitAPI/Tests/WebKitCocoa/LocalStoragePersistence.mm b/Tools/TestWebKitAPI/Tests/WebKitCocoa/LocalStoragePersistence.mm
>index 02a55f490c00eeb0d40d1c72b3f0dbc6a3c88853..1bc78d71949ba9724116bcfd726eca35b66a3ae2 100644
>--- a/Tools/TestWebKitAPI/Tests/WebKitCocoa/LocalStoragePersistence.mm
>+++ b/Tools/TestWebKitAPI/Tests/WebKitCocoa/LocalStoragePersistence.mm
>@@ -98,6 +98,58 @@ TEST(WKWebView, LocalStorageProcessCrashes)
>     TestWebKitAPI::Util::run(&readyToContinue);
> }
> 
>+TEST(WKWebView, LocalStorageProcessSuspends)
>+{
>+    readyToContinue = false;
>+    [[WKWebsiteDataStore defaultDataStore] removeDataOfTypes:[WKWebsiteDataStore allWebsiteDataTypes] modifiedSince:[NSDate distantPast] completionHandler:^() {
>+        readyToContinue = true;
>+    }];
>+    TestWebKitAPI::Util::run(&readyToContinue);
>+
>+    RetainPtr<LocalStorageMessageHandler> handler = adoptNS([[LocalStorageMessageHandler alloc] init]);
>+    RetainPtr<WKWebViewConfiguration> configuration = adoptNS([[WKWebViewConfiguration alloc] init]);
>+    [[configuration userContentController] addScriptMessageHandler:handler.get() name:@"testHandler"];
>+    RetainPtr<WKProcessPool> processPool = adoptNS([[WKProcessPool alloc] init]);
>+    [configuration setProcessPool:processPool.get()];
>+
>+    RetainPtr<WKWebView> webView1 = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:configuration.get()]);
>+    NSURLRequest *request = [NSURLRequest requestWithURL:[[NSBundle mainBundle] URLForResource:@"local-storage-process-suspends-1" withExtension:@"html" subdirectory:@"TestWebKitAPI.resources"]];
>+    [webView1 loadRequest:request];
>+
>+    receivedScriptMessage = false;
>+    TestWebKitAPI::Util::run(&receivedScriptMessage);
>+    EXPECT_WK_STREQ(@"value", [lastScriptMessage body]);
>+    
>+    RetainPtr<WKWebView> webView2 = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:configuration.get()]);
>+    request = [NSURLRequest requestWithURL:[[NSBundle mainBundle] URLForResource:@"local-storage-process-suspends-2" withExtension:@"html" subdirectory:@"TestWebKitAPI.resources"]];
>+    [webView2 loadRequest:request];
>+
>+    receivedScriptMessage = false;
>+    TestWebKitAPI::Util::run(&receivedScriptMessage);
>+    EXPECT_WK_STREQ(@"value", [lastScriptMessage body]);
>+
>+    [processPool.get() _sendNetworkProcessWillSuspendImminently];
>+
>+    readyToContinue = false;
>+    [webView1 evaluateJavaScript:@"window.localStorage.setItem('key', 'newValue')" completionHandler:^(id, NSError *) {
>+        readyToContinue = true;
>+    }];
>+    TestWebKitAPI::Util::run(&readyToContinue);
>+    
>+    readyToContinue = false;
>+    [webView2 evaluateJavaScript:@"window.localStorage.getItem('key')" completionHandler:^(id result, NSError *) {
>+        EXPECT_TRUE([@"value" isEqualToString:result]);
>+        readyToContinue = true;
>+    }];
>+    TestWebKitAPI::Util::run(&readyToContinue);
>+    
>+    [processPool.get() _sendNetworkProcessDidResume];
>+
>+    receivedScriptMessage = false;
>+    TestWebKitAPI::Util::run(&receivedScriptMessage);
>+    EXPECT_WK_STREQ(@"newValue", [lastScriptMessage body]);
>+}
>+
> TEST(WKWebView, LocalStorageEmptyString)
> {
>     readyToContinue = false;
>diff --git a/Tools/TestWebKitAPI/Tests/WebKitCocoa/local-storage-process-suspends-1.html b/Tools/TestWebKitAPI/Tests/WebKitCocoa/local-storage-process-suspends-1.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..11991eade6e615aad0a7e87dcd849b92a8ee7d51
>--- /dev/null
>+++ b/Tools/TestWebKitAPI/Tests/WebKitCocoa/local-storage-process-suspends-1.html
>@@ -0,0 +1,7 @@
>+<!DOCTYPE html>
>+<script>
>+
>+localStorage.setItem('key', 'value');
>+window.webkit.messageHandlers.testHandler.postMessage(localStorage.getItem('key'));
>+
>+</script>
>\ No newline at end of file
>diff --git a/Tools/TestWebKitAPI/Tests/WebKitCocoa/local-storage-process-suspends-2.html b/Tools/TestWebKitAPI/Tests/WebKitCocoa/local-storage-process-suspends-2.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..d3a2764d6e34f9fa45e2dd169e69a40cf55189cb
>--- /dev/null
>+++ b/Tools/TestWebKitAPI/Tests/WebKitCocoa/local-storage-process-suspends-2.html
>@@ -0,0 +1,17 @@
>+<!DOCTYPE html>
>+<script>
>+
>+var startValue = window.localStorage.getItem('key');
>+window.webkit.messageHandlers.testHandler.postMessage(startValue);
>+
>+var tries = 10;
>+var intervalID = setInterval(()=> {
>+    var newValue = window.localStorage.getItem('key');
>+    if (newValue != startValue || tries == 0) {
>+        window.webkit.messageHandlers.testHandler.postMessage(newValue);
>+        clearInterval(intervalID);
>+    }
>+    --tries;
>+}, 100);
>+
>+</script>
>\ No newline at end of file

Actions: View | Formatted Diff | Diff

Attachments on bug 198201: 370532 | 370583 | 370587 | 370867 | 370887 | 370894 | 370976