fileapi: Add modification time check for FileSystemFileStreamReader

The check is added since BlobData has expected_modification_time field.
Add tests (mostly copied from LocalFileStreamReaderTest).

BUG=141835
TEST=content_unittests --gtest_filter="FileSystemFileStreamReaderTest.*"
TBR=jam@chromium.org

Review URL: https://chromiumcodereview.appspot.com/11098067

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

content/content_tests.gypi

@@ -405,6 +405,7 @@
         '../webkit/fileapi/file_system_database_test_helper.h',
         '../webkit/fileapi/file_system_directory_database_unittest.cc',
         '../webkit/fileapi/file_system_dir_url_request_job_unittest.cc',
+        '../webkit/fileapi/file_system_file_stream_reader_unittest.cc',
         '../webkit/fileapi/file_system_file_util_unittest.cc',
         '../webkit/fileapi/file_system_mount_point_provider_unittest.cc',
         '../webkit/fileapi/file_system_origin_database_unittest.cc',

webkit/chromeos/fileapi/cros_mount_point_provider.cc

@@ -270,11 +270,13 @@ fileapi::FileSystemOperation* CrosMountPointProvider::CreateFileSystemOperation(
 webkit_blob::FileStreamReader* CrosMountPointProvider::CreateFileStreamReader(
     const fileapi::FileSystemURL& url,
     int64 offset,
+    const base::Time& expected_modification_time,
     fileapi::FileSystemContext* context) const {
   // For now we return a generic Reader implementation which utilizes
   // CreateSnapshotFile internally (i.e. will download everything first).
   // TODO(satorux,zel): implement more efficient reader for remote cases.
-  return new fileapi::FileSystemFileStreamReader(context, url, offset);
+  return new fileapi::FileSystemFileStreamReader(
+      context, url, offset, expected_modification_time);
 }
 
 fileapi::FileStreamWriter* CrosMountPointProvider::CreateFileStreamWriter(

webkit/chromeos/fileapi/cros_mount_point_provider.h

@@ -67,6 +67,7 @@ class FILEAPI_EXPORT CrosMountPointProvider
   virtual webkit_blob::FileStreamReader* CreateFileStreamReader(
       const fileapi::FileSystemURL& path,
       int64 offset,
+      const base::Time& expected_modification_time,
       fileapi::FileSystemContext* context) const OVERRIDE;
   virtual fileapi::FileStreamWriter* CreateFileStreamWriter(
       const fileapi::FileSystemURL& url,

webkit/fileapi/file_system_context.cc

@@ -259,14 +259,16 @@ FileSystemOperation* FileSystemContext::CreateFileSystemOperation(
 
 webkit_blob::FileStreamReader* FileSystemContext::CreateFileStreamReader(
     const FileSystemURL& url,
-    int64 offset) {
+    int64 offset,
+    const base::Time& expected_modification_time) {
   if (!url.is_valid())
     return NULL;
   FileSystemMountPointProvider* mount_point_provider =
       GetMountPointProvider(url.type());
   if (!mount_point_provider)
     return NULL;
-  return mount_point_provider->CreateFileStreamReader(url, offset, this);
+  return mount_point_provider->CreateFileStreamReader(
+      url, offset, expected_modification_time, this);
 }
 
 void FileSystemContext::RegisterMountPointProvider(

webkit/fileapi/file_system_context.h

@@ -147,14 +147,19 @@ class FILEAPI_EXPORT FileSystemContext
       base::PlatformFileError* error_code);
 
   // Creates new FileStreamReader instance to read a file pointed by the given
-  // filesystem URL |url| starting from |offset|.
+  // filesystem URL |url| starting from |offset|. |expected_modification_time|
+  // specifies the expected last modification if the value is non-null, the
+  // reader will check the underlying file's actual modification time to see if
+  // the file has been modified, and if it does any succeeding read operations
+  // should fail with ERR_UPLOAD_FILE_CHANGED error.
   // This method internally cracks the |url|, get an appropriate
   // MountPointProvider for the URL and call the provider's CreateFileReader.
   // The resolved MountPointProvider could perform further specialization
   // depending on the filesystem type pointed by the |url|.
   webkit_blob::FileStreamReader* CreateFileStreamReader(
       const FileSystemURL& url,
-      int64 offset);
+      int64 offset,
+      const base::Time& expected_modification_time);
 
   // Register a filesystem provider. The ownership of |provider| is
   // transferred to this instance.

webkit/fileapi/file_system_file_stream_reader.cc

@@ -50,10 +50,12 @@ void Int64CallbackAdapter(const net::Int64CompletionCallback& callback,
 FileSystemFileStreamReader::FileSystemFileStreamReader(
     FileSystemContext* file_system_context,
     const FileSystemURL& url,
-    int64 initial_offset)
+    int64 initial_offset,
+    const base::Time& expected_modification_time)
     : file_system_context_(file_system_context),
       url_(url),
       initial_offset_(initial_offset),
+      expected_modification_time_(expected_modification_time),
       has_pending_create_snapshot_(false),
       weak_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)) {
 }
@@ -122,7 +124,7 @@ void FileSystemFileStreamReader::DidCreateSnapshot(
   local_file_reader_.reset(
       new LocalFileStreamReader(
           file_system_context_->task_runners()->file_task_runner(),
-          platform_path, initial_offset_, base::Time()));
+          platform_path, initial_offset_, expected_modification_time_));
 
   callback.Run();
 }

