[webkit-reviews] review granted: [Bug 185984] Minor ApplicationCacheStorage clean up : [Attachment 341294] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri May 25 09:53:34 PDT 2018


youenn fablet <youennf at gmail.com> has granted Chris Dumez <cdumez at apple.com>'s
request for review:
Bug 185984: Minor ApplicationCacheStorage clean up
https://bugs.webkit.org/show_bug.cgi?id=185984

Attachment 341294: Patch

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




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

r = me as long as bots are happy.
Some potential improvements below.

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

> Source/WebCore/loader/appcache/ApplicationCacheStorage.cpp:1474
> +HashSet<RefPtr<SecurityOrigin>> ApplicationCacheStorage::originsWithCache()

Maybe HashSet<Ref<>>?

> Source/WebCore/loader/appcache/ApplicationCacheStorage.h:106
> +    WEBCORE_EXPORT ApplicationCacheStorage(const String& cacheDirectory,
const String& flatFileSubdirectoryName);

Could probably be String&&

> Source/WebCore/loader/appcache/ApplicationCacheStorage.h:110
> +    bool getManifestURLs(Vector<URL>& urls);

Could be std::optional<Vector<>> manifestURLs()

> Source/WebCore/loader/appcache/ApplicationCacheStorage.h:145
> +    int64_t m_maximumSize { noQuota() };

Could it be uint64_t?

> Source/WebCore/loader/appcache/ApplicationCacheStorage.h:148
> +    int64_t m_defaultOriginQuota { noQuota() };

Ditto.


More information about the webkit-reviews mailing list