Commit f7e0cd81 authored by benm@google.com's avatar benm@google.com

Stale database version persists through browser refresh (changeVersion doesn't work)

https://bugs.webkit.org/show_bug.cgi?id=27836

WebCore: 

Reviewed by David Kilzer.

Tests: storage/change-version-handle-reuse.html
       storage/change-version.html

* bindings/v8/custom/V8DatabaseCustom.cpp:
(WebCore::CALLBACK_FUNC_DECL): Implement the V8 binding for database.changeVersion().
(WebCore::createTransaction): Fix a bug that was checking the wrong argument index to save the success callback.
* storage/Database.cpp:
(WebCore::updateGuidVersionMap): Safely update the Guid/version hash map.
(WebCore::Database::~Database): Remove code that removes the database from the guid->database and guid->version maps.
(WebCore::Database::setVersionInDatabase): Add a comment to explain some behaviour.
(WebCore::Database::close): Move the code that updates the maps from the destructor to here.
(WebCore::Database::performOpenAndVerify): Call updateGuidVersionMap instead of setting the hash map directly.
(WebCore::Database::setExpectedVersion): Update the in memory guid->version map when we want to update the database version.

LayoutTests: 

Reviewed by  David Kilzer.

* storage/change-version-expected.txt: Added.
* storage/change-version-handle-reuse-expected.txt: Added.
* storage/change-version-handle-reuse.html: Added.
* storage/change-version.html: Added.



git-svn-id: http://svn.webkit.org/repository/webkit/trunk@49018 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent 30f9bf08
2009-10-02 Ben Murdoch <benm@google.com>
Reviewed by David Kilzer.
Stale database version persists through browser refresh (changeVersion doesn't work)
https://bugs.webkit.org/show_bug.cgi?id=27836
* storage/change-version-expected.txt: Added.
* storage/change-version-handle-reuse-expected.txt: Added.
* storage/change-version-handle-reuse.html: Added.
* storage/change-version.html: Added.
2009-10-01 Drew Wilson <atwilson@chromium.org>
Reviewed by David Levin.
......
This test verifies that the JS database.changeVersion() function works as expected.
Finished tests with version 3; expected version: 3
TEST COMPLETE
This tests that a database can be accessed after changing its version. You should see an error about FooBar table below, not about mismatching versions. Also, reloading the page should not cause an assertion failure.
changeVersion: transaction callback
changeVersion: success callback
transaction: statement error callback: no such table: FooBar
TEST COMPLETE.
<html>
<head>
<script>
function log(message)
{
document.getElementById("result").innerText += message + "\n";
}
function finishTest()
{
log("TEST COMPLETE.");
if (window.layoutTestController)
layoutTestController.notifyDone();
}
function runTest()
{
if (window.layoutTestController) {
layoutTestController.waitUntilDone();
layoutTestController.dumpAsText();
}
document.getElementById("result").innerText = "";
try {
db = openDatabase("ChangeVersion", "", "Test that changing a database version doesn\'t kill our handle to it", 1);
var version = db.version;
var newVersion = version ? (parseInt(version) + 1).toString() : "1";
db.changeVersion(version, newVersion, function(tx) {
log("changeVersion: transaction callback");
}, function(error) {
log("changeVersion: error callback: " + error.message);
}, function() {
log("changeVersion: success callback");
});
setTimeout(runTest2, 1000);
} catch (e) {
log("changeVersion exception: " + e);
finishTest();
}
}
function runTest2()
{
try {
db.transaction(function(tx) {
tx.executeSql("SELECT * from FooBar", [], function(tx) {
log("transaction: statement callback");
finishTest();
}, function(tx, error) {
log("transaction: statement error callback: " + error.message);
finishTest();
});
});
} catch (e) {
log("transaction exception: " + e);
finishTest();
}
}
</script>
</head>
<body onload="runTest()">
<div>This tests that a database can be accessed after changing its version. You should see an error about FooBar table below, not about mismatching versions. Also, reloading the page should not cause an assertion failure.
<pre id="result">
FAILURE: test didn't run.
</pre>
</body>
</html>
<html>
<head>
<title>Test database.changeVersion</title>
<script>
var db;
var EXPECTED_VERSION_AFTER_HIXIE_TEST = '2';
var EXPECTED_VERSION_AFTER_RELOAD = '3';
function emptyFunction() { }
function changeVersionCallback(tx)
{
tx.executeSql("DROP table if exists info;", [], emptyFunction, emptyFunction);
tx.executeSql("CREATE table if not exists info (version INTEGER);", [], emptyFunction, emptyFunction);
tx.executeSql("INSERT into info values(?);", [EXPECTED_VERSION_AFTER_RELOAD], emptyFunction, emptyFunction);
}
function changeVersionSuccess()
{
log("Successfully changed version to ' + db.version + '. Reloading.");
window.location.href = window.location + '?2';
}
function changeVersionError(error)
{
log("Error: " + error.message);
finishTest();
}
function finishTest()
{
if (window.layoutTestController)
layoutTestController.notifyDone();
log("TEST COMPLETE");
}
function log(message)
{
document.getElementById("console").innerText += message + "\n";
}
function runTest()
{
if (window.location.search == "?2") {
db = window.openDatabase("changeversion-test", "", "Test for the database.changeVersion() function", 1024);
log("Finished tests with version " + db.version + "; expected version: " + EXPECTED_VERSION_AFTER_RELOAD);
finishTest();
} else
testPart1();
}
function testPart1() {
if (window.layoutTestController) {
layoutTestController.clearAllDatabases();
layoutTestController.dumpAsText();
layoutTestController.waitUntilDone();
}
db = window.openDatabase("changeversion-test", "", "Test for the database.changeVersion() function", 1024);
if (db.version != EXPECTED_VERSION_AFTER_RELOAD) {
// First run Hixie's test to ensure basic changeVersion functionality works (see bug 28418).
db.changeVersion("", EXPECTED_VERSION_AFTER_HIXIE_TEST, emptyFunction, function (e) {
log('FAIL in changeVersion:' + e);
finishTest();
}, function () {
try {
var db2 = openDatabase("change-version-test", EXPECTED_VERSION_AFTER_HIXIE_TEST, "", 0);
} catch (e) {
log('FAIL in openDatabase: ' + e);
finishTest();
}
// The two database versions should match.
if (db.version == db2.version)
log("PASS: db.version(" + db.version + ") matches db2.version(" + db2.version +") as expected.");
else
log("FAIL: db.version(" + db.version + ") does not match db2.version(" + db2.version +")");
// Now try a test to ensure the version persists after reloading (see bug 27836)
db.changeVersion(EXPECTED_VERSION_AFTER_HIXIE_TEST, EXPECTED_VERSION_AFTER_RELOAD, changeVersionCallback, changeVersionError, changeVersionSuccess);
});
}
}
</script>
</head>
<body onload="runTest();">
This test verifies that the JS database.changeVersion() function works as expected.
<pre id="console"></pre>
</body>
</html>
2009-10-02 Ben Murdoch <benm@google.com>
Reviewed by David Kilzer.
Stale database version persists through browser refresh (changeVersion doesn't work)
https://bugs.webkit.org/show_bug.cgi?id=27836
Tests: storage/change-version-handle-reuse.html
storage/change-version.html
* bindings/v8/custom/V8DatabaseCustom.cpp:
(WebCore::CALLBACK_FUNC_DECL): Implement the V8 binding for database.changeVersion().
(WebCore::createTransaction): Fix a bug that was checking the wrong argument index to save the success callback.
* storage/Database.cpp:
(WebCore::updateGuidVersionMap): Safely update the Guid/version hash map.
(WebCore::Database::~Database): Remove code that removes the database from the guid->database and guid->version maps.
(WebCore::Database::setVersionInDatabase): Add a comment to explain some behaviour.
(WebCore::Database::close): Move the code that updates the maps from the destructor to here.
(WebCore::Database::performOpenAndVerify): Call updateGuidVersionMap instead of setting the hash map directly.
(WebCore::Database::setExpectedVersion): Update the in memory guid->version map when we want to update the database version.
2009-10-02 Janne Koskinen <janne.p.koskinen@digia.com>
Reviewed by Simon Hausmann.
......@@ -45,6 +45,45 @@ namespace WebCore {
CALLBACK_FUNC_DECL(DatabaseChangeVersion)
{
INC_STATS("DOM.Database.changeVersion()");
if (args.Length() < 2)
return throwError("The old and new version strings are required.", V8Proxy::SyntaxError);
if (!(args[0]->IsString() && args[1]->IsString()))
return throwError("The old and new versions must be strings.");
Database* database = V8DOMWrapper::convertToNativeObject<Database>(V8ClassIndex::DATABASE, args.Holder());
Frame* frame = V8Proxy::retrieveFrameForCurrentContext();
if (!frame)
return v8::Undefined();
RefPtr<V8CustomSQLTransactionCallback> callback;
if (args.Length() > 2) {
if (!args[2]->IsObject())
return throwError("changeVersion transaction callback must be of valid type.");
callback = V8CustomSQLTransactionCallback::create(args[2], frame);
}
RefPtr<V8CustomSQLTransactionErrorCallback> errorCallback;
if (args.Length() > 3) {
if (!args[3]->IsObject())
return throwError("changeVersion error callback must be of valid type.");
errorCallback = V8CustomSQLTransactionErrorCallback::create(args[3], frame);
}
RefPtr<V8CustomVoidCallback> successCallback;
if (args.Length() > 4) {
if (!args[4]->IsObject())
return throwError("changeVersion success callback must be of valid type.");
successCallback = V8CustomVoidCallback::create(args[4], frame);
}
database->changeVersion(toWebCoreString(args[0]), toWebCoreString(args[1]), callback.release(), errorCallback.release(), successCallback.release());
return v8::Undefined();
}
......@@ -74,7 +113,7 @@ static v8::Handle<v8::Value> createTransaction(const v8::Arguments& args, bool r
RefPtr<V8CustomVoidCallback> successCallback;
if (args.Length() > 2) {
if (!args[1]->IsObject())
if (!args[2]->IsObject())
return throwError("Transaction success callback must be of valid type.");
successCallback = V8CustomVoidCallback::create(args[2], frame);
......
......@@ -89,6 +89,19 @@ static GuidVersionMap& guidToVersionMap()
return map;
}
// NOTE: Caller must lock guidMutex().
static inline void updateGuidVersionMap(int guid, String newVersion)
{
// Note: It is not safe to put an empty string into the guidToVersionMap() map.
// That's because the map is cross-thread, but empty strings are per-thread.
// The copy() function makes a version of the string you can use on the current
// thread, but we need a string we can keep in a cross-thread data structure.
// FIXME: This is a quite-awkward restriction to have to program with.
// Map null string to empty string (see comment above).
guidToVersionMap().set(guid, newVersion.isEmpty() ? String() : newVersion.copy());
}
typedef HashMap<int, HashSet<Database*>*> GuidDatabaseMap;
static GuidDatabaseMap& guidToDatabaseMap()
{
......@@ -177,20 +190,6 @@ Database::Database(Document* document, const String& name, const String& expecte
Database::~Database()
{
{
MutexLocker locker(guidMutex());
HashSet<Database*>* hashSet = guidToDatabaseMap().get(m_guid);
ASSERT(hashSet);
ASSERT(hashSet->contains(this));
hashSet->remove(this);
if (hashSet->isEmpty()) {
guidToDatabaseMap().remove(m_guid);
delete hashSet;
guidToVersionMap().remove(m_guid);
}
}
if (m_document->databaseThread())
m_document->databaseThread()->unscheduleDatabaseTasks(this);
......@@ -277,6 +276,8 @@ static bool setTextValueInDatabase(SQLiteDatabase& db, const String& query, cons
bool Database::setVersionInDatabase(const String& version)
{
// 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()).
DEFINE_STATIC_LOCAL(String, setVersionQuery, ("INSERT INTO " + databaseInfoTableName() + " (key, value) VALUES ('" + databaseVersionKey() + "', ?);"));
m_databaseAuthorizer->disable();
......@@ -330,6 +331,20 @@ void Database::close()
m_sqliteDatabase.close();
m_document->databaseThread()->recordDatabaseClosed(this);
m_opened = false;
{
MutexLocker locker(guidMutex());
HashSet<Database*>* hashSet = guidToDatabaseMap().get(m_guid);
ASSERT(hashSet);
ASSERT(hashSet->contains(this));
hashSet->remove(this);
if (hashSet->isEmpty()) {
guidToDatabaseMap().remove(m_guid);
delete hashSet;
guidToVersionMap().remove(m_guid);
}
}
}
}
......@@ -449,15 +464,9 @@ bool Database::performOpenAndVerify(ExceptionCode& e)
{
MutexLocker locker(guidMutex());
// Note: It is not safe to put an empty string into the guidToVersionMap() map.
// That's because the map is cross-thread, but empty strings are per-thread.
// The copy() function makes a version of the string you can use on the current
// thread, but we need a string we can keep in a cross-thread data structure.
// FIXME: This is a quite-awkward restriction to have to program with.
GuidVersionMap::iterator entry = guidToVersionMap().find(m_guid);
if (entry != guidToVersionMap().end()) {
// Map null string to empty string (see comment above).
// 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());
} else {
......@@ -488,8 +497,7 @@ bool Database::performOpenAndVerify(ExceptionCode& e)
currentVersion = m_expectedVersion;
}
// Map empty string to null string (see comment above).
guidToVersionMap().set(m_guid, currentVersion.isEmpty() ? String() : currentVersion.copy());
updateGuidVersionMap(m_guid, currentVersion);
}
}
......@@ -633,6 +641,9 @@ Vector<String> Database::tableNames()
void Database::setExpectedVersion(const String& version)
{
m_expectedVersion = version.copy();
// Update the in memory database version map.
MutexLocker locker(guidMutex());
updateGuidVersionMap(m_guid, version);
}
PassRefPtr<SecurityOrigin> Database::securityOriginCopy() const
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment