Diff
Modified: trunk/Source/WebKit/ChangeLog (290032 => 290033)
--- trunk/Source/WebKit/ChangeLog 2022-02-17 17:57:53 UTC (rev 290032)
+++ trunk/Source/WebKit/ChangeLog 2022-02-17 18:19:24 UTC (rev 290033)
@@ -1,3 +1,34 @@
+2022-02-17 Sihui Liu <sihui_...@apple.com>
+
+ Ensure NetworkStorageManager::moveData performs move operation based on data types
+ https://bugs.webkit.org/show_bug.cgi?id=236727
+ rdar://89009881
+
+ Reviewed by Chris Dumez.
+
+ In r286936, we introduced NetworkStorageManager::moveData to move data from one origin to another. However, the
+ function does not take dataTypes parameter as NetworkProcess::renameOriginInWebsiteData does, and
+ NetworkStorageManager would try to move every storage type it manages. This has caused an unexpected failure, as
+ FileSystem storage needs special handling before data can be moved. To address the regression, let's revert
+ the behavior change by making sure NetworkStorageManager checks the types value, and only performs move
+ operation for IndexedDB and LocalStorage data.
+
+ Updated existing API test to ensure only expected data types are moved.
+
+ * NetworkProcess/NetworkProcess.cpp:
+ (WebKit::NetworkProcess::renameOriginInWebsiteData):
+ * NetworkProcess/storage/NetworkStorageManager.cpp:
+ (WebKit::NetworkStorageManager::moveData):
+ * NetworkProcess/storage/NetworkStorageManager.h:
+ * NetworkProcess/storage/OriginStorageManager.cpp:
+ (WebKit::OriginStorageManager::StorageBucket::moveData):
+ (WebKit::OriginStorageManager::StorageBucket::resolvedLocalStoragePath):
+ (WebKit::OriginStorageManager::StorageBucket::deleteIDBStorageData):
+ (WebKit::OriginStorageManager::resolvedLocalStoragePath):
+ (WebKit::OriginStorageManager::resolvedIDBStoragePath):
+ (WebKit::OriginStorageManager::moveData):
+ * NetworkProcess/storage/OriginStorageManager.h:
+
2022-02-17 Kimmo Kinnunen <kkinnu...@apple.com>
MediaPlayer::videoFrameForCurrentTime() should return VideoFrame
Modified: trunk/Source/WebKit/NetworkProcess/NetworkProcess.cpp (290032 => 290033)
--- trunk/Source/WebKit/NetworkProcess/NetworkProcess.cpp 2022-02-17 17:57:53 UTC (rev 290032)
+++ trunk/Source/WebKit/NetworkProcess/NetworkProcess.cpp 2022-02-17 18:19:24 UTC (rev 290033)
@@ -2247,7 +2247,7 @@
auto* session = networkSession(sessionID);
if (auto* manager = session ? session->storageManager() : nullptr)
- manager->moveData(oldOrigin, newOrigin, [aggregator] { });
+ manager->moveData(dataTypes, oldOrigin, newOrigin, [aggregator] { });
}
#if ENABLE(SERVICE_WORKER)
Modified: trunk/Source/WebKit/NetworkProcess/storage/NetworkStorageManager.cpp (290032 => 290033)
--- trunk/Source/WebKit/NetworkProcess/storage/NetworkStorageManager.cpp 2022-02-17 17:57:53 UTC (rev 290032)
+++ trunk/Source/WebKit/NetworkProcess/storage/NetworkStorageManager.cpp 2022-02-17 18:19:24 UTC (rev 290033)
@@ -609,27 +609,22 @@
});
}
-void NetworkStorageManager::moveData(const WebCore::SecurityOriginData& source, const WebCore::SecurityOriginData& target, CompletionHandler<void()>&& completionHandler)
+void NetworkStorageManager::moveData(OptionSet<WebsiteDataType> types, const WebCore::SecurityOriginData& source, const WebCore::SecurityOriginData& target, CompletionHandler<void()>&& completionHandler)
{
ASSERT(RunLoop::isMain());
ASSERT(!m_closed);
- m_queue->dispatch([this, protectedThis = Ref { *this }, source = crossThreadCopy(source), target = crossThreadCopy(target), completionHandler = WTFMove(completionHandler)]() mutable {
+ m_queue->dispatch([this, protectedThis = Ref { *this }, types, source = crossThreadCopy(source), target = crossThreadCopy(target), completionHandler = WTFMove(completionHandler)]() mutable {
auto sourceOrigin = WebCore::ClientOrigin { source, source };
auto targetOrigin = WebCore::ClientOrigin { target, target };
// Clear existing data of target origin.
- OptionSet<WebsiteDataType> types = {
- WebsiteDataType::FileSystem,
- WebsiteDataType::LocalStorage,
- WebsiteDataType::SessionStorage,
- WebsiteDataType::IndexedDBDatabases
- };
localOriginStorageManager(targetOrigin).deleteData(types, -WallTime::infinity());
- removeOriginStorageManagerIfPossible(targetOrigin);
// Move data from source origin to target origin.
- localOriginStorageManager(sourceOrigin).moveData(originDirectoryPath(m_path, targetOrigin, m_salt), LocalStorageManager::localStorageFilePath(m_customLocalStoragePath, targetOrigin), IDBStorageManager::idbStorageOriginDirectory(m_customIDBStoragePath, targetOrigin));
+ localOriginStorageManager(sourceOrigin).moveData(types, localOriginStorageManager(targetOrigin).resolvedLocalStoragePath(), localOriginStorageManager(targetOrigin).resolvedIDBStoragePath());
+
+ removeOriginStorageManagerIfPossible(targetOrigin);
removeOriginStorageManagerIfPossible(sourceOrigin);
RunLoop::main().dispatch(WTFMove(completionHandler));
Modified: trunk/Source/WebKit/NetworkProcess/storage/NetworkStorageManager.h (290032 => 290033)
--- trunk/Source/WebKit/NetworkProcess/storage/NetworkStorageManager.h 2022-02-17 17:57:53 UTC (rev 290032)
+++ trunk/Source/WebKit/NetworkProcess/storage/NetworkStorageManager.h 2022-02-17 18:19:24 UTC (rev 290033)
@@ -84,7 +84,7 @@
void deleteData(OptionSet<WebsiteDataType>, const Vector<WebCore::SecurityOriginData>&, CompletionHandler<void()>&&);
void deleteDataModifiedSince(OptionSet<WebsiteDataType>, WallTime, CompletionHandler<void()>&&);
void deleteDataForRegistrableDomains(OptionSet<WebsiteDataType>, const Vector<WebCore::RegistrableDomain>&, CompletionHandler<void(HashSet<WebCore::RegistrableDomain>&&)>&&);
- void moveData(const WebCore::SecurityOriginData& source, const WebCore::SecurityOriginData& target, CompletionHandler<void()>&&);
+ void moveData(OptionSet<WebsiteDataType>, const WebCore::SecurityOriginData& source, const WebCore::SecurityOriginData& target, CompletionHandler<void()>&&);
void suspend(CompletionHandler<void()>&&);
void resume();
void handleLowMemoryWarning();
Modified: trunk/Source/WebKit/NetworkProcess/storage/OriginStorageManager.cpp (290032 => 290033)
--- trunk/Source/WebKit/NetworkProcess/storage/OriginStorageManager.cpp 2022-02-17 17:57:53 UTC (rev 290032)
+++ trunk/Source/WebKit/NetworkProcess/storage/OriginStorageManager.cpp 2022-02-17 18:19:24 UTC (rev 290033)
@@ -218,29 +218,51 @@
deleteIDBStorageData(modifiedSinceTime);
}
- void moveData(const String& path, const String& localStoragePath, const String& idbStoragePath)
+ void moveData(OptionSet<WebsiteDataType> types, const String& localStoragePath, const String& idbStoragePath)
{
- m_fileSystemStorageManager = nullptr;
- if (m_localStorageManager)
- m_localStorageManager->close();
+ // This is only supported for IndexedDB and LocalStorage now.
+ if (types.contains(WebsiteDataType::LocalStorage) && !localStoragePath.isEmpty()) {
+ if (m_localStorageManager)
+ m_localStorageManager->close();
- if (m_idbStorageManager)
- m_idbStorageManager->closeDatabasesForDeletion();
+ auto currentLocalStoragePath = resolvedLocalStoragePath();
+ if (!currentLocalStoragePath.isEmpty()) {
+ FileSystem::makeAllDirectories(FileSystem::parentPath(localStoragePath));
+ WebCore::SQLiteFileSystem::moveDatabaseFile(currentLocalStoragePath, localStoragePath);
+ }
+ }
- FileSystem::makeAllDirectories(FileSystem::parentPath(path));
- FileSystem::moveFile(m_rootPath, path);
+ if (types.contains(WebsiteDataType::IndexedDBDatabases) && !idbStoragePath.isEmpty()) {
+ if (m_idbStorageManager)
+ m_idbStorageManager->closeDatabasesForDeletion();
- auto currentLocalStoragePath = resolvedLocalStoragePath();
- if (!currentLocalStoragePath.isEmpty() && !localStoragePath.isEmpty()) {
- FileSystem::makeAllDirectories(FileSystem::parentPath(localStoragePath));
- WebCore::SQLiteFileSystem::moveDatabaseFile(currentLocalStoragePath, localStoragePath);
+ auto currentIDBStoragePath = resolvedIDBStoragePath();
+ if (!currentIDBStoragePath.isEmpty()) {
+ FileSystem::makeAllDirectories(FileSystem::parentPath(idbStoragePath));
+ FileSystem::moveFile(currentIDBStoragePath, idbStoragePath);
+ }
}
+ }
- auto currentIDBStoragePath = resolvedIDBStoragePath();
- if (!currentIDBStoragePath.isEmpty() && !idbStoragePath.isEmpty()) {
- FileSystem::makeAllDirectories(FileSystem::parentPath(idbStoragePath));
- WebCore::SQLiteFileSystem::moveDatabaseFile(currentIDBStoragePath, idbStoragePath);
+ String resolvedLocalStoragePath()
+ {
+ if (!m_resolvedLocalStoragePath.isNull())
+ return m_resolvedLocalStoragePath;
+
+ if (m_shouldUseCustomPaths) {
+ ASSERT(m_customLocalStoragePath.isEmpty() == m_rootPath.isEmpty());
+ m_resolvedLocalStoragePath = m_customLocalStoragePath;
+ } else {
+ auto localStoragePath = LocalStorageManager::localStorageFilePath(typeStoragePath(StorageType::LocalStorage));
+ if (!m_customLocalStoragePath.isEmpty() && !FileSystem::fileExists(localStoragePath)) {
+ FileSystem::makeAllDirectories(FileSystem::parentPath(localStoragePath));
+ WebCore::SQLiteFileSystem::moveDatabaseFile(m_customLocalStoragePath, localStoragePath);
+ }
+
+ m_resolvedLocalStoragePath = localStoragePath;
}
+
+ return m_resolvedLocalStoragePath;
}
String resolvedIDBStoragePath()
@@ -352,28 +374,7 @@
FileSystem::deleteAllFilesModifiedSince(resolvedIDBStoragePath(), time);
}
-
- String resolvedLocalStoragePath()
- {
- if (!m_resolvedLocalStoragePath.isNull())
- return m_resolvedLocalStoragePath;
- if (m_shouldUseCustomPaths) {
- ASSERT(m_customLocalStoragePath.isEmpty() == m_rootPath.isEmpty());
- m_resolvedLocalStoragePath = m_customLocalStoragePath;
- } else {
- auto localStoragePath = LocalStorageManager::localStorageFilePath(typeStoragePath(StorageType::LocalStorage));
- if (!m_customLocalStoragePath.isEmpty() && !FileSystem::fileExists(localStoragePath)) {
- FileSystem::makeAllDirectories(FileSystem::parentPath(localStoragePath));
- WebCore::SQLiteFileSystem::moveDatabaseFile(m_customLocalStoragePath, localStoragePath);
- }
-
- m_resolvedLocalStoragePath = localStoragePath;
- }
-
- return m_resolvedLocalStoragePath;
- }
-
String m_rootPath;
String m_identifier;
StorageBucketMode m_mode { StorageBucketMode::BestEffort };
@@ -481,6 +482,16 @@
return defaultBucket().existingIDBStorageManager();
}
+String OriginStorageManager::resolvedLocalStoragePath()
+{
+ return defaultBucket().resolvedLocalStoragePath();
+}
+
+String OriginStorageManager::resolvedIDBStoragePath()
+{
+ return defaultBucket().resolvedIDBStoragePath();
+}
+
bool OriginStorageManager::isActive()
{
return defaultBucket().isActive();
@@ -513,11 +524,11 @@
defaultBucket().deleteData(types, modifiedSince);
}
-void OriginStorageManager::moveData(const String& newPath, const String& localStoragePath, const String& idbStoragePath)
+void OriginStorageManager::moveData(OptionSet<WebsiteDataType> types, const String& localStoragePath, const String& idbStoragePath)
{
ASSERT(!RunLoop::isMain());
- defaultBucket().moveData(newPath, localStoragePath, idbStoragePath);
+ defaultBucket().moveData(types, localStoragePath, idbStoragePath);
}
} // namespace WebKit
Modified: trunk/Source/WebKit/NetworkProcess/storage/OriginStorageManager.h (290032 => 290033)
--- trunk/Source/WebKit/NetworkProcess/storage/OriginStorageManager.h 2022-02-17 17:57:53 UTC (rev 290032)
+++ trunk/Source/WebKit/NetworkProcess/storage/OriginStorageManager.h 2022-02-17 18:19:24 UTC (rev 290033)
@@ -59,11 +59,13 @@
SessionStorageManager* existingSessionStorageManager();
IDBStorageManager& idbStorageManager(IDBStorageRegistry&);
IDBStorageManager* existingIDBStorageManager();
+ String resolvedLocalStoragePath();
+ String resolvedIDBStoragePath();
bool isActive();
bool isEmpty();
OptionSet<WebsiteDataType> fetchDataTypesInList(OptionSet<WebsiteDataType>);
void deleteData(OptionSet<WebsiteDataType>, WallTime);
- void moveData(const String& newPath, const String& localStoragePath, const String& idbStoragePath);
+ void moveData(OptionSet<WebsiteDataType>, const String& localStoragePath, const String& idbStoragePath);
private:
enum class StorageBucketMode : bool;
Modified: trunk/Tools/ChangeLog (290032 => 290033)
--- trunk/Tools/ChangeLog 2022-02-17 17:57:53 UTC (rev 290032)
+++ trunk/Tools/ChangeLog 2022-02-17 18:19:24 UTC (rev 290033)
@@ -1,3 +1,14 @@
+2022-02-17 Sihui Liu <sihui_...@apple.com>
+
+ Ensure NetworkStorageManager::moveData performs move operation based on data types
+ https://bugs.webkit.org/show_bug.cgi?id=236727
+ rdar://89009881
+
+ Reviewed by Chris Dumez.
+
+ * TestWebKitAPI/Tests/WebKitCocoa/WebsiteDataStoreCustomPaths.mm:
+ (TEST):
+
2022-02-17 Angelos Oikonomopoulos <ange...@igalia.com>
[JSC] Increase timeout to account for MIPS slowness
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WebsiteDataStoreCustomPaths.mm (290032 => 290033)
--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WebsiteDataStoreCustomPaths.mm 2022-02-17 17:57:53 UTC (rev 290032)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WebsiteDataStoreCustomPaths.mm 2022-02-17 18:19:24 UTC (rev 290033)
@@ -542,6 +542,7 @@
[webView loadHTMLString:testString baseURL:exampleURL];
EXPECT_WK_STREQ("database is created", getNextMessage().body);
+ [webView stringByEvaluatingJavaScript:@"localStorage.setItem('testkey', 'testvalue')"];
auto dataStore = webView.get().configuration.websiteDataStore;
auto indexedDBType = adoptNS([[NSSet alloc] initWithObjects:WKWebsiteDataTypeIndexedDBDatabases, nil]);
@@ -554,6 +555,8 @@
[webView loadHTMLString:testString baseURL:webKitURL];
EXPECT_WK_STREQ("database exists", getNextMessage().body);
+ // Ensure LocalStorage data is not moved.
+ EXPECT_WK_STREQ("<null>", [webView stringByEvaluatingJavaScript:@"localStorage.getItem('testkey')"]);
[webView loadHTMLString:testString baseURL:exampleURL];
EXPECT_WK_STREQ("database is created", getNextMessage().body);
