Bug 105307 – 2 fast/filesystem test cases ask for more space than the size they request when creating the file system

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Lyon Chen]

Following two test cases ask for more space than they request when making the request to open a file system:
LayoutTests/fast/filesystem/file-writer-abort-continue.html
LayoutTests/fast/filesystem/op-get-metadata.html

For the first test case, in file fast/filesystem/resources/file-writer-abort-continue.js, following code is called to create a WebFileSystem with 2M size:

setupAndRunTest(2*1024*1024, ...),

but then in one of the test, a length of 2200000 is requested as shown below, with blobSize defined as 1100000.

    verifyLength : blobSize * 2, // Add in leftovers from previous method.

For the second test case, in file fast/filesystem/resources/op-tests-helper.js, following code is called to create a WebFileSystem with 100 bytes size:

webkitRequestFileSystem(TEMPORARY, 100, fileSystemCallback, errorCallback);

But then in one of the test file in fast/filesystem/resources/op-get-metadata.js, an 100 bytes file is requested, after a file with 10 bytes size is created.

            {fullPath:'/file3', size:100},

Right now the size used in requestFileSystem() is not enforced, but for platform want to enforce this limit, these 2 tests will fail because the space over usage.

[Lyon Chen]

Created attachment 179952 [details]
patch for update these 2 test cases

[Lyon Chen]

Add original author of these 2 test cases: Eric, Kinuko Yasuda

[Eric U.]

Comment on attachment 179952 [details]
patch for update these 2 test cases

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

> LayoutTests/fast/filesystem/resources/file-writer-abort-continue.js:-177
> -setupAndRunTest(2*1024*1024, 'file-writer-abort',

Why not key this directly off blobSize, e.g. make it "blobSize * 2 + someWiggleRoomForOverhead"?  Different implementations will have different overhead costs [between zero and e.g. an amount used for e.g. storing path names], but it seems safer to use blobSize than to put in an undocumented number that will go stale or just be wrong.  Speaking from experience, obviously, as I wrote this bug ;'>.

[Lyon Chen]

(In reply to comment #3)
> (From update of attachment 179952 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=179952&action=review
> 
> > LayoutTests/fast/filesystem/resources/file-writer-abort-continue.js:-177
> > -setupAndRunTest(2*1024*1024, 'file-writer-abort',
> 
> Why not key this directly off blobSize, e.g. make it "blobSize * 2 + someWiggleRoomForOverhead"?  Different implementations will have different overhead costs [between zero and e.g. an amount used for e.g. storing path names], but it seems safer to use blobSize than to put in an undocumented number that will go stale or just be wrong.  Speaking from experience, obviously, as I wrote this bug ;'>.

Thanks Eric. But the problem is then how to decide the size of the overhead? Does using a percentage, like 8/512 (8 bytes per 512 byte block), make sense to you?

[Eric U.]

> Thanks Eric. But the problem is then how to decide the size of the overhead? Does using a percentage, like 8/512 (8 bytes per 512 byte block), make sense to you?

You can see what chromium does in UsageForPath at http://code.google.com/searchframe#OAMlx_jo-ck/src/webkit/fileapi/obfuscated_file_util.cc&exact_package=chromium&q=UsageForPath

We charge overhead per file plus per character of name.  Whatever you put in will certainly be approximate, but anything's better than just a random number, and if you make your fudge factor large, it won't break for a long time.  And then whoever's got a problem with it can fix it.

[Lyon Chen]

Created attachment 180213 [details]
updated patch for 105307.

Updates based on Eric's comments.

[Eric U.]

Comment on attachment 180213 [details]
updated patch for 105307.

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

> LayoutTests/fast/filesystem/resources/file-writer-abort-continue.js:23
> +var fileSystemSize = blobSize * 2 + fileSystemOverhead;

That ought to be plenty; thanks for the fix.

[Lyon Chen]

(In reply to comment #7)
> (From update of attachment 180213 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=180213&action=review
> 
> > LayoutTests/fast/filesystem/resources/file-writer-abort-continue.js:23
> > +var fileSystemSize = blobSize * 2 + fileSystemOverhead;
> 
> That ought to be plenty; thanks for the fix.

And thanks for the comment and info, Eric.

[Yong Li]

Comment on attachment 180213 [details]
updated patch for 105307.

r+ according to Eric's comments. Thanks Eric

[WebKit Review Bot]

Comment on attachment 180213 [details]
updated patch for 105307.

Clearing flags on attachment: 180213

Committed r138257: <http://trac.webkit.org/changeset/138257>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.