Bug 195302 - Check IDB quota usage through QuotaManager
Summary: Check IDB quota usage through QuotaManager
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: youenn fablet
URL:
Keywords: InRadar
Depends on: 195283 195541 195544
Blocks:
  Show dependency treegraph
 
Reported: 2019-03-04 17:20 PST by youenn fablet
Modified: 2019-03-13 14:48 PDT (History)
12 users (show)

See Also:


Attachments
Patch (52.71 KB, patch)
2019-03-04 17:28 PST, youenn fablet
no flags Details | Formatted Diff | Diff
Patch (64.41 KB, patch)
2019-03-07 22:38 PST, youenn fablet
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews106 for mac-highsierra-wk2 (975.54 KB, application/zip)
2019-03-07 23:31 PST, Build Bot
no flags Details
Archive of layout-test-results from ews102 for mac-highsierra (2.49 MB, application/zip)
2019-03-07 23:43 PST, Build Bot
no flags Details
Archive of layout-test-results from ews114 for mac-highsierra (2.69 MB, application/zip)
2019-03-08 00:29 PST, Build Bot
no flags Details
Archive of layout-test-results from ews124 for ios-simulator-wk2 (1.11 MB, application/zip)
2019-03-08 04:31 PST, Build Bot
no flags Details
Patch (88.39 KB, patch)
2019-03-10 15:27 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews103 for mac-highsierra (2.44 MB, application/zip)
2019-03-10 16:32 PDT, Build Bot
no flags Details
Patch (89.29 KB, patch)
2019-03-10 18:31 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
Patch (72.74 KB, patch)
2019-03-11 13:42 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
Patch (72.77 KB, patch)
2019-03-11 16:47 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
Patch (73.15 KB, patch)
2019-03-11 16:52 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
Patch (77.56 KB, patch)
2019-03-11 21:09 PDT, youenn fablet
ews: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews205 for win-future (12.92 MB, application/zip)
2019-03-12 02:11 PDT, Build Bot
no flags Details
Patch (72.33 KB, patch)
2019-03-12 09:51 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
Patch (72.54 KB, patch)
2019-03-12 11:42 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
Updated according review comments (72.59 KB, patch)
2019-03-13 11:17 PDT, youenn fablet
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description youenn fablet 2019-03-04 17:20:22 PST
Check IDB quota usage through QuotaManager
Comment 1 youenn fablet 2019-03-04 17:28:16 PST
Created attachment 363573 [details]
Patch
Comment 2 youenn fablet 2019-03-07 22:38:36 PST
Created attachment 363988 [details]
Patch
Comment 3 Build Bot 2019-03-07 22:40:18 PST
Attachment 363988 [details] did not pass style-queue:


ERROR: Source/WebCore/Modules/indexeddb/server/IDBServer.cpp:62:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 1 in 31 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 Build Bot 2019-03-07 23:31:07 PST
Comment on attachment 363988 [details]
Patch

Attachment 363988 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/11424062

Number of test failures exceeded the failure limit.
Comment 5 Build Bot 2019-03-07 23:31:09 PST
Created attachment 363990 [details]
Archive of layout-test-results from ews106 for mac-highsierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-highsierra-wk2  Platform: Mac OS X 10.13.6
Comment 6 Build Bot 2019-03-07 23:43:22 PST
Comment on attachment 363988 [details]
Patch

Attachment 363988 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/11424110

New failing tests:
storage/indexeddb/transaction-coordination-across-databases-private.html
storage/indexeddb/transaction-coordination-across-databases.html
storage/indexeddb/storage-limit.html
storage/websql/open-database-while-transaction-in-progress.html
storage/websql/transaction-database-expand-quota.html
storage/indexeddb/storage-limit-2.html
Comment 7 Build Bot 2019-03-07 23:43:23 PST
Created attachment 363991 [details]
Archive of layout-test-results from ews102 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-highsierra  Platform: Mac OS X 10.13.6
Comment 8 Build Bot 2019-03-08 00:29:05 PST
Comment on attachment 363988 [details]
Patch

