[webkit-reviews] review granted: [Bug 201057] IndexedDB: update size of database when database operation is completed : [Attachment 377393] Patch

[bugzilla-daemon at webkit.org]

youenn fablet <youennf at gmail.com> has granted Sihui Liu <sihui_liu at apple.com>'s
request for review:
Bug 201057: IndexedDB: update size of database when database operation is
completed
https://bugs.webkit.org/show_bug.cgi?id=201057

Attachment 377393: Patch

https://bugs.webkit.org/attachment.cgi?id=377393&action=review




--- Comment #18 from youenn fablet <youennf at gmail.com> ---
Comment on attachment 377393
  --> https://bugs.webkit.org/attachment.cgi?id=377393
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=377393&action=review

> Source/WebCore/Modules/indexeddb/server/IDBServer.cpp:783
> +static uint64_t databasesSizeForDirectory(const String& directory)

I quite liked that SQLLiteIDBBackingStore was solely responsible to know that
the extension is sqlite3.
Now, we have two different places, IDBServer and SQLiteIDBBackingStore.

> Source/WebCore/Modules/indexeddb/server/MemoryIDBBackingStore.cpp:606
> +    return;

return; not needed.

> Source/WebCore/Modules/indexeddb/server/MemoryIDBBackingStore.h:82
> +    uint64_t databaseSize() const final;

Could be made private.

> Source/WebCore/Modules/indexeddb/server/MemoryIDBBackingStore.h:89
> +    void close();

final and private?

> Source/WebCore/Modules/indexeddb/server/SQLiteIDBBackingStore.cpp:850
> +    return SQLiteFileSystem::getDatabaseFileSize(fullDatabasePath());

Add ASSERT(!isMainThread());

> Source/WebCore/Modules/indexeddb/server/SQLiteIDBBackingStore.h:85
> +    uint64_t databaseSize() const final;

Could be made private.

> Source/WebCore/Modules/indexeddb/server/SQLiteIDBBackingStore.h:106
> +    void close();

final and private?

> Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.cpp:354
> +	   m_reportedDatabaseSize = backingStore->databaseSize();

m_reportedDatabaseSize is not really reported per se.
Can we have more precise terms like: m_currentDatabaseSize and
m_newDatabaseSize.
Or m_databaseSizeBeforeTask and m_databaseSizeAfterTask

> Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.cpp:789
> +    // QuotaUser should already initiliazes storage usage, which contains
the

s/initiliazes/have initialized/