Bug 195804

Summary: Compute quota after network process restart based on default quota and space used
Product: WebKit Reporter: youenn fablet <youennf>
Component: Service WorkersAssignee: youenn fablet <youennf>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, beidson, cdumez, cgarcia, commit-queue, ews-watchlist, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 195707    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch for landing none

Description youenn fablet 2019-03-15 09:55:13 PDT
Compute quota after network process restart based on default quota and space used.
Comment 1 youenn fablet 2019-03-15 11:14:40 PDT
Created attachment 364815 [details]
Patch
Comment 2 youenn fablet 2019-03-15 13:10:45 PDT
Created attachment 364825 [details]
Patch
Comment 3 Chris Dumez 2019-03-16 08:36:11 PDT
Comment on attachment 364825 [details]
Patch

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

> Source/WebCore/storage/StorageQuotaManager.cpp:71
> +        initializeQuotaAccordingSpaceUsage();

Not a great name. AccordingTo at least to be correct although I wish the name was shorter.

> Source/WebKit/NetworkProcess/cache/CacheStorageEngineCaches.cpp:-60
> -    quotaManager.addUser(*this);

Why this change? I thought it looked in the constructor.
Comment 4 youenn fablet 2019-03-16 09:07:00 PDT
(In reply to Chris Dumez from comment #3)
> Comment on attachment 364825 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=364825&action=review
> 
> > Source/WebCore/storage/StorageQuotaManager.cpp:71
> > +        initializeQuotaAccordingSpaceUsage();
> 
> Not a great name. AccordingTo at least to be correct although I wish the
> name was shorter.

OK.

> > Source/WebKit/NetworkProcess/cache/CacheStorageEngineCaches.cpp:-60
> > -    quotaManager.addUser(*this);
> 
> Why this change? I thought it looked in the constructor.

When added as a quota user, whenInitialized will be called.
This will trigger a call to initialize which will try to ref 'this'.
This must be done once 'this' is adopted.

I think a follow-up patch will try to delay initialization to the first call of requestSpace with a task size greater than zero.
Comment 5 youenn fablet 2019-03-20 10:19:02 PDT
Created attachment 365353 [details]
Patch
Comment 6 youenn fablet 2019-03-20 17:57:01 PDT
Created attachment 365451 [details]
Patch
Comment 7 Chris Dumez 2019-03-20 19:48:25 PDT
Comment on attachment 365451 [details]
Patch

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

> Source/WebCore/storage/StorageQuotaManager.cpp:47
> +void StorageQuotaManager::setQuotaBasedOnSpaceUsage()

maybe updateQuotaBasedOnSpaceUsage() since it is not a one time thing. Also, it makes no parameter.
Comment 8 youenn fablet 2019-03-20 20:59:40 PDT
Created attachment 365477 [details]
Patch for landing
Comment 9 youenn fablet 2019-03-20 21:00:05 PDT
Thanks for the review.

(In reply to Chris Dumez from comment #7)
> Comment on attachment 365451 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=365451&action=review
> 
> > Source/WebCore/storage/StorageQuotaManager.cpp:47
> > +void StorageQuotaManager::setQuotaBasedOnSpaceUsage()
> 
> maybe updateQuotaBasedOnSpaceUsage() since it is not a one time thing. Also,
> it makes no parameter.

OK
Comment 10 WebKit Commit Bot 2019-03-20 21:38:35 PDT
Comment on attachment 365477 [details]
Patch for landing

Clearing flags on attachment: 365477

Committed r243276: <https://trac.webkit.org/changeset/243276>
Comment 11 WebKit Commit Bot 2019-03-20 21:38:37 PDT
All reviewed patches have been landed.  Closing bug.
Comment 12 Radar WebKit Bug Importer 2019-03-20 21:41:23 PDT
<rdar://problem/49093838>