Bug 60558

Created attachment 92974 [details] Patch Uploading patch for initial review. Tests forthcoming.

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.

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.

(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://".

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://".

Created attachment 93042 [details] Patch

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.

Comment on attachment 93042 [details] Patch Clearing flags on attachment: 93042 Committed r86205: <http://trac.webkit.org/changeset/86205>

All reviewed patches have been landed. Closing bug.

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. :(