- Revision
- 290233
- Author
- [email protected]
- Date
- 2022-02-20 15:09:55 -0800 (Sun, 20 Feb 2022)
Log Message
Origin file may not be created after migrating data to general storage directory
https://bugs.webkit.org/show_bug.cgi?id=236901
Reviewed by Chris Dumez.
Source/WebKit:
In r289081, we started writing origin file asynchronouly (we used to write it at creation of
OriginStorageManager), to avoid delay in replying sync WebStorage message. In the async task, we check if the
OriginStorageManager still exists. If it does not, we will not write origin file, because the
OriginStorageManager may be removed after origin data gets deleted, and there's no need to add new origin file.
It's also possible that the OriginStorageManager is created for website data fetch, and removed immediately
after the fetch task is done (see NetworkStorageManager::fetchDataFromDisk). We used to not store
new data in general storage directory during data fetch, so it's fine to not write origin file in this case.
With r289878, existing data can be migrated to general storage directory at data fetch, so it is important to
ensure we write origin to file; otherwise, we will lose track of the origin. To solve this, let's restore the
old behavior that synchronouly writes origin file at creation of OriginStorageManager. To solve the delay issue,
in the sync message handler (NetworkStorageManager::connectToStorageArea), we specifically ask to not write
origin file, and manually write origin file after message is replied.
Updated existing API test to add test coverage.
* NetworkProcess/storage/NetworkStorageManager.cpp:
(WebKit::NetworkStorageManager::localOriginStorageManager):
(WebKit::NetworkStorageManager::connectToStorageArea):
* NetworkProcess/storage/NetworkStorageManager.h:
Tools:
* TestWebKitAPI/Tests/WebKitCocoa/WebsiteDataStoreCustomPaths.mm:
(TEST):
Modified Paths
Diff
Modified: trunk/Source/WebKit/ChangeLog (290232 => 290233)
--- trunk/Source/WebKit/ChangeLog 2022-02-20 20:47:12 UTC (rev 290232)
+++ trunk/Source/WebKit/ChangeLog 2022-02-20 23:09:55 UTC (rev 290233)
@@ -1,3 +1,31 @@
+2022-02-20 Sihui Liu <[email protected]>
+
+ Origin file may not be created after migrating data to general storage directory
+ https://bugs.webkit.org/show_bug.cgi?id=236901
+
+ Reviewed by Chris Dumez.
+
+ In r289081, we started writing origin file asynchronouly (we used to write it at creation of
+ OriginStorageManager), to avoid delay in replying sync WebStorage message. In the async task, we check if the
+ OriginStorageManager still exists. If it does not, we will not write origin file, because the
+ OriginStorageManager may be removed after origin data gets deleted, and there's no need to add new origin file.
+ It's also possible that the OriginStorageManager is created for website data fetch, and removed immediately
+ after the fetch task is done (see NetworkStorageManager::fetchDataFromDisk). We used to not store
+ new data in general storage directory during data fetch, so it's fine to not write origin file in this case.
+
+ With r289878, existing data can be migrated to general storage directory at data fetch, so it is important to
+ ensure we write origin to file; otherwise, we will lose track of the origin. To solve this, let's restore the
+ old behavior that synchronouly writes origin file at creation of OriginStorageManager. To solve the delay issue,
+ in the sync message handler (NetworkStorageManager::connectToStorageArea), we specifically ask to not write
+ origin file, and manually write origin file after message is replied.
+
+ Updated existing API test to add test coverage.
+
+ * NetworkProcess/storage/NetworkStorageManager.cpp:
+ (WebKit::NetworkStorageManager::localOriginStorageManager):
+ (WebKit::NetworkStorageManager::connectToStorageArea):
+ * NetworkProcess/storage/NetworkStorageManager.h:
+
2022-02-20 Brandon Stewart <[email protected]>
Generate compile_commands.json on macOS Builds
Modified: trunk/Source/WebKit/NetworkProcess/storage/NetworkStorageManager.cpp (290232 => 290233)
--- trunk/Source/WebKit/NetworkProcess/storage/NetworkStorageManager.cpp 2022-02-20 20:47:12 UTC (rev 290232)
+++ trunk/Source/WebKit/NetworkProcess/storage/NetworkStorageManager.cpp 2022-02-20 23:09:55 UTC (rev 290233)
@@ -227,21 +227,14 @@
return FileSystem::pathByAppendingComponent(directory, OriginStorageManager::originFileIdentifier());
}
-OriginStorageManager& NetworkStorageManager::localOriginStorageManager(const WebCore::ClientOrigin& origin)
+OriginStorageManager& NetworkStorageManager::localOriginStorageManager(const WebCore::ClientOrigin& origin, ShouldWriteOriginFile shouldWriteOriginFile)
{
ASSERT(!RunLoop::isMain());
return *m_localOriginStorageManagers.ensure(origin, [&] {
auto originDirectory = originDirectoryPath(m_path, origin, m_salt);
- // Write origin file asynchronously to avoid delay in replying sync messages for Web Storage API.
- if (!originDirectory.isEmpty()) {
- m_queue->dispatch([this, protectedThis = Ref { *this }, origin, originDirectory]() mutable {
- if (!m_localOriginStorageManagers.contains(origin))
- return;
-
- writeOriginToFileIfNecessary(originFilePath(originDirectory), origin);
- });
- }
+ if (!originDirectory.isEmpty() && shouldWriteOriginFile == ShouldWriteOriginFile::Yes)
+ writeOriginToFileIfNecessary(originFilePath(originDirectory), origin);
auto localStoragePath = LocalStorageManager::localStorageFilePath(m_customLocalStoragePath, origin);
auto idbStoragePath = IDBStorageManager::idbStorageOriginDirectory(m_customIDBStoragePath, origin);
auto cacheStoragePath = CacheStorage::Engine::storagePath(m_customCacheStoragePath, origin);
@@ -729,7 +722,17 @@
ASSERT(!RunLoop::isMain());
auto connectionIdentifier = connection.uniqueID();
- auto& originStorageManager = localOriginStorageManager(origin);
+ bool willCreateOriginStorageManager = !m_localOriginStorageManagers.contains(origin);
+ // Avoid delay in replying sync message by writing origin file after replying message.
+ auto& originStorageManager = localOriginStorageManager(origin, ShouldWriteOriginFile::No);
+ auto writeOriginFile = makeScopeExit([&] {
+ if (!willCreateOriginStorageManager)
+ return;
+
+ if (auto originDirectory = originDirectoryPath(m_path, origin, m_salt); !originDirectory.isEmpty())
+ writeOriginToFileIfNecessary(originFilePath(originDirectory), origin);
+ });
+
StorageAreaIdentifier resultIdentifier;
switch (type) {
case WebCore::StorageType::Local:
Modified: trunk/Source/WebKit/NetworkProcess/storage/NetworkStorageManager.h (290232 => 290233)
--- trunk/Source/WebKit/NetworkProcess/storage/NetworkStorageManager.h 2022-02-20 20:47:12 UTC (rev 290232)
+++ trunk/Source/WebKit/NetworkProcess/storage/NetworkStorageManager.h 2022-02-20 23:09:55 UTC (rev 290233)
@@ -97,7 +97,8 @@
private:
NetworkStorageManager(PAL::SessionID, IPC::Connection::UniqueID, const String& path, const String& customLocalStoragePath, const String& customIDBStoragePath, const String& customCacheStoragePath, uint64_t defaultOriginQuota, uint64_t defaultThirdPartyOriginQuota, bool shouldUseCustomPaths);
~NetworkStorageManager();
- OriginStorageManager& localOriginStorageManager(const WebCore::ClientOrigin&);
+ enum class ShouldWriteOriginFile : bool { No, Yes };
+ OriginStorageManager& localOriginStorageManager(const WebCore::ClientOrigin&, ShouldWriteOriginFile = ShouldWriteOriginFile::Yes);
bool removeOriginStorageManagerIfPossible(const WebCore::ClientOrigin&);
void deleteOriginDirectoryIfPossible(const WebCore::ClientOrigin&);
Modified: trunk/Tools/ChangeLog (290232 => 290233)
--- trunk/Tools/ChangeLog 2022-02-20 20:47:12 UTC (rev 290232)
+++ trunk/Tools/ChangeLog 2022-02-20 23:09:55 UTC (rev 290233)
@@ -1,3 +1,13 @@
+2022-02-20 Sihui Liu <[email protected]>
+
+ Origin file may not be created after migrating data to general storage directory
+ https://bugs.webkit.org/show_bug.cgi?id=236901
+
+ Reviewed by Chris Dumez.
+
+ * TestWebKitAPI/Tests/WebKitCocoa/WebsiteDataStoreCustomPaths.mm:
+ (TEST):
+
2022-02-20 Brandon Stewart <[email protected]>
Generate compile_commands.json on macOS Builds
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WebsiteDataStoreCustomPaths.mm (290232 => 290233)
--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WebsiteDataStoreCustomPaths.mm 2022-02-20 20:47:12 UTC (rev 290232)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WebsiteDataStoreCustomPaths.mm 2022-02-20 23:09:55 UTC (rev 290233)
@@ -962,8 +962,10 @@
NSURL *webkitGeneralStorageDirectory = [generalStorageDirectory URLByAppendingPathComponent:@"YUn_wgR51VLVo9lc5xiivAzZ8TMmojoa0IbW323qibs/YUn_wgR51VLVo9lc5xiivAzZ8TMmojoa0IbW323qibs"];
NSURL *webkitGeneralLocalStorageDirectory = [webkitGeneralStorageDirectory URLByAppendingPathComponent:@"LocalStorage"];
NSURL *webKitGeneralLocalStorageFile = [webkitGeneralLocalStorageDirectory URLByAppendingPathComponent:@"localstorage.sqlite3"];
- NSURL *appleGeneralLocalStorageFile = [generalStorageDirectory URLByAppendingPathComponent:@"xKK8XQeHebxhSzMyotOOtNKp2jWZ4l_CEN4WggYYXbI/xKK8XQeHebxhSzMyotOOtNKp2jWZ4l_CEN4WggYYXbI/LocalStorage/localstorage.sqlite3"];
- NSURL *appleGeneralIndexedDBDatabaseFile = [generalStorageDirectory URLByAppendingPathComponent:@"xKK8XQeHebxhSzMyotOOtNKp2jWZ4l_CEN4WggYYXbI/xKK8XQeHebxhSzMyotOOtNKp2jWZ4l_CEN4WggYYXbI/IndexedDB/620AD548000B0A49C02D2E8D75C32E0F85697B54DF86146ECAE360DE104AB3F9/IndexedDB.sqlite3"];
+ NSURL *appleGeneralStorageDirectory = [generalStorageDirectory URLByAppendingPathComponent:@"xKK8XQeHebxhSzMyotOOtNKp2jWZ4l_CEN4WggYYXbI/xKK8XQeHebxhSzMyotOOtNKp2jWZ4l_CEN4WggYYXbI"];
+ NSURL *appleGeneralOriginFile = [appleGeneralStorageDirectory URLByAppendingPathComponent:@"origin"];
+ NSURL *appleGeneralLocalStorageFile = [appleGeneralStorageDirectory URLByAppendingPathComponent:@"LocalStorage/localstorage.sqlite3"];
+ NSURL *appleGeneralIndexedDBDatabaseFile = [appleGeneralStorageDirectory URLByAppendingPathComponent:@"IndexedDB/620AD548000B0A49C02D2E8D75C32E0F85697B54DF86146ECAE360DE104AB3F9/IndexedDB.sqlite3"];
// Copy apple.com files to custom path.
NSFileManager *fileManager = [NSFileManager defaultManager];
@@ -989,12 +991,12 @@
// Ensure data is fetched from both custom path and new path.
auto dataTypes = [NSSet setWithObjects:WKWebsiteDataTypeLocalStorage, WKWebsiteDataTypeIndexedDBDatabases, nil];
+ auto sortFunction = ^(WKWebsiteDataRecord *record1, WKWebsiteDataRecord *record2) {
+ return [record1.displayName compare:record2.displayName];
+ };
done = false;
[websiteDataStore fetchDataRecordsOfTypes:dataTypes completionHandler:^(NSArray<WKWebsiteDataRecord *> *records) {
EXPECT_EQ(records.count, 2u);
- auto sortFunction = ^(WKWebsiteDataRecord *record1, WKWebsiteDataRecord *record2) {
- return [record1.displayName compare:record2.displayName];
- };
auto sortedRecords = [records sortedArrayUsingComparator:sortFunction];
auto appleRecord = [sortedRecords objectAtIndex:0];
EXPECT_WK_STREQ(@"apple.com", [appleRecord displayName]);
@@ -1009,8 +1011,18 @@
// Ensure apple.com data is moved to new path after fetch.
EXPECT_FALSE([fileManager fileExistsAtPath:appleCustomLocalStorageFile.path]);
EXPECT_FALSE([fileManager fileExistsAtPath:appleCustomIndexedDBDatabaseFile.path]);
+ EXPECT_TRUE([fileManager fileExistsAtPath:appleGeneralOriginFile.path]);
EXPECT_TRUE([fileManager fileExistsAtPath:appleGeneralLocalStorageFile.path]);
EXPECT_TRUE([fileManager fileExistsAtPath:appleGeneralIndexedDBDatabaseFile.path]);
+
+ done = false;
+ [websiteDataStore fetchDataRecordsOfTypes:dataTypes completionHandler:^(NSArray<WKWebsiteDataRecord *> *records) {
+ EXPECT_EQ(records.count, 2u);
+ auto sortedRecords = [records sortedArrayUsingComparator:sortFunction];
+ EXPECT_WK_STREQ(@"apple.com", [[sortedRecords objectAtIndex:0] displayName]);
+ done = true;
+ }];
+ TestWebKitAPI::Util::run(&done);
}
TEST(WebKit, DeleteDataAfterEnablingGeneralStorageDirectory)
