Extend simple_file_system to use SandboxedFileSystemOperation

so that most of the code paths that run in chromium can be tested
in test_shell.

Also removed SandboxedFileSystemContext::CheckIfFilePathIsSafe that is not used at all.

BUG=60243
TEST=fast/filesystem

Review URL: http://codereview.chromium.org/4879001

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@67158 0039d316-1c4b-4281-b951-d872f2087c98

9 files changed, 154 insertions, 96 deletions

diff --git a/webkit/fileapi/sandboxed_file_system_operation.cc b/webkit/fileapi/sandboxed_file_system_operation.cc
index fc77ce3..dfc2884 100644
--- a/webkit/fileapi/sandboxed_file_system_operation.cc
+++ b/webkit/fileapi/sandboxed_file_system_operation.cc
@@ -115,9 +115,10 @@ void SandboxedFileSystemOperation::Truncate(
   FileSystemOperation::Truncate(path, length);
 }
 
-void SandboxedFileSystemOperation::TouchFile(const FilePath& path,
-                const base::Time& last_access_time,
-                const base::Time& last_modified_time) {
+void SandboxedFileSystemOperation::TouchFile(
+    const FilePath& path,
+    const base::Time& last_access_time,
+    const base::Time& last_modified_time) {
   if (!VerifyFileSystemPathForWrite(path, true /* create */, 0))
     return;
   FileSystemOperation::TouchFile(path, last_access_time, last_modified_time);
@@ -171,14 +172,4 @@ bool SandboxedFileSystemOperation::VerifyFileSystemPathForWrite(
   return true;
 }
 
-bool SandboxedFileSystemOperation::CheckIfFilePathIsSafe(
-    const FilePath& path) {
-  if (file_system_context_->path_manager()->IsRestrictedFileName(
-          path.BaseName())) {
-    dispatcher()->DidFail(base::PLATFORM_FILE_ERROR_SECURITY);
-    return false;
-  }
-  return true;
-}
-
 }  // namespace fileapi
diff --git a/webkit/fileapi/sandboxed_file_system_operation.h b/webkit/fileapi/sandboxed_file_system_operation.h
index 088a5f3..18d47f7 100644
--- a/webkit/fileapi/sandboxed_file_system_operation.h
+++ b/webkit/fileapi/sandboxed_file_system_operation.h
@@ -89,12 +89,6 @@ class SandboxedFileSystemOperation : public FileSystemOperation {
                                     bool create,
                                     int64 growth);
 
-  // Checks if a given |path| does not contain any restricted names/chars
-  // for new files. Returns true if the given |path| is safe.
-  // Otherwise it fires dispatcher's DidFail method with
-  // PLATFORM_FILE_ERROR_SECURITY and returns false.
-  bool CheckIfFilePathIsSafe(const FilePath& path);
-
   // Not owned.  See the comment at the constructor.
   SandboxedFileSystemContext* file_system_context_;
 
diff --git a/webkit/support/webkit_support.cc b/webkit/support/webkit_support.cc
index f6d0b24..55550de 100644
--- a/webkit/support/webkit_support.cc
+++ b/webkit/support/webkit_support.cc
@@ -48,6 +48,7 @@
 #include "webkit/support/test_webplugin_page_delegate.h"
 #include "webkit/support/test_webkit_client.h"
 #include "webkit/tools/test_shell/simple_database_system.h"
+#include "webkit/tools/test_shell/simple_file_system.h"
 #include "webkit/tools/test_shell/simple_resource_loader_bridge.h"
 
 using WebKit::WebCString;
@@ -543,18 +544,11 @@ WebURL GetDevToolsPathAsURL() {
 }
 
 // FileSystem
-void OpenFileSystem(WebFrame*, WebFileSystem::Type type,
-    long long, bool, WebFileSystemCallbacks* callbacks) {
-  // TODO(kinuko): hook up FileSystemPathManager in a way that the code could
-  // be shared with test_shell.
-  if (test_environment->webkit_client()->file_system_root().empty()) {
-    callbacks->didFail(WebKit::WebFileErrorSecurity);
-  } else {
-    callbacks->didOpenFileSystem(
-        "TestShellFileSystem",
-        webkit_glue::FilePathToWebString(
-            test_environment->webkit_client()->file_system_root()));
-  }
+void OpenFileSystem(WebFrame* frame, WebFileSystem::Type type,
+    long long size, bool create, WebFileSystemCallbacks* callbacks) {
+  SimpleFileSystem* fileSystem = static_cast<SimpleFileSystem*>(
+      test_environment->webkit_client()->fileSystem());
+  fileSystem->OpenFileSystem(frame, type, size, create, callbacks);
 }
 
 }  // namespace webkit_support
diff --git a/webkit/tools/layout_tests/test_expectations.txt b/webkit/tools/layout_tests/test_expectations.txt
index e62732c..d3304c4 100644
--- a/webkit/tools/layout_tests/test_expectations.txt
+++ b/webkit/tools/layout_tests/test_expectations.txt
@@ -19,3 +19,9 @@ BUG_JPARENT MAC : fast/lists/001-vertical.html = IMAGE
 // Started to flake since http://src.chromium.org/viewvc/chrome?view=rev&revision=66937
 // (72440; 72514] range
 BUG_ANTON LINUX DEBUG : fast/ruby/nested-ruby.html = CRASH PASS
+
+// Need rebaseline.
+BUG_KINUKO : fast/filesystem/read-directory.html = TEXT
+BUG_KINUKO : fast/filesystem/simple-persistent.html = TEXT
+BUG_KINUKO : fast/filesystem/simple-temporary.html = TEXT
+BUG_KINUKO : fast/filesystem/async-operations.html = TEXT
diff --git a/webkit/tools/test_shell/simple_file_system.cc b/webkit/tools/test_shell/simple_file_system.cc
index 520f709..29ea45e 100644
--- a/webkit/tools/test_shell/simple_file_system.cc
+++ b/webkit/tools/test_shell/simple_file_system.cc
@@ -6,58 +6,85 @@
 
 #include "base/file_path.h"
 #include "base/message_loop_proxy.h"
+#include "base/scoped_callback_factory.h"
 #include "base/time.h"
+#include "base/utf_string_conversions.h"
+#include "googleurl/src/gurl.h"
 #include "third_party/WebKit/WebKit/chromium/public/WebFileInfo.h"
+#include "third_party/WebKit/WebKit/chromium/public/WebFileSystemCallbacks.h"
 #include "third_party/WebKit/WebKit/chromium/public/WebFileSystemEntry.h"
+#include "third_party/WebKit/WebKit/chromium/public/WebFrame.h"
+#include "third_party/WebKit/WebKit/chromium/public/WebSecurityOrigin.h"
 #include "third_party/WebKit/WebKit/chromium/public/WebVector.h"
 #include "webkit/fileapi/file_system_callback_dispatcher.h"
+#include "webkit/fileapi/file_system_path_manager.h"
+#include "webkit/fileapi/file_system_types.h"
+#include "webkit/fileapi/sandboxed_file_system_context.h"
+#include "webkit/fileapi/sandboxed_file_system_operation.h"
 #include "webkit/glue/webkit_glue.h"
 #include "webkit/tools/test_shell/simple_file_writer.h"
 
+using base::WeakPtr;
+
 using WebKit::WebFileInfo;
+using WebKit::WebFileSystem;
 using WebKit::WebFileSystemCallbacks;
 using WebKit::WebFileSystemEntry;
 using WebKit::WebFileWriter;
 using WebKit::WebFileWriterClient;
+using WebKit::WebFrame;
+using WebKit::WebSecurityOrigin;
 using WebKit::WebString;
 using WebKit::WebVector;
 
+using fileapi::FileSystemCallbackDispatcher;
+using fileapi::SandboxedFileSystemContext;
+using fileapi::SandboxedFileSystemOperation;
+
 namespace {
 
-class TestShellFileSystemCallbackDispatcher
-    : public fileapi::FileSystemCallbackDispatcher {
+class SimpleFileSystemCallbackDispatcher
+    : public FileSystemCallbackDispatcher {
  public:
-  TestShellFileSystemCallbackDispatcher(
-      SimpleFileSystem* file_system,
+  SimpleFileSystemCallbackDispatcher(
+      const WeakPtr<SimpleFileSystem>& file_system,
       WebFileSystemCallbacks* callbacks)
       : file_system_(file_system),
-        callbacks_(callbacks),
-        request_id_(-1) {
+        callbacks_(callbacks) {
   }
 
-  void set_request_id(int request_id) { request_id_ = request_id; }
+  ~SimpleFileSystemCallbackDispatcher() {
+    DCHECK(!operation_.get());
+  }
+
+  void set_operation(SandboxedFileSystemOperation* operation) {
+    operation_.reset(operation);
+  }
 
   virtual void DidSucceed() {
-    callbacks_->didSucceed();
-    file_system_->RemoveCompletedOperation(request_id_);
+    if (file_system_)
+      callbacks_->didSucceed();
+    RemoveOperation();
   }
 
   virtual void DidReadMetadata(const base::PlatformFileInfo& info) {
+    DCHECK(file_system_);
     WebFileInfo web_file_info;
     web_file_info.length = info.size;
     web_file_info.modificationTime = info.last_modified.ToDoubleT();
     web_file_info.type = info.is_directory ?
         WebFileInfo::TypeDirectory : WebFileInfo::TypeFile;
     callbacks_->didReadMetadata(web_file_info);
-    file_system_->RemoveCompletedOperation(request_id_);
+    RemoveOperation();
   }
 
   virtual void DidReadDirectory(
       const std::vector<base::FileUtilProxy::Entry>& entries,
       bool has_more) {
+    DCHECK(file_system_);
     std::vector<WebFileSystemEntry> web_entries_vector;
     for (std::vector<base::FileUtilProxy::Entry>::const_iterator it =
-             entries.begin(); it != entries.end(); ++it) {
+            entries.begin(); it != entries.end(); ++it) {
       WebFileSystemEntry entry;
       entry.name = webkit_glue::FilePathStringToWebString(it->name);
       entry.isDirectory = it->is_directory;
@@ -66,17 +93,25 @@ class TestShellFileSystemCallbackDispatcher
     WebVector<WebKit::WebFileSystemEntry> web_entries =
         web_entries_vector;
     callbacks_->didReadDirectory(web_entries, has_more);
-    file_system_->RemoveCompletedOperation(request_id_);
+    RemoveOperation();
   }
 
-  virtual void DidOpenFileSystem(const std::string&, const FilePath&) {
-    NOTREACHED();
+  virtual void DidOpenFileSystem(
+      const std::string& name, const FilePath& path) {
+    DCHECK(file_system_);
+    if (path.empty())
+      callbacks_->didFail(WebKit::WebFileErrorSecurity);
+    else
+      callbacks_->didOpenFileSystem(
+          UTF8ToUTF16(name), webkit_glue::FilePathToWebString(path));
+    RemoveOperation();
   }
 
   virtual void DidFail(base::PlatformFileError error_code) {
+    DCHECK(file_system_);
     callbacks_->didFail(
         webkit_glue::PlatformFileErrorToWebFileError(error_code));
-    file_system_->RemoveCompletedOperation(request_id_);
+    RemoveOperation();
   }
 
   virtual void DidWrite(int64, bool) {
@@ -84,18 +119,61 @@ class TestShellFileSystemCallbackDispatcher
   }
 
  private:
-  SimpleFileSystem* file_system_;
+  void RemoveOperation() {
+    // We need to make sure operation_ is null when we delete the operation
+    // (which in turn deletes this dispatcher instance).
+    scoped_ptr<SandboxedFileSystemOperation> operation;
+    operation.swap(operation_);
+    operation.reset();
+  }
+
+  WeakPtr<SimpleFileSystem> file_system_;
   WebFileSystemCallbacks* callbacks_;
-  int request_id_;
+  scoped_ptr<SandboxedFileSystemOperation> operation_;
 };
 
-} // namespace
+}  // namespace
+
+SimpleFileSystem::SimpleFileSystem() {
+  if (file_system_dir_.CreateUniqueTempDir()) {
+    sandboxed_context_.reset(new SandboxedFileSystemContext(
+        base::MessageLoopProxy::CreateForCurrentThread(),
+        file_system_dir_.path(),
+        false /* incognito */,
+        true /* allow_file_access */,
+        false /* unlimited_quota */));
+  } else {
+    LOG(WARNING) << "Failed to create a temp dir for the filesystem."
+                    "FileSystem feature will be disabled.";
+  }
+}
 
 SimpleFileSystem::~SimpleFileSystem() {
-  // Drop all the operations.
-  for (OperationsMap::const_iterator iter(&operations_);
-       !iter.IsAtEnd(); iter.Advance())
-    operations_.Remove(iter.GetCurrentKey());
+}
+
+void SimpleFileSystem::OpenFileSystem(
+    WebFrame* frame, WebFileSystem::Type web_filesystem_type,
+    long long, bool create,
+    WebFileSystemCallbacks* callbacks) {
+  if (!frame || !sandboxed_context_.get()) {
+    // The FileSystem temp directory was not initialized successfully.
+    callbacks->didFail(WebKit::WebFileErrorSecurity);
+    return;
+  }
+
+  fileapi::FileSystemType type;
+  if (web_filesystem_type == WebFileSystem::TypeTemporary)
+    type = fileapi::kFileSystemTypeTemporary;
+  else if (web_filesystem_type == WebFileSystem::TypePersistent)
+    type = fileapi::kFileSystemTypePersistent;
+  else {
+    // Unknown type filesystem is requested.
+    callbacks->didFail(WebKit::WebFileErrorSecurity);
+    return;
+  }
+
+  GURL origin_url(frame->securityOrigin().toString());
+  GetNewOperation(callbacks)->OpenFileSystem(origin_url, type, create);
 }
 
 void SimpleFileSystem::move(
@@ -177,18 +255,13 @@ WebFileWriter* SimpleFileSystem::createFileWriter(
   return new SimpleFileWriter(path, client);
 }
 
-fileapi::FileSystemOperation* SimpleFileSystem::GetNewOperation(
+SandboxedFileSystemOperation* SimpleFileSystem::GetNewOperation(
     WebFileSystemCallbacks* callbacks) {
-  // This pointer will be owned by |operation|.
-  TestShellFileSystemCallbackDispatcher* dispatcher =
-      new TestShellFileSystemCallbackDispatcher(this, callbacks);
-  fileapi::FileSystemOperation* operation = new fileapi::FileSystemOperation(
-      dispatcher, base::MessageLoopProxy::CreateForCurrentThread());
-  int32 request_id = operations_.Add(operation);
-  dispatcher->set_request_id(request_id);
+  SimpleFileSystemCallbackDispatcher* dispatcher =
+      new SimpleFileSystemCallbackDispatcher(AsWeakPtr(), callbacks);
+  SandboxedFileSystemOperation* operation = new SandboxedFileSystemOperation(
+      dispatcher, base::MessageLoopProxy::CreateForCurrentThread(),
+      sandboxed_context_.get());
+  dispatcher->set_operation(operation);
   return operation;
 }
-
-void SimpleFileSystem::RemoveCompletedOperation(int request_id) {
-  operations_.Remove(request_id);
-}
diff --git a/webkit/tools/test_shell/simple_file_system.h b/webkit/tools/test_shell/simple_file_system.h
index e1a8236..9a6758c 100644
--- a/webkit/tools/test_shell/simple_file_system.h
+++ b/webkit/tools/test_shell/simple_file_system.h
@@ -8,16 +8,33 @@
 #include <vector>
 #include "base/file_util_proxy.h"
 #include "base/id_map.h"
+#include "base/scoped_temp_dir.h"
+#include "base/weak_ptr.h"
 #include "third_party/WebKit/WebKit/chromium/public/WebFileSystem.h"
-#include "third_party/WebKit/WebKit/chromium/public/WebFileSystemCallbacks.h"
-#include "webkit/fileapi/file_system_operation.h"
+#include "webkit/fileapi/file_system_types.h"
 
-class SimpleFileSystem : public WebKit::WebFileSystem {
+namespace WebKit {
+class WebFileSystemCallbacks;
+class WebFrame;
+}
+
+namespace fileapi {
+class SandboxedFileSystemContext;
+class SandboxedFileSystemOperation;
+}
+
+class SimpleFileSystem
+    : public WebKit::WebFileSystem,
+      public base::SupportsWeakPtr<SimpleFileSystem> {
  public:
-  SimpleFileSystem() {}
+  SimpleFileSystem();
   virtual ~SimpleFileSystem();
 
-  void RemoveCompletedOperation(int request_id);
+  void OpenFileSystem(WebKit::WebFrame* frame,
+                      WebKit::WebFileSystem::Type type,
+                      long long size,
+                      bool create,
+                      WebKit::WebFileSystemCallbacks* callbacks);
 
   // WebKit::WebFileSystem methods.
   virtual void move(const WebKit::WebString& src_path,
@@ -49,12 +66,13 @@ class SimpleFileSystem : public WebKit::WebFileSystem {
 
  private:
   // Helpers.
-  fileapi::FileSystemOperation* GetNewOperation(
+  fileapi::SandboxedFileSystemOperation* GetNewOperation(
       WebKit::WebFileSystemCallbacks* callbacks);
 
-  // Keeps ongoing file system operations.
-  typedef IDMap<fileapi::FileSystemOperation, IDMapOwnPointer> OperationsMap;
-  OperationsMap operations_;
+  // A temporary directory for FileSystem API.
+  ScopedTempDir file_system_dir_;
+
+  scoped_ptr<fileapi::SandboxedFileSystemContext> sandboxed_context_;
 
   DISALLOW_COPY_AND_ASSIGN(SimpleFileSystem);
 };
diff --git a/webkit/tools/test_shell/test_shell.cc b/webkit/tools/test_shell/test_shell.cc
index 41f88c2..0b6171d 100644
--- a/webkit/tools/test_shell/test_shell.cc
+++ b/webkit/tools/test_shell/test_shell.cc
@@ -153,12 +153,6 @@ TestShell::TestShell()
     filter->AddHostnameHandler("test-shell-resource", "inspector",
                                &URLRequestTestShellFileJob::InspectorFactory);
     url_util::AddStandardScheme("test-shell-resource");
-
-    if (!file_system_root_.CreateUniqueTempDir()) {
-      LOG(WARNING) << "Failed to create a temp dir for the filesystem."
-                      "FileSystem feature will be disabled.";
-      DCHECK(file_system_root_.path().empty());
-    }
 }
 
 TestShell::~TestShell() {
diff --git a/webkit/tools/test_shell/test_shell.h b/webkit/tools/test_shell/test_shell.h
index 7c320a2..048c5ff 100644
--- a/webkit/tools/test_shell/test_shell.h
+++ b/webkit/tools/test_shell/test_shell.h
@@ -351,10 +351,6 @@ public:
       test_params_ = test_params;
     }
 
-    const FilePath& file_system_root() const {
-      return file_system_root_.path();
-    }
-
 #if defined(OS_MACOSX)
     // handle cleaning up a shell given the associated window
     static void DestroyAssociatedShell(gfx::NativeWindow handle);
@@ -460,9 +456,6 @@ private:
     scoped_ptr<WebKit::WebSpeechInputControllerMock>
         speech_input_controller_mock_;
 
-    // A temporary directory for FileSystem API.
-    ScopedTempDir file_system_root_;
-
     const TestParams* test_params_;
 
     // True while a test is preparing to run
diff --git a/webkit/tools/test_shell/test_webview_delegate.cc b/webkit/tools/test_shell/test_webview_delegate.cc
index f22b6b8..b07186e 100644
--- a/webkit/tools/test_shell/test_webview_delegate.cc
+++ b/webkit/tools/test_shell/test_webview_delegate.cc
@@ -70,6 +70,7 @@
 #include "webkit/tools/test_shell/mock_spellcheck.h"
 #include "webkit/tools/test_shell/notification_presenter.h"
 #include "webkit/tools/test_shell/simple_appcache_system.h"
+#include "webkit/tools/test_shell/simple_file_system.h"
 #include "webkit/tools/test_shell/test_geolocation_service.h"
 #include "webkit/tools/test_shell/test_navigation_controller.h"
 #include "webkit/tools/test_shell/test_shell.h"
@@ -1126,15 +1127,9 @@ bool TestWebViewDelegate::allowScript(WebFrame* frame,
 void TestWebViewDelegate::openFileSystem(
     WebFrame* frame, WebFileSystem::Type type, long long size, bool create,
     WebFileSystemCallbacks* callbacks) {
-  if (shell_->file_system_root().empty()) {
-    // The FileSystem temp directory was not initialized successfully.
-    callbacks->didFail(WebKit::WebFileErrorSecurity);
-  } else {
-    // TODO(michaeln): need to put origin/type in the path.
-    callbacks->didOpenFileSystem(
-        "TestShellFileSystem",
-        webkit_glue::FilePathToWebString(shell_->file_system_root()));
-  }
+  SimpleFileSystem* fileSystem = static_cast<SimpleFileSystem*>(
+      WebKit::webKitClient()->fileSystem());
+  fileSystem->OpenFileSystem(frame, type, size, create, callbacks);
 }
 
 // WebPluginPageDelegate -----------------------------------------------------