Title: [245978] trunk
Revision
245978
Author
ryanhad...@apple.com
Date
2019-05-31 12:49:16 -0700 (Fri, 31 May 2019)

Log Message

Unreviewed, rolling out r245943.

Caused API test WKWebView.LocalStorageProcessSuspends to fail
on release bots.

Reverted changeset:

"WKWebsiteDataStore API fails to fetch web storage data for
non-persistent data store"
https://bugs.webkit.org/show_bug.cgi?id=198317
https://trac.webkit.org/changeset/245943

Modified Paths

Diff

Modified: trunk/Source/WebKit/ChangeLog (245977 => 245978)


--- trunk/Source/WebKit/ChangeLog	2019-05-31 19:07:30 UTC (rev 245977)
+++ trunk/Source/WebKit/ChangeLog	2019-05-31 19:49:16 UTC (rev 245978)
@@ -1,5 +1,19 @@
 2019-05-31  Ryan Haddad  <ryanhad...@apple.com>
 
+        Unreviewed, rolling out r245943.
+
+        Caused API test WKWebView.LocalStorageProcessSuspends to fail
+        on release bots.
+
+        Reverted changeset:
+
+        "WKWebsiteDataStore API fails to fetch web storage data for
+        non-persistent data store"
+        https://bugs.webkit.org/show_bug.cgi?id=198317
+        https://trac.webkit.org/changeset/245943
+
+2019-05-31  Ryan Haddad  <ryanhad...@apple.com>
+
         Unreviewed, rolling out r245946.
 
         Breaks the watchOS build.

Modified: trunk/Source/WebKit/NetworkProcess/WebStorage/StorageManager.cpp (245977 => 245978)


--- trunk/Source/WebKit/NetworkProcess/WebStorage/StorageManager.cpp	2019-05-31 19:07:30 UTC (rev 245977)
+++ trunk/Source/WebKit/NetworkProcess/WebStorage/StorageManager.cpp	2019-05-31 19:49:16 UTC (rev 245978)
@@ -63,7 +63,7 @@
     const HashMap<String, String>& items() const;
     void clear();
 
-    bool isEphemeral() const { return !m_localStorageNamespace; }
+    bool isSessionStorage() const { return !m_localStorageNamespace; }
 
 private:
     explicit StorageArea(LocalStorageNamespace*, const SecurityOriginData&, unsigned quotaInBytes);
@@ -72,7 +72,7 @@
 
     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 or the storage area is in an ephemeral session.
+    // Will be null if the storage area belongs to a session storage namespace.
     LocalStorageNamespace* m_localStorageNamespace;
     mutable RefPtr<LocalStorageDatabase> m_localStorageDatabase;
     mutable bool m_didImportItemsFromDatabase { false };
@@ -91,16 +91,12 @@
 
     StorageManager* storageManager() const { return &m_storageManager; }
 
-    enum class IsEphemeral : bool { No, Yes };
-    Ref<StorageArea> getOrCreateStorageArea(SecurityOriginData&&, IsEphemeral);
+    Ref<StorageArea> getOrCreateStorageArea(SecurityOriginData&&);
     void didDestroyStorageArea(StorageArea*);
 
     void clearStorageAreasMatchingOrigin(const SecurityOriginData&);
     void clearAllStorageAreas();
 
-    Vector<SecurityOriginData> ephemeralOrigins() const;
-    void cloneTo(LocalStorageNamespace& newLocalStorageNamespace);
-
 private:
     LocalStorageNamespace(StorageManager&, uint64_t storageManagerID);
 
@@ -108,7 +104,8 @@
     uint64_t m_storageNamespaceID;
     unsigned m_quotaInBytes;
 
-    HashMap<SecurityOriginData, RefPtr<StorageArea>> m_storageAreaMap;
+    // 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;
 };
 
 // Suggested by https://www.w3.org/TR/webstorage/#disk-space
@@ -199,7 +196,7 @@
 
 void StorageManager::StorageArea::removeListener(IPC::Connection::UniqueID connectionID, uint64_t storageMapID)
 {
-    ASSERT(isEphemeral() || m_eventListeners.contains(std::make_pair(connectionID, storageMapID)));
+    ASSERT(isSessionStorage() || m_eventListeners.contains(std::make_pair(connectionID, storageMapID)));
     m_eventListeners.remove(std::make_pair(connectionID, storageMapID));
 }
 
