WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
60558
StorageTracker should report actual local storage usage on disk
https://bugs.webkit.org/show_bug.cgi?id=60558
Summary
StorageTracker should report actual local storage usage on disk
Anton D'Auria
Reported
2011-05-10 10:13:19 PDT
Local Storage currently checks quota against key value pairs in memory. We need to provide an API to get usage on disk even if the key/value pairs haven't been loaded for an origin.
Attachments
Patch
(7.27 KB, patch)
2011-05-10 10:29 PDT
,
Anton D'Auria
no flags
Details
Formatted Diff
Diff
Patch
(23.08 KB, patch)
2011-05-10 13:12 PDT
,
Anton D'Auria
no flags
Details
Formatted Diff
Diff
Patch
(23.07 KB, patch)
2011-05-10 14:46 PDT
,
Anton D'Auria
no flags
Details
Formatted Diff
Diff
Patch
(23.45 KB, patch)
2011-05-10 16:28 PDT
,
Anton D'Auria
no flags
Details
Formatted Diff
Diff
Show Obsolete
(3)
View All
Add attachment
proposed patch, testcase, etc.
Anton D'Auria
Comment 1
2011-05-10 10:29:38 PDT
Created
attachment 92974
[details]
Patch Uploading patch for initial review. Tests forthcoming.
Anton D'Auria
Comment 2
2011-05-10 13:12:13 PDT
Created
attachment 92998
[details]
Patch Adding tests and stubs. No need to add tests to skip lists on other platforms because storagetracker directory is already skipped in its entirety on all platforms except Mac.
David Levin
Comment 3
2011-05-10 14:13:41 PDT
Comment on
attachment 92998
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=92998&action=review
> Source/WebCore/storage/StorageTracker.cpp:573 > + return 0.0f;
Do you need to do 0.0f here as opposed to 0? (especially because long long is an int type).
> Source/WebCore/storage/StorageTracker.cpp:579 > + return 0.0f;
Ditto.
> LayoutTests/storage/domstorage/localstorage/storagetracker/storage-tracker-6-create-expected.txt:2 > +StorageTracker test - write local storage for this origin. Should be called after storage-tracker-5-delete-one.html.
Interdependencies among tests are really bad. Each test should stand on its own for lots of reasons. The simplest reason is that you can't guarantee that they will be run in the same instance of DRT. Other reasons are that it makes repro's harder. (Also if we can make tests completely independent then we'll be able to parallelize the running of them better and make the test runs go faster.)
> LayoutTests/storage/domstorage/localstorage/storagetracker/storage-tracker-7-usage.html:21 > + shouldBeEqualToString("layoutTestController.originsWithLocalStorage().toString()", "file__0");
Why is this "file__0"? I suspect this is port specific. If so, it would be better to have it as test output (instead of hard coded in the test) so the various ports could have their own results.
Anton D'Auria
Comment 4
2011-05-10 14:32:33 PDT
(In reply to
comment #3
)
> (From update of
attachment 92998
[details]
) > View in context:
https://bugs.webkit.org/attachment.cgi?id=92998&action=review
> > > Source/WebCore/storage/StorageTracker.cpp:573 > > + return 0.0f; > > Do you need to do 0.0f here as opposed to 0? (especially because long long is an int type).
This is a mistake, thanks.
> > LayoutTests/storage/domstorage/localstorage/storagetracker/storage-tracker-6-create-expected.txt:2 > > +StorageTracker test - write local storage for this origin. Should be called after storage-tracker-5-delete-one.html. > > Interdependencies among tests are really bad. > > Each test should stand on its own for lots of reasons. The simplest reason is that you can't guarantee that they will be run in the same instance of DRT. > > Other reasons are that it makes repro's harder. (Also if we can make tests completely independent then we'll be able to parallelize the running of them better and make the test runs go faster.)
Unfortunately, there is no test harness for async WebCore APIs in WebKit1. We previously agreed to these sequential tests for StorageTracker will suffice, and I followed that precedent.
> > > LayoutTests/storage/domstorage/localstorage/storagetracker/storage-tracker-7-usage.html:21 > > + shouldBeEqualToString("layoutTestController.originsWithLocalStorage().toString()", "file__0"); > > Why is this "file__0"? I suspect this is port specific. If so, it would be better to have it as test output (instead of hard coded in the test) so the various ports could have their own results.
This is the cross-platform origin identifier (from WebCore::SecurityOrigin) for "file://".
Anton D'Auria
Comment 5
2011-05-10 14:46:28 PDT
Created
attachment 93010
[details]
Patch Removing 0.0f. Not changing tests because LocalStorage is written to disk asynchronously and we need to guarantee the db has content before getting its size. Not changing "file__0" because that's the WebCore::SecurityOrigin identifier for "file://".
Anton D'Auria
Comment 6
2011-05-10 16:28:09 PDT
Created
attachment 93042
[details]
Patch
WebKit Commit Bot
Comment 7
2011-05-10 18:53:21 PDT
The commit-queue encountered the following flaky tests while processing
attachment 93042
[details]
: http/tests/xmlhttprequest/remember-bad-password.html
bug 51733
(author:
ap@webkit.org
) The commit-queue is continuing to process your patch.
WebKit Commit Bot
Comment 8
2011-05-10 18:53:42 PDT
Comment on
attachment 93042
[details]
Patch Clearing flags on attachment: 93042 Committed
r86205
: <
http://trac.webkit.org/changeset/86205
>
WebKit Commit Bot
Comment 9
2011-05-10 18:53:47 PDT
All reviewed patches have been landed. Closing bug.
Eric Seidel (no email)
Comment 10
2011-09-07 11:46:21 PDT
According to TestFailures:
http://build.webkit.org/TestFailures/#/SnowLeopard
Intel Release (Tests) storage/domstorage/localstorage/storagetracker/storage-tracker-6-create.html: pretty diff (flaky) storage/domstorage/localstorage/storagetracker/storage-tracker-7-usage.html: pretty diff are both flaky/failing on Snow Leopard Mac. Please remove the tests or fix them. :(
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug