[Webkit-unassigned] [Bug 61000] Control Indexeddb backends from LayoutTestController

[bugzilla-daemon at webkit.org]

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





--- Comment #26 from Greg Simon <gregsimon at chromium.org>  2011-06-07 10:33:24 PST ---
(In reply to comment #24)
> (From update of attachment 96144 [details])
> Hi Greg! Again, thank you very much for working on this!
> 
> View in context: https://bugs.webkit.org/attachment.cgi?id=96144&action=review
> 
> > Source/WebCore/storage/IDBBackingStore.h:32
> > +#include "IDBFactoryBackendImpl.h"
> 
> Why is this being included?

Refactored to remove this include.

> > Source/WebCore/storage/IDBBackingStore.h:111
> > +    virtual IDBFactoryBackendInterface::BackingStoreType backingStoreType() = 0;
> 
> let's make this const
Done.

> > Source/WebCore/storage/IDBFactoryBackendImpl.cpp:91
> > +        if (m_migrateEnabled) {
> 
> i'm trying to think why we're making this conditional on m_migrateEnabled, rather than on backingStoreType...
> 
> shouldn't the logic of this function be more like
> 
> "if backingStoreType is LevelDB and the origin has a SQLiteBackingStore, and the origin doesn't have a LevelDBBackingStore, then do migration?"
> 
> as it is now, if m_migrateEnabled is true (which is always is??), the backingStoreType gets ignored and we migrate to LevelDB anyway?

Agreed -- removed the enable/disable.

> > Source/WebCore/storage/IDBFactoryBackendImpl.cpp:93
> > +            bool hasLevelDbBackend = IDBLevelDBBackingStore::backingStoreExists(securityOrigin.get(), dataDir);
> 
> i'd prefer if these were named hasSQLBackingStore and hasLevelDBBackingStore
> 
> > Source/WebCore/storage/IDBSQLiteBackingStore.cpp:996
> > +    if (!makeAllDirectories(pathBase)) {
> 
> I think I've commented five times that we shouldn't be creating directories should to see if the database exists
yes... I had a local git merge failure from when it was fixed before :-) Fixed (again)!

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.