@@ -239,10 +236,7 @@
 
 void StorageManager::StorageArea::setItems(const HashMap<String, String>& items)
 {
-    // Import items from web process if items are not stored on disk.
-    if (!isEphemeral())
-        return;
-
+    ASSERT(!m_localStorageDatabase);
     for (auto& item : items) {
         String oldValue;
         bool quotaException;
@@ -317,10 +311,9 @@
     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, m_securityOrigin);
+        m_localStorageDatabase = LocalStorageDatabase::create(m_localStorageNamespace->storageManager()->m_queue.copyRef(), m_localStorageNamespace->storageManager()->m_localStorageDatabaseTracker.copyRef(), m_securityOrigin);
 
     if (m_didImportItemsFromDatabase)
         return;
@@ -357,13 +350,14 @@
 
 StorageManager::LocalStorageNamespace::~LocalStorageNamespace()
 {
+    ASSERT(m_storageAreaMap.isEmpty());
 }
 
-auto StorageManager::LocalStorageNamespace::getOrCreateStorageArea(SecurityOriginData&& securityOrigin, IsEphemeral isEphemeral) -> Ref<StorageArea>
+auto StorageManager::LocalStorageNamespace::getOrCreateStorageArea(SecurityOriginData&& securityOrigin) -> Ref<StorageArea>
 {
     RefPtr<StorageArea> protectedStorageArea;
     return *m_storageAreaMap.ensure(securityOrigin, [&]() mutable {
-        protectedStorageArea = StorageArea::create(isEphemeral == IsEphemeral::Yes ? nullptr : this, WTFMove(securityOrigin), m_quotaInBytes);
+        protectedStorageArea = StorageArea::create(this, WTFMove(securityOrigin), m_quotaInBytes);
         return protectedStorageArea.get();
     }).iterator->value;
 }
@@ -389,26 +383,10 @@
 
 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);
@@ -505,11 +483,11 @@
 
 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()
@@ -570,13 +548,6 @@
         ASSERT(newSessionStorageNamespace);
 
         sessionStorageNamespace->cloneTo(*newSessionStorageNamespace);
-
-        if (!m_localStorageDatabaseTracker) {
-            if (auto* localStorageNamespace = m_localStorageNamespaces.get(storageNamespaceID)) {
-                LocalStorageNamespace* newlocalStorageNamespace = getOrCreateLocalStorageNamespace(newStorageNamespaceID);
-                localStorageNamespace->cloneTo(*newlocalStorageNamespace);
-            }
-        }
     });
 }
 
