[webkit-reviews] review requested: [Bug 223578] Add logging in IndexedDB to help debug flaky quota tests : [Attachment 423921] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Mar 22 12:48:47 PDT 2021

Alexey Proskuryakov <ap at webkit.org> has asked  for review:
Bug 223578: Add logging in IndexedDB to help debug flaky quota tests

Attachment 423921: Patch


--- Comment #3 from Alexey Proskuryakov <ap at webkit.org> ---
Comment on attachment 423921
  --> https://bugs.webkit.org/attachment.cgi?id=423921

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

I'll leave the real review for experts, but I couldn't stop myself from adding
a couple comments.

> Source/WebCore/Modules/indexeddb/server/IDBServer.cpp:763
> +uint64_t IDBServer::diskUsage(const String& rootDirectory, const
ClientOrigin& origin, bool shouldPrintDetail)

Nit: it would be good to align the terminology ("log" vs. "print detail" vs.
"print usage detail").

> Source/WebCore/Modules/indexeddb/server/SQLiteIDBBackingStore.cpp:1281
> +		  
WTFLogAlways("SQLiteIDBBackingStore::databasesSizeForDirectory filePath='%s',
database='%s', size=%" PRIu64, file.utf8().data(), databaseName.utf8().data(),

Are there no privacy concerns with logging these? Seems questionable even in
regular browsing, and almost certainly not OK in private browsing. Bur perhaps
that's how the top level caller decides whether to pass

> Source/WebKit/NetworkProcess/NetworkProcess.cpp:2533
> +	   usage += IDBServer::IDBServer::diskUsage(idbRootPath, origin,
shouldPrintusageDetail == StorageQuotaManager::ShouldPrintUsageDetail::Yes);

shouldPrintusageDetail - "usage" should be title case too.

More information about the webkit-reviews mailing list