Modified: trunk/Source/WebKit/ChangeLog (257518 => 257519)
--- trunk/Source/WebKit/ChangeLog 2020-02-26 22:41:40 UTC (rev 257518)
+++ trunk/Source/WebKit/ChangeLog 2020-02-26 22:44:03 UTC (rev 257519)
@@ -1,3 +1,41 @@
+2020-02-26 Kate Cheney <[email protected]>
+
+ Tests should each use a unique ResourceLoadStatistics file path for its database store
+ https://bugs.webkit.org/show_bug.cgi?id=208206
+ <rdar://problem/59690272>
+
+ Reviewed by Brady Eidson.
+
+ The ResourceLoadStatistics directory was being set to a non-unique
+ default value before being created at the temporary path generated by
+ WebKitTestRunner. This was causing crashes and concurrency issues in tests
+ when multiple database stores at the same path are accessed
+ simultaneously.
+
+ This patch updates WebProcessPool.cpp to treat the
+ resourceLoadStatisticsDirectory parameter like it does with IndexedDB
+ to avoid initializing multiple databases at the same file path.
+
+ * NetworkProcess/Classifier/ResourceLoadStatisticsDatabaseStore.cpp:
+ (WebKit::ResourceLoadStatisticsDatabaseStore::openAndDropOldDatabaseIfNecessary):
+ We should use SQLiteFileSystem::deleteDatabaseFile because it takes
+ care of additional SQLite files automatically.
+
+ * NetworkProcess/Classifier/WebResourceLoadStatisticsStore.cpp:
+ (WebKit::WebResourceLoadStatisticsStore::WebResourceLoadStatisticsStore):
+ (WebKit::WebResourceLoadStatisticsStore::populateMemoryStoreFromDisk):
+ Checks if the path is empty because we no longer
+ guarantee that the resourceLoadStatisticsDirectory is set. Also checks
+ if m_statisticsStore is initialized before trying to grandfather
+ statistics.
+
+ * UIProcess/WebProcessPool.cpp:
+ (WebKit::WebProcessPool::ensureNetworkProcess):
+ Checks if a default WebsiteDataStore exists before setting the
+ parameter to the default value. This way, the parameter is empty if
+ WebKitTestRunner hasn't yet set the temporary directory and we won't
+ create a new database.
+
2020-02-26 Christopher Reid <[email protected]>
[Win] Implement NetworkCache::Data by using FileSystem::MappedFileData
Modified: trunk/Source/WebKit/NetworkProcess/Classifier/WebResourceLoadStatisticsStore.cpp (257518 => 257519)
--- trunk/Source/WebKit/NetworkProcess/Classifier/WebResourceLoadStatisticsStore.cpp 2020-02-26 22:41:40 UTC (rev 257518)
+++ trunk/Source/WebKit/NetworkProcess/Classifier/WebResourceLoadStatisticsStore.cpp 2020-02-26 22:44:03 UTC (rev 257519)
@@ -52,6 +52,7 @@
#include <WebCore/NetworkStorageSession.h>
#include <WebCore/ResourceLoadStatistics.h>
#include <WebCore/SQLiteDatabase.h>
+#include <WebCore/SQLiteFileSystem.h>
#include <WebCore/SQLiteStatement.h>
#include <wtf/CallbackAggregator.h>
#include <wtf/CrossThreadCopier.h>
@@ -157,36 +158,30 @@
{
RELEASE_ASSERT(RunLoop::isMain());
- postTask([this, databaseEnabled = networkSession.networkProcess().isITPDatabaseEnabled(), resourceLoadStatisticsDirectory = resourceLoadStatisticsDirectory.isolatedCopy(), shouldIncludeLocalhost, sessionID = networkSession.sessionID()] {
- if (databaseEnabled) {
- m_statisticsStore = makeUnique<ResourceLoadStatisticsDatabaseStore>(*this, m_statisticsQueue, shouldIncludeLocalhost, resourceLoadStatisticsDirectory, sessionID);
+ if (!resourceLoadStatisticsDirectory.isEmpty()) {
+ postTask([this, databaseEnabled = networkSession.networkProcess().isITPDatabaseEnabled(), resourceLoadStatisticsDirectory = resourceLoadStatisticsDirectory.isolatedCopy(), shouldIncludeLocalhost, sessionID = networkSession.sessionID()] {
+ if (databaseEnabled) {
+ m_statisticsStore = makeUnique<ResourceLoadStatisticsDatabaseStore>(*this, m_statisticsQueue, shouldIncludeLocalhost, resourceLoadStatisticsDirectory, sessionID);
- auto memoryStore = makeUnique<ResourceLoadStatisticsMemoryStore>(*this, m_statisticsQueue, shouldIncludeLocalhost);
- downcast<ResourceLoadStatisticsDatabaseStore>(*m_statisticsStore.get()).populateFromMemoryStore(*memoryStore);
+ auto memoryStore = makeUnique<ResourceLoadStatisticsMemoryStore>(*this, m_statisticsQueue, shouldIncludeLocalhost);
+ downcast<ResourceLoadStatisticsDatabaseStore>(*m_statisticsStore.get()).populateFromMemoryStore(*memoryStore);
- auto legacyPlistFilePath = FileSystem::pathByAppendingComponent(resourceLoadStatisticsDirectory, "full_browsing_session_resourceLog.plist");
- if (FileSystem::fileExists(legacyPlistFilePath))
- FileSystem::deleteFile(legacyPlistFilePath);
+ auto legacyPlistFilePath = FileSystem::pathByAppendingComponent(resourceLoadStatisticsDirectory, "full_browsing_session_resourceLog.plist");
+ if (FileSystem::fileExists(legacyPlistFilePath))
+ FileSystem::deleteFile(legacyPlistFilePath);
- } else {
- m_statisticsStore = makeUnique<ResourceLoadStatisticsMemoryStore>(*this, m_statisticsQueue, shouldIncludeLocalhost);
- m_persistentStorage = makeUnique<ResourceLoadStatisticsPersistentStorage>(downcast<ResourceLoadStatisticsMemoryStore>(*m_statisticsStore), m_statisticsQueue, resourceLoadStatisticsDirectory);
+ } else {
+ m_statisticsStore = makeUnique<ResourceLoadStatisticsMemoryStore>(*this, m_statisticsQueue, shouldIncludeLocalhost);
+ m_persistentStorage = makeUnique<ResourceLoadStatisticsPersistentStorage>(downcast<ResourceLoadStatisticsMemoryStore>(*m_statisticsStore), m_statisticsQueue, resourceLoadStatisticsDirectory);
- auto databaseStorageFilePath = FileSystem::pathByAppendingComponent(resourceLoadStatisticsDirectory, "observations.db");
- auto databaseStorageTemporaryWalFilePath = FileSystem::pathByAppendingComponent(resourceLoadStatisticsDirectory, "observations.db-wal");
- auto databaseStorageTemporaryShmFilePath = FileSystem::pathByAppendingComponent(resourceLoadStatisticsDirectory, "observations.db-shm");
- if (FileSystem::fileExists(databaseStorageFilePath)) {
- FileSystem::deleteFile(databaseStorageFilePath);
- FileSystem::deleteFile(databaseStorageTemporaryWalFilePath);
- FileSystem::deleteFile(databaseStorageTemporaryShmFilePath);
+ SQLiteFileSystem::deleteDatabaseFile(FileSystem::pathByAppendingComponent(resourceLoadStatisticsDirectory, "observations.db"));
}
- }
- // FIXME(193297): This should be revised after the UIProcess version goes away.
- m_statisticsStore->didCreateNetworkProcess();
- });
+ m_statisticsStore->didCreateNetworkProcess();
+ });
- m_dailyTasksTimer.startRepeating(24_h);
+ m_dailyTasksTimer.startRepeating(24_h);
+ }
}
WebResourceLoadStatisticsStore::~WebResourceLoadStatisticsStore()
@@ -244,7 +239,7 @@
m_persistentStorage->populateMemoryStoreFromDisk([protectedThis = WTFMove(protectedThis), completionHandler = WTFMove(completionHandler)]() mutable {
postTaskReply(WTFMove(completionHandler));
});
- else if (is<ResourceLoadStatisticsDatabaseStore>(*m_statisticsStore)) {
+ else if (m_statisticsStore && is<ResourceLoadStatisticsDatabaseStore>(*m_statisticsStore)) {
auto& databaseStore = downcast<ResourceLoadStatisticsDatabaseStore>(*m_statisticsStore);
if (databaseStore.isNewResourceLoadStatisticsDatabaseFile()) {
m_statisticsStore->grandfatherExistingWebsiteData([protectedThis = WTFMove(protectedThis), completionHandler = WTFMove(completionHandler)]() mutable {
Modified: trunk/Source/WebKit/UIProcess/WebProcessPool.cpp (257518 => 257519)
--- trunk/Source/WebKit/UIProcess/WebProcessPool.cpp 2020-02-26 22:41:40 UTC (rev 257518)
+++ trunk/Source/WebKit/UIProcess/WebProcessPool.cpp 2020-02-26 22:44:03 UTC (rev 257519)
@@ -571,11 +571,12 @@
if (m_websiteDataStore)
parameters.defaultDataStoreParameters.networkSessionParameters.resourceLoadStatisticsDirectory = m_websiteDataStore->resolvedResourceLoadStatisticsDirectory();
- if (parameters.defaultDataStoreParameters.networkSessionParameters.resourceLoadStatisticsDirectory.isEmpty())
- parameters.defaultDataStoreParameters.networkSessionParameters.resourceLoadStatisticsDirectory = WebKit::WebsiteDataStore::defaultResourceLoadStatisticsDirectory();
+ else if (WebKit::WebsiteDataStore::defaultDataStoreExists())
+ parameters.defaultDataStoreParameters.networkSessionParameters.resourceLoadStatisticsDirectory = WebKit::WebsiteDataStore::defaultDataStore()->parameters().networkSessionParameters.resourceLoadStatisticsDirectory;
+
+ if (!parameters.defaultDataStoreParameters.networkSessionParameters.resourceLoadStatisticsDirectory.isEmpty())
+ SandboxExtension::createHandleForReadWriteDirectory(parameters.defaultDataStoreParameters.networkSessionParameters.resourceLoadStatisticsDirectory, parameters.defaultDataStoreParameters.networkSessionParameters.resourceLoadStatisticsDirectoryExtensionHandle);
- SandboxExtension::createHandleForReadWriteDirectory(parameters.defaultDataStoreParameters.networkSessionParameters.resourceLoadStatisticsDirectory, parameters.defaultDataStoreParameters.networkSessionParameters.resourceLoadStatisticsDirectoryExtensionHandle);
-
bool enableResourceLoadStatistics = false;
bool enableResourceLoadStatisticsLogTestingEvent = false;
bool shouldIncludeLocalhost = true;
Modified: trunk/Tools/ChangeLog (257518 => 257519)
--- trunk/Tools/ChangeLog 2020-02-26 22:41:40 UTC (rev 257518)
+++ trunk/Tools/ChangeLog 2020-02-26 22:44:03 UTC (rev 257519)
@@ -1,3 +1,17 @@
+2020-02-26 Kate Cheney <[email protected]>
+
+ Tests should each use a unique ResourceLoadStatistics file path for its database store
+ https://bugs.webkit.org/show_bug.cgi?id=208206
+ <rdar://problem/59690272>
+
+ Reviewed by Brady Eidson.
+
+ Initialize a default website data store before checking for the ITP
+ store to ensure the directory exists after the file has been deleted.
+
+ * TestWebKitAPI/Tests/WebKitCocoa/WebsiteDataStoreCustomPaths.mm:
+ (TEST):
+
2020-02-26 Christopher Reid <[email protected]>
[Win] Implement NetworkCache::Data by using FileSystem::MappedFileData
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WebsiteDataStoreCustomPaths.mm (257518 => 257519)
--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WebsiteDataStoreCustomPaths.mm 2020-02-26 22:41:40 UTC (rev 257518)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WebsiteDataStoreCustomPaths.mm 2020-02-26 22:44:03 UTC (rev 257519)
@@ -428,6 +428,7 @@
handler = nil;
configuration = nil;
+ [WKWebsiteDataStore defaultDataStore];
EXPECT_TRUE([[NSFileManager defaultManager] fileExistsAtPath:defaultResourceLoadStatisticsPath.path]);
EXPECT_FALSE([[NSFileManager defaultManager] fileExistsAtPath:defaultResourceLoadStatisticsFilePath.path]);
@@ -471,6 +472,7 @@
handler = nil;
configuration = nil;
+ [WKWebsiteDataStore defaultDataStore];
EXPECT_TRUE([[NSFileManager defaultManager] fileExistsAtPath:defaultResourceLoadStatisticsPath.path]);
EXPECT_FALSE([[NSFileManager defaultManager] fileExistsAtPath:defaultResourceLoadStatisticsFilePath.path]);
