Diff
Modified: trunk/Source/WebCore/ChangeLog (232409 => 232410)
--- trunk/Source/WebCore/ChangeLog 2018-06-01 19:53:56 UTC (rev 232409)
+++ trunk/Source/WebCore/ChangeLog 2018-06-01 20:13:08 UTC (rev 232410)
@@ -1,3 +1,14 @@
+2018-06-01 Sihui Liu <[email protected]>
+
+ Stop using StorageTracker.db in LocalStorageDatabaseTracker
+ https://bugs.webkit.org/show_bug.cgi?id=186104
+
+ Reviewed by Geoffrey Garen.
+
+ No behavior change.
+
+ * platform/sql/SQLiteFileSystem.h:
+
2018-06-01 Zalan Bujtas <[email protected]>
[LFC] Simplify the formatting class implementation by pushing down some of the logic to the Geometry class
Modified: trunk/Source/WebCore/platform/sql/SQLiteFileSystem.h (232409 => 232410)
--- trunk/Source/WebCore/platform/sql/SQLiteFileSystem.h 2018-06-01 19:53:56 UTC (rev 232409)
+++ trunk/Source/WebCore/platform/sql/SQLiteFileSystem.h 2018-06-01 20:13:08 UTC (rev 232410)
@@ -57,13 +57,13 @@
//
// path - The directory.
// fileName - The file name.
- static String appendDatabaseFileNameToPath(const String& path, const String& fileName);
+ WEBCORE_EXPORT static String appendDatabaseFileNameToPath(const String& path, const String& fileName);
// Makes sure the given directory exists, by creating all missing directories
// on the given path.
//
// path - The directory.
- static bool ensureDatabaseDirectoryExists(const String& path);
+ WEBCORE_EXPORT static bool ensureDatabaseDirectoryExists(const String& path);
// If 'checkPathOnly' is false, then this method only checks if the given file exists.
// If 'checkPathOnly' is true, then this method makes sure all directories on the
@@ -97,8 +97,8 @@
#endif
static long long getDatabaseFileSize(const String& fileName);
- static double databaseCreationTime(const String& fileName);
- static double databaseModificationTime(const String& fileName);
+ WEBCORE_EXPORT static double databaseCreationTime(const String& fileName);
+ WEBCORE_EXPORT static double databaseModificationTime(const String& fileName);
private:
// do not instantiate this class
Modified: trunk/Source/WebKit/ChangeLog (232409 => 232410)
--- trunk/Source/WebKit/ChangeLog 2018-06-01 19:53:56 UTC (rev 232409)
+++ trunk/Source/WebKit/ChangeLog 2018-06-01 20:13:08 UTC (rev 232410)
@@ -1,3 +1,38 @@
+2018-06-01 Sihui Liu <[email protected]>
+
+ Stop using StorageTracker.db in LocalStorageDatabaseTracker
+ https://bugs.webkit.org/show_bug.cgi?id=186104
+
+ Reviewed by Geoffrey Garen.
+
+ Stop using StorageTracker.db and stop caching origins in LocalStorageDatabaseTracker for efficiency
+ and simplicity. Since functions in LocalStorageDatabaseTracker are not frequently called, we get
+ little benefits from caching origins.
+
+ * Platform/Logging.h:
+ * UIProcess/API/C/WKKeyValueStorageManager.cpp:
+ (WKKeyValueStorageManagerGetStorageDetailsByOrigin):
+ * UIProcess/WebStorage/LocalStorageDatabaseTracker.cpp:
+ (WebKit::LocalStorageDatabaseTracker::LocalStorageDatabaseTracker):
+ (WebKit::LocalStorageDatabaseTracker::didOpenDatabaseWithOrigin):
+ (WebKit::LocalStorageDatabaseTracker::deleteDatabaseWithOrigin):
+ (WebKit::LocalStorageDatabaseTracker::deleteAllDatabases):
+ (WebKit::LocalStorageDatabaseTracker::databasesModifiedSince):
+ (WebKit::LocalStorageDatabaseTracker::origins const):
+ (WebKit::LocalStorageDatabaseTracker::originDetails):
+ (WebKit::LocalStorageDatabaseTracker::databasePath const):
+ (WebKit::fileCreationTime): Deleted.
+ (WebKit::fileModificationTime): Deleted.
+ (WebKit::LocalStorageDatabaseTracker::trackerDatabasePath const): Deleted.
+ (WebKit::LocalStorageDatabaseTracker::openTrackerDatabase): Deleted.
+ (WebKit::LocalStorageDatabaseTracker::importOriginIdentifiers): Deleted.
+ (WebKit::LocalStorageDatabaseTracker::updateTrackerDatabaseFromLocalStorageDatabaseFiles): Deleted.
+ (WebKit::LocalStorageDatabaseTracker::addDatabaseWithOriginIdentifier): Deleted.
+ (WebKit::LocalStorageDatabaseTracker::removeDatabaseWithOriginIdentifier): Deleted.
+ (WebKit::LocalStorageDatabaseTracker::pathForDatabaseWithOriginIdentifier): Deleted.
+ * UIProcess/WebStorage/LocalStorageDatabaseTracker.h:
+ * UIProcess/WebStorage/StorageManager.h:
+
2018-06-01 Michael Catanzaro <[email protected]>
[GTK] Crash in WebKitFaviconDatabase when pageURL is unset
Modified: trunk/Source/WebKit/Platform/Logging.h (232409 => 232410)
--- trunk/Source/WebKit/Platform/Logging.h 2018-06-01 19:53:56 UTC (rev 232409)
+++ trunk/Source/WebKit/Platform/Logging.h 2018-06-01 20:13:08 UTC (rev 232410)
@@ -52,6 +52,7 @@
M(KeyHandling) \
M(Layers) \
M(Loading) \
+ M(LocalStorageDatabaseTracker) \
M(MouseHandling) \
M(Network) \
M(NetworkCache) \
Modified: trunk/Source/WebKit/UIProcess/API/C/WKKeyValueStorageManager.cpp (232409 => 232410)
--- trunk/Source/WebKit/UIProcess/API/C/WKKeyValueStorageManager.cpp 2018-06-01 19:53:56 UTC (rev 232409)
+++ trunk/Source/WebKit/UIProcess/API/C/WKKeyValueStorageManager.cpp 2018-06-01 20:13:08 UTC (rev 232410)
@@ -102,9 +102,9 @@
detailsMap.set(toImpl(WKKeyValueStorageManagerGetOriginKey())->string(), origin);
if (originDetails.creationTime)
- detailsMap.set(toImpl(WKKeyValueStorageManagerGetCreationTimeKey())->string(), API::Double::create(originDetails.creationTime.value_or(0)));
+ detailsMap.set(toImpl(WKKeyValueStorageManagerGetCreationTimeKey())->string(), API::Double::create(originDetails.creationTime.secondsSinceEpoch().value()));
if (originDetails.modificationTime)
- detailsMap.set(toImpl(WKKeyValueStorageManagerGetModificationTimeKey())->string(), API::Double::create(originDetails.modificationTime.value_or(0)));
+ detailsMap.set(toImpl(WKKeyValueStorageManagerGetModificationTimeKey())->string(), API::Double::create(originDetails.modificationTime.secondsSinceEpoch().value()));
result.uncheckedAppend(API::Dictionary::create(WTFMove(detailsMap)));
}
Modified: trunk/Source/WebKit/UIProcess/WebStorage/LocalStorageDatabaseTracker.cpp (232409 => 232410)
--- trunk/Source/WebKit/UIProcess/WebStorage/LocalStorageDatabaseTracker.cpp 2018-06-01 19:53:56 UTC (rev 232409)
+++ trunk/Source/WebKit/UIProcess/WebStorage/LocalStorageDatabaseTracker.cpp 2018-06-01 20:13:08 UTC (rev 232410)
@@ -26,11 +26,10 @@
#include "config.h"
#include "LocalStorageDatabaseTracker.h"
+#include "Logging.h"
#include <WebCore/FileSystem.h>
#include <WebCore/SQLiteFileSystem.h>
#include <WebCore/SQLiteStatement.h>
-#include <WebCore/SecurityOrigin.h>
-#include <WebCore/SecurityOriginData.h>
#include <WebCore/TextEncoding.h>
#include <wtf/MainThread.h>
#include <wtf/RunLoop.h>
@@ -56,7 +55,8 @@
UTF8Encoding();
m_queue->dispatch([protectedThis = makeRef(*this)]() mutable {
- protectedThis->importOriginIdentifiers();
+ // Delete legacy storageTracker database file.
+ SQLiteFileSystem::deleteDatabaseFile(protectedThis->databasePath("StorageTracker.db"));
});
}
@@ -71,131 +71,77 @@
void LocalStorageDatabaseTracker::didOpenDatabaseWithOrigin(const SecurityOriginData& securityOrigin)
{
- addDatabaseWithOriginIdentifier(securityOrigin.databaseIdentifier(), databasePath(securityOrigin));
+ // FIXME: Tell clients that the origin was added.
}
void LocalStorageDatabaseTracker::deleteDatabaseWithOrigin(const SecurityOriginData& securityOrigin)
{
- removeDatabaseWithOriginIdentifier(securityOrigin.databaseIdentifier());
+ auto path = databasePath(securityOrigin);
+ if (!path.isEmpty())
+ SQLiteFileSystem::deleteDatabaseFile(path);
+
+ // FIXME: Tell clients that the origin was removed.
}
void LocalStorageDatabaseTracker::deleteAllDatabases()
{
- m_origins.clear();
+ auto paths = FileSystem::listDirectory(m_localStorageDirectory, "*.localstorage");
+ for (auto path : paths) {
+ SQLiteFileSystem::deleteDatabaseFile(path);
- openTrackerDatabase(SkipIfNonExistent);
- if (!m_database.isOpen())
- return;
-
- SQLiteStatement statement(m_database, "SELECT origin, path FROM Origins");
- if (statement.prepare() != SQLITE_OK) {
- LOG_ERROR("Failed to prepare statement.");
- return;
- }
-
- int result;
- while ((result = statement.step()) == SQLITE_ROW) {
- FileSystem::deleteFile(statement.getColumnText(1));
-
// FIXME: Call out to the client.
}
- if (result != SQLITE_DONE)
- LOG_ERROR("Failed to read in all origins from the database.");
-
- if (m_database.isOpen())
- m_database.close();
-
- if (!FileSystem::deleteFile(trackerDatabasePath())) {
- // In the case where it is not possible to delete the database file (e.g some other program
- // like a virus scanner is accessing it), make sure to remove all entries.
- openTrackerDatabase(SkipIfNonExistent);
- if (!m_database.isOpen())
- return;
-
- SQLiteStatement deleteStatement(m_database, "DELETE FROM Origins");
- if (deleteStatement.prepare() != SQLITE_OK) {
- LOG_ERROR("Unable to prepare deletion of all origins");
- return;
- }
- if (!deleteStatement.executeCommand()) {
- LOG_ERROR("Unable to execute deletion of all origins");
- return;
- }
- }
-
- FileSystem::deleteEmptyDirectory(m_localStorageDirectory);
+ SQLiteFileSystem::deleteEmptyDatabaseDirectory(m_localStorageDirectory);
}
-static std::optional<time_t> fileCreationTime(const String& filePath)
-{
- time_t time;
- return FileSystem::getFileCreationTime(filePath, time) ? time : std::optional<time_t>(std::nullopt);
-}
-
-static std::optional<time_t> fileModificationTime(const String& filePath)
-{
- time_t time;
- if (!FileSystem::getFileModificationTime(filePath, time))
- return std::nullopt;
-
- return time;
-}
-
Vector<SecurityOriginData> LocalStorageDatabaseTracker::databasesModifiedSince(WallTime time)
{
- ASSERT(!RunLoop::isMain());
- importOriginIdentifiers();
- Vector<String> originIdentifiersModified;
+ Vector<SecurityOriginData> databaseOriginsModified;
+ auto databaseOrigins = origins();
- for (const String& origin : m_origins) {
- String filePath = pathForDatabaseWithOriginIdentifier(origin);
+ for (auto origin : databaseOrigins) {
+ auto path = databasePath(origin);
- auto modificationTime = FileSystem::getFileModificationTime(filePath);
- if (!modificationTime || modificationTime.value() >= time)
- originIdentifiersModified.append(origin);
+ auto modificationTime = WallTime::fromRawSeconds(SQLiteFileSystem::databaseModificationTime(path));
+ if (modificationTime >= time)
+ databaseOriginsModified.append(origin);
}
-
- Vector<SecurityOriginData> databaseOriginsModified;
- databaseOriginsModified.reserveInitialCapacity(originIdentifiersModified.size());
-
- for (const auto& originIdentifier : originIdentifiersModified) {
- if (auto origin = SecurityOriginData::fromDatabaseIdentifier(originIdentifier))
- databaseOriginsModified.uncheckedAppend(*origin);
- else
- ASSERT_NOT_REACHED();
- }
-
+
return databaseOriginsModified;
}
Vector<SecurityOriginData> LocalStorageDatabaseTracker::origins() const
{
- Vector<SecurityOriginData> origins;
- origins.reserveInitialCapacity(m_origins.size());
-
- for (const String& originIdentifier : m_origins) {
- if (auto origin = SecurityOriginData::fromDatabaseIdentifier(originIdentifier))
- origins.uncheckedAppend(*origin);
+ Vector<SecurityOriginData> databaseOrigins;
+ auto paths = FileSystem::listDirectory(m_localStorageDirectory, "*.localstorage");
+
+ for (auto path : paths) {
+ auto filename = FileSystem::pathGetFileName(path);
+ auto originIdentifier = filename.substring(0, filename.length() - strlen(".localstorage"));
+ auto origin = SecurityOriginData::fromDatabaseIdentifier(originIdentifier);
+ if (origin)
+ databaseOrigins.append(origin.value());
else
- ASSERT_NOT_REACHED();
+ RELEASE_LOG_ERROR(LocalStorageDatabaseTracker, "Unable to extract origin from path %s", path.utf8().data());
}
- return origins;
+ return databaseOrigins;
}
Vector<LocalStorageDatabaseTracker::OriginDetails> LocalStorageDatabaseTracker::originDetails()
{
Vector<OriginDetails> result;
- result.reserveInitialCapacity(m_origins.size());
+ auto databaseOrigins = origins();
+ result.reserveInitialCapacity(databaseOrigins.size());
- for (const String& origin : m_origins) {
- String filePath = pathForDatabaseWithOriginIdentifier(origin);
+ for (auto origin : databaseOrigins) {
+ String path = databasePath(origin);
OriginDetails details;
- details.originIdentifier = origin.isolatedCopy();
- details.creationTime = fileCreationTime(filePath);
- details.modificationTime = fileModificationTime(filePath);
+ details.originIdentifier = origin.databaseIdentifier();
+ details.creationTime = WallTime::fromRawSeconds(SQLiteFileSystem::databaseCreationTime(path));
+ details.modificationTime = WallTime::fromRawSeconds(SQLiteFileSystem::databaseModificationTime(path));
result.uncheckedAppend(details);
}
@@ -204,7 +150,7 @@
String LocalStorageDatabaseTracker::databasePath(const String& filename) const
{
- if (!FileSystem::makeAllDirectories(m_localStorageDirectory)) {
+ if (!SQLiteFileSystem::ensureDatabaseDirectoryExists(m_localStorageDirectory)) {
LOG_ERROR("Unable to create LocalStorage database path %s", m_localStorageDirectory.utf8().data());
return String();
}
@@ -213,172 +159,7 @@
platformMaybeExcludeFromBackup();
#endif
- return FileSystem::pathByAppendingComponent(m_localStorageDirectory, filename);
+ return SQLiteFileSystem::appendDatabaseFileNameToPath(m_localStorageDirectory, filename);
}
-String LocalStorageDatabaseTracker::trackerDatabasePath() const
-{
- return databasePath("StorageTracker.db");
-}
-
-void LocalStorageDatabaseTracker::openTrackerDatabase(DatabaseOpeningStrategy openingStrategy)
-{
- if (m_database.isOpen())
- return;
-
- String databasePath = trackerDatabasePath();
-
- if (!FileSystem::fileExists(databasePath) && openingStrategy == SkipIfNonExistent)
- return;
-
- if (!m_database.open(databasePath)) {
- LOG_ERROR("Failed to open databasePath %s.", databasePath.ascii().data());
- return;
- }
-
- // Since a WorkQueue isn't bound to a specific thread, we have to disable threading checks
- // even though we never access the database from different threads simultaneously.
- m_database.disableThreadingChecks();
-
- if (m_database.tableExists("Origins"))
- return;
-
- if (!m_database.executeCommand("CREATE TABLE Origins (origin TEXT UNIQUE ON CONFLICT REPLACE, path TEXT);"))
- LOG_ERROR("Failed to create Origins table.");
-}
-
-void LocalStorageDatabaseTracker::importOriginIdentifiers()
-{
- openTrackerDatabase(SkipIfNonExistent);
-
- if (m_database.isOpen()) {
- SQLiteStatement statement(m_database, "SELECT origin FROM Origins");
- if (statement.prepare() != SQLITE_OK) {
- LOG_ERROR("Failed to prepare statement.");
- return;
- }
-
- int result;
-
- while ((result = statement.step()) == SQLITE_ROW)
- m_origins.add(statement.getColumnText(0));
-
- if (result != SQLITE_DONE) {
- LOG_ERROR("Failed to read in all origins from the database.");
- return;
- }
- }
-
- updateTrackerDatabaseFromLocalStorageDatabaseFiles();
-}
-
-void LocalStorageDatabaseTracker::updateTrackerDatabaseFromLocalStorageDatabaseFiles()
-{
- Vector<String> paths = FileSystem::listDirectory(m_localStorageDirectory, "*.localstorage");
-
- HashSet<String> origins(m_origins);
- HashSet<String> originsFromLocalStorageDatabaseFiles;
-
- for (size_t i = 0; i < paths.size(); ++i) {
- const String& path = paths[i];
-
- if (!path.endsWith(".localstorage"))
- continue;
-
- String filename = FileSystem::pathGetFileName(path);
-
- String originIdentifier = filename.substring(0, filename.length() - strlen(".localstorage"));
-
- if (!m_origins.contains(originIdentifier))
- addDatabaseWithOriginIdentifier(originIdentifier, path);
-
- originsFromLocalStorageDatabaseFiles.add(originIdentifier);
- }
-
- for (auto it = origins.begin(), end = origins.end(); it != end; ++it) {
- const String& originIdentifier = *it;
- if (origins.contains(originIdentifier))
- continue;
-
- removeDatabaseWithOriginIdentifier(originIdentifier);
- }
-}
-
-void LocalStorageDatabaseTracker::addDatabaseWithOriginIdentifier(const String& originIdentifier, const String& databasePath)
-{
- openTrackerDatabase(CreateIfNonExistent);
- if (!m_database.isOpen())
- return;
-
- SQLiteStatement statement(m_database, "INSERT INTO Origins VALUES (?, ?)");
- if (statement.prepare() != SQLITE_OK) {
- LOG_ERROR("Unable to establish origin '%s' in the tracker", originIdentifier.utf8().data());
- return;
- }
-
- statement.bindText(1, originIdentifier);
- statement.bindText(2, databasePath);
-
- if (statement.step() != SQLITE_DONE)
- LOG_ERROR("Unable to establish origin '%s' in the tracker", originIdentifier.utf8().data());
-
- m_origins.add(originIdentifier);
-
- // FIXME: Tell clients that the origin was added.
-}
-
-void LocalStorageDatabaseTracker::removeDatabaseWithOriginIdentifier(const String& originIdentifier)
-{
- openTrackerDatabase(SkipIfNonExistent);
- if (!m_database.isOpen())
- return;
-
- String path = pathForDatabaseWithOriginIdentifier(originIdentifier);
- if (path.isEmpty())
- return;
-
- SQLiteStatement deleteStatement(m_database, "DELETE FROM Origins where origin=?");
- if (deleteStatement.prepare() != SQLITE_OK) {
- LOG_ERROR("Unable to prepare deletion of origin '%s'", originIdentifier.ascii().data());
- return;
- }
- deleteStatement.bindText(1, originIdentifier);
- if (!deleteStatement.executeCommand()) {
- LOG_ERROR("Unable to execute deletion of origin '%s'", originIdentifier.ascii().data());
- return;
- }
-
- SQLiteFileSystem::deleteDatabaseFile(path);
-
- m_origins.remove(originIdentifier);
- if (m_origins.isEmpty()) {
- // There are no origins left; delete the tracker database.
- m_database.close();
- SQLiteFileSystem::deleteDatabaseFile(trackerDatabasePath());
- FileSystem::deleteEmptyDirectory(m_localStorageDirectory);
- }
-
- // FIXME: Tell clients that the origin was removed.
-}
-
-String LocalStorageDatabaseTracker::pathForDatabaseWithOriginIdentifier(const String& originIdentifier)
-{
- if (!m_database.isOpen())
- return String();
-
- SQLiteStatement pathStatement(m_database, "SELECT path FROM Origins WHERE origin=?");
- if (pathStatement.prepare() != SQLITE_OK) {
- LOG_ERROR("Unable to prepare selection of path for origin '%s'", originIdentifier.utf8().data());
- return String();
- }
-
- pathStatement.bindText(1, originIdentifier);
-
- int result = pathStatement.step();
- if (result != SQLITE_ROW)
- return String();
-
- return pathStatement.getColumnText(0);
-}
-
} // namespace WebKit
Modified: trunk/Source/WebKit/UIProcess/WebStorage/LocalStorageDatabaseTracker.h (232409 => 232410)
--- trunk/Source/WebKit/UIProcess/WebStorage/LocalStorageDatabaseTracker.h 2018-06-01 19:53:56 UTC (rev 232409)
+++ trunk/Source/WebKit/UIProcess/WebStorage/LocalStorageDatabaseTracker.h 2018-06-01 20:13:08 UTC (rev 232410)
@@ -25,7 +25,7 @@
#pragma once
-#include <WebCore/SQLiteDatabase.h>
+#include <WebCore/SecurityOriginData.h>
#include <wtf/HashSet.h>
#include <wtf/Optional.h>
#include <wtf/RefPtr.h>
@@ -35,11 +35,6 @@
#include <wtf/text/StringHash.h>
#include <wtf/text/WTFString.h>
-namespace WebCore {
-class SecurityOrigin;
-struct SecurityOriginData;
-}
-
namespace WebKit {
struct LocalStorageDetails;
@@ -62,8 +57,8 @@
struct OriginDetails {
String originIdentifier;
- std::optional<time_t> creationTime;
- std::optional<time_t> modificationTime;
+ WallTime creationTime;
+ WallTime modificationTime;
};
Vector<OriginDetails> originDetails();
@@ -71,27 +66,15 @@
LocalStorageDatabaseTracker(Ref<WorkQueue>&&, const String& localStorageDirectory);
String databasePath(const String& filename) const;
- String trackerDatabasePath() const;
enum DatabaseOpeningStrategy {
CreateIfNonExistent,
SkipIfNonExistent
};
- void openTrackerDatabase(DatabaseOpeningStrategy);
- void importOriginIdentifiers();
- void updateTrackerDatabaseFromLocalStorageDatabaseFiles();
-
- void addDatabaseWithOriginIdentifier(const String& originIdentifier, const String& databasePath);
- void removeDatabaseWithOriginIdentifier(const String& originIdentifier);
- String pathForDatabaseWithOriginIdentifier(const String& originIdentifier);
-
RefPtr<WorkQueue> m_queue;
String m_localStorageDirectory;
- WebCore::SQLiteDatabase m_database;
- HashSet<String> m_origins;
-
#if PLATFORM(IOS)
void platformMaybeExcludeFromBackup() const;
Modified: trunk/Source/WebKit/UIProcess/WebStorage/StorageManager.h (232409 => 232410)
--- trunk/Source/WebKit/UIProcess/WebStorage/StorageManager.h 2018-06-01 19:53:56 UTC (rev 232409)
+++ trunk/Source/WebKit/UIProcess/WebStorage/StorageManager.h 2018-06-01 20:13:08 UTC (rev 232410)
@@ -27,8 +27,6 @@
#include "Connection.h"
#include "LocalStorageDatabaseTracker.h"
-#include <WebCore/SecurityOriginData.h>
-#include <WebCore/SecurityOriginHash.h>
#include <wtf/Forward.h>
#include <wtf/Function.h>
#include <wtf/HashSet.h>
