Attachment 370897 Details for Bug 198317 – Patch for landing

[patch] Patch for landing

bug-198317-20190529164341.patch (text/plain), 33.39 KB, created by Sihui Liu on 2019-05-29 16:43:41 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Sihui Liu

Created: 2019-05-29 16:43:41 PDT

Size: 33.39 KB

patch

obsolete

>Subversion Revision: 245869
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 24651d538547c9bde082bedda7177bafc70e3887..7fe9c62207d100fa9d261dc401563fc7ce5f8710 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,52 @@
>+2019-05-29  Sihui Liu  <sihui_liu@apple.com>
>+
>+        WKWebsiteDataStore API fails to fetch web storage data for non-persistent data store
>+        https://bugs.webkit.org/show_bug.cgi?id=198317
>+
>+        Reviewed by Alex Christensen.
>+
>+        Use LocalStorageNameSpace instead of SessionStorageNameSpace for localStorage in ephemeral session or 
>+        websiteDataStore.
>+
>+        * NetworkProcess/WebStorage/StorageManager.cpp:
>+        (WebKit::StorageManager::StorageArea::isEphemeral const):
>+        (WebKit::StorageManager::StorageArea::removeListener):
>+        (WebKit::StorageManager::StorageArea::setItems):
>+        (WebKit::StorageManager::StorageArea::openDatabaseAndImportItemsIfNeeded const):
>+        (WebKit::StorageManager::LocalStorageNamespace::~LocalStorageNamespace):
>+        (WebKit::StorageManager::LocalStorageNamespace::getOrCreateStorageArea):
>+        (WebKit::StorageManager::LocalStorageNamespace::clearAllStorageAreas):
>+        (WebKit::StorageManager::LocalStorageNamespace::ephemeralOrigins const):
>+        (WebKit::StorageManager::LocalStorageNamespace::cloneTo):
>+        (WebKit::StorageManager::StorageManager):
>+        (WebKit::StorageManager::cloneSessionStorageNamespace):
>+        (WebKit::StorageManager::getLocalStorageOrigins):
>+        (WebKit::StorageManager::getLocalStorageOriginDetails):
>+        (WebKit::StorageManager::deleteLocalStorageEntriesForOrigin):
>+        (WebKit::StorageManager::deleteLocalStorageOriginsModifiedSince):
>+        (WebKit::StorageManager::deleteLocalStorageEntriesForOrigins):
>+        (WebKit::StorageManager::createLocalStorageMap):
>+        (WebKit::StorageManager::createTransientLocalStorageMap):
>+        (WebKit::StorageManager::createSessionStorageMap):
>+        (WebKit::StorageManager::destroyStorageMap):
>+        (WebKit::StorageManager::getValues):
>+        (WebKit::StorageManager::setItem):
>+        (WebKit::StorageManager::removeItem):
>+        (WebKit::StorageManager::clear):
>+        (WebKit::StorageManager::StorageArea::isSessionStorage const): Deleted.
>+        * NetworkProcess/WebStorage/StorageManager.h:
>+        (): Deleted.
>+        * WebProcess/WebStorage/StorageAreaMap.cpp:
>+        (WebKit::StorageAreaMap::dispatchStorageEvent):
>+        (WebKit::StorageAreaMap::dispatchSessionStorageEvent):
>+        (WebKit::StorageAreaMap::connect):
>+        * WebProcess/WebStorage/StorageNamespaceImpl.cpp:
>+        (WebKit::StorageNamespaceImpl::createEphemeralLocalStorageNamespace):
>+        (WebKit::StorageNamespaceImpl::createLocalStorageNamespace):
>+        * WebProcess/WebStorage/StorageNamespaceImpl.h:
>+        * WebProcess/WebStorage/WebStorageNamespaceProvider.cpp:
>+        (WebKit::WebStorageNamespaceProvider::createLocalStorageNamespace):
>+
> 2019-05-29  Geoffrey Garen  <ggaren@apple.com>
> 
>         WeakPtr breaks vtables when upcasting to base classes
>diff --git a/Source/WebKit/NetworkProcess/WebStorage/StorageManager.cpp b/Source/WebKit/NetworkProcess/WebStorage/StorageManager.cpp
>index 003945053f55034c23087e90798bf9b7dfbe8623..7af6fd765aa6beff7863cd16aa8eaf05339ebcac 100644
>--- a/Source/WebKit/NetworkProcess/WebStorage/StorageManager.cpp
>+++ b/Source/WebKit/NetworkProcess/WebStorage/StorageManager.cpp
>@@ -63,7 +63,7 @@ public:
>     const HashMap<String, String>& items() const;
>     void clear();
> 
>-    bool isSessionStorage() const { return !m_localStorageNamespace; }
>+    bool isEphemeral() const { return !m_localStorageNamespace; }
> 
> private:
>     explicit StorageArea(LocalStorageNamespace*, const SecurityOriginData&, unsigned quotaInBytes);
>@@ -72,7 +72,7 @@ private:
> 
>     void dispatchEvents(IPC::Connection::UniqueID sourceConnection, uint64_t sourceStorageAreaID, const String& key, const String& oldValue, const String& newValue, const String& urlString) const;
> 
>-    // Will be null if the storage area belongs to a session storage namespace.
>+    // Will be null if the storage area belongs to a session storage namespace or the storage area is in an ephemeral session.
>     LocalStorageNamespace* m_localStorageNamespace;
>     mutable RefPtr<LocalStorageDatabase> m_localStorageDatabase;
>     mutable bool m_didImportItemsFromDatabase { false };
>@@ -91,12 +91,16 @@ public:
> 
>     StorageManager* storageManager() const { return &m_storageManager; }
> 
>-    Ref<StorageArea> getOrCreateStorageArea(SecurityOriginData&&);
>+    enum class IsEphemeral : bool { No, Yes };
>+    Ref<StorageArea> getOrCreateStorageArea(SecurityOriginData&&, IsEphemeral);
>     void didDestroyStorageArea(StorageArea*);
> 
>     void clearStorageAreasMatchingOrigin(const SecurityOriginData&);
>     void clearAllStorageAreas();
> 
>+    Vector<SecurityOriginData> ephemeralOrigins() const;
>+    void cloneTo(LocalStorageNamespace& newLocalStorageNamespace);
>+
> private:
>     LocalStorageNamespace(StorageManager&, uint64_t storageManagerID);
> 
>@@ -104,8 +108,7 @@ private:
>     uint64_t m_storageNamespaceID;
>     unsigned m_quotaInBytes;
> 
>-    // We don't hold an explicit reference to the StorageAreas; they are kept alive by the m_storageAreasByConnection map in StorageManager.
>-    HashMap<SecurityOriginData, StorageArea*> m_storageAreaMap;
>+    HashMap<SecurityOriginData, RefPtr<StorageArea>> m_storageAreaMap;
> };
> 
> // Suggested by https://www.w3.org/TR/webstorage/#disk-space
>@@ -196,7 +199,7 @@ void StorageManager::StorageArea::addListener(IPC::Connection::UniqueID connecti
> 
> void StorageManager::StorageArea::removeListener(IPC::Connection::UniqueID connectionID, uint64_t storageMapID)
> {
>-    ASSERT(isSessionStorage() || m_eventListeners.contains(std::make_pair(connectionID, storageMapID)));
>+    ASSERT(isEphemeral() || m_eventListeners.contains(std::make_pair(connectionID, storageMapID)));
>     m_eventListeners.remove(std::make_pair(connectionID, storageMapID));
> }
> 
>@@ -236,7 +239,10 @@ void StorageManager::StorageArea::setItem(IPC::Connection::UniqueID sourceConnec
> 
> void StorageManager::StorageArea::setItems(const HashMap<String, String>& items)
> {
>-    ASSERT(!m_localStorageDatabase);
>+    // Import items from web process if items are not stored on disk.
>+    if (!isEphemeral())
>+        return;
>+
>     for (auto& item : items) {
>         String oldValue;
>         bool quotaException;
>@@ -311,9 +317,10 @@ void StorageManager::StorageArea::openDatabaseAndImportItemsIfNeeded() const
>     if (!m_localStorageNamespace)
>         return;
> 
>+    ASSERT(m_localStorageNamespace->storageManager()->m_localStorageDatabaseTracker);
>     // We open the database here even if we've already imported our items to ensure that the database is open if we need to write to it.
>     if (!m_localStorageDatabase)
>-        m_localStorageDatabase = LocalStorageDatabase::create(m_localStorageNamespace->storageManager()->m_queue.copyRef(), m_localStorageNamespace->storageManager()->m_localStorageDatabaseTracker.copyRef(), m_securityOrigin);
>+        m_localStorageDatabase = LocalStorageDatabase::create(m_localStorageNamespace->storageManager()->m_queue.copyRef(), *m_localStorageNamespace->storageManager()->m_localStorageDatabaseTracker, m_securityOrigin);
> 
>     if (m_didImportItemsFromDatabase)
>         return;
>@@ -350,14 +357,13 @@ StorageManager::LocalStorageNamespace::LocalStorageNamespace(StorageManager& sto
> 
> StorageManager::LocalStorageNamespace::~LocalStorageNamespace()
> {
>-    ASSERT(m_storageAreaMap.isEmpty());
> }
> 
>-auto StorageManager::LocalStorageNamespace::getOrCreateStorageArea(SecurityOriginData&& securityOrigin) -> Ref<StorageArea>
>+auto StorageManager::LocalStorageNamespace::getOrCreateStorageArea(SecurityOriginData&& securityOrigin, IsEphemeral isEphemeral) -> Ref<StorageArea>
> {
>     RefPtr<StorageArea> protectedStorageArea;
>     return *m_storageAreaMap.ensure(securityOrigin, [&]() mutable {
>-        protectedStorageArea = StorageArea::create(this, WTFMove(securityOrigin), m_quotaInBytes);
>+        protectedStorageArea = StorageArea::create(isEphemeral == IsEphemeral::Yes ? nullptr : this, WTFMove(securityOrigin), m_quotaInBytes);
>         return protectedStorageArea.get();
>     }).iterator->value;
> }
>@@ -383,10 +389,26 @@ void StorageManager::LocalStorageNamespace::clearStorageAreasMatchingOrigin(cons
> 
> void StorageManager::LocalStorageNamespace::clearAllStorageAreas()
> {
>-    for (auto* storageArea : m_storageAreaMap.values())
>+    for (auto storageArea : m_storageAreaMap.values())
>         storageArea->clear();
> }
> 
>+Vector<SecurityOriginData> StorageManager::LocalStorageNamespace::ephemeralOrigins() const
>+{
>+    Vector<SecurityOriginData> origins;
>+    for (const auto& storageArea : m_storageAreaMap.values()) {
>+        if (!storageArea->items().isEmpty())
>+            origins.append(storageArea->securityOrigin());
>+    }
>+    return origins;
>+}
>+
>+void StorageManager::LocalStorageNamespace::cloneTo(LocalStorageNamespace& newLocalStorageNamespace)
>+{
>+    for (auto& pair : m_storageAreaMap)
>+        newLocalStorageNamespace.m_storageAreaMap.add(pair.key, pair.value->clone());
>+}
>+
> class StorageManager::SessionStorageNamespace : public ThreadSafeRefCounted<SessionStorageNamespace> {
> public:
>     static Ref<SessionStorageNamespace> create(unsigned quotaInBytes);
>@@ -483,11 +505,11 @@ Ref<StorageManager> StorageManager::create(const String& localStorageDirectory)
> 
> StorageManager::StorageManager(const String& localStorageDirectory)
>     : m_queue(WorkQueue::create("com.apple.WebKit.StorageManager"))
>-    , m_localStorageDatabaseTracker(LocalStorageDatabaseTracker::create(m_queue.copyRef(), localStorageDirectory))
>-    , m_isEphemeral(localStorageDirectory.isNull())
> {
>     // Make sure the encoding is initialized before we start dispatching things to the queue.
>     UTF8Encoding();
>+    if (!localStorageDirectory.isNull())
>+        m_localStorageDatabaseTracker = LocalStorageDatabaseTracker::create(m_queue.copyRef(), localStorageDirectory);
> }
> 
> StorageManager::~StorageManager()
>@@ -548,6 +570,13 @@ void StorageManager::cloneSessionStorageNamespace(uint64_t storageNamespaceID, u
>         ASSERT(newSessionStorageNamespace);
> 
>         sessionStorageNamespace->cloneTo(*newSessionStorageNamespace);
>+
>+        if (!m_localStorageDatabaseTracker) {
>+            if (auto* localStorageNamespace = m_localStorageNamespaces.get(storageNamespaceID)) {
>+                LocalStorageNamespace* newlocalStorageNamespace = getOrCreateLocalStorageNamespace(newStorageNamespaceID);
>+                localStorageNamespace->cloneTo(*newlocalStorageNamespace);
>+            }
>+        }
>     });
> }
> 
>@@ -629,8 +658,15 @@ void StorageManager::getLocalStorageOrigins(Function<void(HashSet<WebCore::Secur
>     m_queue->dispatch([this, protectedThis = makeRef(*this), completionHandler = WTFMove(completionHandler)]() mutable {
>         HashSet<SecurityOriginData> origins;
> 
>-        for (auto& origin : m_localStorageDatabaseTracker->origins())
>-            origins.add(origin);
>+        if (m_localStorageDatabaseTracker) {
>+            for (auto& origin : m_localStorageDatabaseTracker->origins())
>+                origins.add(origin);
>+        } else {
>+            for (const auto& localStorageNameSpace : m_localStorageNamespaces.values()) {
>+                for (auto& origin : localStorageNameSpace->ephemeralOrigins())
>+                    origins.add(origin);
>+            }
>+        }
> 
>         for (auto& transientLocalStorageNamespace : m_transientLocalStorageNamespaces.values()) {
>             for (auto& origin : transientLocalStorageNamespace->origins())
>@@ -646,7 +682,9 @@ void StorageManager::getLocalStorageOrigins(Function<void(HashSet<WebCore::Secur
> void StorageManager::getLocalStorageOriginDetails(Function<void(Vector<LocalStorageDatabaseTracker::OriginDetails>&&)>&& completionHandler)
> {
>     m_queue->dispatch([this, protectedThis = makeRef(*this), completionHandler = WTFMove(completionHandler)]() mutable {
>-        auto originDetails = m_localStorageDatabaseTracker->originDetails();
>+        Vector<LocalStorageDatabaseTracker::OriginDetails> originDetails;
>+        if (m_localStorageDatabaseTracker)
>+            originDetails = m_localStorageDatabaseTracker->originDetails();
> 
>         RunLoop::main().dispatch([originDetails = WTFMove(originDetails), completionHandler = WTFMove(completionHandler)]() mutable {
>             completionHandler(WTFMove(originDetails));
>@@ -663,23 +701,29 @@ void StorageManager::deleteLocalStorageEntriesForOrigin(const SecurityOriginData
>         for (auto& transientLocalStorageNamespace : m_transientLocalStorageNamespaces.values())
>             transientLocalStorageNamespace->clearStorageAreasMatchingOrigin(copiedOrigin);
> 
>-        m_localStorageDatabaseTracker->deleteDatabaseWithOrigin(copiedOrigin);
>+        if (m_localStorageDatabaseTracker)
>+            m_localStorageDatabaseTracker->deleteDatabaseWithOrigin(copiedOrigin);
>     });
> }
> 
> void StorageManager::deleteLocalStorageOriginsModifiedSince(WallTime time, Function<void()>&& completionHandler)
> {
>     m_queue->dispatch([this, protectedThis = makeRef(*this), time, completionHandler = WTFMove(completionHandler)]() mutable {
>-        auto originsToDelete = m_localStorageDatabaseTracker->databasesModifiedSince(time);
>-        
>-        for (auto& transientLocalStorageNamespace : m_transientLocalStorageNamespaces.values())
>-            transientLocalStorageNamespace->clearAllStorageAreas();
>+        if (m_localStorageDatabaseTracker) {
>+            auto originsToDelete = m_localStorageDatabaseTracker->databasesModifiedSince(time);
>+            
>+            for (auto& transientLocalStorageNamespace : m_transientLocalStorageNamespaces.values())
>+                transientLocalStorageNamespace->clearAllStorageAreas();
> 
>-        for (const auto& origin : originsToDelete) {
>+            for (const auto& origin : originsToDelete) {
>+                for (auto& localStorageNamespace : m_localStorageNamespaces.values())
>+                    localStorageNamespace->clearStorageAreasMatchingOrigin(origin);
>+                
>+                m_localStorageDatabaseTracker->deleteDatabaseWithOrigin(origin);
>+            }
>+        } else {
>             for (auto& localStorageNamespace : m_localStorageNamespaces.values())
>-                localStorageNamespace->clearStorageAreasMatchingOrigin(origin);
>-            
>-            m_localStorageDatabaseTracker->deleteDatabaseWithOrigin(origin);
>+                localStorageNamespace->clearAllStorageAreas();
>         }
> 
>         RunLoop::main().dispatch(WTFMove(completionHandler));
>@@ -702,7 +746,8 @@ void StorageManager::deleteLocalStorageEntriesForOrigins(const Vector<WebCore::S
>             for (auto& transientLocalStorageNamespace : m_transientLocalStorageNamespaces.values())
>                 transientLocalStorageNamespace->clearStorageAreasMatchingOrigin(origin);
> 
>-            m_localStorageDatabaseTracker->deleteDatabaseWithOrigin(origin);
>+            if (m_localStorageDatabaseTracker)
>+                m_localStorageDatabaseTracker->deleteDatabaseWithOrigin(origin);
>         }
> 
>         RunLoop::main().dispatch(WTFMove(completionHandler));
>@@ -712,7 +757,6 @@ void StorageManager::deleteLocalStorageEntriesForOrigins(const Vector<WebCore::S
> void StorageManager::createLocalStorageMap(IPC::Connection& connection, uint64_t storageMapID, uint64_t storageNamespaceID, SecurityOriginData&& securityOriginData)
> {
>     m_queue->dispatch([this, protectedThis = makeRef(*this), connectionID = connection.uniqueID(), storageMapID, storageNamespaceID, securityOriginData = securityOriginData.isolatedCopy()]() mutable {
>-        ASSERT(!m_isEphemeral);
>         std::pair<IPC::Connection::UniqueID, uint64_t> connectionAndStorageMapIDPair(connectionID, storageMapID);
> 
>         // FIXME: This should be a message check.
>@@ -729,7 +773,7 @@ void StorageManager::createLocalStorageMap(IPC::Connection& connection, uint64_t
>         // FIXME: This should be a message check.
>         ASSERT(localStorageNamespace);
> 
>-        auto storageArea = localStorageNamespace->getOrCreateStorageArea(WTFMove(securityOriginData));
>+        auto storageArea = localStorageNamespace->getOrCreateStorageArea(WTFMove(securityOriginData), m_localStorageDatabaseTracker ? StorageManager::LocalStorageNamespace::IsEphemeral::No : StorageManager::LocalStorageNamespace::IsEphemeral::Yes);
>         storageArea->addListener(connectionID, storageMapID);
> 
>         result.iterator->value = WTFMove(storageArea);
>@@ -748,7 +792,7 @@ void StorageManager::createTransientLocalStorageMap(IPC::Connection& connection,
>             if (it->key.first != connectionID)
>                 continue;
>             Ref<StorageArea> area = *it->value;
>-            if (!area->isSessionStorage())
>+            if (!area->isEphemeral())
>                 continue;
>             if (!origin.securityOrigin()->isSameSchemeHostPort(area->securityOrigin().securityOrigin().get()))
>                 continue;
>@@ -780,10 +824,6 @@ void StorageManager::createTransientLocalStorageMap(IPC::Connection& connection,
> void StorageManager::createSessionStorageMap(IPC::Connection& connection, uint64_t storageMapID, uint64_t storageNamespaceID, SecurityOriginData&& securityOriginData)
> {
>     m_queue->dispatch([this, protectedThis = makeRef(*this), connectionID = connection.uniqueID(), storageMapID, storageNamespaceID, securityOriginData = securityOriginData.isolatedCopy()]() mutable {
>-        if (m_isEphemeral) {
>-            m_ephemeralStorage.add(securityOriginData, WebCore::StorageMap::create(localStorageDatabaseQuotaInBytes));
>-            return;
>-        }
>         // FIXME: This should be a message check.
>         ASSERT(m_sessionStorageNamespaces.isValidKey(storageNamespaceID));
> 
>@@ -829,7 +869,7 @@ void StorageManager::destroyStorageMap(IPC::Connection& connection, uint64_t sto
>         it->value->removeListener(connectionID, storageMapID);
> 
>         // Don't remove session storage maps. The web process may reconnect and expect the data to still be around.
>-        if (it->value->isSessionStorage())
>+        if (it->value->isEphemeral())
>             return;
> 
>         m_storageAreasByConnection.remove(connectionAndStorageMapIDPair);
>@@ -847,14 +887,11 @@ void StorageManager::getValues(IPC::Connection& connection, WebCore::SecurityOri
> {
>     m_queue->dispatch([this, protectedThis = makeRef(*this), connection = makeRef(connection), securityOriginData = securityOriginData.isolatedCopy(), storageMapID, storageMapSeed, completionHandler = WTFMove(completionHandler)]() mutable {
>         auto* storageArea = findStorageArea(connection.get(), storageMapID);
>-        if (!storageArea) {
>-            if (m_isEphemeral) {
>-                if (auto storageMap = m_ephemeralStorage.get(securityOriginData))
>-                    return didGetValues(connection.get(), storageMapID, storageMap->items(), WTFMove(completionHandler));
>-            }
>-            // This is a session storage area for a page that has already been closed. Ignore it.
>+
>+        // This is a session storage area for a page that has already been closed. Ignore it.
>+        if (!storageArea)
>             return didGetValues(connection.get(), storageMapID, { }, WTFMove(completionHandler));
>-        }
>+
>         didGetValues(connection.get(), storageMapID, storageArea->items(), WTFMove(completionHandler));
>         connection->send(Messages::StorageAreaMap::DidGetValues(storageMapSeed), storageMapID);
>     });
>@@ -864,17 +901,10 @@ void StorageManager::setItem(IPC::Connection& connection, WebCore::SecurityOrigi
> {
>     m_queue->dispatch([this, protectedThis = makeRef(*this), connection = makeRef(connection), securityOriginData = securityOriginData.isolatedCopy(), storageMapID, sourceStorageAreaID, storageMapSeed, key = key.isolatedCopy(), value = value.isolatedCopy(), urlString = urlString.isolatedCopy()]() mutable {
>         auto* storageArea = findStorageArea(connection.get(), storageMapID);
>-        if (!storageArea) {
>-            if (m_isEphemeral) {
>-                if (auto storageMap = m_ephemeralStorage.get(securityOriginData)) {
>-                    String oldValue;
>-                    bool quotaException;
>-                    storageMap->setItem(key, value, oldValue, quotaException);
>-                }
>-            }
>-            // This is a session storage area for a page that has already been closed. Ignore it.
>+
>+        // This is a session storage area for a page that has already been closed. Ignore it.
>+        if (!storageArea)
>             return;
>-        }
> 
>         bool quotaError;
>         storageArea->setItem(connection->uniqueID(), sourceStorageAreaID, key, value, urlString, quotaError);
>@@ -894,16 +924,10 @@ void StorageManager::removeItem(IPC::Connection& connection, WebCore::SecurityOr
> {
>     m_queue->dispatch([this, protectedThis = makeRef(*this), connection = makeRef(connection), securityOriginData = securityOriginData.isolatedCopy(), storageMapID, sourceStorageAreaID, storageMapSeed, key = key.isolatedCopy(), urlString = urlString.isolatedCopy()]() mutable {
>         auto* storageArea = findStorageArea(connection.get(), storageMapID);
>-        if (!storageArea) {
>-            if (m_isEphemeral) {
>-                if (auto storageMap = m_ephemeralStorage.get(securityOriginData)) {
>-                    String oldValue;
>-                    storageMap->removeItem(key, oldValue);
>-                }
>-            }
>-            // This is a session storage area for a page that has already been closed. Ignore it.
>+
>+        // This is a session storage area for a page that has already been closed. Ignore it.
>+        if (!storageArea)
>             return;
>-        }
> 
>         storageArea->removeItem(connection->uniqueID(), sourceStorageAreaID, key, urlString);
>         connection->send(Messages::StorageAreaMap::DidRemoveItem(storageMapSeed, key), storageMapID);
>@@ -914,12 +938,10 @@ void StorageManager::clear(IPC::Connection& connection, WebCore::SecurityOriginD
> {
>     m_queue->dispatch([this, protectedThis = makeRef(*this), connection = makeRef(connection), securityOriginData = securityOriginData.isolatedCopy(), storageMapID, sourceStorageAreaID, storageMapSeed, urlString = urlString.isolatedCopy()]() mutable {
>         auto* storageArea = findStorageArea(connection.get(), storageMapID);
>-        if (!storageArea) {
>-            if (m_isEphemeral)
>-                m_ephemeralStorage.remove(securityOriginData);
>-            // This is a session storage area for a page that has already been closed. Ignore it.
>+
>+        // This is a session storage area for a page that has already been closed. Ignore it.
>+        if (!storageArea)
>             return;
>-        }
> 
>         storageArea->clear(connection->uniqueID(), sourceStorageAreaID, urlString);
>         connection->send(Messages::StorageAreaMap::DidClear(storageMapSeed), storageMapID);
>diff --git a/Source/WebKit/NetworkProcess/WebStorage/StorageManager.h b/Source/WebKit/NetworkProcess/WebStorage/StorageManager.h
>index a888332efcfb594130b2608846af4a182182b790..0a2672f1b0557fb22d1875628b71ceb9eecb11c7 100644
>--- a/Source/WebKit/NetworkProcess/WebStorage/StorageManager.h
>+++ b/Source/WebKit/NetworkProcess/WebStorage/StorageManager.h
>@@ -101,7 +101,7 @@ private:
> 
>     Ref<WorkQueue> m_queue;
> 
>-    Ref<LocalStorageDatabaseTracker> m_localStorageDatabaseTracker;
>+    RefPtr<LocalStorageDatabaseTracker> m_localStorageDatabaseTracker;
>     HashMap<uint64_t, RefPtr<LocalStorageNamespace>> m_localStorageNamespaces;
> 
>     HashMap<std::pair<uint64_t, WebCore::SecurityOriginData>, RefPtr<TransientLocalStorageNamespace>> m_transientLocalStorageNamespaces;
>@@ -110,9 +110,6 @@ private:
>     HashMap<uint64_t, RefPtr<SessionStorageNamespace>> m_sessionStorageNamespaces;
> 
>     HashMap<std::pair<IPC::Connection::UniqueID, uint64_t>, RefPtr<StorageArea>> m_storageAreasByConnection;
>-
>-    HashMap<WebCore::SecurityOriginData, Ref<WebCore::StorageMap>> m_ephemeralStorage;
>-    bool m_isEphemeral { false };
> };
> 
> } // namespace WebKit
>diff --git a/Source/WebKit/WebProcess/WebStorage/StorageAreaMap.cpp b/Source/WebKit/WebProcess/WebStorage/StorageAreaMap.cpp
>index 0e94296049e6c087ab6e8f0c5ebfe4c1a2e2c078..dd743a9f969c4dd015659910bd7365f8b7eb3b46 100644
>--- a/Source/WebKit/WebProcess/WebStorage/StorageAreaMap.cpp
>+++ b/Source/WebKit/WebProcess/WebStorage/StorageAreaMap.cpp
>@@ -294,7 +294,7 @@ void StorageAreaMap::dispatchStorageEvent(uint64_t sourceStorageAreaID, const St
>         applyChange(key, newValue);
>     }
> 
>-    if (storageType() == StorageType::Session)
>+    if (storageType() == StorageType::Session || storageType() == StorageType::EphemeralLocal)
>         dispatchSessionStorageEvent(sourceStorageAreaID, key, oldValue, newValue, urlString);
>     else
>         dispatchLocalStorageEvent(sourceStorageAreaID, key, oldValue, newValue, urlString);
>@@ -307,9 +307,7 @@ void StorageAreaMap::clearCache()
> 
> void StorageAreaMap::dispatchSessionStorageEvent(uint64_t sourceStorageAreaID, const String& key, const String& oldValue, const String& newValue, const String& urlString)
> {
>-    ASSERT(storageType() == StorageType::Session);
>-
>-    // Namespace IDs for session storage namespaces are equivalent to web page IDs
>+    // Namespace IDs for session storage namespaces and ephemeral local storage namespaces are equivalent to web page IDs
>     // so we can get the right page here.
>     WebPage* webPage = WebProcess::singleton().webPage(makeObjectIdentifier<PageIdentifierType>(m_storageNamespaceID));
>     if (!webPage)
>@@ -377,6 +375,7 @@ void StorageAreaMap::connect()
> 
>     switch (m_storageType) {
>     case StorageType::Local:
>+    case StorageType::EphemeralLocal:
>     case StorageType::TransientLocal:
>         if (SecurityOrigin* topLevelOrigin = m_storageNamespace->topLevelOrigin())
>             WebProcess::singleton().ensureNetworkProcessConnection().connection().send(Messages::StorageManager::CreateTransientLocalStorageMap(m_storageMapID, m_storageNamespace->storageNamespaceID(), topLevelOrigin->data(), m_securityOrigin->data()), 0);
>@@ -385,14 +384,10 @@ void StorageAreaMap::connect()
>         break;
>     case StorageType::Session:
>         WebProcess::singleton().ensureNetworkProcessConnection().connection().send(Messages::StorageManager::CreateSessionStorageMap(m_storageMapID, m_storageNamespace->storageNamespaceID(), m_securityOrigin->data()), 0);
>-        if (m_storageMap)
>-            WebProcess::singleton().ensureNetworkProcessConnection().connection().send(Messages::StorageManager::SetItems(m_storageMapID, m_storageMap->items()), 0);
>-        break;
>-    case StorageType::EphemeralLocal:
>-        ASSERT_NOT_REACHED();
>-        return;
>     }
> 
>+    if (m_storageMap)
>+        WebProcess::singleton().ensureNetworkProcessConnection().connection().send(Messages::StorageManager::SetItems(m_storageMapID, m_storageMap->items()), 0);
>     m_isDisconnected = false;
> }
> 
>diff --git a/Source/WebKit/WebProcess/WebStorage/StorageNamespaceImpl.cpp b/Source/WebKit/WebProcess/WebStorage/StorageNamespaceImpl.cpp
>index 7ea5e7dd90242e360ef2f099bdeee679accc5e47..f59c79b7fa2be2fc9a72052c1b9e53cb6a323fd0 100644
>--- a/Source/WebKit/WebProcess/WebStorage/StorageNamespaceImpl.cpp
>+++ b/Source/WebKit/WebProcess/WebStorage/StorageNamespaceImpl.cpp
>@@ -47,12 +47,12 @@ Ref<StorageNamespaceImpl> StorageNamespaceImpl::createSessionStorageNamespace(ui
> 
> Ref<StorageNamespaceImpl> StorageNamespaceImpl::createEphemeralLocalStorageNamespace(uint64_t identifier, unsigned quotaInBytes)
> {
>-    return createSessionStorageNamespace(identifier, quotaInBytes);
>+    return createLocalStorageNamespace(identifier, quotaInBytes, IsEphemeral::Yes);
> }
> 
>-Ref<StorageNamespaceImpl> StorageNamespaceImpl::createLocalStorageNamespace(uint64_t identifier, unsigned quotaInBytes)
>+Ref<StorageNamespaceImpl> StorageNamespaceImpl::createLocalStorageNamespace(uint64_t identifier, unsigned quotaInBytes, IsEphemeral isEphemeral)
> {
>-    return adoptRef(*new StorageNamespaceImpl(StorageType::Local, identifier, nullptr, quotaInBytes));
>+    return adoptRef(*new StorageNamespaceImpl(isEphemeral == IsEphemeral::Yes ? StorageType::EphemeralLocal : StorageType::Local, identifier, nullptr, quotaInBytes));
> }
> 
> Ref<StorageNamespaceImpl> StorageNamespaceImpl::createTransientLocalStorageNamespace(uint64_t identifier, WebCore::SecurityOrigin& topLevelOrigin, uint64_t quotaInBytes)
>diff --git a/Source/WebKit/WebProcess/WebStorage/StorageNamespaceImpl.h b/Source/WebKit/WebProcess/WebStorage/StorageNamespaceImpl.h
>index 6757e3b818886c4ff57bc3e79f4c15800f75b1bc..ea6d5518cdfc99dc3a01d857e3663650f9b666a2 100644
>--- a/Source/WebKit/WebProcess/WebStorage/StorageNamespaceImpl.h
>+++ b/Source/WebKit/WebProcess/WebStorage/StorageNamespaceImpl.h
>@@ -41,7 +41,9 @@ class StorageNamespaceImpl : public WebCore::StorageNamespace {
> public:
>     static Ref<StorageNamespaceImpl> createSessionStorageNamespace(uint64_t identifier, unsigned quotaInBytes);
>     static Ref<StorageNamespaceImpl> createEphemeralLocalStorageNamespace(uint64_t identifier, unsigned quotaInBytes);
>-    static Ref<StorageNamespaceImpl> createLocalStorageNamespace(uint64_t identifier, unsigned quotaInBytes);
>+
>+    enum class IsEphemeral : bool { No, Yes };
>+    static Ref<StorageNamespaceImpl> createLocalStorageNamespace(uint64_t identifier, unsigned quotaInBytes, IsEphemeral isEphemeral);
>     static Ref<StorageNamespaceImpl> createTransientLocalStorageNamespace(uint64_t identifier, WebCore::SecurityOrigin& topLevelOrigin, uint64_t quotaInBytes);
> 
>     virtual ~StorageNamespaceImpl();
>diff --git a/Source/WebKit/WebProcess/WebStorage/WebStorageNamespaceProvider.cpp b/Source/WebKit/WebProcess/WebStorage/WebStorageNamespaceProvider.cpp
>index 9a6b6f3700a0b95851dea8106fcbb32a4b3073f1..1fa0ace154d43eb958cdce3ec956cf01a89eb9f6 100644
>--- a/Source/WebKit/WebProcess/WebStorage/WebStorageNamespaceProvider.cpp
>+++ b/Source/WebKit/WebProcess/WebStorage/WebStorageNamespaceProvider.cpp
>@@ -77,7 +77,7 @@ Ref<WebCore::StorageNamespace> WebStorageNamespaceProvider::createEphemeralLocal
> 
> Ref<WebCore::StorageNamespace> WebStorageNamespaceProvider::createLocalStorageNamespace(unsigned quota)
> {
>-    return StorageNamespaceImpl::createLocalStorageNamespace(m_identifier, quota);
>+    return StorageNamespaceImpl::createLocalStorageNamespace(m_identifier, quota, StorageNamespaceImpl::IsEphemeral::Yes);
> }
> 
> Ref<WebCore::StorageNamespace> WebStorageNamespaceProvider::createTransientLocalStorageNamespace(WebCore::SecurityOrigin& topLevelOrigin, unsigned quota)
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index be614c1f4723f5668cb184ffab739f324572f485..a2aad93be48387428c40a9538f7fb6539b96f4ac 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,13 @@
>+2019-05-29  Sihui Liu  <sihui_liu@apple.com>
>+
>+        WKWebsiteDataStore API fails to fetch web storage data for non-persistent data store
>+        https://bugs.webkit.org/show_bug.cgi?id=198317
>+
>+        Reviewed by Alex Christensen.
>+
>+        * TestWebKitAPI/Tests/WebKitCocoa/WKWebsiteDatastore.mm:
>+        (TestWebKitAPI::TEST):
>+
> 2019-05-29  Geoffrey Garen  <ggaren@apple.com>
> 
>         WeakPtr breaks vtables when upcasting to base classes
>diff --git a/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebsiteDatastore.mm b/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebsiteDatastore.mm
>index 3f953a707518fe3820673ea10323460cb9294d46..24075b725e04dae366ac9b4f560fba8e97e02f29 100644
>--- a/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebsiteDatastore.mm
>+++ b/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebsiteDatastore.mm
>@@ -304,4 +304,46 @@ TEST(WKWebsiteDataStore, RemoveNonPersistentCredentials)
>     TestWebKitAPI::Util::run(&done);
> }
> 
>+TEST(WKWebsiteDataStore, FetchNonPersistentWebStorage)
>+{
>+    auto nonPersistentDataStore = [WKWebsiteDataStore nonPersistentDataStore];
>+    auto configuration = adoptNS([WKWebViewConfiguration new]);
>+    [configuration setWebsiteDataStore:nonPersistentDataStore];
>+    auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:configuration.get()]);
>+    auto navigationDelegate = adoptNS([[NavigationTestDelegate alloc] init]);
>+    [webView setNavigationDelegate:navigationDelegate.get()];
>+    [webView loadHTMLString:@"<script>sessionStorage.setItem('session', 'storage');localStorage.setItem('local', 'storage');</script>" baseURL:[NSURL URLWithString:@"http://localhost"]];
>+    [navigationDelegate waitForDidFinishNavigation];
>+
>+    readyToContinue = false;
>+    [webView evaluateJavaScript:@"window.sessionStorage.getItem('session')" completionHandler:^(id result, NSError *) {
>+        EXPECT_TRUE([@"storage" isEqualToString:result]);
>+        readyToContinue = true;
>+    }];
>+    TestWebKitAPI::Util::run(&readyToContinue);
>+
>+    readyToContinue = false;
>+    [webView evaluateJavaScript:@"window.localStorage.getItem('local')" completionHandler:^(id result, NSError *) {
>+        EXPECT_TRUE([@"storage" isEqualToString:result]);
>+        readyToContinue = true;
>+    }];
>+    TestWebKitAPI::Util::run(&readyToContinue);
>+
>+    readyToContinue = false;
>+    [nonPersistentDataStore fetchDataRecordsOfTypes:[NSSet setWithObject:WKWebsiteDataTypeSessionStorage] completionHandler:^(NSArray<WKWebsiteDataRecord *> *dataRecords) {
>+        EXPECT_EQ((int)dataRecords.count, 1);
>+        EXPECT_TRUE([[[dataRecords objectAtIndex:0] displayName] isEqualToString:@"localhost"]);
>+        readyToContinue = true;
>+    }];
>+    TestWebKitAPI::Util::run(&readyToContinue);
>+
>+    readyToContinue = false;
>+    [nonPersistentDataStore fetchDataRecordsOfTypes:[NSSet setWithObject:WKWebsiteDataTypeLocalStorage] completionHandler:^(NSArray<WKWebsiteDataRecord *> *dataRecords) {
>+        EXPECT_EQ((int)dataRecords.count, 1);
>+        EXPECT_TRUE([[[dataRecords objectAtIndex:0] displayName] isEqualToString:@"localhost"]);
>+        readyToContinue = true;
>+    }];
>+    TestWebKitAPI::Util::run(&readyToContinue);
>+}
>+
> }

Actions: View | Formatted Diff | Diff

Attachments on bug 198317: 370815 | 370897 | 370995 | 371012 | 371325