REGRESSION (iOS 16): LocalStorage writes fail after ~2.5MB #4758

szewai · 2022-09-27T17:43:52Z

`0297f12`

REGRESSION (iOS 16): LocalStorage writes fail after ~2.5MB
https://bugs.webkit.org/show_bug.cgi?id=245479
<rdar://100252023>

Reviewed by Chris Dumez.

We set the maximum size of LocalStorage database to be 5MB, because quota is 5MB. But SQLite has overhead, which means
the file size can reach 5MB when the stored content is much smaller than that. To avoid this issue, now we let
SQLiteStorageArea keep track of the actual content size and use that size to do quota computation, instead of setting
hard limit on database file.

* Source/WebKit/NetworkProcess/storage/SQLiteStorageArea.cpp:
(WebKit::SQLiteStorageArea::isEmpty):
(WebKit::SQLiteStorageArea::prepareDatabase):
(WebKit::SQLiteStorageArea::startTransactionIfNecessary):
(WebKit::SQLiteStorageArea::getItem):
(WebKit::SQLiteStorageArea::getAllItemsFromDatabase):
(WebKit::SQLiteStorageArea::initializeCache):
(WebKit::SQLiteStorageArea::allItems):
(WebKit::SQLiteStorageArea::setItem):
(WebKit::SQLiteStorageArea::removeItem):
(WebKit::SQLiteStorageArea::clear):
* Source/WebKit/NetworkProcess/storage/SQLiteStorageArea.h:

Canonical link: https://commits.webkit.org/255044@main

b7e1da2

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	~~🛠 wpe~~
	✅ 🛠 ios-sim	~~🛠 mac-debug~~	✅ 🛠 gtk	✅ 🛠 wincairo
✅ 🧪 webkitperl	~~🧪 ios-wk2~~	~~🛠 mac-AS-debug~~	~~🧪 gtk-wk2~~
	~~🧪 api-ios~~	~~🧪 api-mac~~	~~🧪 api-gtk~~
	✅ 🛠 tv	~~🧪 mac-wk1~~
	✅ 🛠 tv-sim	✅ 🧪 mac-wk2
✅ 🛠 🧪 merge	✅ 🛠 watch	~~🧪 mac-AS-debug-wk2~~
	✅ 🛠 watch-sim	✅ 🧪 mac-wk2-stress

webkit-early-warning-system · 2022-09-27T17:44:12Z

EWS run on previous version of this PR (hash d95d64c)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 🧪 win
	✅ 🛠 ios-sim	✅ 🛠 mac-debug	✅ 🛠 gtk	✅ 🛠 wincairo
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🛠 mac-AS-debug	✅ 🧪 gtk-wk2
	✅ 🧪 api-ios	✅ 🧪 api-mac	✅ 🧪 api-gtk
	✅ 🛠 tv	❌ 🧪 mac-wk1
	✅ 🛠 tv-sim	❌ 🧪 mac-wk2
	✅ 🛠 watch	✅ 🧪 mac-AS-debug-wk2
	✅ 🛠 watch-sim	✅ 🧪 mac-wk2-stress

webkit-early-warning-system · 2022-09-29T16:53:07Z

EWS run on previous version of this PR (hash 98a2302)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	⏳ 🛠 mac	✅ 🛠 wpe	⏳ 🛠 🧪 win
	✅ 🛠 ios-sim	⏳ 🛠 mac-debug	✅ 🛠 gtk	⏳ 🛠 wincairo
✅ 🧪 webkitperl	⏳ 🧪 ios-wk2	✅ 🛠 mac-AS-debug	✅ 🧪 gtk-wk2
	✅ 🧪 api-ios	⏳ 🧪 api-mac	~~🧪 api-gtk~~
	✅ 🛠 tv	⏳ 🧪 mac-wk1
	✅ 🛠 tv-sim	⏳ 🧪 mac-wk2
	✅ 🛠 watch	~~🧪 mac-AS-debug-wk2~~
	✅ 🛠 watch-sim	⏳ 🧪 mac-wk2-stress

