Changeset 247092 in webkit

Timestamp:

Jul 3, 2019 10:28:53 AM (5 years ago)

Author:

youenn@apple.com

Message:

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 Chris Dumez.

Make sure to isolate copy some strings that may not be isolated in case of ephemeral sessions.
Small refactoring to use crossThreadCopy instead of doing vector copy ourselves.

• NetworkProcess/WebStorage/LocalStorageDatabaseTracker.h:

(WebKit::LocalStorageDatabaseTracker::OriginDetails::isolatedCopy const):

• NetworkProcess/WebStorage/StorageManager.cpp:

(WebKit::StorageManager::deleteSessionStorageEntriesForOrigins):
(WebKit::StorageManager::getLocalStorageOrigins):
(WebKit::StorageManager::getLocalStorageOriginDetails):
(WebKit::StorageManager::deleteLocalStorageEntriesForOrigins):

Location:

trunk/Source/WebKit

Files:

• ChangeLog (modified) (1 diff)
• NetworkProcess/WebStorage/LocalStorageDatabaseTracker.h (modified) (1 diff)
• NetworkProcess/WebStorage/StorageManager.cpp (modified) (5 diffs)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2019-07-03  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 Chris Dumez.
+
+        Make sure to isolate copy some strings that may not be isolated in case of ephemeral sessions.
+        Small refactoring to use crossThreadCopy instead of doing vector copy ourselves.
+
+        * NetworkProcess/WebStorage/LocalStorageDatabaseTracker.h:
+        (WebKit::LocalStorageDatabaseTracker::OriginDetails::isolatedCopy const):
+        * NetworkProcess/WebStorage/StorageManager.cpp:
+        (WebKit::StorageManager::deleteSessionStorageEntriesForOrigins):
+        (WebKit::StorageManager::getLocalStorageOrigins):
+        (WebKit::StorageManager::getLocalStorageOriginDetails):
+        (WebKit::StorageManager::deleteLocalStorageEntriesForOrigins):
+
 2019-07-02  Joonghun Park  <jh718.park@samsung.com>
 

trunk/Source/WebKit/NetworkProcess/WebStorage/LocalStorageDatabaseTracker.h

@@ -59 +59 @@
         template<class Encoder> void encode(Encoder&) const;
         template<class Decoder> static Optional<OriginDetails> decode(Decoder&);
+
+        OriginDetails isolatedCopy() const { return { originIdentifier.isolatedCopy(), creationTime, modificationTime }; }
     };
     Vector<OriginDetails> originDetails();

trunk/Source/WebKit/NetworkProcess/WebStorage/StorageManager.cpp

@@ -622 +622 @@
 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())
@@ -645 +639 @@
         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());
             }
         }
@@ -655 +649 @@
         for (auto& transientLocalStorageNamespace : m_transientLocalStorageNamespaces.values()) {
             for (auto& origin : transientLocalStorageNamespace->origins())
-                origins.add(origin);
+                origins.add(origin.isolatedCopy());
         }
 
@@ -669 +663 @@
         Vector<LocalStorageDatabaseTracker::OriginDetails> originDetails;
         if (m_localStorageDatabaseTracker)
-            originDetails = m_localStorageDatabaseTracker->originDetails();
+            originDetails = m_localStorageDatabaseTracker->originDetails().isolatedCopy();
 
         RunLoop::main().dispatch([originDetails = WTFMove(originDetails), completionHandler = WTFMove(completionHandler)]() mutable {
@@ -717 +711 @@
 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())