Attachment 373364 Details for Bug 199423 – Patch

[patch] Patch

bug-199423-20190702145920.patch (text/plain), 7.06 KB, created by youenn fablet on 2019-07-02 14:59:20 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: youenn fablet

Created: 2019-07-02 14:59:20 PDT

Size: 7.06 KB

patch

obsolete

>Subversion Revision: 246881
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index c631acccbfba505480f5aa70a4d631519f3cce50..f2f568839b310498604e5896a2548193d7e15e8c 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,24 @@
>+2019-07-02  Youenn Fablet  <youenn@apple.com>
>+
>+        Make sure to cross-thread copy in StorageManager when hopping back to the main thread
>+        https://bugs.webkit.org/show_bug.cgi?id=199423
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Make sure to isolate copy some strings that may not be isolated in case of ephemeral sessions.
>+        Rename originDetails to originDetailsIsolatedCopy to clarify the strings are isolated.
>+        Small refactoring to use crossThreadCopy instead of doing vector copy ourselves.
>+
>+        * NetworkProcess/WebStorage/LocalStorageDatabaseTracker.cpp:
>+        (WebKit::LocalStorageDatabaseTracker::originDetailsIsolatedCopy):
>+        * NetworkProcess/WebStorage/LocalStorageDatabaseTracker.h:
>+        * NetworkProcess/WebStorage/StorageManager.cpp:
>+        (WebKit::StorageManager::deleteSessionStorageEntriesForOrigins):
>+        (WebKit::StorageManager::getLocalStorageOrigins):
>+        (WebKit::StorageManager::getLocalStorageOriginDetails):
>+        (WebKit::StorageManager::deleteLocalStorageEntriesForOrigins):
>+        * NetworkProcess/WebStorage/StorageManager.h:
>+
> 2019-07-02  Youenn Fablet  <youenn@apple.com>
> 
>         StorageManager does not need to be a WorkQueueMessageReceiver anymore
>diff --git a/Source/WebKit/NetworkProcess/WebStorage/LocalStorageDatabaseTracker.cpp b/Source/WebKit/NetworkProcess/WebStorage/LocalStorageDatabaseTracker.cpp
>index 88987a45cd4fb9c2cbf1f04d8a2e0085d052e010..139d758c007a93b9f258b3c0ff5b8cf37d5ae8d5 100644
>--- a/Source/WebKit/NetworkProcess/WebStorage/LocalStorageDatabaseTracker.cpp
>+++ b/Source/WebKit/NetworkProcess/WebStorage/LocalStorageDatabaseTracker.cpp
>@@ -129,7 +129,7 @@ Vector<SecurityOriginData> LocalStorageDatabaseTracker::origins() const
>     return databaseOrigins;
> }
> 
>-Vector<LocalStorageDatabaseTracker::OriginDetails> LocalStorageDatabaseTracker::originDetails()
>+Vector<LocalStorageDatabaseTracker::OriginDetails> LocalStorageDatabaseTracker::originDetailsIsolatedCopy()
> {
>     Vector<OriginDetails> result;
>     auto databaseOrigins = origins();
>diff --git a/Source/WebKit/NetworkProcess/WebStorage/LocalStorageDatabaseTracker.h b/Source/WebKit/NetworkProcess/WebStorage/LocalStorageDatabaseTracker.h
>index 2adbe1cdc1a3448e966a17e50e96259330af5a3d..ea9fe125a68f6dedd1b1fb36072831afb69f7de3 100644
>--- a/Source/WebKit/NetworkProcess/WebStorage/LocalStorageDatabaseTracker.h
>+++ b/Source/WebKit/NetworkProcess/WebStorage/LocalStorageDatabaseTracker.h
>@@ -59,7 +59,7 @@ public:
>         template<class Encoder> void encode(Encoder&) const;
>         template<class Decoder> static Optional<OriginDetails> decode(Decoder&);
>     };
>-    Vector<OriginDetails> originDetails();
>+    Vector<OriginDetails> originDetailsIsolatedCopy();
> 
> private:
>     LocalStorageDatabaseTracker(Ref<WorkQueue>&&, const String& localStorageDirectory);
>diff --git a/Source/WebKit/NetworkProcess/WebStorage/StorageManager.cpp b/Source/WebKit/NetworkProcess/WebStorage/StorageManager.cpp
>index 50fde90d615ed58896015c69a2b09152fd2fb27b..6e8c2eab25d38577ea1cd8abda9f5afa858d8c80 100644
>--- a/Source/WebKit/NetworkProcess/WebStorage/StorageManager.cpp
>+++ b/Source/WebKit/NetworkProcess/WebStorage/StorageManager.cpp
>@@ -639,13 +639,7 @@ void StorageManager::deleteSessionStorageOrigins(Function<void()>&& completionHa
> 
> void StorageManager::deleteSessionStorageEntriesForOrigins(const Vector<WebCore::SecurityOriginData>& origins, Function<void()>&& completionHandler)
> {
>-    Vector<WebCore::SecurityOriginData> copiedOrigins;
>-    copiedOrigins.reserveInitialCapacity(origins.size());
>-
>-    for (auto& origin : origins)
>-        copiedOrigins.uncheckedAppend(origin.isolatedCopy());
>-
>-    m_queue->dispatch([this, protectedThis = makeRef(*this), copiedOrigins = WTFMove(copiedOrigins), completionHandler = WTFMove(completionHandler)]() mutable {
>+    m_queue->dispatch([this, protectedThis = makeRef(*this), copiedOrigins = crossThreadCopy(origins), completionHandler = WTFMove(completionHandler)]() mutable {
>         for (auto& origin : copiedOrigins) {
>             for (auto& sessionStorageNamespace : m_sessionStorageNamespaces.values())
>                 sessionStorageNamespace->clearStorageAreasMatchingOrigin(origin);
>@@ -662,18 +656,18 @@ void StorageManager::getLocalStorageOrigins(Function<void(HashSet<WebCore::Secur
> 
>         if (m_localStorageDatabaseTracker) {
>             for (auto& origin : m_localStorageDatabaseTracker->origins())
>-                origins.add(origin);
>+                origins.add(origin.isolatedCopy());
>         } else {
>             std::lock_guard<Lock> lock(m_localStorageNamespacesMutex);
>             for (const auto& localStorageNameSpace : m_localStorageNamespaces.values()) {
>                 for (auto& origin : localStorageNameSpace->ephemeralOrigins())
>-                    origins.add(origin);
>+                    origins.add(origin.isolatedCopy());
>             }
>         }
> 
>         for (auto& transientLocalStorageNamespace : m_transientLocalStorageNamespaces.values()) {
>             for (auto& origin : transientLocalStorageNamespace->origins())
>-                origins.add(origin);
>+                origins.add(origin.isolatedCopy());
>         }
> 
>         RunLoop::main().dispatch([origins = WTFMove(origins), completionHandler = WTFMove(completionHandler)]() mutable {
>@@ -687,7 +681,7 @@ void StorageManager::getLocalStorageOriginDetails(Function<void(Vector<LocalStor
>     m_queue->dispatch([this, protectedThis = makeRef(*this), completionHandler = WTFMove(completionHandler)]() mutable {
>         Vector<LocalStorageDatabaseTracker::OriginDetails> originDetails;
>         if (m_localStorageDatabaseTracker)
>-            originDetails = m_localStorageDatabaseTracker->originDetails();
>+            originDetails = m_localStorageDatabaseTracker->originDetailsIsolatedCopy();
> 
>         RunLoop::main().dispatch([originDetails = WTFMove(originDetails), completionHandler = WTFMove(completionHandler)]() mutable {
>             completionHandler(WTFMove(originDetails));
>@@ -742,13 +736,7 @@ void StorageManager::deleteLocalStorageOriginsModifiedSince(WallTime time, Funct
> 
> void StorageManager::deleteLocalStorageEntriesForOrigins(const Vector<WebCore::SecurityOriginData>& origins, Function<void()>&& completionHandler)
> {
>-    Vector<SecurityOriginData> copiedOrigins;
>-    copiedOrigins.reserveInitialCapacity(origins.size());
>-
>-    for (auto& origin : origins)
>-        copiedOrigins.uncheckedAppend(origin.isolatedCopy());
>-
>-    m_queue->dispatch([this, protectedThis = makeRef(*this), copiedOrigins = WTFMove(copiedOrigins), completionHandler = WTFMove(completionHandler)]() mutable {
>+    m_queue->dispatch([this, protectedThis = makeRef(*this), copiedOrigins = crossThreadCopy(origins), completionHandler = WTFMove(completionHandler)]() mutable {
>         for (auto& origin : copiedOrigins) {
>             {
>                 std::lock_guard<Lock> lock(m_localStorageNamespacesMutex);

Actions: View | Formatted Diff | Diff

Attachments on bug 199423: 373364 | 373367 | 373372