webkit-early-warning-system · 2022-09-29T20:08:16Z

EWS run on previous version of this PR (hash 636c90b)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	⏳ 🛠 mac	✅ 🛠 wpe	~~🛠 🧪 win~~
	✅ 🛠 ios-sim	⏳ 🛠 mac-debug	✅ 🛠 gtk	💥 🛠 wincairo
✅ 🧪 webkitperl	⏳ 🧪 ios-wk2	✅ 🛠 mac-AS-debug	❌ 🧪 gtk-wk2
	✅ 🧪 api-ios	⏳ 🧪 api-mac	~~🧪 api-gtk~~
	✅ 🛠 tv	⏳ 🧪 mac-wk1
	✅ 🛠 tv-sim	⏳ 🧪 mac-wk2
	✅ 🛠 watch	~~🧪 mac-AS-debug-wk2~~
	✅ 🛠 watch-sim	⏳ 🧪 mac-wk2-stress

geoffreygaren · 2022-09-29T20:32:42Z

Is it really true that exceeding quota currently erases all your (under-quota) LocalStorage? If so, does this patch address that symptom?

szewai · 2022-09-29T20:42:06Z

Is it really true that exceeding quota currently erases all your (under-quota) LocalStorage? If so, does this patch address that symptom?

In network process, all updates in the same transaction will be lost because transaction is aborted. Normally we commit 500ms after transaction starts (e3fff83).
For web process, it currently clears cached items in memory on quota error.

geoffreygaren · 2022-09-29T20:50:21Z

For web process, it currently clears all items in memory on quota error.

I see: Not all items are lost, just all items in the current 500ms batch.

webkit-early-warning-system · 2022-09-30T00:12:24Z

EWS run on previous version of this PR (hash 52bbe8f)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	⏳ 🛠 mac	⏳ 🛠 wpe	⏳ 🛠 🧪 win
	⏳ 🛠 ios-sim	⏳ 🛠 mac-debug	✅ 🛠 gtk	⏳ 🛠 wincairo
✅ 🧪 webkitperl	⏳ 🧪 ios-wk2	⏳ 🛠 mac-AS-debug	~~🧪 gtk-wk2~~
	⏳ 🧪 api-ios	⏳ 🧪 api-mac	~~🧪 api-gtk~~
⏳ 🛠 🧪 jsc	✅ 🛠 tv	⏳ 🧪 mac-wk1	⏳ 🛠 jsc-armv7
	⏳ 🛠 tv-sim	⏳ 🧪 mac-wk2	⏳ 🧪 jsc-armv7-tests
	✅ 🛠 watch	⏳ 🧪 mac-AS-debug-wk2
	✅ 🛠 watch-sim	⏳ 🧪 mac-wk2-stress

webkit-early-warning-system · 2022-09-30T00:22:00Z

EWS run on previous version of this PR (hash 7fe083a)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	❌ 🛠 🧪 win
	✅ 🛠 ios-sim	✅ 🛠 mac-debug	✅ 🛠 gtk	✅ 🛠 wincairo
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🛠 mac-AS-debug	✅ 🧪 gtk-wk2
	✅ 🧪 api-ios	~~🧪 api-mac~~	✅ 🧪 api-gtk
	✅ 🛠 tv	✅ 🧪 mac-wk1
	✅ 🛠 tv-sim	❌ 🧪 mac-wk2
	✅ 🛠 watch	✅ 🧪 mac-AS-debug-wk2
	✅ 🛠 watch-sim	✅ 🧪 mac-wk2-stress

cdumez

LGTM

webkit-early-warning-system · 2022-09-30T19:00:43Z

EWS run on current version of this PR (hash b7e1da2)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	~~🛠 wpe~~
	✅ 🛠 ios-sim	~~🛠 mac-debug~~	✅ 🛠 gtk	✅ 🛠 wincairo
