- Revision
- 223419
- Author
- bfulg...@apple.com
- Date
- 2017-10-16 11:08:49 -0700 (Mon, 16 Oct 2017)
Log Message
If we fail to delete any database file, don't remove its information from the tracker database
<rdar://problem/34576132> and https://bugs.webkit.org/show_bug.cgi?id=178251
Patch by Maureen Daum <md...@apple.com> on 2017-10-16
Reviewed by Brady Eidson.
New tests:
DatabaseTracker.DeleteDatabase
DatabaseTracker.DeleteDatabaseWhenDatabaseDoesNotExist
DatabaseTracker.DeleteOrigin
DatabaseTracker.DeleteOriginWhenDeletingADatabaseFails
DatabaseTracker.DeleteOriginWhenDatabaseDoesNotExist
* Modules/webdatabase/DatabaseTracker.cpp:
(WebCore::DatabaseTracker::deleteDatabasesModifiedSince):
If the database doesn't exist, we previously deleted it but failed to remove the
information from the tracker database. We still want to delete all of the information
associated with this database from the tracker database, so add it to databaseNamesToDelete.
(WebCore::DatabaseTracker::deleteOrigin):
If a database doesn't exist, don't try to delete it. We don't need to, but more
importantly, deleteDatabaseFile() will fail if the database doesn't exist, which
will cause us to incorrectly think we failed to remove database information from disk.
If we actually fail to delete any database file, return before we remove the origin
information from the tracker database so we don't lose track of the database.
(WebCore::DatabaseTracker::deleteDatabase):
If a database doesn't exist, don't try to delete it. We don't need to, but also it
will cause us to incorrectly think that we were unable to delete a database, so we
would bail before we remove the database information from the tracker database. We
want to remove the database information from the tracker database because the database
doesn't exist.
* Modules/webdatabase/DatabaseTracker.h:
Expose fullPathForDatabase() for use by tests.
* platform/Logging.h:
Add a logging channel.
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (223418 => 223419)
--- trunk/Source/WebCore/ChangeLog 2017-10-16 18:05:59 UTC (rev 223418)
+++ trunk/Source/WebCore/ChangeLog 2017-10-16 18:08:49 UTC (rev 223419)
@@ -1,3 +1,39 @@
+2017-10-16 Maureen Daum <md...@apple.com>
+
+ If we fail to delete any database file, don't remove its information from the tracker database
+ <rdar://problem/34576132> and https://bugs.webkit.org/show_bug.cgi?id=178251
+
+ Reviewed by Brady Eidson.
+
+ New tests:
+ DatabaseTracker.DeleteDatabase
+ DatabaseTracker.DeleteDatabaseWhenDatabaseDoesNotExist
+ DatabaseTracker.DeleteOrigin
+ DatabaseTracker.DeleteOriginWhenDeletingADatabaseFails
+ DatabaseTracker.DeleteOriginWhenDatabaseDoesNotExist
+
+ * Modules/webdatabase/DatabaseTracker.cpp:
+ (WebCore::DatabaseTracker::deleteDatabasesModifiedSince):
+ If the database doesn't exist, we previously deleted it but failed to remove the
+ information from the tracker database. We still want to delete all of the information
+ associated with this database from the tracker database, so add it to databaseNamesToDelete.
+ (WebCore::DatabaseTracker::deleteOrigin):
+ If a database doesn't exist, don't try to delete it. We don't need to, but more
+ importantly, deleteDatabaseFile() will fail if the database doesn't exist, which
+ will cause us to incorrectly think we failed to remove database information from disk.
+ If we actually fail to delete any database file, return before we remove the origin
+ information from the tracker database so we don't lose track of the database.
+ (WebCore::DatabaseTracker::deleteDatabase):
+ If a database doesn't exist, don't try to delete it. We don't need to, but also it
+ will cause us to incorrectly think that we were unable to delete a database, so we
+ would bail before we remove the database information from the tracker database. We
+ want to remove the database information from the tracker database because the database
+ doesn't exist.
+ * Modules/webdatabase/DatabaseTracker.h:
+ Expose fullPathForDatabase() for use by tests.
+ * platform/Logging.h:
+ Add a logging channel.
+
2017-10-16 Brent Fulgham <bfulg...@apple.com>
REGRESSION(223307): ASSERTION in WebCore::ResourceLoadObserver::logUserInteractionWithReducedTimeResolution
Modified: trunk/Source/WebCore/Modules/webdatabase/DatabaseTracker.cpp (223418 => 223419)
--- trunk/Source/WebCore/Modules/webdatabase/DatabaseTracker.cpp 2017-10-16 18:05:59 UTC (rev 223418)
+++ trunk/Source/WebCore/Modules/webdatabase/DatabaseTracker.cpp 2017-10-16 18:08:49 UTC (rev 223419)
@@ -781,12 +781,17 @@
for (const auto& databaseName : databaseNames) {
auto fullPath = fullPathForDatabase(origin, databaseName, false);
- time_t modificationTime;
- if (!getFileModificationTime(fullPath, modificationTime))
- continue;
+ // If the file doesn't exist, we previously deleted it but failed to remove the information
+ // from the tracker database. We want to delete all of the information associated with this
+ // database from the tracker database, so still add its name to databaseNamesToDelete.
+ if (fileExists(fullPath)) {
+ time_t modificationTime;
+ if (!getFileModificationTime(fullPath, modificationTime))
+ continue;
- if (modificationTime < std::chrono::system_clock::to_time_t(time))
- continue;
+ if (modificationTime < std::chrono::system_clock::to_time_t(time))
+ continue;
+ }
databaseNamesToDelete.uncheckedAppend(databaseName);
}
@@ -830,13 +835,22 @@
}
// We drop the lock here because holding locks during a call to deleteDatabaseFile will deadlock.
+ bool failedToDeleteAnyDatabaseFile = false;
for (auto& name : databaseNames) {
- if (!deleteDatabaseFile(origin, name, deletionMode)) {
+ if (fileExists(fullPathForDatabase(origin, name, false)) && !deleteDatabaseFile(origin, name, deletionMode)) {
// Even if the file can't be deleted, we want to try and delete the rest, don't return early here.
LOG_ERROR("Unable to delete file for database %s in origin %s", name.utf8().data(), origin.databaseIdentifier().utf8().data());
+ 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) {
+ RELEASE_LOG_ERROR(DatabaseTracker, "Failed to delete database for origin");
+ return false;
+ }
+
{
LockHolder lockDatabase(m_databaseGuard);
deleteOriginLockFor(origin);
@@ -1031,7 +1045,7 @@
}
// We drop the lock here because holding locks during a call to deleteDatabaseFile will deadlock.
- if (!deleteDatabaseFile(origin, name, DeletionMode::Default)) {
+ if (fileExists(fullPathForDatabase(origin, name, false)) && !deleteDatabaseFile(origin, name, DeletionMode::Default)) {
LOG_ERROR("Unable to delete file for database %s in origin %s", name.utf8().data(), origin.databaseIdentifier().utf8().data());
LockHolder lockDatabase(m_databaseGuard);
doneDeletingDatabase(origin, name);
Modified: trunk/Source/WebCore/Modules/webdatabase/DatabaseTracker.h (223418 => 223419)
--- trunk/Source/WebCore/Modules/webdatabase/DatabaseTracker.h 2017-10-16 18:05:59 UTC (rev 223418)
+++ trunk/Source/WebCore/Modules/webdatabase/DatabaseTracker.h 2017-10-16 18:08:49 UTC (rev 223419)
@@ -69,7 +69,7 @@
ExceptionOr<void> retryCanEstablishDatabase(DatabaseContext&, const String& name, unsigned estimatedSize);
void setDatabaseDetails(const SecurityOriginData&, const String& name, const String& displayName, unsigned estimatedSize);
- String fullPathForDatabase(const SecurityOriginData&, const String& name, bool createIfDoesNotExist);
+ WEBCORE_TESTSUPPORT_EXPORT String fullPathForDatabase(const SecurityOriginData&, const String& name, bool createIfDoesNotExist);
void addOpenDatabase(Database&);
void removeOpenDatabase(Database&);
Modified: trunk/Source/WebCore/platform/Logging.h (223418 => 223419)
--- trunk/Source/WebCore/platform/Logging.h 2017-10-16 18:05:59 UTC (rev 223418)
+++ trunk/Source/WebCore/platform/Logging.h 2017-10-16 18:08:49 UTC (rev 223419)
@@ -42,6 +42,7 @@
M(Archives) \
M(Compositing) \
M(ContentFiltering) \
+ M(DatabaseTracker) \
M(DisplayLists) \
M(DOMTimers) \
M(Editing) \
