[webkit-reviews] review granted: [Bug 178350] Cache API implementation should be able to compute storage size for WebKit client applications. : [Attachment 323939] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Oct 16 19:15:27 PDT 2017


Chris Dumez <cdumez at apple.com> has granted youenn fablet <youennf at gmail.com>'s
request for review:
Bug 178350: Cache API implementation should be able to compute storage size for
WebKit client applications.
https://bugs.webkit.org/show_bug.cgi?id=178350

Attachment 323939: Patch

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




--- Comment #3 from Chris Dumez <cdumez at apple.com> ---
Comment on attachment 323939
  --> https://bugs.webkit.org/attachment.cgi?id=323939
Patch

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

r=me with changes.

> Source/WebKit/ChangeLog:8
> +	   When gathering data from DOM Cache, we compute the size as follow:

as follows:

> Source/WebKit/ChangeLog:9
> +	   - If caches is not persistent, size is zero

caches (plural) vs is (singular)

> Source/WebKit/ChangeLog:10
> +	   - If caches is persistent, we use the size computed by
NetworkCache::Storage. 

caches (plural) vs is (singular)

> Source/WebCore/page/SecurityOriginData.h:70
> +    bool equals(const SecurityOriginData& data) const

There is already an operator==() on this class.

> Source/WebKit/NetworkProcess/cache/CacheStorageEngine.cpp:402
> +		   taskCounter->addOrigin(WTFMove(origin),
result.value().get().storageSize());

Can .get(). be replace by -> ?

> Source/WebKit/UIProcess/API/C/WKWebsiteDataStoreRef.cpp:379
> +	   for (const auto& dataRecord : dataRecords) {

We usually omit the const.

> Source/WebKit/UIProcess/API/C/WKWebsiteDataStoreRef.cpp:380
> +	       for (const auto& recordOrigin : dataRecord.origins) {

We usually omit the const.

> Source/WebKit/UIProcess/API/C/WKWebsiteDataStoreRef.cpp:381
> +		   if (WebKit::toImpl(origin)->string() ==
recordOrigin.securityOrigin()->toString()) {

Seems inefficient to Stringify just to compare. Assuming recordOrigin is a
SecurityOriginData, we could do:
SecurityOriginData::fromSecurityOrigin(origin) == recordOrigin.

Also, we could cache SecurityOriginData::fromSecurityOrigin(origin) before the
loops.


More information about the webkit-reviews mailing list