webkit/fileapi/file_system_file_stream_reader.h

@@ -8,7 +8,9 @@
 #include "base/bind.h"
 #include "base/memory/ref_counted.h"
 #include "base/platform_file.h"
+#include "base/time.h"
 #include "webkit/fileapi/file_system_url.h"
+#include "webkit/fileapi/fileapi_export.h"
 #include "webkit/blob/file_stream_reader.h"
 #include "webkit/blob/shareable_file_reference.h"
 
@@ -30,12 +32,18 @@ class FileSystemContext;
 // filesystems but remote filesystem should implement its own reader
 // rather than relying on FileSystemOperation::GetSnapshotFile() which
 // may force downloading the entire contents for remote files.
-class FileSystemFileStreamReader : public webkit_blob::FileStreamReader {
+class FILEAPI_EXPORT_PRIVATE FileSystemFileStreamReader
+    : public webkit_blob::FileStreamReader {
  public:
   // Creates a new FileReader for a filesystem URL |url| form |initial_offset|.
+  // |expected_modification_time| specifies the expected last modification if
+  // the value is non-null, the reader will check the underlying file's actual
+  // modification time to see if the file has been modified, and if it does any
+  // succeeding read operations should fail with ERR_UPLOAD_FILE_CHANGED error.
   FileSystemFileStreamReader(FileSystemContext* file_system_context,
                              const FileSystemURL& url,
-                             int64 initial_offset);
+                             int64 initial_offset,
+                             const base::Time& expected_modification_time);
   virtual ~FileSystemFileStreamReader();
 
   // FileStreamReader overrides.
@@ -58,6 +66,7 @@ class FileSystemFileStreamReader : public webkit_blob::FileStreamReader {
   scoped_refptr<FileSystemContext> file_system_context_;
   FileSystemURL url_;
   const int64 initial_offset_;
+  const base::Time expected_modification_time_;
   scoped_ptr<webkit_blob::LocalFileStreamReader> local_file_reader_;
   scoped_refptr<webkit_blob::ShareableFileReference> snapshot_ref_;
   bool has_pending_create_snapshot_;

webkit/fileapi/file_system_file_stream_reader_unittest.cc

@@ -0,0 +1,287 @@
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "webkit/fileapi/file_system_file_stream_reader.h"
+
+#include <string>
+
+#include "base/memory/scoped_ptr.h"
+#include "base/message_loop.h"
+#include "base/platform_file.h"
+#include "base/scoped_temp_dir.h"
+#include "net/base/io_buffer.h"
+#include "net/base/net_errors.h"
+#include "net/base/test_completion_callback.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "webkit/fileapi/file_system_context.h"
+#include "webkit/fileapi/file_system_file_util.h"
+#include "webkit/fileapi/file_system_operation_context.h"
+#include "webkit/fileapi/file_system_task_runners.h"
+#include "webkit/fileapi/mock_file_system_options.h"
+#include "webkit/fileapi/sandbox_mount_point_provider.h"
+#include "webkit/quota/mock_special_storage_policy.h"
+
+namespace fileapi {
+
+namespace {
+
+const char kURLOrigin[] = "http://remote/";
+const char kTestFileName[] = "test.dat";
+const char kTestData[] = "0123456789";
+const int kTestDataSize = arraysize(kTestData) - 1;
+
+void ReadFromReader(FileSystemFileStreamReader* reader,
+                    std::string* data,
+                    size_t size,
+                    int* result) {
+  ASSERT_TRUE(reader != NULL);
+  ASSERT_TRUE(result != NULL);
+  *result = net::OK;
+  net::TestCompletionCallback callback;
+  size_t total_bytes_read = 0;
+  while (total_bytes_read < size) {
+    scoped_refptr<net::IOBufferWithSize> buf(
+        new net::IOBufferWithSize(size - total_bytes_read));
+    int rv = reader->Read(buf, buf->size(), callback.callback());
+    if (rv == net::ERR_IO_PENDING)
+      rv = callback.WaitForResult();
+    if (rv < 0)
+      *result = rv;
+    if (rv <= 0)
+      break;
+    total_bytes_read += rv;
+    data->append(buf->data(), rv);
+  }
+}
+
+void NeverCalled(int) { ADD_FAILURE(); }
+
+}  // namespace
+
+class FileSystemFileStreamReaderTest : public testing::Test {
+ public:
+  FileSystemFileStreamReaderTest()
+      : message_loop_(MessageLoop::TYPE_IO) {}
+
+  virtual void SetUp() OVERRIDE {
+    ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
+
+    special_storage_policy_ = new quota::MockSpecialStoragePolicy;
+    file_system_context_ =
+        new FileSystemContext(
+            FileSystemTaskRunners::CreateMockTaskRunners(),
+            special_storage_policy_,
+            NULL,
+            temp_dir_.path(),
+            CreateDisallowFileAccessOptions());
+
+    file_system_context_->sandbox_provider()->ValidateFileSystemRoot(
+        GURL(kURLOrigin), kFileSystemTypeTemporary, true,  // create
+        base::Bind(&OnValidateFileSystem));
+    MessageLoop::current()->RunAllPending();
+
+    WriteFile(kTestFileName, kTestData, kTestDataSize,
+              &test_file_modification_time_);
+  }
+
+  virtual void TearDown() OVERRIDE {
+    MessageLoop::current()->RunAllPending();
+  }
+
+ protected:
+  FileSystemFileStreamReader* CreateFileReader(
+      const std::string& file_name,
+      int64 initial_offset,
+      const base::Time& expected_modification_time) {
+    return new FileSystemFileStreamReader(file_system_context_,
+                                          GetFileSystemURL(file_name),
+                                          initial_offset,
+                                          expected_modification_time);
+  }
+
+  base::Time test_file_modification_time() const {
+    return test_file_modification_time_;
+  }
+
+  void WriteFile(const std::string& file_name,
+                 const char* buf,
+                 int buf_size,
+                 base::Time* modification_time) {
+    FileSystemFileUtil* file_util = file_system_context_->
+        sandbox_provider()->GetFileUtil(kFileSystemTypeTemporary);
+    FileSystemURL url = GetFileSystemURL(file_name);
+
+    FileSystemOperationContext context(file_system_context_);
+    context.set_allowed_bytes_growth(1024);
+
+    base::PlatformFile handle = base::kInvalidPlatformFileValue;
+    bool created = false;
+    ASSERT_EQ(base::PLATFORM_FILE_OK, file_util->CreateOrOpen(
+        &context,
+        url,
+        base::PLATFORM_FILE_CREATE | base::PLATFORM_FILE_WRITE,
+        &handle,
+        &created));
+    EXPECT_TRUE(created);
+    ASSERT_NE(base::kInvalidPlatformFileValue, handle);
+    ASSERT_EQ(buf_size,
+              base::WritePlatformFile(handle, 0 /* offset */, buf, buf_size));
+    base::ClosePlatformFile(handle);
+
+    base::PlatformFileInfo file_info;
+    FilePath platform_path;
+    ASSERT_EQ(base::PLATFORM_FILE_OK,
+              file_util->GetFileInfo(&context, url, &file_info,
+                                     &platform_path));
+    if (modification_time)
+      *modification_time = file_info.last_modified;
+  }
+
+ private:
+  static void OnValidateFileSystem(base::PlatformFileError result) {
+    ASSERT_EQ(base::PLATFORM_FILE_OK, result);
+  }
+
+  FileSystemURL GetFileSystemURL(const std::string& file_name) {
+    return FileSystemURL(GURL(kURLOrigin),
+                         kFileSystemTypeTemporary,
+                         FilePath().AppendASCII(file_name));
+  }
+
+  MessageLoop message_loop_;
+  ScopedTempDir temp_dir_;
+  scoped_refptr<quota::MockSpecialStoragePolicy> special_storage_policy_;
+  scoped_refptr<FileSystemContext> file_system_context_;
+  base::Time test_file_modification_time_;
+};
+
+TEST_F(FileSystemFileStreamReaderTest, NonExistent) {
+  const char kFileName[] = "nonexistent";
+  scoped_ptr<FileSystemFileStreamReader> reader(
+      CreateFileReader(kFileName, 0, base::Time()));
+  int result = 0;
+  std::string data;
+  ReadFromReader(reader.get(), &data, 10, &result);
+  ASSERT_EQ(net::ERR_FILE_NOT_FOUND, result);
+  ASSERT_EQ(0U, data.size());
+}
+
+TEST_F(FileSystemFileStreamReaderTest, Empty) {
+  const char kFileName[] = "empty";
+  WriteFile(kFileName, NULL, 0, NULL);
+
+  scoped_ptr<FileSystemFileStreamReader> reader(
+      CreateFileReader(kFileName, 0, base::Time()));
+  int result = 0;
+  std::string data;
+  ReadFromReader(reader.get(), &data, 10, &result);
+  ASSERT_EQ(net::OK, result);
+  ASSERT_EQ(0U, data.size());
+
+  net::TestInt64CompletionCallback callback;
+  result = reader->GetLength(callback.callback());
+  if (result == net::ERR_IO_PENDING)
+    result = callback.WaitForResult();
+  ASSERT_EQ(0, result);
+}
+
+TEST_F(FileSystemFileStreamReaderTest, GetLengthNormal) {
+  scoped_ptr<FileSystemFileStreamReader> reader(
+      CreateFileReader(kTestFileName, 0, test_file_modification_time()));
+  net::TestInt64CompletionCallback callback;
+  int result = reader->GetLength(callback.callback());
+  if (result == net::ERR_IO_PENDING)
+    result = callback.WaitForResult();
+  ASSERT_EQ(kTestDataSize, result);
+}
+
+TEST_F(FileSystemFileStreamReaderTest, GetLengthAfterModified) {
+  // Pass a fake expected modifictaion time so that the expectation fails.
+  base::Time fake_expected_modification_time =
+      test_file_modification_time() - base::TimeDelta::FromSeconds(10);
+
+  scoped_ptr<FileSystemFileStreamReader> reader(
+      CreateFileReader(kTestFileName, 0, fake_expected_modification_time));
+  net::TestInt64CompletionCallback callback;
+  int result = reader->GetLength(callback.callback());
+  if (result == net::ERR_IO_PENDING)
+    result = callback.WaitForResult();
+  ASSERT_EQ(net::ERR_UPLOAD_FILE_CHANGED, result);
+
+  // With NULL expected modification time this should work.
+  reader.reset(CreateFileReader(kTestFileName, 0, base::Time()));
+  result = reader->GetLength(callback.callback());
+  if (result == net::ERR_IO_PENDING)
+    result = callback.WaitForResult();
+  ASSERT_EQ(kTestDataSize, result);
+}
+
+TEST_F(FileSystemFileStreamReaderTest, GetLengthWithOffset) {
+  scoped_ptr<FileSystemFileStreamReader> reader(
+      CreateFileReader(kTestFileName, 3, base::Time()));
+  net::TestInt64CompletionCallback callback;
+  int result = reader->GetLength(callback.callback());
+  if (result == net::ERR_IO_PENDING)
+    result = callback.WaitForResult();
+  // Initial offset does not affect the result of GetLength.
+  ASSERT_EQ(kTestDataSize, result);
+}
+
+TEST_F(FileSystemFileStreamReaderTest, ReadNormal) {
+  scoped_ptr<FileSystemFileStreamReader> reader(
+      CreateFileReader(kTestFileName, 0, test_file_modification_time()));
+  int result = 0;
+  std::string data;
+  ReadFromReader(reader.get(), &data, kTestDataSize, &result);
+  ASSERT_EQ(net::OK, result);
+  ASSERT_EQ(kTestData, data);
+}
+
+TEST_F(FileSystemFileStreamReaderTest, ReadAfterModified) {
+  // Pass a fake expected modifictaion time so that the expectation fails.
+  base::Time fake_expected_modification_time =
+      test_file_modification_time() - base::TimeDelta::FromSeconds(10);
+
+  scoped_ptr<FileSystemFileStreamReader> reader(
+      CreateFileReader(kTestFileName, 0, fake_expected_modification_time));
+  int result = 0;
+  std::string data;
+  ReadFromReader(reader.get(), &data, kTestDataSize, &result);
+  ASSERT_EQ(net::ERR_UPLOAD_FILE_CHANGED, result);
+  ASSERT_EQ(0U, data.size());
+
+  // With NULL expected modification time this should work.
+  data.clear();
+  reader.reset(CreateFileReader(kTestFileName, 0, base::Time()));
+  ReadFromReader(reader.get(), &data, kTestDataSize, &result);
+  ASSERT_EQ(net::OK, result);
+  ASSERT_EQ(kTestData, data);
+}
+
+TEST_F(FileSystemFileStreamReaderTest, ReadWithOffset) {
+  scoped_ptr<FileSystemFileStreamReader> reader(
+      CreateFileReader(kTestFileName, 3, base::Time()));
+  int result = 0;
+  std::string data;
+  ReadFromReader(reader.get(), &data, kTestDataSize, &result);
+  ASSERT_EQ(net::OK, result);
+  ASSERT_EQ(&kTestData[3], data);
+}
+
+TEST_F(FileSystemFileStreamReaderTest, DeleteWithUnfinishedRead) {
+  scoped_ptr<FileSystemFileStreamReader> reader(
+      CreateFileReader(kTestFileName, 0, base::Time()));
+
+  net::TestCompletionCallback callback;
+  scoped_refptr<net::IOBufferWithSize> buf(
+      new net::IOBufferWithSize(kTestDataSize));
+  int rv = reader->Read(buf, buf->size(), base::Bind(&NeverCalled));
+  ASSERT_TRUE(rv == net::ERR_IO_PENDING || rv >= 0);
+
+  // Delete immediately.
+  // Should not crash; nor should NeverCalled be callback.
+  reader.reset();
+}
+
+}  // namespace fileapi

webkit/fileapi/file_system_mount_point_provider.h

@@ -87,13 +87,18 @@ class FILEAPI_EXPORT FileSystemMountPointProvider {
       base::PlatformFileError* error_code) const = 0;
 
   // Creates a new file stream reader for a given filesystem URL |url| with an
-  // offset |offset|.
+  // offset |offset|. |expected_modification_time| specifies the expected last
+  // modification if the value is non-null, the reader will check the underlying
+  // file's actual modification time to see if the file has been modified, and
+  // if it does any succeeding read operations should fail with
+  // ERR_UPLOAD_FILE_CHANGED error.
   // The returned object must be owned and managed by the caller.
   // This method itself does *not* check if the given path exists and is a
   // regular file.
   virtual webkit_blob::FileStreamReader* CreateFileStreamReader(
     const FileSystemURL& url,
     int64 offset,
+    const base::Time& expected_modification_time,
     FileSystemContext* context) const = 0;
 
   // Creates a new file stream writer for a given filesystem URL |url| with an

webkit/fileapi/file_system_url_request_job.cc

@@ -205,7 +205,8 @@ void FileSystemURLRequestJob::DidGetMetadata(
   reader_.reset(
       file_system_context_->CreateFileStreamReader(
           url_,
-          byte_range_.first_byte_position()));
+          byte_range_.first_byte_position(),
+          base::Time()));
 
   set_expected_content_size(remaining_bytes_);
   response_info_.reset(new net::HttpResponseInfo());

webkit/fileapi/isolated_mount_point_provider.cc

@@ -144,10 +144,11 @@ webkit_blob::FileStreamReader*
 IsolatedMountPointProvider::CreateFileStreamReader(
     const FileSystemURL& url,
     int64 offset,
+    const base::Time& expected_modification_time,
     FileSystemContext* context) const {
   return new webkit_blob::LocalFileStreamReader(
       context->task_runners()->file_task_runner(),
-      url.path(), offset, base::Time());
+      url.path(), offset, expected_modification_time);
 }
 
 FileStreamWriter* IsolatedMountPointProvider::CreateFileStreamWriter(

webkit/fileapi/isolated_mount_point_provider.h

@@ -52,6 +52,7 @@ class IsolatedMountPointProvider : public FileSystemMountPointProvider {
   virtual webkit_blob::FileStreamReader* CreateFileStreamReader(
       const FileSystemURL& url,
       int64 offset,
+      const base::Time& expected_modification_time,
       FileSystemContext* context) const OVERRIDE;
   virtual FileStreamWriter* CreateFileStreamWriter(
       const FileSystemURL& url,

webkit/fileapi/sandbox_mount_point_provider.cc

@@ -470,8 +470,10 @@ webkit_blob::FileStreamReader*
 SandboxMountPointProvider::CreateFileStreamReader(
     const FileSystemURL& url,
     int64 offset,
+    const base::Time& expected_modification_time,
     FileSystemContext* context) const {
-  return new FileSystemFileStreamReader(context, url, offset);
+  return new FileSystemFileStreamReader(
+      context, url, offset, expected_modification_time);
 }
 
 fileapi::FileStreamWriter* SandboxMountPointProvider::CreateFileStreamWriter(

webkit/fileapi/sandbox_mount_point_provider.h

@@ -101,6 +101,7 @@ class FILEAPI_EXPORT SandboxMountPointProvider
   virtual webkit_blob::FileStreamReader* CreateFileStreamReader(
       const FileSystemURL& url,
       int64 offset,
+      const base::Time& expected_modification_time,
       FileSystemContext* context) const OVERRIDE;
   virtual FileStreamWriter* CreateFileStreamWriter(
       const FileSystemURL& url,

webkit/fileapi/test_mount_point_provider.cc

@@ -134,8 +134,10 @@ FileSystemOperation* TestMountPointProvider::CreateFileSystemOperation(
 webkit_blob::FileStreamReader* TestMountPointProvider::CreateFileStreamReader(
     const FileSystemURL& url,
     int64 offset,
+    const base::Time& expected_modification_time,
     FileSystemContext* context) const {
-  return new FileSystemFileStreamReader(context, url, offset);
+  return new FileSystemFileStreamReader(
+      context, url, offset, expected_modification_time);
 }
 
 fileapi::FileStreamWriter* TestMountPointProvider::CreateFileStreamWriter(

webkit/fileapi/test_mount_point_provider.h

@@ -58,6 +58,7 @@ class FILEAPI_EXPORT_PRIVATE TestMountPointProvider
   virtual webkit_blob::FileStreamReader* CreateFileStreamReader(
       const FileSystemURL& url,
       int64 offset,
+      const base::Time& expected_modification_time,
       FileSystemContext* context) const OVERRIDE;
   virtual FileStreamWriter* CreateFileStreamWriter(
       const FileSystemURL& url,