@@ -658,15 +629,8 @@
     m_queue->dispatch([this, protectedThis = makeRef(*this), completionHandler = WTFMove(completionHandler)]() mutable {
         HashSet<SecurityOriginData> origins;
 
-        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& origin : m_localStorageDatabaseTracker->origins())
+            origins.add(origin);
 
         for (auto& transientLocalStorageNamespace : m_transientLocalStorageNamespaces.values()) {
             for (auto& origin : transientLocalStorageNamespace->origins())
@@ -682,9 +646,7 @@
 void StorageManager::getLocalStorageOriginDetails(Function<void(Vector<LocalStorageDatabaseTracker::OriginDetails>&&)>&& completionHandler)
 {
     m_queue->dispatch([this, protectedThis = makeRef(*this), completionHandler = WTFMove(completionHandler)]() mutable {
-        Vector<LocalStorageDatabaseTracker::OriginDetails> originDetails;
-        if (m_localStorageDatabaseTracker)
-            originDetails = m_localStorageDatabaseTracker->originDetails();
+        auto originDetails = m_localStorageDatabaseTracker->originDetails();
 
         RunLoop::main().dispatch([originDetails = WTFMove(originDetails), completionHandler = WTFMove(completionHandler)]() mutable {
             completionHandler(WTFMove(originDetails));
@@ -701,8 +663,7 @@
         for (auto& transientLocalStorageNamespace : m_transientLocalStorageNamespaces.values())
             transientLocalStorageNamespace->clearStorageAreasMatchingOrigin(copiedOrigin);
 
-        if (m_localStorageDatabaseTracker)
-            m_localStorageDatabaseTracker->deleteDatabaseWithOrigin(copiedOrigin);
+        m_localStorageDatabaseTracker->deleteDatabaseWithOrigin(copiedOrigin);
     });
 }
 
@@ -709,21 +670,16 @@
 void StorageManager::deleteLocalStorageOriginsModifiedSince(WallTime time, Function<void()>&& completionHandler)
 {
     m_queue->dispatch([this, protectedThis = makeRef(*this), time, completionHandler = WTFMove(completionHandler)]() mutable {
-        if (m_localStorageDatabaseTracker) {
-            auto originsToDelete = m_localStorageDatabaseTracker->databasesModifiedSince(time);
-            
-            for (auto& transientLocalStorageNamespace : m_transientLocalStorageNamespaces.values())
-                transientLocalStorageNamespace->clearAllStorageAreas();
+        auto originsToDelete = m_localStorageDatabaseTracker->databasesModifiedSince(time);
+        
+        for (auto& transientLocalStorageNamespace : m_transientLocalStorageNamespaces.values())
+            transientLocalStorageNamespace->clearAllStorageAreas();
 
-            for (const auto& origin : originsToDelete) {
-                for (auto& localStorageNamespace : m_localStorageNamespaces.values())
-                    localStorageNamespace->clearStorageAreasMatchingOrigin(origin);
-                
-                m_localStorageDatabaseTracker->deleteDatabaseWithOrigin(origin);
-            }
-        } else {
+        for (const auto& origin : originsToDelete) {
             for (auto& localStorageNamespace : m_localStorageNamespaces.values())
-                localStorageNamespace->clearAllStorageAreas();
+                localStorageNamespace->clearStorageAreasMatchingOrigin(origin);
+            
+            m_localStorageDatabaseTracker->deleteDatabaseWithOrigin(origin);
         }
 
         RunLoop::main().dispatch(WTFMove(completionHandler));
@@ -746,8 +702,7 @@
             for (auto& transientLocalStorageNamespace : m_transientLocalStorageNamespaces.values())
                 transientLocalStorageNamespace->clearStorageAreasMatchingOrigin(origin);
 
-            if (m_localStorageDatabaseTracker)
-                m_localStorageDatabaseTracker->deleteDatabaseWithOrigin(origin);
+            m_localStorageDatabaseTracker->deleteDatabaseWithOrigin(origin);
         }
 
         RunLoop::main().dispatch(WTFMove(completionHandler));
@@ -757,6 +712,7 @@
 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.
@@ -773,7 +729,7 @@
         // FIXME: This should be a message check.
         ASSERT(localStorageNamespace);
 
-        auto storageArea = localStorageNamespace->getOrCreateStorageArea(WTFMove(securityOriginData), m_localStorageDatabaseTracker ? StorageManager::LocalStorageNamespace::IsEphemeral::No : StorageManager::LocalStorageNamespace::IsEphemeral::Yes);
+        auto storageArea = localStorageNamespace->getOrCreateStorageArea(WTFMove(securityOriginData));
         storageArea->addListener(connectionID, storageMapID);
 
         result.iterator->value = WTFMove(storageArea);
@@ -792,7 +748,7 @@
             if (it->key.first != connectionID)
                 continue;
             Ref<StorageArea> area = *it->value;
-            if (!area->isEphemeral())
+            if (!area->isSessionStorage())
                 continue;
             if (!origin.securityOrigin()->isSameSchemeHostPort(area->securityOrigin().securityOrigin().get()))
                 continue;
@@ -824,6 +780,10 @@
 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));
 
@@ -869,7 +829,7 @@
         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->isEphemeral())
+        if (it->value->isSessionStorage())
             return;
 
         m_storageAreasByConnection.remove(connectionAndStorageMapIDPair);
@@ -887,11 +847,14 @@
 {
     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);
-
-        // This is a session storage area for a page that has already been closed. Ignore it.
-        if (!storageArea)
+        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.
             return didGetValues(connection.get(), storageMapID, { }, WTFMove(completionHandler));
-
+        }
         didGetValues(connection.get(), storageMapID, storageArea->items(), WTFMove(completionHandler));
         connection->send(Messages::StorageAreaMap::DidGetValues(storageMapSeed), storageMapID);
     });
@@ -901,10 +864,17 @@
 {
     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);
-
-        // This is a session storage area for a page that has already been closed. Ignore it.
-        if (!storageArea)
+        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.
             return;
+        }
 
         bool quotaError;
         storageArea->setItem(connection->uniqueID(), sourceStorageAreaID, key, value, urlString, quotaError);
@@ -924,10 +894,16 @@
 {
     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);
-
-        // This is a session storage area for a page that has already been closed. Ignore it.
-        if (!storageArea)
+        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.
             return;
+        }
 
         storageArea->removeItem(connection->uniqueID(), sourceStorageAreaID, key, urlString);
         connection->send(Messages::StorageAreaMap::DidRemoveItem(storageMapSeed, key), storageMapID);
@@ -938,10 +914,12 @@
 {
     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);
