Changeset 85122 in webkit


Ignore:
Timestamp:
Apr 27, 2011 4:52:09 PM (13 years ago)
Author:
abarth@webkit.org
Message:

2011-04-27 Adam Barth <​abarth@webkit.org>

Reviewed by David Levin.

Fix OwnPtr issues in IndexedDB
​https://bugs.webkit.org/show_bug.cgi?id=59656

I didn't do an exhaustive review of this code, but I fixed the problems
caught by turning on strict OwnPtr and all their antecedents. This
patch is entirely tighter bookkeeping. There shouldn't be any actual
behavior change.

  • platform/leveldb/LevelDBDatabase.cpp: (WebCore::LevelDBDatabase::LevelDBDatabase): (WebCore::LevelDBDatabase::open): (WebCore::LevelDBDatabase::createIterator):
  • platform/leveldb/LevelDBDatabase.h:
  • platform/leveldb/LevelDBIterator.cpp: (WebCore::LevelDBIterator::LevelDBIterator):
  • platform/leveldb/LevelDBIterator.h:
  • storage/IDBLevelDBBackingStore.cpp: (WebCore::IDBLevelDBBackingStore::IDBLevelDBBackingStore): (WebCore::IDBLevelDBBackingStore::open): (WebCore::getNewDatabaseId): (WebCore::IDBLevelDBBackingStore::getObjectStores): (WebCore::getNewObjectStoreId): (WebCore::deleteRange): (WebCore::IDBLevelDBBackingStore::nextAutoIncrementNumber): (WebCore::IDBLevelDBBackingStore::forEachObjectStoreRecord): (WebCore::IDBLevelDBBackingStore::getIndexes): (WebCore::getNewIndexId): (WebCore::findGreatestKeyLessThan): (WebCore::IDBLevelDBBackingStore::getPrimaryKeyViaIndex): (WebCore::IDBLevelDBBackingStore::keyExistsInIndex): (WebCore::findLastIndexKeyEqualTo):
  • storage/IDBLevelDBBackingStore.h:
Location:
trunk/Source/WebCore
Files:
7 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r85118 r85122  
     12011-04-27  Adam Barth  <abarth@webkit.org>
     2
     3        Reviewed by David Levin.
     4
     5        Fix OwnPtr issues in IndexedDB
     6        https://bugs.webkit.org/show_bug.cgi?id=59656
     7
     8        I didn't do an exhaustive review of this code, but I fixed the problems
     9        caught by turning on strict OwnPtr and all their antecedents.  This
     10        patch is entirely tighter bookkeeping.  There shouldn't be any actual
     11        behavior change.
     12
     13        * platform/leveldb/LevelDBDatabase.cpp:
     14        (WebCore::LevelDBDatabase::LevelDBDatabase):
     15        (WebCore::LevelDBDatabase::open):
     16        (WebCore::LevelDBDatabase::createIterator):
     17        * platform/leveldb/LevelDBDatabase.h:
     18        * platform/leveldb/LevelDBIterator.cpp:
     19        (WebCore::LevelDBIterator::LevelDBIterator):
     20        * platform/leveldb/LevelDBIterator.h:
     21        * storage/IDBLevelDBBackingStore.cpp:
     22        (WebCore::IDBLevelDBBackingStore::IDBLevelDBBackingStore):
     23        (WebCore::IDBLevelDBBackingStore::open):
     24        (WebCore::getNewDatabaseId):
     25        (WebCore::IDBLevelDBBackingStore::getObjectStores):
     26        (WebCore::getNewObjectStoreId):
     27        (WebCore::deleteRange):
     28        (WebCore::IDBLevelDBBackingStore::nextAutoIncrementNumber):
     29        (WebCore::IDBLevelDBBackingStore::forEachObjectStoreRecord):
     30        (WebCore::IDBLevelDBBackingStore::getIndexes):
     31        (WebCore::getNewIndexId):
     32        (WebCore::findGreatestKeyLessThan):
     33        (WebCore::IDBLevelDBBackingStore::getPrimaryKeyViaIndex):
     34        (WebCore::IDBLevelDBBackingStore::keyExistsInIndex):
     35        (WebCore::findLastIndexKeyEqualTo):
     36        * storage/IDBLevelDBBackingStore.h:
     37
    1382011-04-19  MORITA Hajime  <morrita@google.com>
    239

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

    r84149 r85122  
    8989
    9090LevelDBDatabase::LevelDBDatabase()
    91     : m_db(0)
    9291{
    9392}
    … …  
    9796}
    9897
    99 LevelDBDatabase* LevelDBDatabase::open(const String& fileName, const LevelDBComparator* comparator)
     98PassOwnPtr<LevelDBDatabase> LevelDBDatabase::open(const String& fileName, const LevelDBComparator* comparator)
    10099{
    101     OwnPtr<ComparatorAdapter> comparatorAdapter(new ComparatorAdapter(comparator));
     100    OwnPtr<ComparatorAdapter> comparatorAdapter = adoptPtr(new ComparatorAdapter(comparator));
    102101
    103     LevelDBDatabase* result = new LevelDBDatabase();
     102    OwnPtr<LevelDBDatabase> result = adoptPtr(new LevelDBDatabase);
    104103
    105104    leveldb::Options options;
    … …  
    109108    leveldb::Status s = leveldb::DB::Open(options, fileName.utf8().data(), &db);
    110109
    111     if (!s.ok()) {
    112         delete result;
    113         return 0;
    114     }
     110    if (!s.ok())
     111        return PassOwnPtr<LevelDBDatabase>();
    115112
    116     result->m_db = WTF::adoptPtr(db);
     113    result->m_db = adoptPtr(db);
    117114    result->m_comparatorAdapter = comparatorAdapter.release();
    118115
    119     return result;
     116    return result.release();
    120117}
    121118
    … …  
    147144}
    148145
    149 LevelDBIterator* LevelDBDatabase::newIterator()
     146PassOwnPtr<LevelDBIterator> LevelDBDatabase::createIterator()
    150147{
    151     leveldb::Iterator* i = m_db->NewIterator(leveldb::ReadOptions());
     148    OwnPtr<leveldb::Iterator> i = adoptPtr(m_db->NewIterator(leveldb::ReadOptions()));
    152149    if (!i) // FIXME: Double check if we actually need to check this.
    153150        return 0;
    154     return new LevelDBIterator(i);
     151    return adoptPtr(new LevelDBIterator(i.release()));
    155152}
    156153
    157 } // namespace WebCore
     154}
    158155
    159 #endif // ENABLE(LEVELDB)
     156#endif

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

    r84149 r85122  
    2929#if ENABLE(LEVELDB)
    3030
    31 #include "PlatformString.h"
    32 #include <OwnPtr.h>
    33 #include <Vector.h>
     31#include <wtf/OwnPtr.h>
     32#include <wtf/PassOwnPtr.h>
     33#include <wtf/Vector.h>
     34#include <wtf/text/WTFString.h>
    3435
    3536namespace leveldb {
    … …  
    4647class LevelDBDatabase {
    4748public:
    48     static LevelDBDatabase* open(const String& fileName, const LevelDBComparator*);
     49    static PassOwnPtr<LevelDBDatabase> open(const String& fileName, const LevelDBComparator*);
    4950    ~LevelDBDatabase();
    5051
    … …  
    5253    bool remove(const LevelDBSlice& key);
    5354    bool get(const LevelDBSlice& key, Vector<char>& value);
    54     LevelDBIterator* newIterator();
     55    PassOwnPtr<LevelDBIterator> createIterator();
    5556
    5657private:
    … …  
    6162};
    6263
    63 } // namespace WebCore
     64}
    6465
    65 #endif // ENABLE(LEVELDB)
    66 #endif // LevelDBDatabase_h
     66#endif
     67#endif

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

    r84149 r85122  
    3131#include <leveldb/iterator.h>
    3232#include <leveldb/slice.h>
     33#include <wtf/PassOwnPtr.h>
    3334#include <wtf/text/CString.h>
    3435#include <wtf/text/WTFString.h>
    … …  
    4041}
    4142
    42 LevelDBIterator::LevelDBIterator(leveldb::Iterator* it)
     43LevelDBIterator::LevelDBIterator(PassOwnPtr<leveldb::Iterator> it)
    4344    : m_iterator(it)
    4445{

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

    r84149 r85122  
    5353
    5454private:
    55     LevelDBIterator(leveldb::Iterator*);
     55    LevelDBIterator(PassOwnPtr<leveldb::Iterator>);
    5656    friend class LevelDBDatabase;
    5757

  • trunk/Source/WebCore/storage/IDBLevelDBBackingStore.cpp

    r85046 r85122  
    116116}
    117117
    118 IDBLevelDBBackingStore::IDBLevelDBBackingStore(String identifier, IDBFactoryBackendImpl* factory, LevelDBDatabase* db)
     118IDBLevelDBBackingStore::IDBLevelDBBackingStore(String identifier, IDBFactoryBackendImpl* factory, PassOwnPtr<LevelDBDatabase> db)
    119119    : m_identifier(identifier)
    120120    , m_factory(factory)
    … …  
    135135    if (pathBase.isEmpty()) {
    136136        ASSERT_NOT_REACHED(); // FIXME: We need to handle this case for incognito and DumpRenderTree.
    137         return 0;
     137        return PassRefPtr<IDBBackingStore>();
    138138    }
    139139
    140140    if (!makeAllDirectories(pathBase)) {
    141141        LOG_ERROR("Unable to create IndexedDB database path %s", pathBase.utf8().data());
    142         return 0;
     142        return PassRefPtr<IDBBackingStore>();
    143143    }
    144144    // FIXME: We should eventually use the same LevelDB database for all origins.
    145145    String path = pathByAppendingComponent(pathBase, securityOrigin->databaseIdentifier() + ".indexeddb.leveldb");
    146146
    147     OwnPtr<LevelDBComparator> comparator(new Comparator());
    148     LevelDBDatabase* db = LevelDBDatabase::open(path, comparator.get());
     147    OwnPtr<LevelDBComparator> comparator = adoptPtr(new Comparator());
     148    OwnPtr<LevelDBDatabase> db = LevelDBDatabase::open(path, comparator.get());
    149149    if (!db)
    150         return 0;
     150        return PassRefPtr<IDBBackingStore>();
    151151
    152152    // FIXME: Handle comparator name changes.
    153153
    154     RefPtr<IDBLevelDBBackingStore> backingStore(adoptRef(new IDBLevelDBBackingStore(fileIdentifier, factory, db)));
     154    RefPtr<IDBLevelDBBackingStore> backingStore(adoptRef(new IDBLevelDBBackingStore(fileIdentifier, factory, db.release())));
    155155    backingStore->m_comparator = comparator.release();
    156156
    157157    if (!setUpMetadata(backingStore->m_db.get()))
    158         return 0;
     158        return PassRefPtr<IDBBackingStore>();
    159159
    160160    return backingStore.release();
    … …  
    181181    const Vector<char> freeListStopKey = DatabaseFreeListKey::encode(INT64_MAX);
    182182
    183     OwnPtr<LevelDBIterator> it(db->newIterator());
     183    OwnPtr<LevelDBIterator> it = db->createIterator();
    184184    for (it->seek(freeListStartKey); it->isValid() && compareKeys(it->key(), freeListStopKey) < 0; it->next()) {
    185185        const char *p = it->key().begin();
    … …  
    232232    const Vector<char> stopKey = ObjectStoreMetaDataKey::encode(databaseId, INT64_MAX, 0);
    233233
    234     OwnPtr<LevelDBIterator> it(m_db->newIterator());
     234    OwnPtr<LevelDBIterator> it = m_db->createIterator();
    235235    for (it->seek(startKey); it->isValid() && compareKeys(it->key(), stopKey) < 0; it->next()) {
    236236        const char *p = it->key().begin();
    … …  
    289289    const Vector<char> freeListStopKey = ObjectStoreFreeListKey::encode(databaseId, INT64_MAX);
    290290
    291     OwnPtr<LevelDBIterator> it(db->newIterator());
     291    OwnPtr<LevelDBIterator> it = db->createIterator();
    292292    for (it->seek(freeListStartKey); it->isValid() && compareKeys(it->key(), freeListStopKey) < 0; it->next()) {
    293293        const char* p = it->key().begin();
    … …  
    380380{
    381381    // FIXME: LevelDB may be able to provide a bulk operation that we can do first.
    382     OwnPtr<LevelDBIterator> it(db->newIterator());
     382    OwnPtr<LevelDBIterator> it = db->createIterator();
    383383    for (it->seek(begin); it->isValid() && compareKeys(it->key(), end) < 0; it->next()) {
    384384        if (!db->remove(it->key()))
    … …  
    512512    const Vector<char> stopKey = ObjectStoreDataKey::encode(databaseId, objectStoreId, maxIDBKey());
    513513
    514     OwnPtr<LevelDBIterator> it(m_db->newIterator());
     514    OwnPtr<LevelDBIterator> it = m_db->createIterator();
    515515
    516516    int maxNumericKey = 0;
    … …  
    559559    const Vector<char> stopKey = ObjectStoreDataKey::encode(databaseId, objectStoreId, maxIDBKey());
    560560
    561     OwnPtr<LevelDBIterator> it(m_db->newIterator());
     561    OwnPtr<LevelDBIterator> it = m_db->createIterator();
    562562    for (it->seek(startKey); it->isValid() && compareKeys(it->key(), stopKey) < 0; it->next()) {
    563563        const char *p = it->key().begin();
    … …  
    588588    const Vector<char> stopKey = IndexMetaDataKey::encode(databaseId, objectStoreId + 1, 0, 0);
    589589
    590     OwnPtr<LevelDBIterator> it(m_db->newIterator());
     590    OwnPtr<LevelDBIterator> it = m_db->createIterator();
    591591    for (it->seek(startKey); it->isValid() && compareKeys(it->key(), stopKey) < 0; it->next()) {
    592592        const char* p = it->key().begin();
    … …  
    628628    const Vector<char> stopKey = IndexFreeListKey::encode(databaseId, objectStoreId, INT64_MAX);
    629629
    630     OwnPtr<LevelDBIterator> it(db->newIterator());
     630    OwnPtr<LevelDBIterator> it = db->createIterator();
    631631    for (it->seek(startKey); it->isValid() && compareKeys(it->key(), stopKey) < 0; it->next()) {
    632632        const char* p = it->key().begin();
    … …  
    711711static bool findGreatestKeyLessThan(LevelDBDatabase* db, const Vector<char>& target, Vector<char>& foundKey)
    712712{
    713     OwnPtr<LevelDBIterator> it(db->newIterator());
     713    OwnPtr<LevelDBIterator> it = db->createIterator();
    714714    it->seek(target);
    715715
    … …  
    760760{
    761761    const Vector<char> leveldbKey = IndexDataKey::encode(databaseId, objectStoreId, indexId, key, 0);
    762     OwnPtr<LevelDBIterator> it(m_db->newIterator());
     762    OwnPtr<LevelDBIterator> it = m_db->createIterator();
    763763    it->seek(leveldbKey);
    764764
    … …  
    792792{
    793793    const Vector<char> levelDBKey = IndexDataKey::encode(databaseId, objectStoreId, indexId, key, 0);
    794     OwnPtr<LevelDBIterator> it(m_db->newIterator());
     794    OwnPtr<LevelDBIterator> it = m_db->createIterator();
    795795
    796796    bool found = false;
    … …  
    841841bool CursorImplCommon::firstSeek()
    842842{
    843     m_iterator = m_db->newIterator();
     843    m_iterator = m_db->createIterator();
    844844
    845845    if (m_forward)
    … …  
    11181118static bool findLastIndexKeyEqualTo(LevelDBDatabase* db, const Vector<char>& target, Vector<char>& foundKey)
    11191119{
    1120     OwnPtr<LevelDBIterator> it(db->newIterator());
     1120    OwnPtr<LevelDBIterator> it = db->createIterator();
    11211121    it->seek(target);
    11221122

  • trunk/Source/WebCore/storage/IDBLevelDBBackingStore.h

    r84149 r85122  
    7575
    7676private:
    77     IDBLevelDBBackingStore(String identifier, IDBFactoryBackendImpl*, LevelDBDatabase*);
     77    IDBLevelDBBackingStore(String identifier, IDBFactoryBackendImpl*, PassOwnPtr<LevelDBDatabase>);
    7878
    7979    String m_identifier;
Note: See TracChangeset for help on using the changeset viewer.