Title: [223442] trunk
- Revision
- 223442
- Author
- [email protected]
- Date
- 2017-10-16 15:22:40 -0700 (Mon, 16 Oct 2017)
Log Message
If an origin doesn't have databases in the Databases table we should still remove its information from disk in DatabaseTracker::deleteOrigin()
https://bugs.webkit.org/show_bug.cgi?id=178281
<rdar://problem/34576132>
Patch by Maureen Daum <[email protected]> on 2017-10-16
Reviewed by Brent Fulgham.
Source/WebCore:
New test:
DatabaseTracker.DeleteOriginWithMissingEntryInDatabasesTable
* Modules/webdatabase/DatabaseTracker.cpp:
(WebCore::DatabaseTracker::deleteOrigin):
If databaseNames is empty, don't bail early. Instead, delete everything in the directory
containing the databases for this origin. This condition indicates that we previously
tried to remove the origin but didn't get all of the way through the deletion process.
Because we have lost track of the databases for this origin, we can assume that no
other process is accessing them. This means it should be safe to delete them outright.
Tools:
Verify that if there is an entry in the Origins table but no entries in the Databases
table that we still remove the directory for the origin, and that we remove the
entry from the Origins table.
* TestWebKitAPI/Tests/WebCore/cocoa/DatabaseTrackerTest.mm:
(TestWebKitAPI::TEST):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (223441 => 223442)
--- trunk/Source/WebCore/ChangeLog 2017-10-16 21:57:27 UTC (rev 223441)
+++ trunk/Source/WebCore/ChangeLog 2017-10-16 22:22:40 UTC (rev 223442)
@@ -1,3 +1,22 @@
+2017-10-16 Maureen Daum <[email protected]>
+
+ If an origin doesn't have databases in the Databases table we should still remove its information from disk in DatabaseTracker::deleteOrigin()
+ https://bugs.webkit.org/show_bug.cgi?id=178281
+ <rdar://problem/34576132>
+
+ Reviewed by Brent Fulgham.
+
+ New test:
+ DatabaseTracker.DeleteOriginWithMissingEntryInDatabasesTable
+
+ * Modules/webdatabase/DatabaseTracker.cpp:
+ (WebCore::DatabaseTracker::deleteOrigin):
+ If databaseNames is empty, don't bail early. Instead, delete everything in the directory
+ containing the databases for this origin. This condition indicates that we previously
+ tried to remove the origin but didn't get all of the way through the deletion process.
+ Because we have lost track of the databases for this origin, we can assume that no
+ other process is accessing them. This means it should be safe to delete them outright.
+
2017-10-16 Youenn Fablet <[email protected]>
[FETCH] Remove Request.type getter
Modified: trunk/Source/WebCore/Modules/webdatabase/DatabaseTracker.cpp (223441 => 223442)
--- trunk/Source/WebCore/Modules/webdatabase/DatabaseTracker.cpp 2017-10-16 21:57:27 UTC (rev 223441)
+++ trunk/Source/WebCore/Modules/webdatabase/DatabaseTracker.cpp 2017-10-16 22:22:40 UTC (rev 223442)
@@ -823,10 +823,9 @@
return false;
databaseNames = databaseNamesNoLock(origin);
- if (databaseNames.isEmpty()) {
+ if (databaseNames.isEmpty())
LOG_ERROR("Unable to retrieve list of database names for origin %s", origin.databaseIdentifier().utf8().data());
- return false;
- }
+
if (!canDeleteOrigin(origin)) {
LOG_ERROR("Tried to delete an origin (%s) while either creating database in it or already deleting it", origin.databaseIdentifier().utf8().data());
ASSERT_NOT_REACHED();
@@ -845,6 +844,20 @@
}
}
+ // If databaseNames is empty, delete everything in the directory containing the databases for this origin.
+ // This condition indicates that we previously tried to remove the origin but didn't get all of the way
+ // through the deletion process. Because we have lost track of the databases for this origin,
+ // we can assume that no other process is accessing them. This means it should be safe to delete them outright.
+ if (databaseNames.isEmpty()) {
+#if PLATFORM(COCOA)
+ RELEASE_LOG_ERROR(DatabaseTracker, "Unable to retrieve list of database names for origin");
+#endif
+ for (const auto& file : listDirectory(originPath(origin), "*")) {
+ if (!deleteFile(file))
+ failedToDeleteAnyDatabaseFile = true;
+ }
+ }
+
// If we failed to delete any database file, don't remove the origin from the tracker
// database because we didn't successfully remove all of its data.
if (failedToDeleteAnyDatabaseFile) {
Modified: trunk/Tools/ChangeLog (223441 => 223442)
--- trunk/Tools/ChangeLog 2017-10-16 21:57:27 UTC (rev 223441)
+++ trunk/Tools/ChangeLog 2017-10-16 22:22:40 UTC (rev 223442)
@@ -1,3 +1,18 @@
+2017-10-16 Maureen Daum <[email protected]>
+
+ If an origin doesn't have databases in the Databases table we should still remove its information from disk in DatabaseTracker::deleteOrigin()
+ https://bugs.webkit.org/show_bug.cgi?id=178281
+ <rdar://problem/34576132>
+
+ Reviewed by Brent Fulgham.
+
+ Verify that if there is an entry in the Origins table but no entries in the Databases
+ table that we still remove the directory for the origin, and that we remove the
+ entry from the Origins table.
+
+ * TestWebKitAPI/Tests/WebCore/cocoa/DatabaseTrackerTest.mm:
+ (TestWebKitAPI::TEST):
+
2017-10-15 Ryosuke Niwa <[email protected]>
Cannot access images included in the content pasted from Microsoft Word
Modified: trunk/Tools/TestWebKitAPI/Tests/WebCore/cocoa/DatabaseTrackerTest.mm (223441 => 223442)
--- trunk/Tools/TestWebKitAPI/Tests/WebCore/cocoa/DatabaseTrackerTest.mm 2017-10-16 21:57:27 UTC (rev 223441)
+++ trunk/Tools/TestWebKitAPI/Tests/WebCore/cocoa/DatabaseTrackerTest.mm 2017-10-16 22:22:40 UTC (rev 223442)
@@ -228,6 +228,45 @@
EXPECT_FALSE(fileExists(databaseDirectoryPath));
}
+TEST(DatabaseTracker, DeleteOriginWithMissingEntryInDatabasesTable)
+{
+ // Test the case where there is an entry in the Origins table but not
+ // the Databases table. There is an actual database on disk.
+ // The information should still be removed from the Origins table, and the
+ // database should be deleted from disk.
+ NSString *webSQLDirectory = createTemporaryDirectory(@"WebSQL");
+ String databaseDirectoryPath(webSQLDirectory.UTF8String);
+
+ std::unique_ptr<DatabaseTracker> databaseTracker = DatabaseTracker::trackerWithDatabasePath(databaseDirectoryPath);
+ SecurityOriginData origin("https", "webkit.org", 443);
+
+ databaseTracker->setQuota(origin, 5242880);
+ EXPECT_EQ((unsigned)1, databaseTracker->origins().size());
+
+ String databasePath = pathByAppendingComponent(databaseDirectoryPath, "Databases.db");
+ EXPECT_TRUE(fileExists(databasePath));
+
+ EXPECT_TRUE(databaseTracker->databaseNames(origin).isEmpty());
+
+ String originPath = pathByAppendingComponent(databaseDirectoryPath, origin.databaseIdentifier());
+ EXPECT_TRUE(makeAllDirectories(originPath));
+ EXPECT_TRUE(fileExists(originPath));
+
+ String webDatabasePath = pathByAppendingComponent(originPath, "database.db");
+ createFileAtPath(webDatabasePath);
+
+ EXPECT_TRUE(databaseTracker->deleteOrigin(origin));
+
+ EXPECT_FALSE(fileExists(webDatabasePath));
+ EXPECT_FALSE(fileExists(originPath));
+
+ EXPECT_TRUE(databaseTracker->origins().isEmpty());
+ EXPECT_TRUE(databaseTracker->databaseNames(origin).isEmpty());
+
+ removeDirectoryAndAllContents(databaseDirectoryPath);
+ EXPECT_FALSE(fileExists(databaseDirectoryPath));
+}
+
TEST(DatabaseTracker, DeleteDatabase)
{
// Test the expected case. There is an entry in the Databases table
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