-
-        // This is a session storage area for a page that has already been closed. Ignore it.
-        if (!storageArea)
+        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.
             return;
+        }
 
         storageArea->clear(connection->uniqueID(), sourceStorageAreaID, urlString);
         connection->send(Messages::StorageAreaMap::DidClear(storageMapSeed), storageMapID);
@@ -968,7 +946,7 @@
 
 void StorageManager::suspend(CompletionHandler<void()>&& completionHandler)
 {
-    if (!m_localStorageDatabaseTracker)
+    if (m_isEphemeral)
         return;
 
     Locker<Lock> stateLocker(m_stateLock);
@@ -993,7 +971,7 @@
 
 void StorageManager::resume()
 {
-    if (!m_localStorageDatabaseTracker)
+    if (m_isEphemeral)
         return;
 
     Locker<Lock> stateLocker(m_stateLock);

Modified: trunk/Source/WebKit/NetworkProcess/WebStorage/StorageManager.h (245977 => 245978)


--- trunk/Source/WebKit/NetworkProcess/WebStorage/StorageManager.h	2019-05-31 19:07:30 UTC (rev 245977)
+++ trunk/Source/WebKit/NetworkProcess/WebStorage/StorageManager.h	2019-05-31 19:49:16 UTC (rev 245978)
@@ -103,7 +103,7 @@
 
     Ref<WorkQueue> m_queue;
 
-    RefPtr<LocalStorageDatabaseTracker> m_localStorageDatabaseTracker;
+    Ref<LocalStorageDatabaseTracker> m_localStorageDatabaseTracker;
     HashMap<uint64_t, RefPtr<LocalStorageNamespace>> m_localStorageNamespaces;
 
     HashMap<std::pair<uint64_t, WebCore::SecurityOriginData>, RefPtr<TransientLocalStorageNamespace>> m_transientLocalStorageNamespaces;
@@ -113,6 +113,9 @@
 
     HashMap<std::pair<IPC::Connection::UniqueID, uint64_t>, RefPtr<StorageArea>> m_storageAreasByConnection;
 
+    HashMap<WebCore::SecurityOriginData, Ref<WebCore::StorageMap>> m_ephemeralStorage;
+    bool m_isEphemeral { false };
+
     enum class State {
         Running,
         WillSuspend,

Modified: trunk/Source/WebKit/WebProcess/WebStorage/StorageAreaMap.cpp (245977 => 245978)


--- trunk/Source/WebKit/WebProcess/WebStorage/StorageAreaMap.cpp	2019-05-31 19:07:30 UTC (rev 245977)
+++ trunk/Source/WebKit/WebProcess/WebStorage/StorageAreaMap.cpp	2019-05-31 19:49:16 UTC (rev 245978)
@@ -294,7 +294,7 @@
         applyChange(key, newValue);
     }
 
-    if (storageType() == StorageType::Session || storageType() == StorageType::EphemeralLocal)
+    if (storageType() == StorageType::Session)
         dispatchSessionStorageEvent(sourceStorageAreaID, key, oldValue, newValue, urlString);
     else
         dispatchLocalStorageEvent(sourceStorageAreaID, key, oldValue, newValue, urlString);
@@ -307,7 +307,9 @@
 
 void StorageAreaMap::dispatchSessionStorageEvent(uint64_t sourceStorageAreaID, const String& key, const String& oldValue, const String& newValue, const String& urlString)
 {
-    // Namespace IDs for session storage namespaces and ephemeral local storage namespaces are equivalent to web page IDs
+    ASSERT(storageType() == StorageType::Session);
+
+    // Namespace IDs for session 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)
@@ -375,7 +377,6 @@
 
     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);
@@ -384,10 +385,14 @@
         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;
 }
 

Modified: trunk/Source/WebKit/WebProcess/WebStorage/StorageNamespaceImpl.cpp (245977 => 245978)


--- trunk/Source/WebKit/WebProcess/WebStorage/StorageNamespaceImpl.cpp	2019-05-31 19:07:30 UTC (rev 245977)
+++ trunk/Source/WebKit/WebProcess/WebStorage/StorageNamespaceImpl.cpp	2019-05-31 19:49:16 UTC (rev 245978)
@@ -47,12 +47,12 @@
 
 Ref<StorageNamespaceImpl> StorageNamespaceImpl::createEphemeralLocalStorageNamespace(uint64_t identifier, unsigned quotaInBytes)
 {
-    return createLocalStorageNamespace(identifier, quotaInBytes, IsEphemeral::Yes);
+    return createSessionStorageNamespace(identifier, quotaInBytes);
 }
 
-Ref<StorageNamespaceImpl> StorageNamespaceImpl::createLocalStorageNamespace(uint64_t identifier, unsigned quotaInBytes, IsEphemeral isEphemeral)
+Ref<StorageNamespaceImpl> StorageNamespaceImpl::createLocalStorageNamespace(uint64_t identifier, unsigned quotaInBytes)
 {
-    return adoptRef(*new StorageNamespaceImpl(isEphemeral == IsEphemeral::Yes ? StorageType::EphemeralLocal : StorageType::Local, identifier, nullptr, quotaInBytes));
+    return adoptRef(*new StorageNamespaceImpl(StorageType::Local, identifier, nullptr, quotaInBytes));
 }
 
 Ref<StorageNamespaceImpl> StorageNamespaceImpl::createTransientLocalStorageNamespace(uint64_t identifier, WebCore::SecurityOrigin& topLevelOrigin, uint64_t quotaInBytes)

Modified: trunk/Source/WebKit/WebProcess/WebStorage/StorageNamespaceImpl.h (245977 => 245978)


--- trunk/Source/WebKit/WebProcess/WebStorage/StorageNamespaceImpl.h	2019-05-31 19:07:30 UTC (rev 245977)
+++ trunk/Source/WebKit/WebProcess/WebStorage/StorageNamespaceImpl.h	2019-05-31 19:49:16 UTC (rev 245978)
@@ -41,9 +41,7 @@
 public:
     static Ref<StorageNamespaceImpl> createSessionStorageNamespace(uint64_t identifier, unsigned quotaInBytes);
     static Ref<StorageNamespaceImpl> createEphemeralLocalStorageNamespace(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> createLocalStorageNamespace(uint64_t identifier, unsigned quotaInBytes);
     static Ref<StorageNamespaceImpl> createTransientLocalStorageNamespace(uint64_t identifier, WebCore::SecurityOrigin& topLevelOrigin, uint64_t quotaInBytes);
 
     virtual ~StorageNamespaceImpl();

Modified: trunk/Source/WebKit/WebProcess/WebStorage/WebStorageNamespaceProvider.cpp (245977 => 245978)


--- trunk/Source/WebKit/WebProcess/WebStorage/WebStorageNamespaceProvider.cpp	2019-05-31 19:07:30 UTC (rev 245977)
+++ trunk/Source/WebKit/WebProcess/WebStorage/WebStorageNamespaceProvider.cpp	2019-05-31 19:49:16 UTC (rev 245978)
@@ -77,7 +77,7 @@
 
 Ref<WebCore::StorageNamespace> WebStorageNamespaceProvider::createLocalStorageNamespace(unsigned quota)
 {
-    return StorageNamespaceImpl::createLocalStorageNamespace(m_identifier, quota, StorageNamespaceImpl::IsEphemeral::No);
+    return StorageNamespaceImpl::createLocalStorageNamespace(m_identifier, quota);
 }
 
 Ref<WebCore::StorageNamespace> WebStorageNamespaceProvider::createTransientLocalStorageNamespace(WebCore::SecurityOrigin& topLevelOrigin, unsigned quota)

Modified: trunk/Tools/ChangeLog (245977 => 245978)


--- trunk/Tools/ChangeLog	2019-05-31 19:07:30 UTC (rev 245977)
+++ trunk/Tools/ChangeLog	2019-05-31 19:49:16 UTC (rev 245978)
@@ -1,3 +1,17 @@
+2019-05-31  Ryan Haddad  <ryanhad...@apple.com>
+
+        Unreviewed, rolling out r245943.
+
+        Caused API test WKWebView.LocalStorageProcessSuspends to fail
+        on release bots.
+
+        Reverted changeset:
+
+        "WKWebsiteDataStore API fails to fetch web storage data for
+        non-persistent data store"
+        https://bugs.webkit.org/show_bug.cgi?id=198317
+        https://trac.webkit.org/changeset/245943
+
 2019-05-31  Alexey Proskuryakov  <a...@apple.com>
 
         WebKitTestRunner sometimes freezes under -[NSWindow release]

Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebsiteDatastore.mm (245977 => 245978)


--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebsiteDatastore.mm	2019-05-31 19:07:30 UTC (rev 245977)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebsiteDatastore.mm	2019-05-31 19:49:16 UTC (rev 245978)
@@ -304,46 +304,4 @@
     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);
 }
-
-}
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to