FileAPI: Add Initialize() function to FileSystemBackend

FileAPI: Add Initialize() function to FileSystemBackend

This is a preliminary change for adding SyncFileSystemBackend.

At first, we planed to do initializing process for SyncFileSystemBackend in
InitializeFileSystem(), but it revealed that there are some hassles on that
way (see [*]), so we decieded to introduce a separate initialze function and
to rename InitializeFileSystem() to its original name (i.e. OpenFileSystem()).

[*] https://codereview.chromium.org/18668003/


BUG=242422
TEST=should pass all existing tests
NOTRY=true

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=213382

[nhiroki, 2013-07-23 07:21:14]

Hi, can you take a look? Thanks!

[kinuko, 2013-07-23 07:33:49]

Can we merge the revert change into this one (i.e. splitting the current
InitializeFileSystem into two: Initialize and OpenFileSystem), or land the
revert change first?  Having both Initialize and InitializeFileSystem look very
chaotic.

https://codereview.chromium.org/19632004/diff/5001/webkit/browser/fileapi/fil...
File webkit/browser/fileapi/file_system_backend.h (right):

https://codereview.chromium.org/19632004/diff/5001/webkit/browser/fileapi/fil...
webkit/browser/fileapi/file_system_backend.h:56: // Initializes the filesystem.
Can you elaborate more?  E.g.

This method if called right after the backend is registered in the
FileSystemContext and before any other methods are called.   Each backend can
implement additional initialization which depends on FileSystemContext.

[nhiroki, 2013-07-23 08:11:22]

Thanks! PTAL.

On 2013/07/23 07:33:49, kinuko wrote:
> Can we merge the revert change into this one (i.e. splitting the current
> InitializeFileSystem into two: Initialize and OpenFileSystem), or land the
> revert change first?  Having both Initialize and InitializeFileSystem look
very
> chaotic.

Merged it into this change.

https://codereview.chromium.org/19632004/diff/5001/webkit/browser/fileapi/fil...
File webkit/browser/fileapi/file_system_backend.h (right):

https://codereview.chromium.org/19632004/diff/5001/webkit/browser/fileapi/fil...
webkit/browser/fileapi/file_system_backend.h:56: // Initializes the filesystem.
On 2013/07/23 07:33:49, kinuko wrote:
> Can you elaborate more?  E.g.
> 
> This method if called right after the backend is registered in the
> FileSystemContext and before any other methods are called.   Each backend can
> implement additional initialization which depends on FileSystemContext.

Done.

[kinuko, 2013-07-23 08:22:15]

lgtm

https://codereview.chromium.org/19632004/diff/15001/webkit/browser/fileapi/fi...
File webkit/browser/fileapi/file_system_backend.h (right):

https://codereview.chromium.org/19632004/diff/15001/webkit/browser/fileapi/fi...
webkit/browser/fileapi/file_system_backend.h:56: // This method should be called
right after the backend is registered in the
should be -> is

https://codereview.chromium.org/19632004/diff/15001/webkit/browser/fileapi/fi...
webkit/browser/fileapi/file_system_backend.h:58: // implement additional
initialization which depends on FileSystemContext.
'implement' -> 'do' might be simpler

can you append ' here' at the end of the sentence?

[nhiroki, 2013-07-23 08:38:08]

Updated!

https://codereview.chromium.org/19632004/diff/15001/webkit/browser/fileapi/fi...
File webkit/browser/fileapi/file_system_backend.h (right):

https://codereview.chromium.org/19632004/diff/15001/webkit/browser/fileapi/fi...
webkit/browser/fileapi/file_system_backend.h:56: // This method should be called
right after the backend is registered in the
On 2013/07/23 08:22:15, kinuko wrote:
> should be -> is

Done.

