Attachment 373367 Details for Bug 199423 – Patch

[patch] Patch

bug-199423-20190702154639.patch (text/plain), 7.05 KB, created by youenn fablet on 2019-07-02 15:46:39 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: youenn fablet

Created: 2019-07-02 15:46:39 PDT

Size: 7.05 KB

patch

obsolete

>Subversion Revision: 247073
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 8deca8e0352190c5e5e2f500fb772c1c701a1ab6..cb7ef1be98a969c30d44c224c1a8511f3f188008 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  Tim Horton  <timothy_horton@apple.com>
> 
>         REGRESSION (r243240): Unable to swipe back in Safari
>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 76c0fe53928e377877b15a11b0c0ce929f5833eb..df0dba9b1779c4b813fa9d8ffbb3cc93f9508eb5 100644
>--- a/Source/WebKit/NetworkProcess/WebStorage/StorageManager.cpp
>+++ b/Source/WebKit/NetworkProcess/WebStorage/StorageManager.cpp
>@@ -621,13 +621,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);
>@@ -644,17 +638,17 @@ 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 {
>             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 {
>@@ -668,7 +662,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));
>@@ -716,13 +710,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) {
>             for (auto& localStorageNamespace : m_localStorageNamespaces.values())
>                 localStorageNamespace->clearStorageAreasMatchingOrigin(origin);

Actions: View | Formatted Diff | Diff

Attachments on bug 199423: 373364 | 373367 | 373372