Diff
Modified: branches/chromium/835/Source/WebCore/storage/AbstractDatabase.cpp (92304 => 92305)
--- branches/chromium/835/Source/WebCore/storage/AbstractDatabase.cpp 2011-08-03 19:54:22 UTC (rev 92304)
+++ branches/chromium/835/Source/WebCore/storage/AbstractDatabase.cpp 2011-08-03 19:55:43 UTC (rev 92305)
@@ -34,6 +34,7 @@
#include "DatabaseTracker.h"
#include "Logging.h"
#include "SQLiteStatement.h"
+#include "SQLiteTransaction.h"
#include "ScriptExecutionContext.h"
#include "SecurityOrigin.h"
#include <wtf/HashMap.h>
@@ -235,13 +236,16 @@
String AbstractDatabase::version() const
{
- MutexLocker locker(guidMutex());
- return guidToVersionMap().get(m_guid).threadsafeCopy();
+ // Note: In multi-process browsers the cached value may be accurate, but we cannot read the
+ // actual version from the database without potentially inducing a deadlock.
+ // FIXME: Add an async version getter to the DatabaseAPI.
+ return getCachedVersion();
}
-static const int maxSqliteBusyWaitTime = 30000;
bool AbstractDatabase::performOpenAndVerify(bool shouldSetVersionInNewDatabase, ExceptionCode& ec)
{
+ const int maxSqliteBusyWaitTime = 30000;
+
if (!m_sqliteDatabase.open(m_filename, true)) {
LOG_ERROR("Unable to open database at path %s", m_filename.ascii().data());
ec = INVALID_STATE_ERR;
@@ -250,8 +254,6 @@
if (!m_sqliteDatabase.turnOnIncrementalAutoVacuum())
LOG_ERROR("Unable to turn on incremental auto-vacuum for database %s", m_filename.ascii().data());
- ASSERT(m_databaseAuthorizer);
- m_sqliteDatabase.setAuthorizer(m_databaseAuthorizer);
m_sqliteDatabase.setBusyTimeout(maxSqliteBusyWaitTime);
String currentVersion;
@@ -263,43 +265,68 @@
// Map null string to empty string (see updateGuidVersionMap()).
currentVersion = entry->second.isNull() ? String("") : entry->second;
LOG(StorageAPI, "Current cached version for guid %i is %s", m_guid, currentVersion.ascii().data());
+
+#if PLATFORM(CHROMIUM)
+ // Note: In multi-process browsers the cached value may be inaccurate, but
+ // we cannot read the actual version from the database without potentially
+ // inducing a form of deadlock, a busytimeout error when trying to
+ // access the database. So we'll use the cached value if we're able to read
+ // the value without waiting, and otherwise use the cached value (which may be off).
+ // FIXME: Add an async openDatabase method to the DatabaseAPI.
+ const int noSqliteBusyWaitTime = 0;
+ m_sqliteDatabase.setBusyTimeout(noSqliteBusyWaitTime);
+ String versionFromDatabase;
+ if (getVersionFromDatabase(versionFromDatabase, false)) {
+ currentVersion = versionFromDatabase;
+ updateGuidVersionMap(m_guid, currentVersion);
+ }
+ m_sqliteDatabase.setBusyTimeout(maxSqliteBusyWaitTime);
+#endif
} else {
LOG(StorageAPI, "No cached version for guid %i", m_guid);
+ SQLiteTransaction transaction(m_sqliteDatabase);
+ transaction.begin();
+ if (!transaction.inProgress()) {
+ LOG_ERROR("Unable to begin transaction while opening %s", databaseDebugName().ascii().data());
+ ec = INVALID_STATE_ERR;
+ m_sqliteDatabase.close();
+ return false;
+ }
+
if (!m_sqliteDatabase.tableExists(databaseInfoTableName())) {
m_new = true;
if (!m_sqliteDatabase.executeCommand("CREATE TABLE " + databaseInfoTableName() + " (key TEXT NOT NULL ON CONFLICT FAIL UNIQUE ON CONFLICT REPLACE,value TEXT NOT NULL ON CONFLICT FAIL);")) {
LOG_ERROR("Unable to create table %s in database %s", databaseInfoTableName().ascii().data(), databaseDebugName().ascii().data());
ec = INVALID_STATE_ERR;
- // Close the handle to the database file.
+ transaction.rollback();
m_sqliteDatabase.close();
return false;
}
- }
-
- if (!getVersionFromDatabase(currentVersion)) {
+ } else if (!getVersionFromDatabase(currentVersion, false)) {
LOG_ERROR("Failed to get current version from database %s", databaseDebugName().ascii().data());
ec = INVALID_STATE_ERR;
- // Close the handle to the database file.
+ transaction.rollback();
m_sqliteDatabase.close();
return false;
}
+
if (currentVersion.length()) {
LOG(StorageAPI, "Retrieved current version %s from database %s", currentVersion.ascii().data(), databaseDebugName().ascii().data());
} else if (!m_new || shouldSetVersionInNewDatabase) {
LOG(StorageAPI, "Setting version %s in database %s that was just created", m_expectedVersion.ascii().data(), databaseDebugName().ascii().data());
- if (!setVersionInDatabase(m_expectedVersion)) {
+ if (!setVersionInDatabase(m_expectedVersion, false)) {
LOG_ERROR("Failed to set version %s in database %s", m_expectedVersion.ascii().data(), databaseDebugName().ascii().data());
ec = INVALID_STATE_ERR;
- // Close the handle to the database file.
+ transaction.rollback();
m_sqliteDatabase.close();
return false;
}
currentVersion = m_expectedVersion;
}
-
updateGuidVersionMap(m_guid, currentVersion);
+ transaction.commit();
}
}
@@ -314,13 +341,18 @@
LOG(StorageAPI, "page expects version %s from database %s, which actually has version name %s - openDatabase() call will fail", m_expectedVersion.ascii().data(),
databaseDebugName().ascii().data(), currentVersion.ascii().data());
ec = INVALID_STATE_ERR;
- // Close the handle to the database file.
m_sqliteDatabase.close();
return false;
}
+ ASSERT(m_databaseAuthorizer);
+ m_sqliteDatabase.setAuthorizer(m_databaseAuthorizer);
+
m_opened = true;
+ if (m_new && !shouldSetVersionInNewDatabase)
+ m_expectedVersion = ""; // The caller provided a creationCallback which will set the expected version.
+
return true;
}
@@ -364,14 +396,17 @@
return key;
}
-bool AbstractDatabase::getVersionFromDatabase(String& version)
+bool AbstractDatabase::getVersionFromDatabase(String& version, bool shouldCacheVersion)
{
DEFINE_STATIC_LOCAL(String, getVersionQuery, ("SELECT value FROM " + databaseInfoTableName() + " WHERE key = '" + databaseVersionKey() + "';"));
m_databaseAuthorizer->disable();
bool result = retrieveTextResultFromDatabase(m_sqliteDatabase, getVersionQuery.threadsafeCopy(), version);
- if (!result)
+ if (result) {
+ if (shouldCacheVersion)
+ setCachedVersion(version);
+ } else
LOG_ERROR("Failed to retrieve version from database %s", databaseDebugName().ascii().data());
m_databaseAuthorizer->enable();
@@ -379,7 +414,7 @@
return result;
}
-bool AbstractDatabase::setVersionInDatabase(const String& version)
+bool AbstractDatabase::setVersionInDatabase(const String& version, bool shouldCacheVersion)
{
// The INSERT will replace an existing entry for the database with the new version number, due to the UNIQUE ON CONFLICT REPLACE
// clause in the CREATE statement (see Database::performOpenAndVerify()).
@@ -388,7 +423,10 @@
m_databaseAuthorizer->disable();
bool result = setTextValueInDatabase(m_sqliteDatabase, setVersionQuery.threadsafeCopy(), version);
- if (!result)
+ if (result) {
+ if (shouldCacheVersion)
+ setCachedVersion(version);
+ } else
LOG_ERROR("Failed to set version %s in database (%s)", version.ascii().data(), setVersionQuery.ascii().data());
m_databaseAuthorizer->enable();
@@ -396,24 +434,37 @@
return result;
}
-bool AbstractDatabase::versionMatchesExpected() const
+void AbstractDatabase::setExpectedVersion(const String& version)
{
- if (!m_expectedVersion.isEmpty()) {
- MutexLocker locker(guidMutex());
- return m_expectedVersion == guidToVersionMap().get(m_guid);
- }
+ m_expectedVersion = version.threadsafeCopy();
+}
- return true;
+String AbstractDatabase::getCachedVersion() const
+{
+ MutexLocker locker(guidMutex());
+ return guidToVersionMap().get(m_guid).threadsafeCopy();
}
-void AbstractDatabase::setExpectedVersion(const String& version)
+void AbstractDatabase::setCachedVersion(const String& actualVersion)
{
- m_expectedVersion = version.threadsafeCopy();
// Update the in memory database version map.
MutexLocker locker(guidMutex());
- updateGuidVersionMap(m_guid, version);
+ updateGuidVersionMap(m_guid, actualVersion);
}
+bool AbstractDatabase::getActualVersionForTransaction(String &actualVersion)
+{
+ ASSERT(m_sqliteDatabase.transactionInProgress());
+#if PLATFORM(CHROMIUM)
+ // Note: In multi-process browsers the cached value may be inaccurate.
+ // So we retrieve the value from the database and update the cached value here.
+ return getVersionFromDatabase(actualVersion, true);
+#else
+ actualVersion = getCachedVersion();
+ return true;
+#endif
+}
+
void AbstractDatabase::disableAuthorizer()
{
ASSERT(m_databaseAuthorizer);
Modified: branches/chromium/835/Source/WebCore/storage/AbstractDatabase.h (92304 => 92305)
--- branches/chromium/835/Source/WebCore/storage/AbstractDatabase.h 2011-08-03 19:54:22 UTC (rev 92304)
+++ branches/chromium/835/Source/WebCore/storage/AbstractDatabase.h 2011-08-03 19:55:43 UTC (rev 92305)
@@ -71,12 +71,6 @@
void interrupt();
bool isInterrupted();
- // FIXME: move all version-related methods to a DatabaseVersionTracker class
- bool versionMatchesExpected() const;
- void setExpectedVersion(const String& version);
- bool getVersionFromDatabase(String& version);
- bool setVersionInDatabase(const String& version);
-
void disableAuthorizer();
void enableAuthorizer();
void setAuthorizerReadOnly();
@@ -91,6 +85,10 @@
virtual void closeImmediately() = 0;
protected:
+ friend class SQLTransactionSync;
+ friend class SQLTransaction;
+ friend class ChangeVersionWrapper;
+
AbstractDatabase(ScriptExecutionContext*, const String& name, const String& expectedVersion,
const String& displayName, unsigned long estimatedSize);
@@ -98,6 +96,14 @@
virtual bool performOpenAndVerify(bool shouldSetVersionInNewDatabase, ExceptionCode& ec);
+ bool getVersionFromDatabase(String& version, bool shouldCacheVersion = true);
+ bool setVersionInDatabase(const String& version, bool shouldCacheVersion = true);
+ void setExpectedVersion(const String&);
+ const String& expectedVersion() const { return m_expectedVersion; }
+ String getCachedVersion()const;
+ void setCachedVersion(const String&);
+ bool getActualVersionForTransaction(String& version);
+
static const String& databaseInfoTableName();
RefPtr<ScriptExecutionContext> m_scriptExecutionContext;
Modified: branches/chromium/835/Source/WebCore/storage/ChangeVersionWrapper.cpp (92304 => 92305)
--- branches/chromium/835/Source/WebCore/storage/ChangeVersionWrapper.cpp 2011-08-03 19:54:22 UTC (rev 92304)
+++ branches/chromium/835/Source/WebCore/storage/ChangeVersionWrapper.cpp 2011-08-03 19:55:43 UTC (rev 92305)
@@ -78,6 +78,11 @@
return true;
}
+void ChangeVersionWrapper::handleCommitFailedAfterPostflight(SQLTransaction* transaction)
+{
+ transaction->database()->setCachedVersion(m_oldVersion);
+}
+
} // namespace WebCore
#endif // ENABLE(DATABASE)
Modified: branches/chromium/835/Source/WebCore/storage/ChangeVersionWrapper.h (92304 => 92305)
--- branches/chromium/835/Source/WebCore/storage/ChangeVersionWrapper.h 2011-08-03 19:54:22 UTC (rev 92304)
+++ branches/chromium/835/Source/WebCore/storage/ChangeVersionWrapper.h 2011-08-03 19:55:43 UTC (rev 92305)
@@ -44,8 +44,8 @@
virtual bool performPreflight(SQLTransaction*);
virtual bool performPostflight(SQLTransaction*);
-
virtual SQLError* sqlError() const { return m_sqlError.get(); }
+ virtual void handleCommitFailedAfterPostflight(SQLTransaction*);
private:
ChangeVersionWrapper(const String& oldVersion, const String& newVersion);
Modified: branches/chromium/835/Source/WebCore/storage/Database.cpp (92304 => 92305)
--- branches/chromium/835/Source/WebCore/storage/Database.cpp 2011-08-03 19:54:22 UTC (rev 92304)
+++ branches/chromium/835/Source/WebCore/storage/Database.cpp 2011-08-03 19:55:43 UTC (rev 92305)
@@ -109,12 +109,7 @@
InspectorInstrumentation::didOpenDatabase(context, database, context->securityOrigin()->host(), name, expectedVersion);
- // If it's a new database and a creation callback was provided, reset the expected
- // version to "" and schedule the creation callback. Because of some subtle String
- // implementation issues, we have to reset m_expectedVersion here instead of doing
- // it inside performOpenAndVerify() which is run on the DB thread.
if (database->isNew() && creationCallback.get()) {
- database->m_expectedVersion = "";
LOG(StorageAPI, "Scheduling DatabaseCreationCallbackTask for database %p\n", database.get());
database->m_scriptExecutionContext->postTask(DatabaseCreationCallbackTask::create(database, creationCallback));
}
Modified: branches/chromium/835/Source/WebCore/storage/DatabaseSync.cpp (92304 => 92305)
--- branches/chromium/835/Source/WebCore/storage/DatabaseSync.cpp 2011-08-03 19:54:22 UTC (rev 92304)
+++ branches/chromium/835/Source/WebCore/storage/DatabaseSync.cpp 2011-08-03 19:55:43 UTC (rev 92305)
@@ -67,7 +67,6 @@
DatabaseTracker::tracker().setDatabaseDetails(context->securityOrigin(), name, displayName, estimatedSize);
if (database->isNew() && creationCallback.get()) {
- database->m_expectedVersion = "";
LOG(StorageAPI, "Invoking the creation callback for database %p\n", database.get());
creationCallback->handleEvent(database.get());
}
@@ -123,8 +122,10 @@
return;
}
- if ((ec = transaction->commit()))
+ if ((ec = transaction->commit())) {
+ setCachedVersion(oldVersion);
return;
+ }
setExpectedVersion(newVersion);
}
Modified: branches/chromium/835/Source/WebCore/storage/SQLTransaction.cpp (92304 => 92305)
--- branches/chromium/835/Source/WebCore/storage/SQLTransaction.cpp 2011-08-03 19:54:22 UTC (rev 92304)
+++ branches/chromium/835/Source/WebCore/storage/SQLTransaction.cpp 2011-08-03 19:55:43 UTC (rev 92305)
@@ -78,6 +78,7 @@
, m_modifiedDatabase(false)
, m_lockAcquired(false)
, m_readOnly(readOnly)
+ , m_hasVersionMismatch(false)
{
ASSERT(m_database);
}
@@ -105,9 +106,6 @@
if (m_database->deleted())
statement->setDatabaseDeletedError();
- if (!m_database->versionMatchesExpected())
- statement->setVersionMismatchedError();
-
enqueueStatement(statement);
}
@@ -272,9 +270,26 @@
return;
}
+ // Note: We intentionally retrieve the actual version even with an empty expected version.
+ // In multi-process browsers, we take this opportinutiy to update the cached value for
+ // the actual version. In single-process browsers, this is just a map lookup.
+ String actualVersion;
+ if (!m_database->getActualVersionForTransaction(actualVersion)) {
+ m_database->disableAuthorizer();
+ m_sqliteTransaction.clear();
+ m_database->enableAuthorizer();
+ m_transactionError = SQLError::create(SQLError::DATABASE_ERR, "unable to read version");
+ handleTransactionError(false);
+ return;
+ }
+ m_hasVersionMismatch = !m_database->expectedVersion().isEmpty()
+ && (m_database->expectedVersion() != actualVersion);
+
// Transaction Steps 3 - Peform preflight steps, jumping to the error callback if they fail
if (m_wrapper && !m_wrapper->performPreflight(this)) {
+ m_database->disableAuthorizer();
m_sqliteTransaction.clear();
+ m_database->enableAuthorizer();
m_transactionError = m_wrapper->sqlError();
if (!m_transactionError)
m_transactionError = SQLError::create(SQLError::UNKNOWN_ERR, "unknown error occured setting up transaction");
@@ -369,6 +384,9 @@
m_database->resetAuthorizer();
+ if (m_hasVersionMismatch)
+ m_currentStatement->setVersionMismatchedError();
+
if (m_currentStatement->execute(m_database.get())) {
if (m_database->lastActionChangedDatabase()) {
// Flag this transaction as having changed the database for later delegate notification
@@ -465,6 +483,8 @@
// If the commit failed, the transaction will still be marked as "in progress"
if (m_sqliteTransaction->inProgress()) {
+ if (m_wrapper)
+ m_wrapper->handleCommitFailedAfterPostflight(this);
m_successCallbackWrapper.clear();
m_transactionError = SQLError::create(SQLError::DATABASE_ERR, "failed to commit the transaction");
handleTransactionError(false);
Modified: branches/chromium/835/Source/WebCore/storage/SQLTransaction.h (92304 => 92305)
--- branches/chromium/835/Source/WebCore/storage/SQLTransaction.h 2011-08-03 19:54:22 UTC (rev 92304)
+++ branches/chromium/835/Source/WebCore/storage/SQLTransaction.h 2011-08-03 19:55:43 UTC (rev 92305)
@@ -55,8 +55,8 @@
virtual ~SQLTransactionWrapper() { }
virtual bool performPreflight(SQLTransaction*) = 0;
virtual bool performPostflight(SQLTransaction*) = 0;
-
virtual SQLError* sqlError() const = 0;
+ virtual void handleCommitFailedAfterPostflight(SQLTransaction*) = 0;
};
class SQLTransaction : public ThreadSafeRefCounted<SQLTransaction> {
@@ -123,6 +123,7 @@
bool m_modifiedDatabase;
bool m_lockAcquired;
bool m_readOnly;
+ bool m_hasVersionMismatch;
Mutex m_statementMutex;
Deque<RefPtr<SQLStatement> > m_statementQueue;
Modified: branches/chromium/835/Source/WebCore/storage/SQLTransactionSync.cpp (92304 => 92305)
--- branches/chromium/835/Source/WebCore/storage/SQLTransactionSync.cpp 2011-08-03 19:54:22 UTC (rev 92304)
+++ branches/chromium/835/Source/WebCore/storage/SQLTransactionSync.cpp 2011-08-03 19:55:43 UTC (rev 92305)
@@ -58,6 +58,7 @@
: m_database(db)
, m_callback(callback)
, m_readOnly(readOnly)
+ , m_hasVersionMismatch(false)
, m_modifiedDatabase(false)
, m_transactionClient(adoptPtr(new SQLTransactionClient()))
{
@@ -79,7 +80,7 @@
return 0;
}
- if (!m_database->versionMatchesExpected()) {
+ if (m_hasVersionMismatch) {
ec = SQLException::VERSION_ERR;
return 0;
}
@@ -150,6 +151,16 @@
return SQLException::DATABASE_ERR;
}
+ // Note: We intentionally retrieve the actual version even with an empty expected version.
+ // In multi-process browsers, we take this opportinutiy to update the cached value for
+ // the actual version. In single-process browsers, this is just a map lookup.
+ String actualVersion;
+ if (!m_database->getActualVersionForTransaction(actualVersion)) {
+ rollback();
+ return SQLException::DATABASE_ERR;
+ }
+ m_hasVersionMismatch = !m_database->expectedVersion().isEmpty()
+ && (m_database->expectedVersion() != actualVersion);
return 0;
}
Modified: branches/chromium/835/Source/WebCore/storage/SQLTransactionSync.h (92304 => 92305)
--- branches/chromium/835/Source/WebCore/storage/SQLTransactionSync.h 2011-08-03 19:54:22 UTC (rev 92304)
+++ branches/chromium/835/Source/WebCore/storage/SQLTransactionSync.h 2011-08-03 19:55:43 UTC (rev 92305)
@@ -34,6 +34,7 @@
#if ENABLE(DATABASE)
#include "ExceptionCode.h"
+#include "PlatformString.h"
#include <wtf/Forward.h>
#include <wtf/RefCounted.h>
#include <wtf/Vector.h>
@@ -70,6 +71,7 @@
RefPtr<DatabaseSync> m_database;
RefPtr<SQLTransactionSyncCallback> m_callback;
bool m_readOnly;
+ bool m_hasVersionMismatch;
bool m_modifiedDatabase;
OwnPtr<SQLTransactionClient> m_transactionClient;
