Add FileReader interface and implement FileSystemFileReader

webkit/fileapi/file_system_file_reader.cc

+// 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_reader.h"
+
+#include "base/file_util_proxy.h"
+#include "base/message_loop_proxy.h"
+#include "base/platform_file.h"
+#include "net/base/file_stream.h"
+#include "net/base/io_buffer.h"
+#include "net/base/net_errors.h"
+#include "webkit/blob/local_file_reader.h"
+#include "webkit/fileapi/file_system_context.h"
+#include "webkit/fileapi/file_system_operation_interface.h"
+
+using webkit_blob::LocalFileReader;
+
+namespace fileapi {
+
+namespace {
+
+void ReadAdapter(base::WeakPtr<FileSystemFileReader> reader,
+                 net::IOBuffer* buf, int buf_len,
+                 const net::CompletionCallback& callback) {
+  if (!reader.get())
+    return;
+  int rv = reader->Read(buf, buf_len, callback);
+  if (rv != net::ERR_IO_PENDING)
+    callback.Run(rv);
+}
+
+}
+
+FileSystemFileReader::FileSystemFileReader(
+    base::MessageLoopProxy* file_thread_proxy,
+    FileSystemContext* file_system_context,
+    const GURL& url,
+    int64 initial_offset)
+    : file_thread_proxy_(file_thread_proxy),
+      file_system_context_(file_system_context),
+      url_(url),
+      initial_offset_(initial_offset),
+      has_pending_operation_(false),
+      weak_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)) {
+}
+
+FileSystemFileReader::~FileSystemFileReader() {
+}
+
+int FileSystemFileReader::Read(
+    net::IOBuffer* buf, int buf_len,
+    const net::CompletionCallback& callback) {
+  if (local_file_reader_.get())
+    return local_file_reader_->Read(buf, buf_len, callback);
+  DCHECK(!has_pending_operation_);
+  FileSystemOperationInterface* operation =
+      file_system_context_->CreateFileSystemOperation(
+          url_, file_thread_proxy_);
+  if (!operation)
+    return net::ERR_INVALID_URL;
+  has_pending_operation_ = true;

[michaeln, 2012/04/13 21:14:59]

maybe clarify this referes to a pending snapshot and not a pending read...
has_pending_snapshot_operation_

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

Done.

+  operation->CreateSnapshotFile(
+      url_,
+      base::Bind(&FileSystemFileReader::DidCreateSnapshot,
+                 weak_factory_.GetWeakPtr(),
+                 base::Bind(&ReadAdapter, weak_factory_.GetWeakPtr(),
+                            make_scoped_refptr(buf), buf_len, callback),
+                 callback));
+  return net::ERR_IO_PENDING;
+}
+
+void FileSystemFileReader::DidCreateSnapshot(
+    const base::Closure& post_task_closure,

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

? maybe call this the 'read_closure' or 'read_continuation_closure'

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

Done.

+    const net::CompletionCallback& callback,
+    base::PlatformFileError file_error,
+    const base::PlatformFileInfo& file_info,
+    const FilePath& platform_path,
+    const scoped_refptr<webkit_blob::ShareableFileReference>& file_ref) {
+  DCHECK(has_pending_operation_);
+  DCHECK(!local_file_reader_.get());
+  has_pending_operation_ = false;
+
+  if (file_error != base::PLATFORM_FILE_OK) {
+    weak_factory_.InvalidateWeakPtrs();

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

Same question about why call invalidate, is it important to do so?

The reason i'm asking is because i think it would be easy to neglect doing so
(in future changes)... if its important to do so code comments might help, and
if its not important to, maybe just don't do it to avoid a few lines of code
that could be confusing in the future. 

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

Makes sense, dropped the line.

+    callback.Run(LocalFileReader::PlatformFileErrorToNetError(file_error));
+    return;
+  }
+
+  // Keep the reference (if it's non-null) so that the file won't go away.
+  snapshot_ref_ = file_ref;
+
+  local_file_reader_.reset(
+      new LocalFileReader(file_thread_proxy_,
+                          platform_path,
+                          initial_offset_,
+                          base::Time()));
+
+  post_task_closure.Run();
+  weak_factory_.InvalidateWeakPtrs();
+}
+
+}  // namespace fileapi