✅ 🧪 webkitperl	~~🧪 ios-wk2~~	~~🛠 mac-AS-debug~~	~~🧪 gtk-wk2~~
	~~🧪 api-ios~~	~~🧪 api-mac~~	~~🧪 api-gtk~~
	✅ 🛠 tv	~~🧪 mac-wk1~~
	✅ 🛠 tv-sim	✅ 🧪 mac-wk2
✅ 🛠 🧪 merge	✅ 🛠 watch	~~🧪 mac-AS-debug-wk2~~
	✅ 🛠 watch-sim	✅ 🧪 mac-wk2-stress

https://bugs.webkit.org/show_bug.cgi?id=245479 <rdar://100252023> Reviewed by Chris Dumez. We set the maximum size of LocalStorage database to be 5MB, because quota is 5MB. But SQLite has overhead, which means the file size can reach 5MB when the stored content is much smaller than that. To avoid this issue, now we let SQLiteStorageArea keep track of the actual content size and use that size to do quota computation, instead of setting hard limit on database file. * Source/WebKit/NetworkProcess/storage/SQLiteStorageArea.cpp: (WebKit::SQLiteStorageArea::isEmpty): (WebKit::SQLiteStorageArea::prepareDatabase): (WebKit::SQLiteStorageArea::startTransactionIfNecessary): (WebKit::SQLiteStorageArea::getItem): (WebKit::SQLiteStorageArea::getAllItemsFromDatabase): (WebKit::SQLiteStorageArea::initializeCache): (WebKit::SQLiteStorageArea::allItems): (WebKit::SQLiteStorageArea::setItem): (WebKit::SQLiteStorageArea::removeItem): (WebKit::SQLiteStorageArea::clear): * Source/WebKit/NetworkProcess/storage/SQLiteStorageArea.h: Canonical link: https://commits.webkit.org/255044@main

webkit-commit-queue · 2022-09-30T20:37:10Z

Committed 255044@main (0297f12): https://commits.webkit.org/255044@main

Reviewed commits have been landed. Closing PR #4758 and removing active labels.

szewai requested a review from cdumez as a code owner September 27, 2022 17:43

szewai self-assigned this Sep 27, 2022

szewai added WebKit Nightly Build Website Storage Bugs related to storage APIs, including IndexedDB and localStorage labels Sep 27, 2022

webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Sep 27, 2022

szewai removed the merging-blocked Applied to prevent a change from being merged label Sep 29, 2022

szewai force-pushed the eng/REGRESSION-iOS-16-LocalStorage-is-cleared-after-writing-data-of-more-than-2-5MB branch from d95d64c to 98a2302 Compare September 29, 2022 16:52

szewai requested review from achristensen07, geoffreygaren, bnham and beidson September 29, 2022 20:04

szewai force-pushed the eng/REGRESSION-iOS-16-LocalStorage-is-cleared-after-writing-data-of-more-than-2-5MB branch from 98a2302 to 636c90b Compare September 29, 2022 20:08

geoffreygaren changed the title ~~REGRESSION (iOS 16): LocalStorage is cleared after writing data of more than ~2.5MB~~ REGRESSION (iOS 16): LocalStorage writes fail after ~2.5MB Sep 29, 2022

webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Sep 29, 2022

szewai removed the merging-blocked Applied to prevent a change from being merged label Sep 30, 2022

szewai force-pushed the eng/REGRESSION-iOS-16-LocalStorage-is-cleared-after-writing-data-of-more-than-2-5MB branch from 636c90b to 52bbe8f Compare September 30, 2022 00:11

szewai changed the title ~~REGRESSION (iOS 16): LocalStorage writes fail after ~2.5MB~~ REGRESSION (iOS 16): LocalStorage is cleared after writing data of more than ~2.5MB Sep 30, 2022

szewai force-pushed the eng/REGRESSION-iOS-16-LocalStorage-is-cleared-after-writing-data-of-more-than-2-5MB branch from 52bbe8f to 7fe083a Compare September 30, 2022 00:20

