[fsp] Introduce BufferingFileStreamReader to read files in bigger chunks.

chrome/browser/chromeos/file_system_provider/fileapi/buffering_file_stream_reader.h

+// Copyright 2014 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.
+
+#ifndef CHROME_BROWSER_CHROMEOS_FILE_SYSTEM_PROVIDER_FILEAPI_BUFFERING_FILE_STREAM_READER_H_
+#define CHROME_BROWSER_CHROMEOS_FILE_SYSTEM_PROVIDER_FILEAPI_BUFFERING_FILE_STREAM_READER_H_
+
+#include "base/basictypes.h"
+#include "base/memory/ref_counted.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/memory/weak_ptr.h"
+#include "net/base/io_buffer.h"

[hashimoto, 2014/06/05 03:18:36]

nit: forward declaration is enough.

[mtomasz, 2014/06/05 09:22:27]

Done.

+#include "webkit/browser/blob/file_stream_reader.h"
+
+namespace fileapi {
+class AsyncFileUtil;

[hashimoto, 2014/06/05 03:18:36]

nit: unused forward declaration.

[mtomasz, 2014/06/05 09:22:27]

Done.

+}  // namespace fileapi
+
+namespace chromeos {
+namespace file_system_provider {
+
+// Wraps the file stream reader implementation with a prefetching buffer. This
+// limits number of IPC to providing extensions significantly. For remote (HTTP
+// base) file systems, this also reduces overhead when creating HTTP requests
+// for each chunk. The chunks are bigger, so number of requests is smaller.
+class BufferingFileStreamReader : public webkit_blob::FileStreamReader {
+ public:
+  explicit BufferingFileStreamReader(

[hashimoto, 2014/06/05 03:18:36]

nit: "explicit" unneeded.

[mtomasz, 2014/06/05 09:22:27]

Done.

+      scoped_ptr<webkit_blob::FileStreamReader> file_stream_reader,
+      int buffer_size);
+
+  virtual ~BufferingFileStreamReader();
+
+  // webkit_blob::FileStreamReader overrides.
+  virtual int Read(net::IOBuffer* buf,
+                   int buf_len,
+                   const net::CompletionCallback& callback) OVERRIDE;
+  virtual int64 GetLength(
+      const net::Int64CompletionCallback& callback) OVERRIDE;
+
+ private:
+  // Copies data from the preloading buffer and updates the internal iterator.
+  // Returns number of bytes successfully copied.
+  int CopyFromBuffer(scoped_refptr<net::IOBuffer> buffer, int buffer_length);
+
+  // Preloads data from the internal stream reader and calls the |callback|.
+  void Preload(const net::CompletionCallback& callback);
+
+  // Called when preloading of a buffer chunk is finished. Updates state of the
+  // preloading buffer and copied requested data to the |buffer|.
+  void OnPreloadCompleted(scoped_refptr<net::IOBuffer> buffer,
+                          int buffer_length,
+                          const net::CompletionCallback& callback,
+                          int result);
+
+  scoped_ptr<webkit_blob::FileStreamReader> file_stream_reader_;
+  int buffer_size_;
+  scoped_refptr<net::IOBuffer> preloading_buffer_;

[hashimoto, 2014/06/05 03:18:36]

How about using DrainableIOBuffer?

[mtomasz, 2014/06/05 09:22:27]

Net::DrainableIOBuffer works well if the buffer is always fully filled out. In
our case, the preloading_buffer_ is often not. So, we would have to recreate the
net::DrainableIOBuffer every time it is filled out. Otherwise,
DrainableIOBuffer::BytesRemaining() would return an incorrect value for not
fully-filled out buffers. I'm not very sure if it would be better here, since we
would need two members:

scoped_refptr<net::IOBuffer> internal_buffer;  // internal buffer for
current_buffer.
scoped_refptr<net::DrainableIOBuffer> current_buffer;  // recreated every
Preload() call.

WDYT?

[hashimoto, 2014/06/05 10:07:10]

I meant by using DrailableIOBuffer there is no need to have
|preloading_buffer_offset_| and manually offset the pointer.
You can use SetOffset() and DidConsume() to do the same thing as you do by
changing the value of preloading_buffer_offset_.

You can reuse the same DrainableIOBuffer by resetting the offset with
SetOffset(0).

[mtomasz, 2014/06/05 11:27:43]

This won't work since the buffer is not always full.
Eg. if there is only 10 bytes in the buffer of size 50, then BytesRemaining()
will return 50, once it should return 10.

[hashimoto, 2014/06/05 12:31:09]

You don't need BytesRemaining().
What I suggested was replacing:
- "preloading_buffer_offset_ += read_bytes;" with DidConsume(read_bytes)
- "preloading_buffer_offset_ = 0;" with SetOffset(0)
so that you can manage the offset with the buffer itself.
This is just a suggestion.  You can use whatever you think reasonable.

[mtomasz, 2014/06/06 01:31:02]

DrailableIOBuffer is designed for fully filled out buffers. I think it is
confusing to use it here, since some of methods would return misleading values
(even if we don't use them). Let me go with the simpler solution, using
preloading_buffer_offset_.

+  int preloading_buffer_offset_;
+  int buffered_bytes_;
+
+  base::WeakPtrFactory<BufferingFileStreamReader> weak_ptr_factory_;
+  DISALLOW_COPY_AND_ASSIGN(BufferingFileStreamReader);
+};
+
+}  // namespace file_system_provider
+}  // namespace chromeos
+
+#endif  // CHROME_BROWSER_CHROMEOS_FILE_SYSTEM_PROVIDER_FILEAPI_BUFFERING_FILE_STREAM_READER_H_