[webkit-reviews] review granted: [Bug 79413] IndexedDB: Handle LevelDB database corruption : [Attachment 130211] Patch

[bugzilla-daemon at webkit.org]

Tony Chang <tony at chromium.org> has granted Joshua Bell <jsbell at chromium.org>'s
request for review:
Bug 79413: IndexedDB: Handle LevelDB database corruption
https://bugs.webkit.org/show_bug.cgi?id=79413

Attachment 130211: Patch
https://bugs.webkit.org/attachment.cgi?id=130211&action=review

------- Additional Comments from Tony Chang <tony at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=130211&action=review


> Source/WebCore/Modules/indexeddb/IDBLevelDBBackingStore.cpp:166
> +		   LOG_ERROR("IndexedDB backing store cleanup failed");
> +		   return PassRefPtr<IDBBackingStore>();

Nit: The flow might be a bit more clear if you early returned on failure and
didn't indent the success case.  *shrug*

> Source/WebKit/chromium/tests/LevelDBTest.cpp:44
> +    virtual int compare(const LevelDBSlice& a, const LevelDBSlice& b) const

Nit: OVERRIDE?

> Source/WebKit/chromium/tests/LevelDBTest.cpp:49
> +    virtual const char* name() const { return "temp_comparator"; }

Nit: OVERRIDE?

> Source/WebKit/chromium/tests/LevelDBTest.cpp:89
> +    PlatformFileHandle handle = openFile(filepath, OpenForWrite);
> +    truncateFile(handle, 0);
> +    closeFile(handle);

Do we want to add tests for more subtle corruption like 0 bytes somewhere in
the file?