Summary: | Cache API implementation should be able to compute storage size for WebKit client applications. | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | youenn fablet <youennf> | ||||||
Component: | WebCore Misc. | Assignee: | youenn fablet <youennf> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | achristensen, buildbot, cdumez, cgarcia, commit-queue, webkit-bug-importer | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
youenn fablet
2017-10-16 10:55:25 PDT
Created attachment 323939 [details]
Patch
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. Created attachment 324025 [details]
Patch
Comment on attachment 324025 [details] Patch Clearing flags on attachment: 324025 Committed r223558: <https://trac.webkit.org/changeset/223558> All reviewed patches have been landed. Closing bug. 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. (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. (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. |