Cleanup: Move FileSystem API type definitions into a separate header file. Currently they are defined in AsyncFileSystem.h (and AsyncFileSystemChromium.cpp for chromium), but by moving them into a separate header file we can: - make it clearer that all FileSystem API types can be found in the single separate header file - save including entire AsyncFileSystem definition just for referring type enum values
Created attachment 140424 [details] Patch
Another patch separated out from http://webkit.org/b/84135 (for cross-filesystem support).
Created attachment 140429 [details] Patch
Comment on attachment 140429 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=140429&action=review > Source/WebKit/chromium/src/AsyncFileSystemChromium.cpp:59 > const size_t externalPathPrefixLength = sizeof(externalPathPrefix) - 1; I think the one place in the code that used this went away. Feel free to delete it.
Comment on attachment 140429 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=140429&action=review > Source/WebCore/platform/FileSystemType.h:1 > +/* I'm surprised that there aren't some build file changes to go along with this. it would be nice to include it in build files. (As long as this not being the those files doesn't break anything and it doesn't seem to, I would be fine with that as a follow up patch.)
(In reply to comment #5) > (From update of attachment 140429 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=140429&action=review > > > Source/WebCore/platform/FileSystemType.h:1 > > +/* > > I'm surprised that there aren't some build file changes to go along with this. > > it would be nice to include it in build files. (As long as this not being the those files doesn't break anything and it doesn't seem to, I would be fine with that as a follow up patch.) Yes I should have added the new header to the build files. I'm going to make a follow up patch (unless this breaks other builds on waterfall).
Committed r116280: <http://trac.webkit.org/changeset/116280>
Comment on attachment 140429 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=140429&action=review > Source/WebKit/chromium/src/AsyncFileSystemChromium.cpp:124 > return true; This cleanup didn't make it through your CL-splitting quite right. Now if we get an invalid URL that has no inner_url, we'll return true; it should be false.
(In reply to comment #8) > (From update of attachment 140429 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=140429&action=review > > > Source/WebKit/chromium/src/AsyncFileSystemChromium.cpp:124 > > return true; > > This cleanup didn't make it through your CL-splitting quite right. Now if we get an invalid URL that has no inner_url, we'll return true; it should be false. Thanks for looking it over again Eric!
Thanks Eric, I'm uploading a follow-up fix.
(In reply to comment #10) > Thanks Eric, I'm uploading a follow-up fix. Looks like the new patch (which is implementing crackFileSystemURL in DOMFileSystemChromium) seems to be handling this part correctly. (Returning earlier if url.innerURL() is null) Closing this again... thanks for the catch.