- Revision
- 213547
- Author
- [email protected]
- Date
- 2017-03-07 15:12:34 -0800 (Tue, 07 Mar 2017)
Log Message
Correctly check for an empty database file.
<rdar://problem/30542242> Removing Website Data not working (WebSQL directories being left behind)
https://bugs.webkit.org/show_bug.cgi?id=169256
Patch by Maureen Daum <[email protected]> on 2017-03-07
Reviewed by Brady Eidson.
Source/WebCore:
Tests: DatabaseTrackerTest.DeleteDatabaseFileIfEmpty verifies that we actually delete an empty file.
Instead of checking that a database file's size is zero bytes, we should check if it contains
any tables. Once we open an empty database file and set its journal mode to WAL, it will have a
valid SQLite header and therefore will no longer be empty. We can know that the database was empty
if it does not contain any tables.
* Modules/webdatabase/DatabaseTracker.cpp:
(WebCore::DatabaseTracker::deleteDatabaseFileIfEmpty):
Use BEGIN EXCLUSIVE TRANSACTION in order to obtain the exclusive lock. If the database doesn't contain
any tables, it is empty and can be deleted.
Source/WebKit/mac:
Check if the folder for an origin's WebSQL databases is empty after trying to delete
all of its files. Currently we check if the deletedDatabaseFileCount matches the number
of files that were in the folder. However, when we delete the actual database file in
DatabaseTracker::deleteDatabaseFileIfEmpty(), the shm and wal files get deleted along with
the database file, but deletedDatabaseFileCount only gets incremented once.
* Storage/WebDatabaseManager.mm:
(+[WebDatabaseManager removeEmptyDatabaseFiles]):
Delete the folder if it is empty.
Tools:
Add a test for DatabaseTracker::deleteDatabaseFileIfEmpty that verifies
that if we pass in an empty file it actually gets deleted.
* TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
Add TestWebKitAPI/Tests/WebCore/DatabaseTrackerTest.cpp.
* TestWebKitAPI/Tests/WebCore/DatabaseTrackerTest.cpp: Added.
(TestWebKitAPI::TEST):
Modified Paths
Added Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (213546 => 213547)
--- trunk/Source/WebCore/ChangeLog 2017-03-07 23:07:56 UTC (rev 213546)
+++ trunk/Source/WebCore/ChangeLog 2017-03-07 23:12:34 UTC (rev 213547)
@@ -1,3 +1,23 @@
+2017-03-07 Maureen Daum <[email protected]>
+
+ Correctly check for an empty database file.
+ <rdar://problem/30542242> Removing Website Data not working (WebSQL directories being left behind)
+ https://bugs.webkit.org/show_bug.cgi?id=169256
+
+ Reviewed by Brady Eidson.
+
+ Tests: DatabaseTrackerTest.DeleteDatabaseFileIfEmpty verifies that we actually delete an empty file.
+
+ Instead of checking that a database file's size is zero bytes, we should check if it contains
+ any tables. Once we open an empty database file and set its journal mode to WAL, it will have a
+ valid SQLite header and therefore will no longer be empty. We can know that the database was empty
+ if it does not contain any tables.
+
+ * Modules/webdatabase/DatabaseTracker.cpp:
+ (WebCore::DatabaseTracker::deleteDatabaseFileIfEmpty):
+ Use BEGIN EXCLUSIVE TRANSACTION in order to obtain the exclusive lock. If the database doesn't contain
+ any tables, it is empty and can be deleted.
+
2017-03-07 Alex Christensen <[email protected]>
[URLParser] Fix file URLs that are just file:// and a Windows drive letter
Modified: trunk/Source/WebCore/Modules/webdatabase/DatabaseTracker.cpp (213546 => 213547)
--- trunk/Source/WebCore/Modules/webdatabase/DatabaseTracker.cpp 2017-03-07 23:07:56 UTC (rev 213546)
+++ trunk/Source/WebCore/Modules/webdatabase/DatabaseTracker.cpp 2017-03-07 23:12:34 UTC (rev 213547)
@@ -1216,7 +1216,7 @@
if (!database.open(path))
return false;
- // Specify that we want the exclusive locking mode, so after the next read,
+ // Specify that we want the exclusive locking mode, so after the next write,
// we'll be holding the lock to this database file.
SQLiteStatement lockStatement(database, "PRAGMA locking_mode=EXCLUSIVE;");
if (lockStatement.prepare() != SQLITE_OK)
@@ -1226,21 +1226,18 @@
return false;
lockStatement.finalize();
- // Every sqlite database has a sqlite_master table that contains the schema for the database.
- // http://www.sqlite.org/faq.html#q7
- SQLiteStatement readStatement(database, "SELECT * FROM sqlite_master LIMIT 1;");
- if (readStatement.prepare() != SQLITE_OK)
+ if (!database.executeCommand("BEGIN EXCLUSIVE TRANSACTION;"))
return false;
- // We shouldn't expect any result.
- if (readStatement.step() != SQLITE_DONE)
+
+ // At this point, we hold the exclusive lock to this file.
+ // Check that the database doesn't contain any tables.
+ if (!database.executeCommand("SELECT name FROM sqlite_master WHERE type='table';"))
return false;
- readStatement.finalize();
-
- // At this point, we hold the exclusive lock to this file. Double-check again to make sure
- // it's still zero bytes.
- if (!isZeroByteFile(path))
- return false;
-
+
+ database.executeCommand("COMMIT TRANSACTION;");
+
+ database.close();
+
return SQLiteFileSystem::deleteDatabaseFile(path);
}
Modified: trunk/Source/WebKit/mac/ChangeLog (213546 => 213547)
--- trunk/Source/WebKit/mac/ChangeLog 2017-03-07 23:07:56 UTC (rev 213546)
+++ trunk/Source/WebKit/mac/ChangeLog 2017-03-07 23:12:34 UTC (rev 213547)
@@ -1,3 +1,21 @@
+2017-03-07 Maureen Daum <[email protected]>
+
+ Correctly check for an empty database file.
+ <rdar://problem/30542242> Removing Website Data not working (WebSQL directories being left behind)
+ https://bugs.webkit.org/show_bug.cgi?id=169256
+
+ Reviewed by Brady Eidson.
+
+ Check if the folder for an origin's WebSQL databases is empty after trying to delete
+ all of its files. Currently we check if the deletedDatabaseFileCount matches the number
+ of files that were in the folder. However, when we delete the actual database file in
+ DatabaseTracker::deleteDatabaseFileIfEmpty(), the shm and wal files get deleted along with
+ the database file, but deletedDatabaseFileCount only gets incremented once.
+
+ * Storage/WebDatabaseManager.mm:
+ (+[WebDatabaseManager removeEmptyDatabaseFiles]):
+ Delete the folder if it is empty.
+
2017-03-06 Michael Saboff <[email protected]>
Unreviewed build fix to add
Modified: trunk/Source/WebKit/mac/Storage/WebDatabaseManager.mm (213546 => 213547)
--- trunk/Source/WebKit/mac/Storage/WebDatabaseManager.mm 2017-03-07 23:07:56 UTC (rev 213546)
+++ trunk/Source/WebKit/mac/Storage/WebDatabaseManager.mm 2017-03-07 23:12:34 UTC (rev 213547)
@@ -213,7 +213,7 @@
}
// If we have removed every database file for this origin, delete the folder for this origin.
- if (databaseFileCount == deletedDatabaseFileCount) {
+ if (databaseFileCount == deletedDatabaseFileCount || ![fileManager contentsOfDirectoryAtPath:path error:nullptr].count) {
// Use rmdir - we don't want the deletion to happen if the folder is not empty.
rmdir([path fileSystemRepresentation]);
}
Modified: trunk/Tools/ChangeLog (213546 => 213547)
--- trunk/Tools/ChangeLog 2017-03-07 23:07:56 UTC (rev 213546)
+++ trunk/Tools/ChangeLog 2017-03-07 23:12:34 UTC (rev 213547)
@@ -1,3 +1,19 @@
+2017-03-07 Maureen Daum <[email protected]>
+
+ Correctly check for an empty database file.
+ <rdar://problem/30542242> Removing Website Data not working (WebSQL directories being left behind)
+ https://bugs.webkit.org/show_bug.cgi?id=169256
+
+ Reviewed by Brady Eidson.
+
+ Add a test for DatabaseTracker::deleteDatabaseFileIfEmpty that verifies
+ that if we pass in an empty file it actually gets deleted.
+
+ * TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
+ Add TestWebKitAPI/Tests/WebCore/DatabaseTrackerTest.cpp.
+ * TestWebKitAPI/Tests/WebCore/DatabaseTrackerTest.cpp: Added.
+ (TestWebKitAPI::TEST):
+
2017-03-07 Alex Christensen <[email protected]>
[URLParser] Fix file URLs that are just file:// and a Windows drive letter
Modified: trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj (213546 => 213547)
--- trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj 2017-03-07 23:07:56 UTC (rev 213546)
+++ trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj 2017-03-07 23:12:34 UTC (rev 213547)
@@ -188,6 +188,7 @@
5C9E59431D3EB5AC00E3C62E /* ApplicationCache.db-wal in Copy Resources */ = {isa = PBXBuildFile; fileRef = 5C9E59401D3EB1DE00E3C62E /* ApplicationCache.db-wal */; };
5E4B1D2E1D404C6100053621 /* WKScrollViewDelegateCrash.mm in Sources */ = {isa = PBXBuildFile; fileRef = 5E4B1D2C1D404C6100053621 /* WKScrollViewDelegateCrash.mm */; };
6BFD294C1D5E6C1D008EC968 /* HashCountedSet.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 7A38D7E51C752D5F004F157D /* HashCountedSet.cpp */; };
+ 755A20AF1E6E38630093C69F /* DatabaseTrackerTest.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 755A20AE1E6E38630093C69F /* DatabaseTrackerTest.cpp */; };
764322D71B61CCC30024F801 /* WordBoundaryTypingAttributes.mm in Sources */ = {isa = PBXBuildFile; fileRef = 764322D51B61CCA40024F801 /* WordBoundaryTypingAttributes.mm */; };
7673499D1930C5BB00E44DF9 /* StopLoadingDuringDidFailProvisionalLoad_bundle.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 7673499A1930182E00E44DF9 /* StopLoadingDuringDidFailProvisionalLoad_bundle.cpp */; };
76E182DD1547569100F1FADD /* WillSendSubmitEvent_Bundle.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 76E182DC1547569100F1FADD /* WillSendSubmitEvent_Bundle.cpp */; };
@@ -1086,6 +1087,7 @@
5C9E593F1D3EB1DE00E3C62E /* ApplicationCache.db-shm */ = {isa = PBXFileReference; lastKnownFileType = file; path = "ApplicationCache.db-shm"; sourceTree = "<group>"; };
5C9E59401D3EB1DE00E3C62E /* ApplicationCache.db-wal */ = {isa = PBXFileReference; lastKnownFileType = file; path = "ApplicationCache.db-wal"; sourceTree = "<group>"; };
5E4B1D2C1D404C6100053621 /* WKScrollViewDelegateCrash.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; name = WKScrollViewDelegateCrash.mm; path = ../ios/WKScrollViewDelegateCrash.mm; sourceTree = "<group>"; };
+ 755A20AE1E6E38630093C69F /* DatabaseTrackerTest.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = DatabaseTrackerTest.cpp; sourceTree = "<group>"; };
7560917719259C59009EF06E /* MemoryCacheAddImageToCacheIOS.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = MemoryCacheAddImageToCacheIOS.mm; sourceTree = "<group>"; };
75F3133F18C171B70041CAEC /* EphemeralSessionPushStateNoHistoryCallback.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = EphemeralSessionPushStateNoHistoryCallback.cpp; sourceTree = "<group>"; };
764322D51B61CCA40024F801 /* WordBoundaryTypingAttributes.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = WordBoundaryTypingAttributes.mm; sourceTree = "<group>"; };
@@ -1681,6 +1683,7 @@
1C9EB8401E380DA1005C6442 /* ComplexTextController.cpp */,
7CB184C41AA3F2100066EDFD /* ContentExtensions.cpp */,
CD5451E919E41F9D0016936F /* CSSParser.cpp */,
+ 755A20AE1E6E38630093C69F /* DatabaseTrackerTest.cpp */,
260BA5781B1D2E7B004FA07C /* DFACombiner.cpp */,
260BA57A1B1D2EE2004FA07C /* DFAHelpers.h */,
26F6E1EF1ADC749B00DE696B /* DFAMinimizer.cpp */,
@@ -2799,6 +2802,7 @@
7C83E0C11D0A652F00FEBCF3 /* ProvisionalURLNotChange.mm in Sources */,
7CCE7EC81A411A7E00447C4C /* PublicSuffix.mm in Sources */,
7C83E0C21D0A653500FEBCF3 /* QuickLook.mm in Sources */,
+ 755A20AF1E6E38630093C69F /* DatabaseTrackerTest.cpp in Sources */,
7CCE7F0D1A411AE600447C4C /* ReloadPageAfterCrash.cpp in Sources */,
7C83E0C31D0A653A00FEBCF3 /* RemoteObjectRegistry.mm in Sources */,
7CCE7EC91A411A7E00447C4C /* RenderedImageFromDOMNode.mm in Sources */,
Added: trunk/Tools/TestWebKitAPI/Tests/WebCore/DatabaseTrackerTest.cpp (0 => 213547)
--- trunk/Tools/TestWebKitAPI/Tests/WebCore/DatabaseTrackerTest.cpp (rev 0)
+++ trunk/Tools/TestWebKitAPI/Tests/WebCore/DatabaseTrackerTest.cpp 2017-03-07 23:12:34 UTC (rev 213547)
@@ -0,0 +1,58 @@
+/*
+ * Copyright (C) 2017 Apple Inc. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS''
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+ * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS
+ * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
+ * THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include "config.h"
+
+#if PLATFORM(IOS)
+
+#include <WebCore/DatabaseTracker.h>
+#include <WebCore/FileSystem.h>
+
+using namespace WebCore;
+
+namespace TestWebKitAPI {
+
+TEST(DatabaseTracker, DeleteDatabaseFileIfEmpty)
+{
+ PlatformFileHandle handle;
+ String databaseFilePath = openTemporaryFile("tempEmptyDatabase", handle);
+ closeFile(handle);
+
+ long long fileSize;
+ getFileSize(databaseFilePath, fileSize);
+ EXPECT_EQ(0, fileSize);
+
+ EXPECT_TRUE(DatabaseTracker::deleteDatabaseFileIfEmpty(databaseFilePath));
+
+ bool fileStillExists = fileExists(databaseFilePath);
+ EXPECT_FALSE(fileStillExists);
+
+ if (fileStillExists)
+ deleteFile(databaseFilePath);
+}
+
+} // namespace TestWebKitAPI
+
+#endif // PLATFORM(IOS)
