Bug 89981 – Add tests to verify Write operation is disallowed in dropped filesystem

• Status: NEW
• Component: WebCore Misc.

[Kinuko Yasuda]

Add tests to verify Write operation is disallowed in dropped filesystem.

Any write operations must fail on dropped FileEntry and DirectoryEntry.

[Kinuko Yasuda]

Created attachment 149533 [details]
Patch

[Kinuko Yasuda]

Created attachment 149535 [details]
Patch

[Yuta Kitamura]

Comment on attachment 149535 [details]
Patch

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

It seems the test is well written. I just have a bunch of style issues. r- because of them.

> LayoutTests/fast/filesystem/dropped-filesystem-non-writable.html:43
> +function expect(value, msg)

nit: msg -> message?

A few other functions that also take |msg| should be fixed, too.

> LayoutTests/fast/filesystem/dropped-filesystem-non-writable.html:65
> +function startTest(e)

nit: e -> event, maybe?

> LayoutTests/fast/filesystem/dropped-filesystem-non-writable.html:74
> +      expect('file' == items[i].kind, 'kind is "file"');

Wrong indent.

> LayoutTests/fast/filesystem/dropped-filesystem-non-writable.html:117
> +    window[test](entry, function() { testFile(entry, testIndex+1, callback); });

nit: spaces needed around a "+". Also in line 128.

> LayoutTests/fast/filesystem/dropped-filesystem-non-writable.html:143
> +    entry.getFile('child-file', {create:true}, unexpectSuccess(), expectError(callback, 'getFile(create:true)'));

"{ create: true }" may be better (spaces within braces). I'm not really sure if this is a common WebKit style, but at least you need to be self-consistent with your code in line 11-12.

This also happens below (in line 148).

> LayoutTests/fast/filesystem/dropped-filesystem-non-writable.html:189
> +              expect(progress.target.error.code == 2, 'writer: Got expected security error');

Indent.

> LayoutTests/fast/filesystem/dropped-filesystem-non-writable.html:201
> +        expect(error.code == FileError.SECURITY_ERR || error.code == FileError.INVALID_MODIFICATION_ERR, msg + ': got expected error');

Are these error codes (SECURITY_ERR or INVALID_MODIFICATION_ERR) returned in a deterministic way? If so, it's probably better to log the value of |error.code|, too.

> LayoutTests/fast/filesystem/dropped-filesystem-non-writable.html:206
> +function unexpectSuccess()

"successUnexpectedly()" or "unexpectedSuccess()" may sound better. "Unexpect" does not seem a valid verb.