Bug 178350 – Cache API implementation should be able to compute storage size for WebKit client applications.

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[youenn fablet]

Cache API implementation should be able to compute storage size for WebKit client applications.

[Radar WebKit Bug Importer]

<rdar://problem/35014434>

[youenn fablet]

Created attachment 323939 [details]
Patch

[Chris Dumez]

Comment on attachment 323939 [details]
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.

[youenn fablet]

Created attachment 324025 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 324025 [details]
Patch

Clearing flags on attachment: 324025

Committed r223558: <https://trac.webkit.org/changeset/223558>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Alex Christensen]

Youenn, please stop adding C SPI without corresponding ObjC SPI.  It takes my project a step in the wrong direction.
Chris, please r- any patches that add C SPI without adding corresponding ObjC SPI.

[Chris Dumez]

(In reply to Alex Christensen from comment #7)
> Youenn, please stop adding C SPI without corresponding ObjC SPI.  It takes
> my project a step in the wrong direction.
> Chris, please r- any patches that add C SPI without adding corresponding
> ObjC SPI.

Ok.

[youenn fablet]

(In reply to Alex Christensen from comment #7)
> Youenn, please stop adding C SPI without corresponding ObjC SPI.  It takes
> my project a step in the wrong direction.
> Chris, please r- any patches that add C SPI without adding corresponding
> ObjC SPI.

There is no plan to make use of this SPI outside WebKitTestRunner.
I do not plan to add a corresponding ObjC SPI.
Maybe we can move it elsewhere to ensure it never gets used outside WebKitTestRunner.