Bug 207959

Summary: Regression(r247567) HTTP Disk cache capacity is no longer set
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: WebKit2Assignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, beidson, cgarcia, commit-queue, ews-watchlist, ggaren, koivisto, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=199817
Bug Depends on:    
Bug Blocks: 207967    
Attachments:
Description Flags
Patch
none
Patch none

Description Chris Dumez 2020-02-19 13:41:20 PST 
HTTP Disk cache capacity is no longer set since r247567 so we end up with a default capacity for std::numeric_limit<size_t>::max().
Comment 1 Radar WebKit Bug Importer 2020-02-19 13:41:50 PST 
<rdar://problem/59603972>
Comment 2 Chris Dumez 2020-02-19 13:58:49 PST 
Created attachment 391197 [details]
Patch
Comment 3 Antti Koivisto 2020-02-19 14:12:27 PST 
Comment on attachment 391197 [details]
Patch

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

> Source/WebKit/NetworkProcess/cache/NetworkCache.cpp:76
> +        calculateURLCacheSizes(cacheModel, diskFreeSize, urlCacheMemoryCapacity, urlCacheDiskCapacity);
> +    }
> +    return urlCacheDiskCapacity;

This stuff could use a cleanups at some point. We have no "URL cache" and urlCacheMemoryCapacity is never used.
Comment 4 Alex Christensen 2020-02-19 14:13:09 PST 
Comment on attachment 391197 [details]
Patch

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

> Source/WebKit/NetworkProcess/NetworkProcess.cpp:2047
> +    if (m_hasSetCacheModel && cacheModel == m_cacheModel)

m_cacheModel should be an Optional instead of a bool m_hasSetCacheModel
Comment 5 Chris Dumez 2020-02-19 14:49:46 PST 
Comment on attachment 391197 [details]
Patch

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

>> Source/WebKit/NetworkProcess/NetworkProcess.cpp:2047
>> +    if (m_hasSetCacheModel && cacheModel == m_cacheModel)
> 
> m_cacheModel should be an Optional instead of a bool m_hasSetCacheModel

I agree but since the patch will likely get cherry-picked, I think it is good to keep the change minimal. I will drop changes to this line. Not sure why I dropped the parentheses.
Comment 6 Chris Dumez 2020-02-19 14:50:36 PST 
Created attachment 391204 [details]
Patch
Comment 7 WebKit Commit Bot 2020-02-19 15:34:01 PST 
Comment on attachment 391204 [details]
Patch

Clearing flags on attachment: 391204

Committed r256967: <https://trac.webkit.org/changeset/256967>
Comment 8 WebKit Commit Bot 2020-02-19 15:34:03 PST 
All reviewed patches have been landed.  Closing bug.