Bug 213292

Summary: [ Catalina WK2 Release ] http/tests/IndexedDB/storage-limit-1.https.html is a flaky failure
Product: WebKit Reporter: Truitt Savell <tsavell>
Component: New BugsAssignee: Sihui Liu <sihui_liu>
Status: RESOLVED FIXED    
Severity: Normal CC: alecflett, beidson, ews-watchlist, ggaren, jsbell, sihui_liu, webkit-bot-watchers-bugzilla, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Description Truitt Savell 2020-06-17 07:32:23 PDT
http/tests/IndexedDB/storage-limit-1.https.html

This test is a flaky failure on Catalina WK2.

History:
https://results.webkit.org/?suite=layout-tests&test=http%2Ftests%2FIndexedDB%2Fstorage-limit-1.https.html

Diff:
--- /Volumes/Data/slave/catalina-release-tests-wk2/build/layout-test-results/http/tests/IndexedDB/storage-limit-1.https-expected.txt
+++ /Volumes/Data/slave/catalina-release-tests-wk2/build/layout-test-results/http/tests/IndexedDB/storage-limit-1.https-actual.txt
@@ -1,21 +1,12 @@
+CONSOLE MESSAGE: Cache API operation failed: Quota exceeded
 This test makes sure that storage of indexedDB and Cache API do not grow unboundedly.
 
 On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
 
 
-indexedDB = self.indexedDB || self.webkitIndexedDB || self.mozIndexedDB || self.msIndexedDB || self.OIndexedDB;
-
-indexedDB.deleteDatabase(dbname)
-indexedDB.open(dbname)
-db = event.target.result
-store = db.createObjectStore('store')
-db = event.target.result
-store = db.transaction('store', 'readwrite').objectStore('store')
-request = store.add(new Uint8Array(300 * 1024), 'key')
-PASS 'error' in request is true
-PASS request.error.code is DOMException.QUOTA_EXCEEDED_ERR
-PASS request.error.name is "QuotaExceededError"
+FAIL Cache API store operation failed: QuotaExceededError: Quota exceeded
 PASS successfullyParsed is true
+Some tests failed.
 
 TEST COMPLETE
Comment 1 Radar WebKit Bug Importer 2020-06-17 07:33:09 PDT
<rdar://problem/64447734>
Comment 2 Truitt Savell 2020-06-17 08:35:56 PDT
Marked this test as failing while it is investigated: https://trac.webkit.org/changeset/263148/webkit
Comment 3 Sihui Liu 2021-04-06 13:17:23 PDT
Created attachment 425317 [details]
Patch
Comment 4 Geoffrey Garen 2021-04-06 13:32:12 PDT
Comment on attachment 425317 [details]
Patch

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

> Source/WebCore/Modules/indexeddb/server/SQLiteIDBBackingStore.cpp:1275
> +    String dbFilename = fullDatabasePath();
> +    if (FileSystem::fileExists(dbFilename))
> +        return 0;

Was this supposed to be !FileSystem::fileExists()?

I believe the behavior of creating the file and then checking its version would return the most up-to-date version, rather than zero, right? Should we mimic that behavior here, instead of returning zero?
Comment 5 Sihui Liu 2021-04-06 14:32:08 PDT
(In reply to Geoffrey Garen from comment #4)
> Comment on attachment 425317 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=425317&action=review
> 
> > Source/WebCore/Modules/indexeddb/server/SQLiteIDBBackingStore.cpp:1275
> > +    String dbFilename = fullDatabasePath();
> > +    if (FileSystem::fileExists(dbFilename))
> > +        return 0;
> 
> Was this supposed to be !FileSystem::fileExists()?

ah right! will change

> 
> I believe the behavior of creating the file and then checking its version
> would return the most up-to-date version, rather than zero, right? Should we
> mimic that behavior here, instead of returning zero?

The up-to-date version number is stored in the database file. If it does not exist, we don't have the version number. 

Our old behavior is creating a new db, setting its version to currentMetadataVersion (1), and returning its version. We can directly returning 1 in this case, but 0 seems to be what other browsers use
Comment 6 Sihui Liu 2021-04-06 14:34:03 PDT
Created attachment 425326 [details]
Patch
Comment 7 Geoffrey Garen 2021-04-06 14:41:17 PDT
Comment on attachment 425326 [details]
Patch

r=me
Comment 8 Geoffrey Garen 2021-04-06 14:42:24 PDT
> > Was this supposed to be !FileSystem::fileExists()?
> 
> ah right! will change

Looks like tests caught this too. Nice.

> Our old behavior is creating a new db, setting its version to
> currentMetadataVersion (1), and returning its version. We can directly
> returning 1 in this case, but 0 seems to be what other browsers use

Sounds good -- nice to match other browsers.
Comment 9 EWS 2021-04-06 18:28:57 PDT
Committed r275583: <https://commits.webkit.org/r275583>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 425326 [details].