[webkit-reviews] review granted: [Bug 211929] ITP database should finalize all prepared statements before closing : [Attachment 399762] Patch

[bugzilla-daemon at webkit.org]

John Wilander <wilander at apple.com> has granted katherine_cheney at apple.com's
request for review:
Bug 211929: ITP database should finalize all prepared statements before closing
https://bugs.webkit.org/show_bug.cgi?id=211929

Attachment 399762: Patch

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




--- Comment #14 from John Wilander <wilander at apple.com> ---
Comment on attachment 399762
  --> https://bugs.webkit.org/attachment.cgi?id=399762
Patch

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

Given that Sihui has looked this over, I'm OK with it too. See comments though.

>
Source/WebKit/NetworkProcess/Classifier/ResourceLoadStatisticsDatabaseStore.cpp
:504
> +SQLiteStatementAutoResetScope
ResourceLoadStatisticsDatabaseStore::getScopedStatement(std::unique_ptr<WebCore
::SQLiteStatement>& statement, const String query, const String& logString)
const

Our style guide says no get prefix. If you get a name conflict with a local
variable, you can call it with this->scopedStatement(). Can the query
parameters be ref?

>
Source/WebKit/NetworkProcess/Classifier/ResourceLoadStatisticsDatabaseStore.cpp
:510
> +	       RELEASE_LOG_ERROR(Network, "%p -
ResourceLoadStatisticsDatabaseStore::%s failed to prepare statement, error
message: %{private}s", this, logString.ascii().data(),
m_database.lastErrorMsg());

I don't remember the reason for keeping these error messages private. Can they
leak something about the user's browsing?