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

[bugzilla-daemon at webkit.org]

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





--- Comment #24 from Hans Wennborg <hans at chromium.org>  2011-06-07 07:03:24 PST ---
(From update of attachment 96144)
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?

> Source/WebCore/storage/IDBBackingStore.h:111
> +    virtual IDBFactoryBackendInterface::BackingStoreType backingStoreType() = 0;

let's make this const

> 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?

> 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

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