Second try at submitting 61462.

BUG=none
TEST=in file_system_operation_unittest.cc

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

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

6 files changed, 153 insertions, 53 deletions

diff --git a/webkit/fileapi/file_system_callback_dispatcher.h b/webkit/fileapi/file_system_callback_dispatcher.h
index c85c68a..e33dd69 100644
--- a/webkit/fileapi/file_system_callback_dispatcher.h
+++ b/webkit/fileapi/file_system_callback_dispatcher.h
@@ -41,6 +41,9 @@ class FileSystemCallbackDispatcher {
 
   // Called with an error code when a requested operation has failed.
   virtual void DidFail(base::PlatformFileError error_code) = 0;
+
+  // Callback for FileWriter's write() call.
+  virtual void DidWrite(int64 bytes, bool complete) = 0;
 };
 
 } // namespace fileapi
diff --git a/webkit/fileapi/file_system_operation.cc b/webkit/fileapi/file_system_operation.cc
index 140ade4..e636420 100644
--- a/webkit/fileapi/file_system_operation.cc
+++ b/webkit/fileapi/file_system_operation.cc
@@ -15,10 +15,11 @@ FileSystemOperation::FileSystemOperation(
     scoped_refptr<base::MessageLoopProxy> proxy)
     : proxy_(proxy),
       dispatcher_(dispatcher),
-      callback_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)) {
+      callback_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)),
+      cancel_operation_(NULL) {
   DCHECK(dispatcher);
 #ifndef NDEBUG
-  operation_pending_ = false;
+  pending_operation_ = kOperationNone;
 #endif
 }
 
