Bug 139996 – Get rid of some PageGroup storage functions

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Anders Carlsson]

Get rid of some PageGroup storage functions

[Anders Carlsson]

Created attachment 243809 [details]
Patch

[Anders Carlsson]

Committed r177814: <http://trac.webkit.org/changeset/177814>

[Alexey Proskuryakov]

This changed some test results:

https://build.webkit.org/results/Apple%20Mavericks%20Release%20WK1%20(Tests)/r177825%20(10978)/storage/domstorage/localstorage/storagetracker/storage-tracker-4-create-diff.txt

https://build.webkit.org/results/Apple%20Mavericks%20Release%20WK1%20(Tests)/r177825%20(10978)/storage/domstorage/localstorage/storagetracker/storage-tracker-6-create-diff.txt

The tests used to be flaky timeouts, but this doesn't seem expected.

[Alexey Proskuryakov]

Updated test expectations in r177830.

Anders, do you know what these tests are flaky in the first place? Each DumpRenderTree instance has its own local storage on disk, so what is the source of flakiness?

And why did this patch change the tests to fail instead of time out? There is nothing in ChangeLog that explains behavior change. Should this be rolled out instead?

[Anders Carlsson]

I'm taking a look at this now.

[Anders Carlsson]

OK, I've looked at the tests and they're pretty badly written.

They expect to be run in order, and will randomly fail if they're not. 

I tried merging the tests into a single file, but due to the way the storage tracker SPI works, there's no way to make the tests not be flaky.

Then I tried rewriting them into an API test, but for the same API "design" reasons, there's no easy way to write a non-flaky test.

Given this, and given that this is a test for a legacy SPI that nobody is using anymore I think it's fine to just remove the tests and the associated DumpRenderTree code.

[Alexey Proskuryakov]

It's fine with me to remove the tests, however they do always run sequentially and in order (unless one forces a non-default order, which bots don't do).

Is it still expected that they are flaky, given that?

[Anders Carlsson]

(In reply to comment #7)
> It's fine with me to remove the tests, however they do always run
> sequentially and in order (unless one forces a non-default order, which bots
> don't do).
> 
> Is it still expected that they are flaky, given that?

If multiple tests were running at the same time and they were touching storage, then yes - they could be flaky.

[Alexey Proskuryakov]

> If multiple tests were running at the same time and they were touching storage, then yes - they could be flaky.

That would be horrible. What is the mechanism that shares state across DumpRenderTree/WebKitTestRunner instances? Can anything else be shared (e.g. the actual records)?

Disk storage at least is supposed to be in unique directories for each process.