| Summary: | Check IDB quota usage through QuotaManager | ||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | youenn fablet <youennf> | ||||||||||||||||||||||||||||||||||||
| Component: | WebCore Misc. | Assignee: | youenn fablet <youennf> | ||||||||||||||||||||||||||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||||||||||||||||||||||||||
| Severity: | Normal | CC: | achristensen, alecflett, beidson, cdumez, commit-queue, dbates, ews-watchlist, japhet, jsbell, rniwa, sihui_liu, sroberts, webkit-bug-importer | ||||||||||||||||||||||||||||||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||||||||||||||||||||||||||||||
| Version: | WebKit Nightly Build | ||||||||||||||||||||||||||||||||||||||
| Hardware: | Unspecified | ||||||||||||||||||||||||||||||||||||||
| OS: | Unspecified | ||||||||||||||||||||||||||||||||||||||
| Bug Depends on: | 195283, 195541, 195544 | ||||||||||||||||||||||||||||||||||||||
| Bug Blocks: | |||||||||||||||||||||||||||||||||||||||
| Attachments: |
|
||||||||||||||||||||||||||||||||||||||
|
Description
youenn fablet
2019-03-04 17:20:22 PST
Created attachment 363573 [details]
Patch
Created attachment 363988 [details]
Patch
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 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. 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 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 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 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. 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 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 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
Created attachment 364201 [details]
Patch
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.
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
Created attachment 364217 [details]
Patch
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.
Created attachment 364283 [details]
Patch
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.
Created attachment 364308 [details]
Patch
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.
Created attachment 364310 [details]
Patch
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.
Created attachment 364345 [details]
Patch
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 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 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
Created attachment 364395 [details]
Patch
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.
Created attachment 364418 [details]
Patch
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.
Wincairo error seems unrelated. ping review 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. 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. Created attachment 364551 [details]
Updated according review comments
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 on attachment 364551 [details] Updated according review comments Clearing flags on attachment: 364551 Committed r242911: <https://trac.webkit.org/changeset/242911> All reviewed patches have been landed. Closing bug. http/tests/security/cross-origin-indexeddb.html imported/blink/fast/text/output-isolate-at-end-of-line-crash.html legacy-animation-engine/imported/blink/fast/text/output-isolate-at-end-of-line-crash.html is crashing on Mac and iOS Sim Debug bots. Crashing started to happen after this revision. I cannot repro locally. Dashboard : https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=http%2Ftests%2Fsecurity%2Fcross-origin-indexeddb.html%20imported%2Fblink%2Ffast%2Ftext%2Foutput-isolate-at-end-of-line-crash.html%20legacy-animation-engine%2Fimported%2Fblink%2Ffast%2Ftext%2Foutput-isolate-at-end-of-line-crash.html Crash log : https://build.webkit.org/results/Apple%20Mojave%20Debug%20WK2%20(Tests)/r243285%20(2035)/http/tests/security/cross-origin-worker-indexeddb-crash-log.txt |