@@ -28,8 +29,8 @@ FileSystemOperation::~FileSystemOperation() {
 void FileSystemOperation::CreateFile(const FilePath& path,
                                      bool exclusive) {
 #ifndef NDEBUG
-  DCHECK(!operation_pending_);
-  operation_pending_ = true;
+  DCHECK(kOperationNone == pending_operation_);
+  pending_operation_ = kOperationCreateFile;
 #endif
 
   base::FileUtilProxy::CreateOrOpen(
@@ -43,8 +44,8 @@ void FileSystemOperation::CreateDirectory(const FilePath& path,
                                           bool exclusive,
                                           bool recursive) {
 #ifndef NDEBUG
-  DCHECK(!operation_pending_);
-  operation_pending_ = true;
+  DCHECK(kOperationNone == pending_operation_);
+  pending_operation_ = kOperationCreateDirectory;
 #endif
 
   base::FileUtilProxy::CreateDirectory(
@@ -55,8 +56,8 @@ void FileSystemOperation::CreateDirectory(const FilePath& path,
 void FileSystemOperation::Copy(const FilePath& src_path,
                                const FilePath& dest_path) {
 #ifndef NDEBUG
-  DCHECK(!operation_pending_);
-  operation_pending_ = true;
+  DCHECK(kOperationNone == pending_operation_);
+  pending_operation_ = kOperationCopy;
 #endif
 
   base::FileUtilProxy::Copy(proxy_, src_path, dest_path,
@@ -67,8 +68,8 @@ void FileSystemOperation::Copy(const FilePath& src_path,
 void FileSystemOperation::Move(const FilePath& src_path,
                                const FilePath& dest_path) {
 #ifndef NDEBUG
-  DCHECK(!operation_pending_);
-  operation_pending_ = true;
+  DCHECK(kOperationNone == pending_operation_);
+  pending_operation_ = kOperationMove;
 #endif
 
   base::FileUtilProxy::Move(proxy_, src_path, dest_path,
@@ -78,8 +79,8 @@ void FileSystemOperation::Move(const FilePath& src_path,
 
 void FileSystemOperation::DirectoryExists(const FilePath& path) {
 #ifndef NDEBUG
-  DCHECK(!operation_pending_);
-  operation_pending_ = true;
+  DCHECK(kOperationNone == pending_operation_);
+  pending_operation_ = kOperationDirectoryExists;
 #endif
 
   base::FileUtilProxy::GetFileInfo(proxy_, path, callback_factory_.NewCallback(
@@ -88,8 +89,8 @@ void FileSystemOperation::DirectoryExists(const FilePath& path) {
 
 void FileSystemOperation::FileExists(const FilePath& path) {
 #ifndef NDEBUG
-  DCHECK(!operation_pending_);
-  operation_pending_ = true;
+  DCHECK(kOperationNone == pending_operation_);
+  pending_operation_ = kOperationFileExists;
 #endif
 
   base::FileUtilProxy::GetFileInfo(proxy_, path, callback_factory_.NewCallback(
@@ -98,8 +99,8 @@ void FileSystemOperation::FileExists(const FilePath& path) {
 
 void FileSystemOperation::GetMetadata(const FilePath& path) {
 #ifndef NDEBUG
-  DCHECK(!operation_pending_);
-  operation_pending_ = true;
+  DCHECK(kOperationNone == pending_operation_);
+  pending_operation_ = kOperationGetMetadata;
 #endif
 
   base::FileUtilProxy::GetFileInfo(proxy_, path, callback_factory_.NewCallback(
@@ -108,8 +109,8 @@ void FileSystemOperation::GetMetadata(const FilePath& path) {
 
 void FileSystemOperation::ReadDirectory(const FilePath& path) {
 #ifndef NDEBUG
-  DCHECK(!operation_pending_);
-  operation_pending_ = true;
+  DCHECK(kOperationNone == pending_operation_);
+  pending_operation_ = kOperationReadDirectory;
 #endif
 
   base::FileUtilProxy::ReadDirectory(proxy_, path,
@@ -119,8 +120,8 @@ void FileSystemOperation::ReadDirectory(const FilePath& path) {
 
 void FileSystemOperation::Remove(const FilePath& path) {
 #ifndef NDEBUG
-  DCHECK(!operation_pending_);
-  operation_pending_ = true;
+  DCHECK(kOperationNone == pending_operation_);
+  pending_operation_ = kOperationRemove;
 #endif
 
   base::FileUtilProxy::Delete(proxy_, path, callback_factory_.NewCallback(
@@ -128,36 +129,32 @@ void FileSystemOperation::Remove(const FilePath& path) {
 }
 
 void FileSystemOperation::Write(
-    const FilePath&,
-    const GURL&,
-    int64) {
+    const FilePath& path,
+    const GURL& blob_url,
+    int64 offset) {
 #ifndef NDEBUG
-  DCHECK(!operation_pending_);
-  operation_pending_ = true;
+  DCHECK(kOperationNone == pending_operation_);
+  pending_operation_ = kOperationWrite;
 #endif
   NOTREACHED();
-  // TODO(ericu):
-  // Set up a loop that, via multiple callback invocations, reads from a
-  // URLRequest wrapping blob_url, writes the bytes to the file, reports
-  // progress events no more frequently than some set rate, and periodically
-  // checks to see if it's been cancelled.
 }
 
 void FileSystemOperation::Truncate(const FilePath& path, int64 length) {
 #ifndef NDEBUG
-  DCHECK(!operation_pending_);
-  operation_pending_ = true;
+  DCHECK(kOperationNone == pending_operation_);
+  pending_operation_ = kOperationTruncate;
 #endif
-  // TODO(ericu):
-  NOTREACHED();
+  base::FileUtilProxy::Truncate(proxy_, path, length,
+      callback_factory_.NewCallback(
+          &FileSystemOperation::DidFinishFileOperation));
 }
 
 void FileSystemOperation::TouchFile(const FilePath& path,
                                     const base::Time& last_access_time,
                                     const base::Time& last_modified_time) {
 #ifndef NDEBUG
-  DCHECK(!operation_pending_);
-  operation_pending_ = true;
+  DCHECK(kOperationNone == pending_operation_);
+  pending_operation_ = kOperationTouchFile;
 #endif
 
   base::FileUtilProxy::Touch(
@@ -165,15 +162,18 @@ void FileSystemOperation::TouchFile(const FilePath& path,
       callback_factory_.NewCallback(&FileSystemOperation::DidTouchFile));
 }
 
-void FileSystemOperation::Cancel() {
+// We can only get here on a write or truncate that's not yet completed.
+// We don't support cancelling any other operation at this time.
+void FileSystemOperation::Cancel(FileSystemOperation* cancel_operation) {
 #ifndef NDEBUG
-  DCHECK(operation_pending_);
+  DCHECK(kOperationTruncate == pending_operation_);
+  // FIXME(ericu): Cancelling for writes coming soon.
 #endif
-  NOTREACHED();
-  // TODO(ericu):
-  // Make sure this was done on a FileSystemOperation used for a Write.
-  // Then set a flag that ensures that the Write loop will exit without
-  // reporting any more progress, with a failure notification.
+  // We're cancelling a truncate operation, but we can't actually stop it
+  // since it's been proxied to another thread.  We need to save the
+  // cancel_operation so that when the truncate returns, it can see that it's
+  // been cancelled, report it, and report that the cancel has succeeded.
+  cancel_operation_ = cancel_operation;
 }
 
 void FileSystemOperation::DidCreateFileExclusive(
@@ -197,10 +197,19 @@ void FileSystemOperation::DidCreateFileNonExclusive(
 
 void FileSystemOperation::DidFinishFileOperation(
     base::PlatformFileError rv) {
-  if (rv == base::PLATFORM_FILE_OK)
+  if (cancel_operation_) {
+#ifndef NDEBUG
+    DCHECK(kOperationTruncate == pending_operation_);
+#endif
+    FileSystemOperation *cancel_op = cancel_operation_;
+    // This call deletes us, so we have to extract cancel_op first.
+    dispatcher_->DidFail(base::PLATFORM_FILE_ERROR_ABORT);
+    cancel_op->dispatcher_->DidSucceed();
+  } else if (rv == base::PLATFORM_FILE_OK) {
     dispatcher_->DidSucceed();
-  else
+  } else {
     dispatcher_->DidFail(rv);
+  }
 }
 
 void FileSystemOperation::DidDirectoryExists(
@@ -250,7 +259,7 @@ void FileSystemOperation::DidWrite(
     int64 bytes,
     bool complete) {
   if (rv == base::PLATFORM_FILE_OK)
-    /* dispatcher_->DidWrite(bytes, complete) TODO(ericu): Coming soon. */ {}
+    dispatcher_->DidWrite(bytes, complete);
   else
     dispatcher_->DidFail(rv);
 }
diff --git a/webkit/fileapi/file_system_operation.h b/webkit/fileapi/file_system_operation.h
index 1107303..ab6d56c 100644
--- a/webkit/fileapi/file_system_operation.h
+++ b/webkit/fileapi/file_system_operation.h
@@ -61,8 +61,7 @@ class FileSystemOperation {
 
   void Remove(const FilePath& path);
 
-  void Write(
-      const FilePath& path, const GURL& blob_url, int64 offset);
+  void Write(const FilePath& path, const GURL& blob_url, int64 offset);
 
   void Truncate(const FilePath& path, int64 length);
 
@@ -70,9 +69,10 @@ class FileSystemOperation {
                  const base::Time& last_access_time,
                  const base::Time& last_modified_time);
 
-  // Used to attempt to cancel the current operation.  This currently does
-  // nothing for any operation other than Write().
-  void Cancel();
+  // Try to cancel the current operation [we support cancelling write or
+  // truncate only].  Report failure for the current operation, then tell the
+  // passed-in operation to report success.
+  void Cancel(FileSystemOperation* cancel_operation);
 
  protected:
   // Proxy for calling file_util_proxy methods.
@@ -114,9 +114,28 @@ class FileSystemOperation {
 
   base::ScopedCallbackFactory<FileSystemOperation> callback_factory_;
 
+  FileSystemOperation* cancel_operation_;
+
 #ifndef NDEBUG
+  enum OperationType {
+    kOperationNone,
+    kOperationCreateFile,
+    kOperationCreateDirectory,
+    kOperationCopy,
+    kOperationMove,
+    kOperationDirectoryExists,
+    kOperationFileExists,
+    kOperationGetMetadata,
+    kOperationReadDirectory,
+    kOperationRemove,
+    kOperationWrite,
+    kOperationTruncate,
+    kOperationTouchFile,
+    kOperationCancel,
+  };
+
   // A flag to make sure we call operation only once per instance.
-  bool operation_pending_;
+  OperationType pending_operation_;
 #endif
 
   DISALLOW_COPY_AND_ASSIGN(FileSystemOperation);
diff --git a/webkit/fileapi/file_system_operation_unittest.cc b/webkit/fileapi/file_system_operation_unittest.cc
index 93680a0..60db566 100644
--- a/webkit/fileapi/file_system_operation_unittest.cc
+++ b/webkit/fileapi/file_system_operation_unittest.cc
@@ -52,6 +52,10 @@ class MockDispatcher : public fileapi::FileSystemCallbackDispatcher {
     NOTREACHED();
   }
 
+  virtual void DidWrite(int64 bytes, bool complete) {
+    NOTREACHED();
+  }
+
   // Helpers for testing.
   int status() const { return status_; }
   int request_id() const { return request_id_; }
@@ -553,3 +557,60 @@ TEST_F(FileSystemOperationTest, TestRemoveSuccess) {
   EXPECT_FALSE(file_util::DirectoryExists(empty_dir.path()));
   EXPECT_EQ(request_id_, mock_dispatcher_->request_id());
 }
+
+TEST_F(FileSystemOperationTest, TestTruncate) {
+  ScopedTempDir dir;
+  ASSERT_TRUE(dir.CreateUniqueTempDir());
+  FilePath file;
+  file_util::CreateTemporaryFileInDir(dir.path(), &file);
+
+  char test_data[] = "test data";
+  int data_size = static_cast<int>(sizeof(test_data));
+  EXPECT_EQ(data_size,
+            file_util::WriteFile(file, test_data, data_size));
+
+  // Check that its length is the size of the data written.
+  operation()->GetMetadata(file);
+  MessageLoop::current()->RunAllPending();
+  EXPECT_EQ(kFileOperationSucceeded, mock_dispatcher_->status());
+  EXPECT_FALSE(mock_dispatcher_->info().is_directory);
+  EXPECT_EQ(data_size, mock_dispatcher_->info().size);
+  EXPECT_EQ(request_id_, mock_dispatcher_->request_id());
+
+  // Extend the file by truncating it.
+  int length = 17;
+  operation()->Truncate(file, length);
+  MessageLoop::current()->RunAllPending();
+  EXPECT_EQ(kFileOperationSucceeded, mock_dispatcher_->status());
+  EXPECT_EQ(request_id_, mock_dispatcher_->request_id());
+
+  // Check that its length is now 17 and that it's all zeroes after the test
+  // data.
+  base::PlatformFileInfo info;
+
+  EXPECT_TRUE(file_util::GetFileInfo(file, &info));
+  EXPECT_EQ(length, info.size);
+  char data[100];
+  EXPECT_EQ(length, file_util::ReadFile(file, data, length));
+  for (int i = 0; i < length; ++i) {
+    if (i < data_size)
+      EXPECT_EQ(test_data[i], data[i]);
+    else
+      EXPECT_EQ(0, data[i]);
+  }
+
+  // Shorten the file by truncating it.
+  length = 3;
+  operation()->Truncate(file, length);
+  MessageLoop::current()->RunAllPending();
+  EXPECT_EQ(kFileOperationSucceeded, mock_dispatcher_->status());
+  EXPECT_EQ(request_id_, mock_dispatcher_->request_id());
+
+  // Check that its length is now 3 and that it contains only bits of test data.
+  EXPECT_TRUE(file_util::GetFileInfo(file, &info));
+  EXPECT_EQ(length, info.size);
+  EXPECT_EQ(length, file_util::ReadFile(file, data, length));
+  for (int i = 0; i < length; ++i)
+    EXPECT_EQ(test_data[i], data[i]);
+}
+
diff --git a/webkit/glue/plugins/pepper_file_system.cc b/webkit/glue/plugins/pepper_file_system.cc
index 82a2fc8..6068ca5 100644
--- a/webkit/glue/plugins/pepper_file_system.cc
+++ b/webkit/glue/plugins/pepper_file_system.cc
@@ -54,6 +54,10 @@ class StatusCallback : public fileapi::FileSystemCallbackDispatcher {
     RunCallback(error_code);
   }
 
+  virtual void DidWrite(int64 bytes, bool complete) {
+    NOTREACHED();
+  }
+
  private:
   void RunCallback(base::PlatformFileError error_code) {
     if (!module_.get() || !callback_.func)
@@ -105,6 +109,10 @@ class QueryInfoCallback : public fileapi::FileSystemCallbackDispatcher {
     RunCallback(error_code, base::PlatformFileInfo());
   }
 
+  virtual void DidWrite(int64 bytes, bool complete) {
+    NOTREACHED();
+  }
+
  private:
   void RunCallback(base::PlatformFileError error_code,
                    const base::PlatformFileInfo& file_info) {
diff --git a/webkit/tools/test_shell/simple_file_system.cc b/webkit/tools/test_shell/simple_file_system.cc
index 3739a62..8e059ea 100644
--- a/webkit/tools/test_shell/simple_file_system.cc
+++ b/webkit/tools/test_shell/simple_file_system.cc
@@ -92,7 +92,7 @@ class TestShellFileSystemCallbackDispatcher
     file_system_->RemoveCompletedOperation(request_id_);
   }
 
-  virtual void DidWrite(int64, bool, fileapi::FileSystemOperation*) {
+  virtual void DidWrite(int64, bool) {
     NOTREACHED();
   }