Modified: trunk/Source/WebCore/ChangeLog (145430 => 145431)
--- trunk/Source/WebCore/ChangeLog 2013-03-11 23:40:58 UTC (rev 145430)
+++ trunk/Source/WebCore/ChangeLog 2013-03-11 23:44:01 UTC (rev 145431)
@@ -1,3 +1,28 @@
+2013-03-11 Mark Lam <[email protected]>
+
+ Remove the use of the quotaMap cache in DatabaseTracker.
+ https://bugs.webkit.org/show_bug.cgi?id=111805.
+
+ Reviewed by Geoffrey Garen.
+
+ This change is needed because using the quotaMap cache can result in
+ an inaccurate read (in a multi-process system) on what database files
+ are actually present on the filesystem. Instead of using the quotaMap
+ cache, we rely on the tracker database and query it every time we need
+ the origin/quota information.
+
+ No new tests.
+
+ * Modules/webdatabase/DatabaseTracker.cpp:
+ (WebCore::DatabaseTracker::hasAdequateQuotaForOrigin):
+ (WebCore::DatabaseTracker::hasEntryForOriginNoLock):
+ (WebCore::DatabaseTracker::origins):
+ (WebCore::DatabaseTracker::quotaForOriginNoLock):
+ (WebCore::DatabaseTracker::setQuota):
+ (WebCore::DatabaseTracker::addDatabase):
+ (WebCore::DatabaseTracker::deleteOrigin):
+ * Modules/webdatabase/DatabaseTracker.h:
+
2013-03-11 Morten Stenshorne <[email protected]>
Don't rely on the fact that StaticPosition happens to be defined as 0
Modified: trunk/Source/WebCore/Modules/webdatabase/DatabaseTracker.cpp (145430 => 145431)
--- trunk/Source/WebCore/Modules/webdatabase/DatabaseTracker.cpp 2013-03-11 23:40:58 UTC (rev 145430)
+++ trunk/Source/WebCore/Modules/webdatabase/DatabaseTracker.cpp 2013-03-11 23:44:01 UTC (rev 145431)
@@ -139,6 +139,7 @@
bool DatabaseTracker::hasAdequateQuotaForOrigin(SecurityOrigin* origin, unsigned long estimatedSize, DatabaseError& err)
{
+ ASSERT(!m_databaseGuard.tryLock());
unsigned long long usage = usageForOrigin(origin);
// If the database will fit, allow its creation.
@@ -239,9 +240,19 @@
bool DatabaseTracker::hasEntryForOriginNoLock(SecurityOrigin* origin)
{
ASSERT(!m_databaseGuard.tryLock());
- populateOriginsIfNeeded();
- ASSERT(m_quotaMap);
- return m_quotaMap->contains(origin);
+ openTrackerDatabase(DontCreateIfDoesNotExist);
+ if (!m_database.isOpen())
+ return false;
+
+ SQLiteStatement statement(m_database, "SELECT origin FROM Origins where origin=?;");
+ if (statement.prepare() != SQLResultOk) {
+ LOG_ERROR("Failed to prepare statement.");
+ return false;
+ }
+
+ statement.bindText(1, origin->databaseIdentifier());
+
+ return statement.step() == SQLResultRow;
}
bool DatabaseTracker::hasEntryForOrigin(SecurityOrigin* origin)
@@ -382,20 +393,15 @@
return fullPathForDatabaseNoLock(origin, name, createIfNotExists).isolatedCopy();
}
-void DatabaseTracker::populateOriginsIfNeeded()
+void DatabaseTracker::origins(Vector<RefPtr<SecurityOrigin> >& originsResult)
{
- ASSERT(!m_databaseGuard.tryLock());
- if (m_quotaMap)
- return;
+ MutexLocker lockDatabase(m_databaseGuard);
- m_quotaMap = adoptPtr(new QuotaMap);
-
openTrackerDatabase(DontCreateIfDoesNotExist);
if (!m_database.isOpen())
return;
- SQLiteStatement statement(m_database, "SELECT origin, quota FROM Origins");
-
+ SQLiteStatement statement(m_database, "SELECT origin FROM Origins");
if (statement.prepare() != SQLResultOk) {
LOG_ERROR("Failed to prepare statement.");
return;
@@ -404,21 +410,14 @@
int result;
while ((result = statement.step()) == SQLResultRow) {
RefPtr<SecurityOrigin> origin = SecurityOrigin::createFromDatabaseIdentifier(statement.getColumnText(0));
- m_quotaMap->set(origin.get()->isolatedCopy(), statement.getColumnInt64(1));
+ originsResult.append(origin->isolatedCopy());
}
+ originsResult.shrinkToFit();
if (result != SQLResultDone)
LOG_ERROR("Failed to read in all origins from the database.");
}
-void DatabaseTracker::origins(Vector<RefPtr<SecurityOrigin> >& result)
-{
- MutexLocker lockDatabase(m_databaseGuard);
- populateOriginsIfNeeded();
- ASSERT(m_quotaMap);
- copyKeysToVector(*m_quotaMap, result);
-}
-
bool DatabaseTracker::databaseNamesForOriginNoLock(SecurityOrigin* origin, Vector<String>& resultVector)
{
ASSERT(!m_databaseGuard.tryLock());
@@ -717,9 +716,23 @@
unsigned long long DatabaseTracker::quotaForOriginNoLock(SecurityOrigin* origin)
{
ASSERT(!m_databaseGuard.tryLock());
- populateOriginsIfNeeded();
- ASSERT(m_quotaMap);
- return m_quotaMap->get(origin);
+ unsigned long long quota = 0;
+
+ openTrackerDatabase(DontCreateIfDoesNotExist);
+ if (!m_database.isOpen())
+ return quota;
+
+ SQLiteStatement statement(m_database, "SELECT quota FROM Origins where origin=?;");
+ if (statement.prepare() != SQLResultOk) {
+ LOG_ERROR("Failed to prepare statement.");
+ return quota;
+ }
+ statement.bindText(1, origin->databaseIdentifier());
+
+ if (statement.step() == SQLResultRow)
+ quota = statement.getColumnInt64(0);
+
+ return quota;
}
unsigned long long DatabaseTracker::quotaForOrigin(SecurityOrigin* origin)
@@ -739,7 +752,8 @@
if (!m_database.isOpen())
return;
- if (!m_quotaMap->contains(origin)) {
+ bool originEntryExists = hasEntryForOriginNoLock(origin);
+ if (!originEntryExists) {
SQLiteStatement statement(m_database, "INSERT INTO Origins VALUES (?, ?)");
if (statement.prepare() != SQLResultOk) {
LOG_ERROR("Unable to establish origin %s in the tracker", origin->databaseIdentifier().ascii().data());
@@ -768,9 +782,6 @@
#endif
}
- // FIXME: Is it really OK to update the quota in memory if we failed to update it on disk?
- m_quotaMap->set(origin->isolatedCopy(), quota);
-
if (m_client)
m_client->dispatchDidModifyOrigin(origin);
}
@@ -781,8 +792,6 @@
openTrackerDatabase(CreateIfDoesNotExist);
if (!m_database.isOpen())
return false;
- populateOriginsIfNeeded();
- ASSERT(m_quotaMap);
// New database should never be added until the origin has been established
ASSERT(hasEntryForOriginNoLock(origin));
@@ -880,12 +889,20 @@
SQLiteFileSystem::deleteEmptyDatabaseDirectory(originPath(origin));
- populateOriginsIfNeeded();
RefPtr<SecurityOrigin> originPossiblyLastReference = origin;
- m_quotaMap->remove(origin);
+ bool isEmpty = true;
+ openTrackerDatabase(DontCreateIfDoesNotExist);
+ if (m_database.isOpen()) {
+ SQLiteStatement statement(m_database, "SELECT origin FROM Origins");
+ if (statement.prepare() != SQLResultOk)
+ LOG_ERROR("Failed to prepare statement.");
+ else if (statement.step() == SQLResultRow)
+ isEmpty = false;
+ }
+
// If we removed the last origin, do some additional deletion.
- if (m_quotaMap->isEmpty()) {
+ if (isEmpty) {
if (m_database.isOpen())
m_database.close();
SQLiteFileSystem::deleteDatabaseFile(trackerDatabasePath());
Modified: trunk/Source/WebCore/Modules/webdatabase/DatabaseTracker.h (145430 => 145431)
--- trunk/Source/WebCore/Modules/webdatabase/DatabaseTracker.h 2013-03-11 23:40:58 UTC (rev 145430)
+++ trunk/Source/WebCore/Modules/webdatabase/DatabaseTracker.h 2013-03-11 23:44:01 UTC (rev 145431)
@@ -137,7 +137,6 @@
bool hasEntryForDatabase(SecurityOrigin*, const String& databaseIdentifier);
bool addDatabase(SecurityOrigin*, const String& name, const String& path);
- void populateOriginsIfNeeded();
bool deleteDatabaseFile(SecurityOrigin*, const String& name);
@@ -150,13 +149,10 @@
Mutex m_openDatabaseMapGuard;
mutable OwnPtr<DatabaseOriginMap> m_openDatabaseMap;
- // This lock protects m_database, m_quotaMap, m_databaseDirectoryPath, m_originsBeingDeleted, m_beingCreated, and m_beingDeleted.
+ // This lock protects m_database, m_originLockMap, m_databaseDirectoryPath, m_originsBeingDeleted, m_beingCreated, and m_beingDeleted.
Mutex m_databaseGuard;
SQLiteDatabase m_database;
- typedef HashMap<RefPtr<SecurityOrigin>, unsigned long long> QuotaMap;
- mutable OwnPtr<QuotaMap> m_quotaMap;
-
typedef HashMap<String, RefPtr<OriginLock> > OriginLockMap;
OriginLockMap m_originLockMap;
