Bug 38228 – [chromium] Add WebFileSystem interface and hook up with all FileSystem methods

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Jian Li]

Add WebFileSystem interface and hook up with all FileSystem methods

[Jian Li]

Created attachment 54474 [details]
Proposed Patch

[Jian Li]

Created attachment 54475 [details]
Proposed Patch

Fixed style errors.

[Jian Li]

Created attachment 54655 [details]
Proposed Patch

Updated the patch to keep existing file system methods to call WebKitClient methods, instead of new WebFileSystem methods.

[WebKit Review Bot]

Attachment 54655 [details] did not build on chromium:
Build output: http://webkit-commit-queue.appspot.com/results/1827167

[Jian Li]

Created attachment 54720 [details]
Proposed Patch

Fixed chromium bot error by leaving out the calls of WebFileSystem methods.

[WebKit Review Bot]

Attachment 54720 [details] did not build on chromium:
Build output: http://webkit-commit-queue.appspot.com/results/1849200

[Jian Li]

Created attachment 54730 [details]
Proposed Patch

Added the new file not included last time.

[Darin Fisher (:fishd, Google)]

Comment on attachment 54730 [details]
Proposed Patch

WebKit/chromium/public/WebFileSystem.h:61
 +      virtual void closeFile(FileHandle&) = 0;
so closeFile will reset the FileHandle to a special invalid value, but there does not appear to be any method here to initialize a FileHandle to hold the invalid value.  it would be nice if closeFile had some documentation describing this subtle behavior.

WebKit/chromium/public/WebFileSystem.h:49
 +      virtual bool fileExists(const WebString& path) = 0;
please provide default implementations for all of these methods.  that way you can implement ChromiumBridge methods in terms of these without having to land something on the Chromium side.  in general, any method implemented by the embedder should have a default implementation in the WebKit API.

WebKit/chromium/src/ChromiumBridge.cpp:328
 +      return webKitClient()->filePathToURL(path);
you can modify all of the existing file-related ChromiumBridge methods to first null-test fileSystem().  if that is null, then fallback to the old methods.

[Jian Li]

Created attachment 54763 [details]
Proposed Patch

All fixed.

[Jian Li]

Created attachment 54764 [details]
Proposed Patch

All fixed. Also checked patch option which I forgot to check last time.

[WebKit Review Bot]

Attachment 54764 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1
WebKit/chromium/src/ChromiumBridge.cpp:274:  An else statement can be removed when the prior "if" concludes with a return, break, continue or goto statement.  [readability/control_flow] [4]
WebKit/chromium/src/ChromiumBridge.cpp:282:  An else statement can be removed when the prior "if" concludes with a return, break, continue or goto statement.  [readability/control_flow] [4]
WebKit/chromium/src/ChromiumBridge.cpp:290:  An else statement can be removed when the prior "if" concludes with a return, break, continue or goto statement.  [readability/control_flow] [4]
WebKit/chromium/src/ChromiumBridge.cpp:298:  An else statement can be removed when the prior "if" concludes with a return, break, continue or goto statement.  [readability/control_flow] [4]
WebKit/chromium/src/ChromiumBridge.cpp:320:  An else statement can be removed when the prior "if" concludes with a return, break, continue or goto statement.  [readability/control_flow] [4]
WebKit/chromium/src/ChromiumBridge.cpp:328:  An else statement can be removed when the prior "if" concludes with a return, break, continue or goto statement.  [readability/control_flow] [4]
WebKit/chromium/src/ChromiumBridge.cpp:336:  An else statement can be removed when the prior "if" concludes with a return, break, continue or goto statement.  [readability/control_flow] [4]
WebKit/chromium/src/ChromiumBridge.cpp:344:  An else statement can be removed when the prior "if" concludes with a return, break, continue or goto statement.  [readability/control_flow] [4]
WebKit/chromium/src/ChromiumBridge.cpp:352:  An else statement can be removed when the prior "if" concludes with a return, break, continue or goto statement.  [readability/control_flow] [4]
WebKit/chromium/src/ChromiumBridge.cpp:360:  An else statement can be removed when the prior "if" concludes with a return, break, continue or goto statement.  [readability/control_flow] [4]
Total errors found: 10 in 7 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Jian Li]

Created attachment 54767 [details]
Proposed Patch

Fixed style errors.

[WebKit Review Bot]

Attachment 54763 [details] did not build on chromium:
Build output: http://webkit-commit-queue.appspot.com/results/1857203

[Jian Li]

Created attachment 54770 [details]
Proposed Patch

Fixed chromium build errors.

[WebKit Review Bot]

Attachment 54767 [details] did not build on chromium:
Build output: http://webkit-commit-queue.appspot.com/results/1857204

[Darin Fisher (:fishd, Google)]

Comment on attachment 54770 [details]
Proposed Patch

WebKit/chromium/public/WebFileSystem.h:60
 +      // Should set the FileHandle to a inavalid value if the file is closed successfully.
inavalid -> invalid

R=me

[Jian Li]

Fixed and landed as http://trac.webkit.org/changeset/58599.