Attachment 370867 Details for Bug 198201 – Patch

[patch] Patch

bug-198201-20190529112824.patch (text/plain), 16.15 KB, created by Sihui Liu on 2019-05-29 11:28:25 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Sihui Liu

Created: 2019-05-29 11:28:25 PDT

Size: 16.15 KB

patch

obsolete

>Subversion Revision: 245849
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 900bbe8f18da4eb8056c2e0bac6d50800c85a7c6..5911bb4357149d8252d456ab311cc77a75b82c73 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,24 @@
>+2019-05-29  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 NOBODY (OOPS!).
>+
>+        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::waitUntilResume):
>+        * NetworkProcess/WebStorage/StorageManager.h:
>+
> 2019-05-28  Fujii Hironori  <Hironori.Fujii@sony.com>
> 
>         [WinCairo] REGRESSION(r245186) Crash in NetworkCache::IOChannel::read in http/tests/IndexedDB some tests
>diff --git a/Source/WebKit/NetworkProcess/NetworkProcess.cpp b/Source/WebKit/NetworkProcess/NetworkProcess.cpp
>index e5c2beae58c8da7ba99a230c392493aae73296f6..05fd7eab372441f1e1050af4028e5a8dcd4c4c7e 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().waitUntilResume();
> }
> 
> void NetworkProcess::prefetchDNS(const String& hostname)
>diff --git a/Source/WebKit/NetworkProcess/NetworkSession.cpp b/Source/WebKit/NetworkProcess/NetworkSession.cpp
>index 94a123700e5f1709995439e29cb5f7c42b8ddd88..5dfa71a29879473ecb8f1bd5945f133887f4386c 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->waitUntilResume();
>     m_storageManager->waitUntilWritesFinished();
> }
> 
>diff --git a/Source/WebKit/NetworkProcess/WebStorage/StorageManager.cpp b/Source/WebKit/NetworkProcess/WebStorage/StorageManager.cpp
>index 003945053f55034c23087e90798bf9b7dfbe8623..3ba31771a6574b1cd3d888ba318e4e01c55019f1 100644
>--- a/Source/WebKit/NetworkProcess/WebStorage/StorageManager.cpp
>+++ b/Source/WebKit/NetworkProcess/WebStorage/StorageManager.cpp
>@@ -37,7 +37,6 @@
> #include <WebCore/TextEncoding.h>
> #include <memory>
> #include <wtf/WorkQueue.h>
>-#include <wtf/threads/BinarySemaphore.h>
> 
> namespace WebKit {
> using namespace WebCore;
>@@ -944,6 +943,51 @@ void StorageManager::waitUntilWritesFinished()
>     semaphore.wait();
> }
> 
>+void StorageManager::suspend(CompletionHandler<void()>&& completionHandler)
>+{
>+    if (m_isEphemeral)
>+        return;
>+
>+    {
>+        Locker<Lock> shouldSuspendLocker(m_shouldSuspendLock);
>+        if (m_shouldSuspend)
>+            return;
>+        m_shouldSuspend = true;
>+    }
>+    m_queue->dispatch([this, protectedThis = makeRef(*this), completionHandler = WTFMove(completionHandler)] () mutable {
>+        {
>+            Locker<Lock> suspendLocker(m_suspendedLock);
>+            m_suspended = true;
>+        }
>+        completionHandler();
>+        {
>+            Locker<Lock> shouldSuspendLocker(m_shouldSuspendLock);
>+            while (m_shouldSuspend)
>+                m_shouldSuspendCondition.wait(m_shouldSuspendLock);
>+        }
>+
>+        Locker<Lock> suspendedLocker(m_suspendedLock);
>+        m_suspended = false;
>+        m_suspendedCondition.notifyOne();
>+    });
>+}
>+
>+void StorageManager::waitUntilResume()
>+{
>+    if (m_isEphemeral)
>+        return;
>+
>+    {
>+        Locker<Lock> shouldSuspendLocker(m_shouldSuspendLock);
>+        m_shouldSuspend = false;
>+        m_shouldSuspendCondition.notifyOne();
>+    }
>+
>+    Locker<Lock> suspendedLocker(m_suspendedLock);
>+    while (m_suspended)
>+        m_suspendedCondition.wait(m_suspendedLock);
>+}
>+
> 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..8c1e90fff84699900425608d21a28f727413cfa6 100644
>--- a/Source/WebKit/NetworkProcess/WebStorage/StorageManager.h
>+++ b/Source/WebKit/NetworkProcess/WebStorage/StorageManager.h
>@@ -33,6 +33,7 @@
> #include <wtf/Function.h>
> #include <wtf/HashSet.h>
> #include <wtf/text/StringHash.h>
>+#include <wtf/threads/BinarySemaphore.h>
> 
> namespace WebCore {
> class SecurityOrigin;
>@@ -58,6 +59,8 @@ public:
> 
>     void processDidCloseConnection(IPC::Connection&);
>     void waitUntilWritesFinished();
>+    void suspend(CompletionHandler<void()>&&);
>+    void waitUntilResume();
> 
>     void getSessionStorageOrigins(Function<void(HashSet<WebCore::SecurityOriginData>&&)>&& completionHandler);
>     void deleteSessionStorageOrigins(Function<void()>&& completionHandler);
>@@ -113,6 +116,14 @@ private:
> 
>     HashMap<WebCore::SecurityOriginData, Ref<WebCore::StorageMap>> m_ephemeralStorage;
>     bool m_isEphemeral { false };
>+
>+    bool m_suspended { false };
>+    Condition m_suspendedCondition;
>+    Lock m_suspendedLock;
>+
>+    bool m_shouldSuspend { false };
>+    Condition m_shouldSuspendCondition;
>+    Lock m_shouldSuspendLock;
> };
> 
> } // namespace WebKit
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index 5cb771224b0e071926c2ed6766563804be0c0cb7..131f064d7cbfe6e9ccfc3a701411cb51c951d542 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,17 @@
>+2019-05-29  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 NOBODY (OOPS!).
>+
>+        * 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-28  Justin Michaud  <justin_michaud@apple.com>
> 
>         Attempt to fix JSC test timeouts after adding collectContinuously to WASM tests.
>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..21bb2ed7defad4b2538b44e8e5fa60e2ffdbdad3 100644
>--- a/Tools/TestWebKitAPI/Tests/WebKitCocoa/LocalStoragePersistence.mm
>+++ b/Tools/TestWebKitAPI/Tests/WebKitCocoa/LocalStoragePersistence.mm
>@@ -98,6 +98,48 @@ 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(@"Continue", [lastScriptMessage body]);
>+
>+    [processPool.get() _sendNetworkProcessWillSuspendImminently];
>+
>+    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];
>+    
>+    [processPool.get() _sendNetworkProcessDidResume];
>+
>+    receivedScriptMessage = false;
>+    TestWebKitAPI::Util::run(&receivedScriptMessage);
>+    RetainPtr<NSString> value1 = (NSString *)[lastScriptMessage body];
>+    EXPECT_GT((int)[value1 length], 0);
>+
>+    receivedScriptMessage = false;
>+    TestWebKitAPI::Util::run(&receivedScriptMessage);
>+    RetainPtr<NSString> value2 = (NSString *)[lastScriptMessage body];
>+    EXPECT_GT((int)[value2 length], 0);
>+    EXPECT_NE(value1.get(), value2.get());
>+}
>+
> 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..1aad8108ac0ea79208b16a9b822ca43b524a1a99
>--- /dev/null
>+++ b/Tools/TestWebKitAPI/Tests/WebKitCocoa/local-storage-process-suspends-1.html
>@@ -0,0 +1,14 @@
>+<!DOCTYPE html>
>+<script>
>+
>+var started = false;
>+var count = 0;
>+window.setInterval(function() {
>+    localStorage.setItem('key', 'value' + count++);
>+    if (!started) {
>+        started = true;
>+        window.webkit.messageHandlers.testHandler.postMessage("Continue");
>+    }
>+}, 100);
>+
>+</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..9b965f53af9e593c868ddf360b57896297249561
>--- /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 = 20;
>+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