szewai changed the title ~~REGRESSION (iOS 16): LocalStorage is cleared after writing data of more than ~2.5MB~~ REGRESSION (iOS 16): LocalStorage writes fail after ~2.5MB Sep 30, 2022

webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Sep 30, 2022

cdumez approved these changes Sep 30, 2022

View reviewed changes

szewai removed the merging-blocked Applied to prevent a change from being merged label Sep 30, 2022

szewai force-pushed the eng/REGRESSION-iOS-16-LocalStorage-is-cleared-after-writing-data-of-more-than-2-5MB branch from 7fe083a to b7e1da2 Compare September 30, 2022 19:00

szewai added the merge-queue Applied to send a pull request to merge-queue label Sep 30, 2022

webkit-early-warning-system force-pushed the eng/REGRESSION-iOS-16-LocalStorage-is-cleared-after-writing-data-of-more-than-2-5MB branch from b7e1da2 to 0297f12 Compare September 30, 2022 20:37

webkit-early-warning-system merged commit 0297f12 into WebKit:main Sep 30, 2022

webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Sep 30, 2022

szewai deleted the eng/REGRESSION-iOS-16-LocalStorage-is-cleared-after-writing-data-of-more-than-2-5MB branch November 14, 2022 20:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

REGRESSION (iOS 16): LocalStorage writes fail after ~2.5MB #4758

REGRESSION (iOS 16): LocalStorage writes fail after ~2.5MB #4758

szewai commented Sep 27, 2022 •

edited by webkit-early-warning-system

Loading

webkit-early-warning-system commented Sep 27, 2022 •

edited

Loading

webkit-early-warning-system commented Sep 29, 2022 •

edited

Loading

webkit-early-warning-system commented Sep 29, 2022 •

edited

Loading

geoffreygaren commented Sep 29, 2022

szewai commented Sep 29, 2022 •

edited

Loading

geoffreygaren commented Sep 29, 2022

webkit-early-warning-system commented Sep 30, 2022 •

edited

Loading

webkit-early-warning-system commented Sep 30, 2022 •

edited

Loading

cdumez left a comment

webkit-early-warning-system commented Sep 30, 2022 •

edited

Loading

webkit-commit-queue commented Sep 30, 2022

REGRESSION (iOS 16): LocalStorage writes fail after ~2.5MB #4758

REGRESSION (iOS 16): LocalStorage writes fail after ~2.5MB #4758

Conversation

szewai commented Sep 27, 2022 • edited by webkit-early-warning-system Loading

0297f12

webkit-early-warning-system commented Sep 27, 2022 • edited Loading

webkit-early-warning-system commented Sep 29, 2022 • edited Loading

webkit-early-warning-system commented Sep 29, 2022 • edited Loading

geoffreygaren commented Sep 29, 2022

szewai commented Sep 29, 2022 • edited Loading

geoffreygaren commented Sep 29, 2022

webkit-early-warning-system commented Sep 30, 2022 • edited Loading

webkit-early-warning-system commented Sep 30, 2022 • edited Loading

cdumez left a comment

Choose a reason for hiding this comment

webkit-early-warning-system commented Sep 30, 2022 • edited Loading

webkit-commit-queue commented Sep 30, 2022

szewai commented Sep 27, 2022 •

edited by webkit-early-warning-system

Loading

`0297f12`

webkit-early-warning-system commented Sep 27, 2022 •

edited

Loading

webkit-early-warning-system commented Sep 29, 2022 •

edited

Loading

webkit-early-warning-system commented Sep 29, 2022 •

edited

Loading

szewai commented Sep 29, 2022 •

edited

Loading

webkit-early-warning-system commented Sep 30, 2022 •

edited

Loading

webkit-early-warning-system commented Sep 30, 2022 •

edited

Loading

webkit-early-warning-system commented Sep 30, 2022 •

edited

Loading