[webkit-reviews] review denied: [Bug 174942] [Curl] Use SQLite database in cookie jar implementation for Curl port : [Attachment 331552] Patch

[bugzilla-daemon at webkit.org]

Alex Christensen <achristensen at apple.com> has denied Christopher Reid
<christopher.reid at am.sony.com>'s request for review:
Bug 174942: [Curl] Use SQLite database in cookie jar implementation for Curl
port
https://bugs.webkit.org/show_bug.cgi?id=174942

Attachment 331552: Patch

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




--- Comment #2 from Alex Christensen <achristensen at apple.com> ---
Comment on attachment 331552
  --> https://bugs.webkit.org/attachment.cgi?id=331552
Patch

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

I feel like this patch has a lot of parsers.  Are there not existing parsers
for these things?

> Source/WebCore/ChangeLog:3
> +	   [Curl] Use SQLite database in cookie jar implementation for Curl
port

Cool!

> Source/WebCore/platform/network/curl/CookieJarCurlDatabase.cpp:89
> +    CookieJarDB* cookieJarDB = CookieJarDB::getInstance();

This should be implemented by NetworkStorageSession instead of having a global
singleton.

> Source/WebCore/platform/network/curl/CookieJarDB.cpp:76
> +CookieJarDB* CookieJarDB::getInstance()

This could return a reference.

> Source/WebCore/platform/network/curl/CookieJarDB.cpp:80
> +	   CookieJarDB::s_instance = new CookieJarDB();

This should use smart pointers or NeverDestroyed to avoid explicit new/delete
calls.

> Source/WebCore/platform/network/curl/CookieJarDB.cpp:84
> +void CookieJarDB::releaseInstance()

This is never called.

> Source/WebCore/platform/network/curl/CookieJarDB.cpp:191
> +void CookieJarDB::detectDatabaseCorruption()

Does this really detect anything?

> Source/WebCore/platform/network/curl/CookieUtil.cpp:82
> +static bool parseExpires(const char* expires, time_t& time)

This could return std::optional<time_t>