Attachment 363988 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/11424175

Number of test failures exceeded the failure limit.
Comment 9 Build Bot 2019-03-08 00:29:06 PST
Created attachment 363993 [details]
Archive of layout-test-results from ews114 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews114  Port: mac-highsierra  Platform: Mac OS X 10.13.6
Comment 10 Build Bot 2019-03-08 04:31:31 PST
Comment on attachment 363988 [details]
Patch

Attachment 363988 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/11425298

New failing tests:
imported/w3c/web-platform-tests/webrtc/simplecall-no-ssrcs.https.html
Comment 11 Build Bot 2019-03-08 04:31:33 PST
Created attachment 364005 [details]
Archive of layout-test-results from ews124 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews124  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6
Comment 12 youenn fablet 2019-03-10 15:27:34 PDT
Created attachment 364201 [details]
Patch
Comment 13 Build Bot 2019-03-10 15:31:48 PDT
Attachment 364201 [details] did not pass style-queue:


ERROR: Source/WebCore/Modules/indexeddb/server/IDBServer.cpp:62:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
ERROR: Tools/WebKitTestRunner/cocoa/TestWebsiteDataStoreDelegate.mm:38:  The parameter name "quota" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 2 in 44 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 14 Build Bot 2019-03-10 16:32:04 PDT
Created attachment 364211 [details]
Archive of layout-test-results from ews103 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-highsierra  Platform: Mac OS X 10.13.6
Comment 15 youenn fablet 2019-03-10 18:31:22 PDT
Created attachment 364217 [details]
Patch
Comment 16 Build Bot 2019-03-10 18:32:36 PDT
Attachment 364217 [details] did not pass style-queue:


ERROR: Source/WebCore/Modules/indexeddb/server/IDBServer.cpp:62:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
ERROR: Tools/WebKitTestRunner/cocoa/TestWebsiteDataStoreDelegate.mm:38:  The parameter name "quota" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 2 in 45 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 17 youenn fablet 2019-03-11 13:42:33 PDT
Created attachment 364283 [details]
Patch
Comment 18 Build Bot 2019-03-11 13:45:41 PDT
Attachment 364283 [details] did not pass style-queue:


ERROR: Source/WebCore/Modules/indexeddb/server/IDBServer.cpp:62:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 1 in 32 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 19 youenn fablet 2019-03-11 16:47:15 PDT
Created attachment 364308 [details]
Patch
Comment 20 Build Bot 2019-03-11 16:49:33 PDT
Attachment 364308 [details] did not pass style-queue:


ERROR: Source/WebCore/Modules/indexeddb/server/IDBServer.cpp:62:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 1 in 32 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 21 youenn fablet 2019-03-11 16:52:27 PDT
Created attachment 364310 [details]
Patch
Comment 22 Build Bot 2019-03-11 16:54:31 PDT
Attachment 364310 [details] did not pass style-queue:


ERROR: Source/WebCore/Modules/indexeddb/server/IDBServer.cpp:62:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 1 in 32 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 23 youenn fablet 2019-03-11 21:09:25 PDT
Created attachment 364345 [details]
Patch
Comment 24 Build Bot 2019-03-11 21:12:33 PDT
Attachment 364345 [details] did not pass style-queue:


ERROR: Source/WebCore/Modules/indexeddb/server/IDBServer.cpp:62:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 1 in 32 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 25 Build Bot 2019-03-12 02:11:38 PDT
Comment on attachment 364345 [details]
Patch

Attachment 364345 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/11470550

New failing tests:
storage/indexeddb/storage-limit.html
http/tests/IndexedDB/storage-limit.https.html
Comment 26 Build Bot 2019-03-12 02:11:49 PDT
Created attachment 364368 [details]
Archive of layout-test-results from ews205 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews205  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
Comment 27 youenn fablet 2019-03-12 09:51:41 PDT
Created attachment 364395 [details]
Patch
Comment 28 Build Bot 2019-03-12 09:54:18 PDT
Attachment 364395 [details] did not pass style-queue:


