⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 136194 in webkit


Ignore:
Timestamp:
Nov 29, 2012, 6:22:20 PM (14 years ago)
Author:
dgrogan@chromium.org
Message:

IndexedDB: Propagate more leveldb errors to script
​https://bugs.webkit.org/show_bug.cgi?id=103580

Reviewed by Tony Chang.

Source/WebCore:

LevelDBDatabase used a single return value to indicate both I/O problems
and a missing key. Now an out variable is used to indicate if the
requested key was found. The return value is used to report corruption
or disk error.

This is a small step toward propagating low level errors everywhere
possible. So far only one scenario will newly cause script to receive
an error: when leveldb has trouble looking for existing keys during an
objectstore->add.

  • Modules/indexeddb/IDBBackingStore.cpp:

(WebCore::getInt):
(WebCore::getVarInt):
(WebCore::getString):
(WebCore::IDBBackingStore::getKeyGeneratorCurrentNumber):
(WebCore::IDBBackingStore::maybeUpdateKeyGeneratorCurrentNumber):
(WebCore::IDBBackingStore::keyExistsInObjectStore):

  • Modules/indexeddb/IDBBackingStore.h:

(IDBBackingStore):

  • Modules/indexeddb/IDBObjectStoreBackendImpl.cpp:

(WebCore::IDBObjectStoreBackendImpl::setIndexKeys):
(WebCore::IDBObjectStoreBackendImpl::putInternal):
(WebCore::IDBObjectStoreBackendImpl::generateKey):
(WebCore::IDBObjectStoreBackendImpl::updateKeyGenerator):

  • Modules/indexeddb/IDBObjectStoreBackendImpl.h:

(IDBObjectStoreBackendImpl):

  • platform/leveldb/LevelDBDatabase.cpp:

(WebCore::LevelDBDatabase::safeGet):

  • platform/leveldb/LevelDBDatabase.h:

(LevelDBDatabase):

  • platform/leveldb/LevelDBTransaction.cpp:

(WebCore::LevelDBTransaction::safeGet):
(WebCore):
(WebCore::LevelDBTransaction::get):

  • platform/leveldb/LevelDBTransaction.h:

(LevelDBTransaction):

Source/WebKit/chromium:

  • tests/IDBFakeBackingStore.h: Update method signatures.
