Changeset 49018 in webkit


Ignore:
Timestamp:
Oct 2, 2009 3:19:45 AM (15 years ago)
Author:
benm@google.com
Message:

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.
Location:
trunk
Files:
4 added
4 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r49009 r49018  
     12009-10-02  Ben Murdoch  <benm@google.com>
     2
     3        Reviewed by  David Kilzer.
     4
     5        Stale database version persists through browser refresh (changeVersion doesn't work)
     6        https://bugs.webkit.org/show_bug.cgi?id=27836
     7
     8        * storage/change-version-expected.txt: Added.
     9        * storage/change-version-handle-reuse-expected.txt: Added.
     10        * storage/change-version-handle-reuse.html: Added.
     11        * storage/change-version.html: Added.
     12
    1132009-10-01  Drew Wilson  <atwilson@chromium.org>
    214

  • trunk/WebCore/ChangeLog

    r49017 r49018  
     12009-10-02  Ben Murdoch  <benm@google.com>
     2
     3        Reviewed by David Kilzer.
     4
     5        Stale database version persists through browser refresh (changeVersion doesn't work)
     6        https://bugs.webkit.org/show_bug.cgi?id=27836
     7
     8        Tests: storage/change-version-handle-reuse.html
     9               storage/change-version.html
     10
     11        * bindings/v8/custom/V8DatabaseCustom.cpp:
     12        (WebCore::CALLBACK_FUNC_DECL): Implement the V8 binding for database.changeVersion().
     13        (WebCore::createTransaction): Fix a bug that was checking the wrong argument index to save the success callback.
     14        * storage/Database.cpp:
     15        (WebCore::updateGuidVersionMap): Safely update the Guid/version hash map.
     16        (WebCore::Database::~Database): Remove code that removes the database from the guid->database and guid->version maps.
     17        (WebCore::Database::setVersionInDatabase): Add a comment to explain some behaviour.
     18        (WebCore::Database::close): Move the code that updates the maps from the destructor to here.
     19        (WebCore::Database::performOpenAndVerify): Call updateGuidVersionMap instead of setting the hash map directly.
     20        (WebCore::Database::setExpectedVersion): Update the in memory guid->version map when we want to update the database version.
     21
    1222009-10-02  Janne Koskinen <janne.p.koskinen@digia.com>
    223

  • trunk/WebCore/bindings/v8/custom/V8DatabaseCustom.cpp

    r48227 r49018  
    4646{
    4747    INC_STATS("DOM.Database.changeVersion()");
     48
     49    if (args.Length() < 2)
     50        return throwError("The old and new version strings are required.", V8Proxy::SyntaxError);
     51
     52    if (!(args[0]->IsString() && args[1]->IsString()))
     53        return throwError("The old and new versions must be strings.");
     54
     55    Database* database = V8DOMWrapper::convertToNativeObject<Database>(V8ClassIndex::DATABASE, args.Holder());
     56
     57    Frame* frame = V8Proxy::retrieveFrameForCurrentContext();
     58    if (!frame)
     59        return v8::Undefined();
     60
     61    RefPtr<V8CustomSQLTransactionCallback> callback;
     62    if (args.Length() > 2) {
     63        if (!args[2]->IsObject())
     64            return throwError("changeVersion transaction callback must be of valid type.");
     65
     66        callback = V8CustomSQLTransactionCallback::create(args[2], frame);
     67    }
     68
     69    RefPtr<V8CustomSQLTransactionErrorCallback> errorCallback;
     70    if (args.Length() > 3) {
     71        if (!args[3]->IsObject())
     72            return throwError("changeVersion error callback must be of valid type.");
     73
     74        errorCallback = V8CustomSQLTransactionErrorCallback::create(args[3], frame);
     75    }
     76
     77    RefPtr<V8CustomVoidCallback> successCallback;
     78    if (args.Length() > 4) {
     79        if (!args[4]->IsObject())
     80            return throwError("changeVersion success callback must be of valid type.");
     81
     82        successCallback = V8CustomVoidCallback::create(args[4], frame);
     83    }
     84
     85    database->changeVersion(toWebCoreString(args[0]), toWebCoreString(args[1]), callback.release(), errorCallback.release(), successCallback.release());
     86
    4887    return v8::Undefined();
    4988}
    … …  
    75114    RefPtr<V8CustomVoidCallback> successCallback;
    76115    if (args.Length() > 2) {
    77         if (!args[1]->IsObject())
     116        if (!args[2]->IsObject())
    78117            return throwError("Transaction success callback must be of valid type.");
    79118

  • trunk/WebCore/storage/Database.cpp

    r48430 r49018  
    9090}
    9191
     92// NOTE: Caller must lock guidMutex().
     93static inline void updateGuidVersionMap(int guid, String newVersion)
     94{
     95    // Note: It is not safe to put an empty string into the guidToVersionMap() map.
     96    // That's because the map is cross-thread, but empty strings are per-thread.
     97    // The copy() function makes a version of the string you can use on the current
     98    // thread, but we need a string we can keep in a cross-thread data structure.
     99    // FIXME: This is a quite-awkward restriction to have to program with.
     100
     101    // Map null string to empty string (see comment above).
     102    guidToVersionMap().set(guid, newVersion.isEmpty() ? String() : newVersion.copy());
     103}
     104
    92105typedef HashMap<int, HashSet<Database*>*> GuidDatabaseMap;
    93106static GuidDatabaseMap& guidToDatabaseMap()
    … …  
    178191Database::~Database()
    179192{
    180     {
    181         MutexLocker locker(guidMutex());
    182 
    183         HashSet<Database*>* hashSet = guidToDatabaseMap().get(m_guid);
    184         ASSERT(hashSet);
    185         ASSERT(hashSet->contains(this));
    186         hashSet->remove(this);
    187         if (hashSet->isEmpty()) {
    188             guidToDatabaseMap().remove(m_guid);
    189             delete hashSet;
    190             guidToVersionMap().remove(m_guid);
    191         }
    192     }
    193 
    194193    if (m_document->databaseThread())
    195194        m_document->databaseThread()->unscheduleDatabaseTasks(this);
    … …  
    278277bool Database::setVersionInDatabase(const String& version)
    279278{
     279    // The INSERT will replace an existing entry for the database with the new version number, due to the UNIQUE ON CONFLICT REPLACE
     280    // clause in the CREATE statement (see Database::performOpenAndVerify()).
    280281    DEFINE_STATIC_LOCAL(String, setVersionQuery, ("INSERT INTO " + databaseInfoTableName() + " (key, value) VALUES ('" + databaseVersionKey() + "', ?);"));
    281282
    … …  
    331332        m_document->databaseThread()->recordDatabaseClosed(this);
    332333        m_opened = false;
     334
     335        {
     336            MutexLocker locker(guidMutex());
     337
     338            HashSet<Database*>* hashSet = guidToDatabaseMap().get(m_guid);
     339            ASSERT(hashSet);
     340            ASSERT(hashSet->contains(this));
     341            hashSet->remove(this);
     342            if (hashSet->isEmpty()) {
     343                guidToDatabaseMap().remove(m_guid);
     344                delete hashSet;
     345                guidToVersionMap().remove(m_guid);
     346            }
     347        }
    333348    }
    334349}
    … …  
    450465        MutexLocker locker(guidMutex());
    451466
    452         // Note: It is not safe to put an empty string into the guidToVersionMap() map.
    453         // That's because the map is cross-thread, but empty strings are per-thread.
    454         // The copy() function makes a version of the string you can use on the current
    455         // thread, but we need a string we can keep in a cross-thread data structure.
    456         // FIXME: This is a quite-awkward restriction to have to program with.
    457 
    458467        GuidVersionMap::iterator entry = guidToVersionMap().find(m_guid);
    459468        if (entry != guidToVersionMap().end()) {
    460             // Map null string to empty string (see comment above).
     469            // Map null string to empty string (see updateGuidVersionMap()).
    461470            currentVersion = entry->second.isNull() ? String("") : entry->second;
    462471            LOG(StorageAPI, "Current cached version for guid %i is %s", m_guid, currentVersion.ascii().data());
    … …  
    489498            }
    490499
    491             // Map empty string to null string (see comment above).
    492             guidToVersionMap().set(m_guid, currentVersion.isEmpty() ? String() : currentVersion.copy());
     500            updateGuidVersionMap(m_guid, currentVersion);
    493501        }
    494502    }
    … …  
    634642{
    635643    m_expectedVersion = version.copy();
     644    // Update the in memory database version map.
     645    MutexLocker locker(guidMutex());
     646    updateGuidVersionMap(m_guid, version);
    636647}
    637648
Note: See TracChangeset for help on using the changeset viewer.