ERROR: Source/WebCore/Modules/indexeddb/server/IDBServer.cpp:62:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 1 in 31 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 29 youenn fablet 2019-03-12 11:42:20 PDT
Created attachment 364418 [details]
Patch
Comment 30 Build Bot 2019-03-12 11:44:23 PDT
Attachment 364418 [details] did not pass style-queue:


ERROR: Source/WebCore/Modules/indexeddb/server/IDBServer.cpp:62:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 1 in 31 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 31 youenn fablet 2019-03-12 14:02:49 PDT
Wincairo error seems unrelated.
Comment 32 youenn fablet 2019-03-13 09:20:28 PDT
ping review
Comment 33 Chris Dumez 2019-03-13 10:04:46 PDT
Comment on attachment 364418 [details]
Patch

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

Would be good if Brady could take a look but it seems fine.

> Source/WebCore/Modules/indexeddb/server/IDBServer.cpp:703
> +void IDBServer::QuotaUser::clearSpaceUsed()

I'd call it 'resetSpaceSpaced'. Clear makes it sound like it might actually remove files.

> Source/WebCore/Modules/indexeddb/server/IDBServer.cpp:782
> +    quotaManager->requestSpace(taskSize, [callback = WTFMove(callback)](auto decision) mutable {

Doesn't this work?

quotaManager->requestSpace(taskSize, WTFMove(callback));

> Source/WebCore/Modules/indexeddb/shared/InProcessIDBServer.cpp:74
> +    return [&server](PAL::SessionID, const auto& origin) {

Seems risky to capture server by reference here given that you are not calling the lambda right away and leaving it up to the caller. If the caller were to store this lambda and dispatch then call it, we could crash.
Comment 34 youenn fablet 2019-03-13 10:53:21 PDT
Thanks for the review.

(In reply to Chris Dumez from comment #33)
> Comment on attachment 364418 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=364418&action=review
> 
> Would be good if Brady could take a look but it seems fine.

Sure, I'll wait to land this patch until this afternoon.

> > Source/WebCore/Modules/indexeddb/server/IDBServer.cpp:703
> > +void IDBServer::QuotaUser::clearSpaceUsed()
> 
> I'd call it 'resetSpaceSpaced'. Clear makes it sound like it might actually
> remove files.

OK

> > Source/WebCore/Modules/indexeddb/server/IDBServer.cpp:782
> > +    quotaManager->requestSpace(taskSize, [callback = WTFMove(callback)](auto decision) mutable {
> 
> Doesn't this work?
> 
> quotaManager->requestSpace(taskSize, WTFMove(callback));

Right.

> > Source/WebCore/Modules/indexeddb/shared/InProcessIDBServer.cpp:74
> > +    return [&server](PAL::SessionID, const auto& origin) {
> 
> Seems risky to capture server by reference here given that you are not
> calling the lambda right away and leaving it up to the caller. If the caller
> were to store this lambda and dispatch then call it, we could crash.

Well InProcessIDBServer is passing *this in the IDBServer constructor and IDBServer keeps it as a reference so I thought that was fine.
Looking more closely though, it seems that the *this reference might never be used in the case of InProcessIDBServer. I'll make InProcessIDBServer WeakPtr-able to make things stronger.
Comment 35 youenn fablet 2019-03-13 11:17:36 PDT
Created attachment 364551 [details]
Updated according review comments
Comment 36 Build Bot 2019-03-13 11:19:56 PDT
Attachment 364551 [details] did not pass style-queue:


ERROR: Source/WebCore/Modules/indexeddb/server/IDBServer.cpp:62:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 1 in 31 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 37 WebKit Commit Bot 2019-03-13 14:47:19 PDT
Comment on attachment 364551 [details]
Updated according review comments

Clearing flags on attachment: 364551

Committed r242911: <https://trac.webkit.org/changeset/242911>
Comment 38 WebKit Commit Bot 2019-03-13 14:47:21 PDT
All reviewed patches have been landed.  Closing bug.
Comment 39 Radar WebKit Bug Importer 2019-03-13 14:48:28 PDT
<rdar://problem/48864151>