Location:
trunk/Source
Files:
12 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r136191 r136194  
     12012-11-29  David Grogan  <dgrogan@chromium.org>
     2
     3        IndexedDB: Propagate more leveldb errors to script
     4        https://bugs.webkit.org/show_bug.cgi?id=103580
     5
     6        Reviewed by Tony Chang.
     7
     8        LevelDBDatabase used a single return value to indicate both I/O problems
     9        and a missing key. Now an out variable is used to indicate if the
     10        requested key was found. The return value is used to report corruption
     11        or disk error.
     12
     13        This is a small step toward propagating low level errors everywhere
     14        possible. So far only one scenario will newly cause script to receive
     15        an error: when leveldb has trouble looking for existing keys during an
     16        objectstore->add.
     17
     18        * Modules/indexeddb/IDBBackingStore.cpp:
     19        (WebCore::getInt):
     20        (WebCore::getVarInt):
     21        (WebCore::getString):
     22        (WebCore::IDBBackingStore::getKeyGeneratorCurrentNumber):
     23        (WebCore::IDBBackingStore::maybeUpdateKeyGeneratorCurrentNumber):
     24        (WebCore::IDBBackingStore::keyExistsInObjectStore):
     25        * Modules/indexeddb/IDBBackingStore.h:
     26        (IDBBackingStore):
     27        * Modules/indexeddb/IDBObjectStoreBackendImpl.cpp:
     28        (WebCore::IDBObjectStoreBackendImpl::setIndexKeys):
     29        (WebCore::IDBObjectStoreBackendImpl::putInternal):
     30        (WebCore::IDBObjectStoreBackendImpl::generateKey):
     31        (WebCore::IDBObjectStoreBackendImpl::updateKeyGenerator):
     32        * Modules/indexeddb/IDBObjectStoreBackendImpl.h:
     33        (IDBObjectStoreBackendImpl):
     34        * platform/leveldb/LevelDBDatabase.cpp:
     35        (WebCore::LevelDBDatabase::safeGet):
     36        * platform/leveldb/LevelDBDatabase.h:
     37        (LevelDBDatabase):
     38        * platform/leveldb/LevelDBTransaction.cpp:
     39        (WebCore::LevelDBTransaction::safeGet):
     40        (WebCore):
     41        (WebCore::LevelDBTransaction::get):
     42        * platform/leveldb/LevelDBTransaction.h:
     43        (LevelDBTransaction):
     44
    1452012-11-29  Sheriff Bot  <webkit.review.bot@gmail.com>
    246

  • trunk/Source/WebCore/Modules/indexeddb/IDBBackingStore.cpp

    r135177 r136194  
    9292{
    9393    Vector<char> result;
    94     if (!db->get(key, result))
     94    bool found = false;
     95    bool ok = db->safeGet(key, result, found);
     96    // FIXME: Notify the caller if !ok.
     97    ASSERT_UNUSED(ok, ok);
     98    if (!found)
    9599        return false;
    96100
    … …  
    109113{
    110114    Vector<char> result;
    111     if (!db->get(key, result))
     115    bool found = false;
     116    bool ok = db->safeGet(key, result, found);
     117    // FIXME: Notify the caller if !ok.
     118    ASSERT_UNUSED(ok, ok);
     119    if (!found)
    112120        return false;
    113121
    … …  
    124132{
    125133    Vector<char> result;
    126     if (!db->get(key, result))
     134    bool found = false;
     135    bool ok = db->safeGet(key, result, found);
     136    // FIXME: Notify the caller if !ok.
     137    ASSERT_UNUSED(ok, ok);
     138    if (!found)
    127139        return false;
    128140
    … …  
    700712
    701713
    702 int64_t IDBBackingStore::getKeyGeneratorCurrentNumber(IDBBackingStore::Transaction* transaction, int64_t databaseId, int64_t objectStoreId)
     714bool IDBBackingStore::getKeyGeneratorCurrentNumber(IDBBackingStore::Transaction* transaction, int64_t databaseId, int64_t objectStoreId, int64_t& keyGeneratorCurrentNumber)
    703715{
    704716    LevelDBTransaction* levelDBTransaction = IDBBackingStore::Transaction::levelDBTransactionFrom(transaction);
    … …  
    706718    const Vector<char> keyGeneratorCurrentNumberKey = ObjectStoreMetaDataKey::encode(databaseId, objectStoreId, ObjectStoreMetaDataKey::KeyGeneratorCurrentNumber);
    707719
    708     int64_t keyGeneratorCurrentNumber = -1;
     720    keyGeneratorCurrentNumber = -1;
    709721    Vector<char> data;
    710722
    711     if (levelDBTransaction->get(keyGeneratorCurrentNumberKey, data))
     723    bool found = false;
     724    bool ok = levelDBTransaction->safeGet(keyGeneratorCurrentNumberKey, data, found);
     725    if (!ok) {
     726        InternalError(IDBLevelDBBackingStoreReadError);
     727        return false;
     728    }
     729    if (found)
    712730        keyGeneratorCurrentNumber = decodeInt(data.begin(), data.end());
    713731    else {
    … …  
    742760}
    743761
    744 void IDBBackingStore::maybeUpdateKeyGeneratorCurrentNumber(IDBBackingStore::Transaction* transaction, int64_t databaseId, int64_t objectStoreId, int64_t newNumber, bool checkCurrent)
     762bool IDBBackingStore::maybeUpdateKeyGeneratorCurrentNumber(IDBBackingStore::Transaction* transaction, int64_t databaseId, int64_t objectStoreId, int64_t newNumber, bool checkCurrent)
    745763{
    746764    LevelDBTransaction* levelDBTransaction = IDBBackingStore::Transaction::levelDBTransactionFrom(transaction);
    747765
    748766    if (checkCurrent) {
    749         int64_t currentNumber = getKeyGeneratorCurrentNumber(transaction, databaseId, objectStoreId);
     767        int64_t currentNumber;
     768        bool ok = getKeyGeneratorCurrentNumber(transaction, databaseId, objectStoreId, currentNumber);
     769        if (!ok)
     770            return false;
    750771        if (newNumber <= currentNumber)
    751             return;
     772            return true;
    752773    }
    753774
    754775    const Vector<char> keyGeneratorCurrentNumberKey = ObjectStoreMetaDataKey::encode(databaseId, objectStoreId, ObjectStoreMetaDataKey::KeyGeneratorCurrentNumber);
    755776    putInt(levelDBTransaction, keyGeneratorCurrentNumberKey, newNumber);
    756 }
    757 
    758 bool IDBBackingStore::keyExistsInObjectStore(IDBBackingStore::Transaction* transaction, int64_t databaseId, int64_t objectStoreId, const IDBKey& key, RecordIdentifier* foundRecordIdentifier)
     777    return true;
     778}
     779
     780bool IDBBackingStore::keyExistsInObjectStore(IDBBackingStore::Transaction* transaction, int64_t databaseId, int64_t objectStoreId, const IDBKey& key, RecordIdentifier* foundRecordIdentifier, bool& found)
    759781{
    760782    IDB_TRACE("IDBBackingStore::keyExistsInObjectStore");
     783    found = false;
    761784    LevelDBTransaction* levelDBTransaction = IDBBackingStore::Transaction::levelDBTransactionFrom(transaction);
    762785    const Vector<char> leveldbKey = ObjectStoreDataKey::encode(databaseId, objectStoreId, key);
    763786    Vector<char> data;
    764787
    765     if (!levelDBTransaction->get(leveldbKey, data))
    766         return false;
     788    bool ok = levelDBTransaction->safeGet(leveldbKey, data, found);
     789    if (!ok) {
     790        InternalError(IDBLevelDBBackingStoreReadError);
     791        return false;
     792    }
     793    if (!found)
     794        return true;
    767795
    768796    int64_t version;

  • trunk/Source/WebCore/Modules/indexeddb/IDBBackingStore.h

    r136079 r136194  
    8282    virtual void clearObjectStore(IDBBackingStore::Transaction*, int64_t databaseId, int64_t objectStoreId);
    8383    virtual void deleteRecord(IDBBackingStore::Transaction*, int64_t databaseId, int64_t objectStoreId, const RecordIdentifier&);
    84     virtual int64_t getKeyGeneratorCurrentNumber(IDBBackingStore::Transaction*, int64_t databaseId, int64_t objectStoreId);
    85     virtual void maybeUpdateKeyGeneratorCurrentNumber(IDBBackingStore::Transaction*, int64_t databaseId, int64_t objectStoreId, int64_t newState, bool checkCurrent);
    86     virtual bool keyExistsInObjectStore(IDBBackingStore::Transaction*, int64_t databaseId, int64_t objectStoreId, const IDBKey&, RecordIdentifier* foundRecordIdentifier);
     84    virtual bool getKeyGeneratorCurrentNumber(IDBBackingStore::Transaction*, int64_t databaseId, int64_t objectStoreId, int64_t& currentNumber) WARN_UNUSED_RETURN;
     85    virtual bool maybeUpdateKeyGeneratorCurrentNumber(IDBBackingStore::Transaction*, int64_t databaseId, int64_t objectStoreId, int64_t newState, bool checkCurrent) WARN_UNUSED_RETURN;
     86    virtual bool keyExistsInObjectStore(IDBBackingStore::Transaction*, int64_t databaseId, int64_t objectStoreId, const IDBKey&, RecordIdentifier* foundRecordIdentifier, bool& found) WARN_UNUSED_RETURN;
    8787
    8888    virtual Vector<IDBIndexMetadata> getIndexes(int64_t databaseId, int64_t objectStoreId);

  • trunk/Source/WebCore/Modules/indexeddb/IDBObjectStoreBackendImpl.cpp

    r136084 r136194  
    232232    // FIXME: This method could be asynchronous, but we need to evaluate if it's worth the extra complexity.
    233233    IDBBackingStore::RecordIdentifier recordIdentifier;
    234     if (!backingStore()->keyExistsInObjectStore(transaction->backingStoreTransaction(), databaseId(), id(), *primaryKey, &recordIdentifier)) {
     234    bool found = false;
     235    bool ok = backingStore()->keyExistsInObjectStore(transaction->backingStoreTransaction(), databaseId(), id(), *primaryKey, &recordIdentifier, found);
     236    if (!ok) {
     237        LOG_ERROR("keyExistsInObjectStore reported an error");
     238        transaction->abort(IDBDatabaseError::create(IDBDatabaseException::UNKNOWN_ERR, "Internal error setting index keys."));
     239        return;
     240    }
     241    if (!found) {
    235242        transaction->abort();
    236243        return;
    … …  
    301308
    302309    IDBBackingStore::RecordIdentifier recordIdentifier;
    303     if (putMode == AddOnly && objectStore->backingStore()->keyExistsInObjectStore(transaction->backingStoreTransaction(), objectStore->databaseId(), objectStore->id(), *key, &recordIdentifier)) {
    304         callbacks->onError(IDBDatabaseError::create(IDBDatabaseException::CONSTRAINT_ERR, "Key already exists in the object store."));
    305         return;
     310    if (putMode == AddOnly) {
     311        bool found = false;
     312        bool ok = objectStore->backingStore()->keyExistsInObjectStore(transaction->backingStoreTransaction(), objectStore->databaseId(), objectStore->id(), *key, &recordIdentifier, found);
     313        if (!ok) {
     314            callbacks->onError(IDBDatabaseError::create(IDBDatabaseException::UNKNOWN_ERR, "Internal error checking key existence."));
     315            return;
     316        }
     317        if (found) {
     318            callbacks->onError(IDBDatabaseError::create(IDBDatabaseException::CONSTRAINT_ERR, "Key already exists in the object store."));
     319            return;
     320        }
    306321    }
    307322
    … …  
    322337    }
    323338
    324     if (autoIncrement && putMode != CursorUpdate && key->type() == IDBKey::NumberType)
    325         objectStore->updateKeyGenerator(transaction, key.get(), !keyWasGenerated);
     339    if (autoIncrement && putMode != CursorUpdate && key->type() == IDBKey::NumberType) {
     340        bool ok = objectStore->updateKeyGenerator(transaction, key.get(), !keyWasGenerated);
     341        if (!ok) {
     342            callbacks->onError(IDBDatabaseError::create(IDBDatabaseException::UNKNOWN_ERR, "Internal error updating key generator."));
     343            return;
     344        }
     345    }
    326346
    327347    callbacks->onSuccess(key.release());
    … …  
    526546{
    527547    const int64_t maxGeneratorValue = 9007199254740992LL; // Maximum integer storable as ECMAScript number.
    528     int64_t currentNumber = backingStore()->getKeyGeneratorCurrentNumber(transaction->backingStoreTransaction(), databaseId(), id());
     548    int64_t currentNumber;
     549    bool ok = backingStore()->getKeyGeneratorCurrentNumber(transaction->backingStoreTransaction(), databaseId(), id(), currentNumber);
     550    if (!ok) {
     551        LOG_ERROR("Failed to getKeyGeneratorCurrentNumber");
     552        return IDBKey::createInvalid();
     553    }
    529554    if (currentNumber < 0 || currentNumber > maxGeneratorValue)
    530555        return IDBKey::createInvalid();
    … …  
    533558}
    534559
    535 void IDBObjectStoreBackendImpl::updateKeyGenerator(PassRefPtr<IDBTransactionBackendImpl> transaction, const IDBKey* key, bool checkCurrent)
     560bool IDBObjectStoreBackendImpl::updateKeyGenerator(PassRefPtr<IDBTransactionBackendImpl> transaction, const IDBKey* key, bool checkCurrent)
    536561{
    537562    ASSERT(key && key->type() == IDBKey::NumberType);
    538     backingStore()->maybeUpdateKeyGeneratorCurrentNumber(transaction->backingStoreTransaction(), databaseId(), id(), static_cast<int64_t>(floor(key->number())) + 1, checkCurrent);
     563    return backingStore()->maybeUpdateKeyGeneratorCurrentNumber(transaction->backingStoreTransaction(), databaseId(), id(), static_cast<int64_t>(floor(key->number())) + 1, checkCurrent);
    539564}
    540565

  • trunk/Source/WebCore/Modules/indexeddb/IDBObjectStoreBackendImpl.h

    r134095 r136194  
    9797    void loadIndexes();
    9898    PassRefPtr<IDBKey> generateKey(PassRefPtr<IDBTransactionBackendImpl>);
    99     void updateKeyGenerator(PassRefPtr<IDBTransactionBackendImpl>, const IDBKey*, bool checkCurrent);
     99    bool updateKeyGenerator(PassRefPtr<IDBTransactionBackendImpl>, const IDBKey*, bool checkCurrent);
    100100
    101101    static void getInternal(ScriptExecutionContext*, PassRefPtr<IDBObjectStoreBackendImpl>, PassRefPtr<IDBKeyRange>, PassRefPtr<IDBCallbacks>, PassRefPtr<IDBTransactionBackendImpl>);

  • trunk/Source/WebCore/platform/leveldb/LevelDBDatabase.cpp

    r133963 r136194  
    199199}
    200200
    201 bool LevelDBDatabase::get(const LevelDBSlice& key, Vector<char>& value, const LevelDBSnapshot* snapshot)
    202 {
     201bool LevelDBDatabase::safeGet(const LevelDBSlice& key, Vector<char>& value, bool& found, const LevelDBSnapshot* snapshot)
     202{
     203    found = false;
    203204    std::string result;
    204205    leveldb::ReadOptions readOptions;
    … …  
    208209    const leveldb::Status s = m_db->Get(readOptions, makeSlice(key), &result);
    209210    if (s.ok()) {
     211        found = true;
    210212        value = makeVector(result);
    211213        return true;
    212214    }
    213215    if (s.IsNotFound())
    214         return false;
     216        return true;
    215217    LOG_ERROR("LevelDB get failed: %s", s.ToString().c_str());
    216218    return false;

  • trunk/Source/WebCore/platform/leveldb/LevelDBDatabase.h

    r133963 r136194  
    7070    bool put(const LevelDBSlice& key, const Vector<char>& value);
    7171    bool remove(const LevelDBSlice& key);
    72     bool get(const LevelDBSlice& key, Vector<char>& value, const LevelDBSnapshot* = 0);
     72    bool safeGet(const LevelDBSlice& key, Vector<char>& value, bool& found, const LevelDBSnapshot* = 0);
    7373    bool write(LevelDBWriteBatch&);
    7474    PassOwnPtr<LevelDBIterator> createIterator(const LevelDBSnapshot* = 0);

  • trunk/Source/WebCore/platform/leveldb/LevelDBTransaction.cpp

    r133963 r136194  
    108108}
    109109
    110 bool LevelDBTransaction::get(const LevelDBSlice& key, Vector<char>& value)
    111 {
     110bool LevelDBTransaction::safeGet(const LevelDBSlice& key, Vector<char>& value, bool& found)
     111{
     112    found = false;
    112113    ASSERT(!m_finished);
    113114    AVLTreeNode* node = m_tree.search(key);
    … …  
    115116    if (node) {
    116117        if (node->deleted)
    117             return false;
     118            return true;
    118119
    119120        value = node->value;
     121        found = true;
    120122        return true;
    121123    }
    122124
    123     return m_db->get(key, value, &m_snapshot);
     125    bool ok = m_db->safeGet(key, value, found, &m_snapshot);
     126    if (!ok) {
     127        ASSERT(!found);
     128        return false;
     129    }
     130    return true;
     131}
     132
     133bool LevelDBTransaction::get(const LevelDBSlice& key, Vector<char>& value)
     134{
     135    bool found = false;
     136    bool ok = safeGet(key, value, found);
     137    if (!ok) {
     138        ASSERT(!found);
     139        ASSERT_NOT_REACHED();
     140    }
     141    return ok && found;
    124142}
    125143

  • trunk/Source/WebCore/platform/leveldb/LevelDBTransaction.h

    r133963 r136194  
    5555    void put(const LevelDBSlice& key, const Vector<char>& value);
    5656    void remove(const LevelDBSlice& key);
     57    bool safeGet(const LevelDBSlice& key, Vector<char>& value, bool& found);
     58    // FIXME: Convert all callers of get to safeGet then remove get.
    5759    bool get(const LevelDBSlice& key, Vector<char>& value);
    5860    bool commit();

  • trunk/Source/WebKit/chromium/ChangeLog

    r136191 r136194  
     12012-11-29  David Grogan  <dgrogan@chromium.org>
     2
     3        IndexedDB: Propagate more leveldb errors to script
     4        https://bugs.webkit.org/show_bug.cgi?id=103580
     5
     6        Reviewed by Tony Chang.
     7
     8        * tests/IDBFakeBackingStore.h:
     9         Update method signatures.
     10
    1112012-11-29  Sheriff Bot  <webkit.review.bot@gmail.com>
    212

  • trunk/Source/WebKit/chromium/tests/IDBFakeBackingStore.h

    r135177 r136194  
    4848    virtual void clearObjectStore(Transaction*, int64_t databaseId, int64_t objectStoreId) OVERRIDE { }
    4949    virtual void deleteRecord(Transaction*, int64_t databaseId, int64_t objectStoreId, const RecordIdentifier&) OVERRIDE { }
    50     virtual int64_t getKeyGeneratorCurrentNumber(Transaction*, int64_t databaseId, int64_t objectStoreId) OVERRIDE { return 0; }
    51     virtual void maybeUpdateKeyGeneratorCurrentNumber(Transaction*, int64_t databaseId, int64_t objectStoreId, int64_t newNumber, bool checkCurrent) OVERRIDE { }
    52     virtual bool keyExistsInObjectStore(Transaction*, int64_t databaseId, int64_t objectStoreId, const IDBKey&, RecordIdentifier* foundRecordIdentifier) OVERRIDE { return false; }
     50    virtual bool getKeyGeneratorCurrentNumber(Transaction*, int64_t databaseId, int64_t objectStoreId, int64_t& currentNumber) OVERRIDE { return true; }
     51    virtual bool maybeUpdateKeyGeneratorCurrentNumber(Transaction*, int64_t databaseId, int64_t objectStoreId, int64_t newNumber, bool checkCurrent) OVERRIDE { return true; }
     52    virtual bool keyExistsInObjectStore(Transaction*, int64_t databaseId, int64_t objectStoreId, const IDBKey&, RecordIdentifier* foundRecordIdentifier, bool& found) OVERRIDE { return true; }
    5353
    5454    virtual Vector<IDBIndexMetadata> getIndexes(int64_t databaseId, int64_t objectStoreId) OVERRIDE { return Vector<IDBIndexMetadata>(); }

  • trunk/Source/WebKit/chromium/tests/LevelDBTest.cpp

    r133963 r136194  
    8080    leveldb = LevelDBDatabase::open(path, &comparator);
    8181    EXPECT_TRUE(leveldb);
    82     success = leveldb->get(key, gotValue);
    83     EXPECT_TRUE(success);
     82    bool found = false;
     83    success = leveldb->safeGet(key, gotValue, found);
     84    EXPECT_TRUE(success);
     85    EXPECT_TRUE(found);
    8486    EXPECT_EQ(putValue, gotValue);
    8587    leveldb.release();
    … …  
    99101    leveldb = LevelDBDatabase::open(path, &comparator);
    100102    EXPECT_TRUE(leveldb);
    101     success = leveldb->get(key, gotValue);
    102     EXPECT_FALSE(success);
     103    success = leveldb->safeGet(key, gotValue, found);
     104    EXPECT_TRUE(success);
     105    EXPECT_FALSE(found);
    103106}
    104107
    … …  
    130133    EXPECT_EQ(comparator.compare(gotValue, oldValue), 0);
    131134
    132     success = leveldb->get(key, gotValue);
    133     EXPECT_TRUE(success);
     135    bool found = false;
     136    success = leveldb->safeGet(key, gotValue, found);
     137    EXPECT_TRUE(success);
     138    EXPECT_TRUE(found);
    134139    EXPECT_EQ(comparator.compare(gotValue, newValue), 0);
    135140
    … …  
    139144    EXPECT_TRUE(success);
    140145
    141     success = leveldb->get(addedKey, gotValue);
    142     EXPECT_TRUE(success);
     146    success = leveldb->safeGet(addedKey, gotValue, found);
     147    EXPECT_TRUE(success);
     148    EXPECT_TRUE(found);
    143149    EXPECT_EQ(comparator.compare(gotValue, addedValue), 0);
    144150
Note: See TracChangeset for help on using the changeset viewer.