Add FileReader interface and implement FileSystemFileReader

webkit/blob/local_file_reader.h

Index: webkit/blob/local_file_reader.h
diff --git a/webkit/blob/local_file_reader.h b/webkit/blob/local_file_reader.h
index f7d3b27338bab6469bb80c27e1f4a33c7fbe533f..12e85921cea6fbff04ac49ece80609912c110d53 100644
--- a/webkit/blob/local_file_reader.h
+++ b/webkit/blob/local_file_reader.h
@@ -9,29 +9,28 @@
 #include "base/basictypes.h"
 #include "base/compiler_specific.h"
 #include "base/file_path.h"
+#include "base/platform_file.h"
 #include "base/memory/weak_ptr.h"
 #include "base/time.h"
-#include "net/base/completion_callback.h"
 #include "webkit/blob/blob_export.h"
+#include "webkit/blob/file_reader.h"
 
 namespace base {
 class MessageLoopProxy;
 }
 
-namespace net {
-class FileStream;
-class IOBuffer;
-}
-
 namespace webkit_blob {
 
 // A thin wrapper of net::FileStream with range support for sliced file
 // handling.
-class BLOB_EXPORT LocalFileReader {
+class BLOB_EXPORT LocalFileReader : public FileReader {
  public:
   typedef base::Callback<void(int error, scoped_ptr<net::FileStream> stream)>
       OpenFileStreamCallback;
 
+  // A convenient method to translate platform file error to net error code.
+  static int PlatformFileErrorToNetError(base::PlatformFileError file_error);
+
   // Creates a new FileReader for a local file |file_path|.
   // |initial_offset| specifies the offset in the file where the first read
   // should start.  If the given offset is out of the file range any
@@ -42,26 +41,24 @@ class BLOB_EXPORT LocalFileReader {
   // 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.
+  // TODO(kinuko): Consider using SequencedWorkerPool.
   LocalFileReader(base::MessageLoopProxy* file_thread_proxy,
                   const FilePath& file_path,
                   int64 initial_offset,
                   const base::Time& expected_modification_time);
+  virtual ~LocalFileReader();
 
-  ~LocalFileReader();
-
-  // Reads from the current cursor position asynchronously.
-  // This works mostly same as how net::FileStream::Read() works except that
-  // it internally opens (and seeks) the file if it is not opened yet.
-  // It is invalid to call Read while there is an in-flight Read operation.
-  int Read(net::IOBuffer* buf, int buf_len,
-           const net::CompletionCallback& callback);
+  // FileReader overrides.
+  virtual int Read(net::IOBuffer* buf, int buf_len,
+                   const net::CompletionCallback& callback) OVERRIDE;
 
-  // Returns the number of bytes available to read from the beginning of
-  // the file (or initial_offset) until the end of the file (rv >= 0 cases).
+  // Returns the length of the file if it could successfully retrieve the
+  // file info *and* its last modification time equals to
+  // expected_modification_time_ (rv >= 0 cases).
   // Otherwise, a negative error code is returned (rv < 0 cases).

[michaeln, 2012/04/13 20:24:51]

Looks like your changing the semantics of this method. Now it always returns the
full length of file_path whereas before it returned file_len - initial_offset.
Wondering why?

[kinuko, 2012/04/16 10:24:21]

I thought always returning the file length would be more intuitive or less error
prone.  (On the other hand I'm feeling that we may want to revisit here if we
want to expose a method like this, GetLength() or Available() or something, on
FileReader.)

-  int GetLength(const net::CompletionCallback& callback);
+  int GetLength(const net::Int64CompletionCallback& callback);
 
- private:
+ protected:

[michaeln, 2012/04/13 20:24:51]

Are there any derivatives of this class that need access to these data members?
If not... keep it private?

[kinuko, 2012/04/16 10:24:21]

Nope, fixed.

   class OpenFileStreamHelper;
 
   int Open(const OpenFileStreamCallback& callback);