[SCM] WebKit Debian packaging branch, webkit-1.1, updated. upstream/1.1.15.1-1414-gc69ee75

benm at google.com benm at google.com
Thu Oct 29 20:38:04 UTC 2009


The following commit has been merged in the webkit-1.1 branch:
commit f7e0cd81d8bbf769fa410b29e0f4a3a9788a0f69
Author: benm at google.com <benm at google.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date:   Fri Oct 2 10:19:45 2009 +0000

    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

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 2af1b45..6edc83a 100644
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,15 @@
+2009-10-02  Ben Murdoch  <benm at 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 at chromium.org>
 
         Reviewed by David Levin.
diff --git a/LayoutTests/storage/change-version-expected.txt b/LayoutTests/storage/change-version-expected.txt
new file mode 100644
index 0000000..c939e36
--- /dev/null
+++ b/LayoutTests/storage/change-version-expected.txt
@@ -0,0 +1,4 @@
+This test verifies that the JS database.changeVersion() function works as expected.
+Finished tests with version 3; expected version: 3
+TEST COMPLETE
+
diff --git a/LayoutTests/storage/change-version-handle-reuse-expected.txt b/LayoutTests/storage/change-version-handle-reuse-expected.txt
new file mode 100644
index 0000000..805c9fb
--- /dev/null
+++ b/LayoutTests/storage/change-version-handle-reuse-expected.txt
@@ -0,0 +1,6 @@
+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.
+
diff --git a/LayoutTests/storage/change-version-handle-reuse.html b/LayoutTests/storage/change-version-handle-reuse.html
new file mode 100644
index 0000000..de3a5f4
--- /dev/null
+++ b/LayoutTests/storage/change-version-handle-reuse.html
@@ -0,0 +1,71 @@
+<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>
diff --git a/LayoutTests/storage/change-version.html b/LayoutTests/storage/change-version.html
new file mode 100644
index 0000000..d81c6b5
--- /dev/null
+++ b/LayoutTests/storage/change-version.html
@@ -0,0 +1,90 @@
+<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>
diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
index aa5f0a6..b90fd50 100644
--- a/WebCore/ChangeLog
+++ b/WebCore/ChangeLog
@@ -1,3 +1,24 @@
+2009-10-02  Ben Murdoch  <benm at 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 at digia.com>
 
         Reviewed by Simon Hausmann.
diff --git a/WebCore/bindings/v8/custom/V8DatabaseCustom.cpp b/WebCore/bindings/v8/custom/V8DatabaseCustom.cpp
index 259e702..9ddd620 100644
--- a/WebCore/bindings/v8/custom/V8DatabaseCustom.cpp
+++ b/WebCore/bindings/v8/custom/V8DatabaseCustom.cpp
@@ -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);
diff --git a/WebCore/storage/Database.cpp b/WebCore/storage/Database.cpp
index be0c490..be5cdcc 100644
--- a/WebCore/storage/Database.cpp
+++ b/WebCore/storage/Database.cpp
@@ -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

-- 
WebKit Debian packaging



More information about the Pkg-webkit-commits mailing list