105307 2012-12-18 08:21:43 -0800 2 fast/filesystem test cases ask for more space than the size they request when creating the file system 2012-12-20 08:23:48 -0800 1 1 1 Unclassified WebKit Tools / Tests 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 liachen liachen ericu joenotcharles kinuko leoyang rwlbuis webkit.review.bot yong.li.webkit oldest_to_newest 793672 0 liachen 2012-12-18 08:21:43 -0800 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. 793694 1 179952 liachen 2012-12-18 08:42:35 -0800 Created attachment 179952 patch for update these 2 test cases 793701 2 liachen 2012-12-18 08:47:01 -0800 Add original author of these 2 test cases: Eric, Kinuko Yasuda 794073 3 179952 ericu 2012-12-18 14:42:27 -0800 Comment on attachment 179952 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 ;'>. 794779 4 liachen 2012-12-19 10:07:30 -0800 (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? 794814 5 ericu 2012-12-19 11:09:33 -0800 > 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. 794874 6 180213 liachen 2012-12-19 13:06:20 -0800 Created attachment 180213 updated patch for 105307. Updates based on Eric's comments. 794877 7 180213 ericu 2012-12-19 13:14:55 -0800 Comment on attachment 180213 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. 794889 8 liachen 2012-12-19 13:29:21 -0800 (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. 795455 9 180213 yong.li.webkit 2012-12-20 08:16:20 -0800 Comment on attachment 180213 updated patch for 105307. r+ according to Eric's comments. Thanks Eric 795457 10 180213 webkit.review.bot 2012-12-20 08:23:44 -0800 Comment on attachment 180213 updated patch for 105307. Clearing flags on attachment: 180213 Committed r138257: <http://trac.webkit.org/changeset/138257> 795458 11 webkit.review.bot 2012-12-20 08:23:48 -0800 All reviewed patches have been landed. Closing bug. 179952 2012-12-18 08:42:35 -0800 2012-12-19 13:06:20 -0800 patch for update these 2 test cases 105307.patch text/plain 2157 liachen ZGlmZiAtLWdpdCBhL0xheW91dFRlc3RzL0NoYW5nZUxvZyBiL0xheW91dFRlc3RzL0NoYW5nZUxv ZwppbmRleCA0Yzg0MGY1Li5iNDQ1ZjQxIDEwMDY0NAotLS0gYS9MYXlvdXRUZXN0cy9DaGFuZ2VM b2cKKysrIGIvTGF5b3V0VGVzdHMvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTcgQEAKKzIwMTItMTIt MTggIExpYW5naHVpIENoZW4gIDxsaWFjaGVuQHJpbS5jb20+CisKKyAgICAgICAgMiBmYXN0L2Zp bGVzeXN0ZW0gdGVzdCBjYXNlcyBhc2sgZm9yIG1vcmUgc3BhY2UgdGhhbiB0aGUgc2l6ZSB0aGV5 IHJlcXVlc3Qgd2hlbiBjcmVhdGluZyB0aGUgZmlsZSBzeXN0ZW0KKyAgICAgICAgaHR0cHM6Ly9i dWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTEwNTMwNworCisgICAgICAgIFJldmlld2Vk IGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIFRoZSBwdXJwb3NlIG9mIHRoaXMgcGF0Y2gg aXMgdG8gbWFrZSB0aGVzZSAyIHRlc3QgY2FzZXMgY29uZm9ybSB0byB0aGUKKyAgICAgICAgc2l6 ZSBsaW1pdCwgc28gZm9yIHBsYXRmb3JtIHRoYXQgd2FudHMgdG8gZW5mb3JjZSB0aGUgc2l6ZSBs aW1pdCB0aGV5CisgICAgICAgIHdpbGwgbm90IGZhaWwgZHVlIHRvIHF1b3RhIGxpbWl0LgorCisg ICAgICAgICogZmFzdC9maWxlc3lzdGVtL3Jlc291cmNlcy9maWxlLXdyaXRlci1hYm9ydC1jb250 aW51ZS5qczoKKyAgICAgICAgKiBmYXN0L2ZpbGVzeXN0ZW0vcmVzb3VyY2VzL29wLWdldC1tZXRh ZGF0YS5qczoKKwogMjAxMi0xMi0xOCAgVy4gSmFtZXMgTWFjTGVhbiAgPHdqbWFjbGVhbkBjaHJv bWl1bS5vcmc+CiAKICAgICAgICAgW2Nocm9taXVtXSBVbnJldmlld2VkIGdhcmRlbmluZzogbWFy a2luZyB0ZXN0IGFzIGZsYWt5IG9uIExpbnV4IERiZyBib3QuCmRpZmYgLS1naXQgYS9MYXlvdXRU ZXN0cy9mYXN0L2ZpbGVzeXN0ZW0vcmVzb3VyY2VzL2ZpbGUtd3JpdGVyLWFib3J0LWNvbnRpbnVl LmpzIGIvTGF5b3V0VGVzdHMvZmFzdC9maWxlc3lzdGVtL3Jlc291cmNlcy9maWxlLXdyaXRlci1h Ym9ydC1jb250aW51ZS5qcwppbmRleCBiZGQ0NmJmLi5iM2RhYjI5IDEwMDY0NAotLS0gYS9MYXlv dXRUZXN0cy9mYXN0L2ZpbGVzeXN0ZW0vcmVzb3VyY2VzL2ZpbGUtd3JpdGVyLWFib3J0LWNvbnRp bnVlLmpzCisrKyBiL0xheW91dFRlc3RzL2Zhc3QvZmlsZXN5c3RlbS9yZXNvdXJjZXMvZmlsZS13 cml0ZXItYWJvcnQtY29udGludWUuanMKQEAgLTE3NCw3ICsxNzQsNyBAQCBmdW5jdGlvbiBydW5U ZXN0KCkgewogfQogCiB2YXIganNUZXN0SXNBc3luYyA9IHRydWU7Ci1zZXR1cEFuZFJ1blRlc3Qo MioxMDI0KjEwMjQsICdmaWxlLXdyaXRlci1hYm9ydCcsCitzZXR1cEFuZFJ1blRlc3QoMyoxMDI0 KjEwMjQsICdmaWxlLXdyaXRlci1hYm9ydCcsCiAgICAgICAgICAgICAgICAgZnVuY3Rpb24gKGZp bGVFbnRyeSwgZmlsZVdyaXRlcikgewogICAgICAgICAgICAgICAgICAgICBmaWxlRW50cnlGb3JD bGVhbnVwID0gZmlsZUVudHJ5OwogICAgICAgICAgICAgICAgICAgICB3cml0ZXIgPSBmaWxlV3Jp dGVyOwpkaWZmIC0tZ2l0IGEvTGF5b3V0VGVzdHMvZmFzdC9maWxlc3lzdGVtL3Jlc291cmNlcy9v cC1nZXQtbWV0YWRhdGEuanMgYi9MYXlvdXRUZXN0cy9mYXN0L2ZpbGVzeXN0ZW0vcmVzb3VyY2Vz L29wLWdldC1tZXRhZGF0YS5qcwppbmRleCAzYzI5MWYzLi44MzU2MDMwIDEwMDY0NAotLS0gYS9M YXlvdXRUZXN0cy9mYXN0L2ZpbGVzeXN0ZW0vcmVzb3VyY2VzL29wLWdldC1tZXRhZGF0YS5qcwor KysgYi9MYXlvdXRUZXN0cy9mYXN0L2ZpbGVzeXN0ZW0vcmVzb3VyY2VzL29wLWdldC1tZXRhZGF0 YS5qcwpAQCAtNSw3ICs1LDcgQEAgdmFyIHRlc3RDYXNlcyA9IFsKICAgICAgICAgICAgIHtmdWxs UGF0aDonL3RtcCd9LAogICAgICAgICAgICAge2Z1bGxQYXRoOicvZmlsZTEnLCBzaXplOjB9LAog ICAgICAgICAgICAge2Z1bGxQYXRoOicvZmlsZTInLCBzaXplOjEwfSwKLSAgICAgICAgICAgIHtm dWxsUGF0aDonL2ZpbGUzJywgc2l6ZToxMDB9LAorICAgICAgICAgICAge2Z1bGxQYXRoOicvZmls ZTMnLCBzaXplOjkwfSwKICAgICAgICAgXSwKICAgICAgICAgdGVzdHM6IFsKICAgICAgICAgICAg IGZ1bmN0aW9uKGhlbHBlcikgeyBoZWxwZXIuZ2V0TWV0YWRhdGEoJy8nKTsgfSwK 180213 2012-12-19 13:06:20 -0800 2012-12-20 08:23:44 -0800 updated patch for 105307. 105307-2.patch text/plain 2665 liachen ZGlmZiAtLWdpdCBhL0xheW91dFRlc3RzL0NoYW5nZUxvZyBiL0xheW91dFRlc3RzL0NoYW5nZUxv ZwppbmRleCA0Yzg0MGY1Li5iNDQ1ZjQxIDEwMDY0NAotLS0gYS9MYXlvdXRUZXN0cy9DaGFuZ2VM b2cKKysrIGIvTGF5b3V0VGVzdHMvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTcgQEAKKzIwMTItMTIt MTggIExpYW5naHVpIENoZW4gIDxsaWFjaGVuQHJpbS5jb20+CisKKyAgICAgICAgMiBmYXN0L2Zp bGVzeXN0ZW0gdGVzdCBjYXNlcyBhc2sgZm9yIG1vcmUgc3BhY2UgdGhhbiB0aGUgc2l6ZSB0aGV5 IHJlcXVlc3Qgd2hlbiBjcmVhdGluZyB0aGUgZmlsZSBzeXN0ZW0KKyAgICAgICAgaHR0cHM6Ly9i dWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTEwNTMwNworCisgICAgICAgIFJldmlld2Vk IGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIFRoZSBwdXJwb3NlIG9mIHRoaXMgcGF0Y2gg aXMgdG8gbWFrZSB0aGVzZSAyIHRlc3QgY2FzZXMgY29uZm9ybSB0byB0aGUKKyAgICAgICAgc2l6 ZSBsaW1pdCwgc28gZm9yIHBsYXRmb3JtIHRoYXQgd2FudHMgdG8gZW5mb3JjZSB0aGUgc2l6ZSBs aW1pdCB0aGV5CisgICAgICAgIHdpbGwgbm90IGZhaWwgZHVlIHRvIHF1b3RhIGxpbWl0LgorCisg ICAgICAgICogZmFzdC9maWxlc3lzdGVtL3Jlc291cmNlcy9maWxlLXdyaXRlci1hYm9ydC1jb250 aW51ZS5qczoKKyAgICAgICAgKiBmYXN0L2ZpbGVzeXN0ZW0vcmVzb3VyY2VzL29wLWdldC1tZXRh ZGF0YS5qczoKKwogMjAxMi0xMi0xOCAgVy4gSmFtZXMgTWFjTGVhbiAgPHdqbWFjbGVhbkBjaHJv bWl1bS5vcmc+CiAKICAgICAgICAgW2Nocm9taXVtXSBVbnJldmlld2VkIGdhcmRlbmluZzogbWFy a2luZyB0ZXN0IGFzIGZsYWt5IG9uIExpbnV4IERiZyBib3QuCmRpZmYgLS1naXQgYS9MYXlvdXRU ZXN0cy9mYXN0L2ZpbGVzeXN0ZW0vcmVzb3VyY2VzL2ZpbGUtd3JpdGVyLWFib3J0LWNvbnRpbnVl LmpzIGIvTGF5b3V0VGVzdHMvZmFzdC9maWxlc3lzdGVtL3Jlc291cmNlcy9maWxlLXdyaXRlci1h Ym9ydC1jb250aW51ZS5qcwppbmRleCBiZGQ0NmJmLi41MjJjNDM0IDEwMDY0NAotLS0gYS9MYXlv dXRUZXN0cy9mYXN0L2ZpbGVzeXN0ZW0vcmVzb3VyY2VzL2ZpbGUtd3JpdGVyLWFib3J0LWNvbnRp bnVlLmpzCisrKyBiL0xheW91dFRlc3RzL2Zhc3QvZmlsZXN5c3RlbS9yZXNvdXJjZXMvZmlsZS13 cml0ZXItYWJvcnQtY29udGludWUuanMKQEAgLTE2LDYgKzE2LDEyIEBAIHZhciBibG9iU2l6ZSA9 IDExMDAwMDA7CiB2YXIgY3VycmVudFRlc3QgPSAwOwogdmFyIGJsb2IgPSBnZXRCbG9iKCk7CiAK Ky8vIGZpbGVTeXN0ZW1PdmVyaGVhZCBpcyBhbiBlc3RpbWF0ZSBvZiBleHRyYSBzaXplIG5lZWRl ZCB0byBzYXZlIGEgZnVsbCBmaWxlCisvLyBzeXN0ZW0sIGl0IG5lZWQgdG8gYmUgbGFyZ2UgZW5v dWdoIHRvIGF2b2lkIHRlc3QgZmFpbHVyZSBkdWUgdG8gZmlsZSBzeXN0ZW0KKy8vIHF1b3RhIGxp bWl0LgordmFyIGZpbGVTeXN0ZW1PdmVyaGVhZCA9IGJsb2JTaXplICogNSAvIDEwMDAgKyAxMDI0 OwordmFyIGZpbGVTeXN0ZW1TaXplID0gYmxvYlNpemUgKiAyICsgZmlsZVN5c3RlbU92ZXJoZWFk OworCiB2YXIgbWV0aG9kU2V0ID0gWwogICB7ICAvLyBTZXR1cCBtZXRob2Qgc2V0IHRoYXQgd3Jp dGVzLCB0aGVuIGFib3J0cyB0aGF0IHdyaXRlIGJlZm9yZSBjb21wbGV0aW9uLgogICAgIGFjdGlv biA6IHN0YXJ0V3JpdGUsCkBAIC0xNzQsNyArMTgwLDcgQEAgZnVuY3Rpb24gcnVuVGVzdCgpIHsK IH0KIAogdmFyIGpzVGVzdElzQXN5bmMgPSB0cnVlOwotc2V0dXBBbmRSdW5UZXN0KDIqMTAyNCox MDI0LCAnZmlsZS13cml0ZXItYWJvcnQnLAorc2V0dXBBbmRSdW5UZXN0KGZpbGVTeXN0ZW1TaXpl LCAnZmlsZS13cml0ZXItYWJvcnQnLAogICAgICAgICAgICAgICAgIGZ1bmN0aW9uIChmaWxlRW50 cnksIGZpbGVXcml0ZXIpIHsKICAgICAgICAgICAgICAgICAgICAgZmlsZUVudHJ5Rm9yQ2xlYW51 cCA9IGZpbGVFbnRyeTsKICAgICAgICAgICAgICAgICAgICAgd3JpdGVyID0gZmlsZVdyaXRlcjsK ZGlmZiAtLWdpdCBhL0xheW91dFRlc3RzL2Zhc3QvZmlsZXN5c3RlbS9yZXNvdXJjZXMvb3AtZ2V0 LW1ldGFkYXRhLmpzIGIvTGF5b3V0VGVzdHMvZmFzdC9maWxlc3lzdGVtL3Jlc291cmNlcy9vcC1n ZXQtbWV0YWRhdGEuanMKaW5kZXggM2MyOTFmMy4uODM1NjAzMCAxMDA2NDQKLS0tIGEvTGF5b3V0 VGVzdHMvZmFzdC9maWxlc3lzdGVtL3Jlc291cmNlcy9vcC1nZXQtbWV0YWRhdGEuanMKKysrIGIv TGF5b3V0VGVzdHMvZmFzdC9maWxlc3lzdGVtL3Jlc291cmNlcy9vcC1nZXQtbWV0YWRhdGEuanMK QEAgLTUsNyArNSw3IEBAIHZhciB0ZXN0Q2FzZXMgPSBbCiAgICAgICAgICAgICB7ZnVsbFBhdGg6 Jy90bXAnfSwKICAgICAgICAgICAgIHtmdWxsUGF0aDonL2ZpbGUxJywgc2l6ZTowfSwKICAgICAg ICAgICAgIHtmdWxsUGF0aDonL2ZpbGUyJywgc2l6ZToxMH0sCi0gICAgICAgICAgICB7ZnVsbFBh dGg6Jy9maWxlMycsIHNpemU6MTAwfSwKKyAgICAgICAgICAgIHtmdWxsUGF0aDonL2ZpbGUzJywg c2l6ZTo5MH0sCiAgICAgICAgIF0sCiAgICAgICAgIHRlc3RzOiBbCiAgICAgICAgICAgICBmdW5j dGlvbihoZWxwZXIpIHsgaGVscGVyLmdldE1ldGFkYXRhKCcvJyk7IH0sCg==