https://codereview.chromium.org/19632004/diff/15001/webkit/browser/fileapi/fi...
webkit/browser/fileapi/file_system_backend.h:58: // implement additional
initialization which depends on FileSystemContext.
On 2013/07/23 08:22:15, kinuko wrote:
> 'implement' -> 'do' might be simpler
> 
> can you append ' here' at the end of the sentence?

Done.

[nhiroki, 2013-07-23 08:41:24]

+Satoru-san, can you review c/b/cros/fileapi/*? Thanks!

[tzik, 2013-07-24 04:03:11]

lgtm

[satorux1, 2013-07-24 04:57:45]

c/b/chromeos/fileapi LGTM

[commit-bot: I haz the power, 2013-07-24 05:22:22]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/nhiroki@chromium.org/19632004/31001

[commit-bot: I haz the power, 2013-07-24 05:22:27]

Failed to apply patch for
webkit/browser/fileapi/copy_or_move_operation_delegate_unittest.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file
webkit/browser/fileapi/copy_or_move_operation_delegate_unittest.cc
  Hunk #1 FAILED at 72.
  1 out of 1 hunk FAILED -- saving rejects to file
webkit/browser/fileapi/copy_or_move_operation_delegate_unittest.cc.rej

Patch:       webkit/browser/fileapi/copy_or_move_operation_delegate_unittest.cc
Index: webkit/browser/fileapi/copy_or_move_operation_delegate_unittest.cc
diff --git a/webkit/browser/fileapi/copy_or_move_operation_delegate_unittest.cc
b/webkit/browser/fileapi/copy_or_move_operation_delegate_unittest.cc
index
3008a3be42d7ed9db81e807705c975ed8483c4e7..60fb70d5c10a7d48794a5a29b337a1b7b6c56824
100644
--- a/webkit/browser/fileapi/copy_or_move_operation_delegate_unittest.cc
+++ b/webkit/browser/fileapi/copy_or_move_operation_delegate_unittest.cc
@@ -72,17 +72,15 @@ class CopyOrMoveOperationTestHelper {
     // Prepare the origin's root directory.
     FileSystemBackend* mount_point_provider =
         file_system_context_->GetFileSystemBackend(src_type_);
-    mount_point_provider->InitializeFileSystem(
+    mount_point_provider->OpenFileSystem(
         origin_, src_type_,
         OPEN_FILE_SYSTEM_CREATE_IF_NONEXISTENT,
-        NULL /* context */,
         base::Bind(&ExpectOk));
     mount_point_provider =
         file_system_context_->GetFileSystemBackend(dest_type_);
-    mount_point_provider->InitializeFileSystem(
+    mount_point_provider->OpenFileSystem(
         origin_, dest_type_,
         OPEN_FILE_SYSTEM_CREATE_IF_NONEXISTENT,
-        NULL /* context */,
         base::Bind(&ExpectOk));
     base::MessageLoop::current()->RunUntilIdle();

[commit-bot: I haz the power, 2013-07-24 05:29:24]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/nhiroki@chromium.org/19632004/43001

[kinuko, 2013-07-24 06:20:07]

Looking at the other CL, it looks we may rather want to pass non-const
FileSystemContext to Initialize here?

[nhiroki, 2013-07-24 06:26:21]

On 2013/07/24 06:20:07, kinuko wrote:
> Looking at the other CL, it looks we may rather want to pass non-const
> FileSystemContext to Initialize here?

Right. Removed those const qualifiers.

[kinuko, 2013-07-24 07:20:20]

On 2013/07/24 06:26:21, nhiroki wrote:
> On 2013/07/24 06:20:07, kinuko wrote:
> > Looking at the other CL, it looks we may rather want to pass non-const
> > FileSystemContext to Initialize here?
> 
> Right. Removed those const qualifiers.

Ok, still lgtm

[commit-bot: I haz the power, 2013-07-24 08:10:06]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/nhiroki@chromium.org/19632004/56001

[commit-bot: I haz the power, 2013-07-24 10:01:58]

Change committed as 213382