Pepper quota support

webkit/plugins/ppapi/quota_file_io.cc

+// Copyright (c) 2011 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/plugins/ppapi/quota_file_io.h"
+
+#include "base/stl_util.h"
+#include "webkit/plugins/ppapi/ppapi_plugin_instance.h"
+
+using base::PlatformFile;
+using base::PlatformFileError;
+using quota::StorageType;
+
+namespace webkit {
+namespace ppapi {
+
+namespace {
+StorageType PPFileSystemTypeToQuotaStorageType(PP_FileSystemType type) {
+  switch (type) {
+    case PP_FILESYSTEMTYPE_LOCALPERSISTENT:
+      return quota::kStorageTypePersistent;
+    case PP_FILESYSTEMTYPE_LOCALTEMPORARY:
+      return quota::kStorageTypeTemporary;
+    default:
+      return quota::kStorageTypeUnknown;
+  }
+  NOTREACHED();
+}
+}  // namespace
+
+class QuotaFileIO::PendingOperationBase {
+ public:
+  virtual ~PendingOperationBase() {}
+
+  // Either one of Run() or DidFail() is called (the latter is called when
+  // there was more than one error during quota queries).
+  virtual void Run() = 0;
+  virtual void DidFail(PlatformFileError error) = 0;
+
+ protected:
+  PendingOperationBase(QuotaFileIO* quota_io, bool is_will_operation)
+      : quota_io_(quota_io), is_will_operation_(is_will_operation) {
+    DCHECK(quota_io_);
+    quota_io_->RegisterOperation(this);
+    quota_io_->WillUpdate();
+  }
+
+  QuotaFileIO* quota_io_;
+  const bool is_will_operation_;
+};
+
+class QuotaFileIO::WriteOperation : public PendingOperationBase {
+ public:
+  WriteOperation(QuotaFileIO* quota_io,
+                 bool is_will_operation,
+                 int64_t offset,
+                 const char* buffer,
+                 int32_t bytes_to_write,
+                 WriteCallback* callback)
+      : PendingOperationBase(quota_io, is_will_operation),
+        offset_(offset),
+        bytes_to_write_(bytes_to_write),
+        callback_(callback),
+        callback_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)) {
+    if (!is_will_operation) {
+      DCHECK(buffer);
+      buffer_.reset(new char[bytes_to_write]);

[michaeln, 2011/07/26 19:25:08]

Do we need the copy the bytes? If the caller is required to keep the ptr valid
until the callback is invoked, is this copy really needed? I see that
PPB_FileIO_Impl does not may a copy.

[kinuko, 2011/07/27 09:33:14]

Right.  I wanted to keep the pointers to make my tests run, but if that's the
spec I should rather change the test code...

[yzshen1, 2011/07/27 18:26:07]

The reason why PPB_FileIO_Impl doesn't need to make a copy is that
FileUtilProxy::Write makes a copy.

I am not sure whether it is okay to assume that the buffer will stay valid until
the callback is invoked. The API definition doesn't mention it.

I agree that it looks bad to copy the buffer (especially later
FileUtilProxy::Write will do it one more time), but please double check that the
assumption (buffer stays valid until callback returns) is consistent with the
overall API convention.

On 2011/07/27 09:33:14, kinuko wrote:

[kinuko, 2011/07/28 14:11:00]

Ok, let me revert this change for now and keep a copy of the buffer until/unless
we can be sure not having copy is ok.  (I wasn't able to find out the particular
notion about that in the api header document)
I put a TODO comment too.

+      memcpy(buffer_.get(), buffer, bytes_to_write);
+    }
+  }
+
+  virtual ~WriteOperation() OVERRIDE {

[yzshen1, 2011/07/26 19:26:44]

I think it is not OVERRIDE, is it?

[kinuko, 2011/07/27 09:33:14]

Done.

+    if (callback_.get())
+      callback_->Run(base::PLATFORM_FILE_ERROR_ABORT, 0);
+  }
+  virtual void Run() OVERRIDE {
+    DCHECK(quota_io_);
+    if (quota_io_->CheckIfExceedsQuota(offset_ + bytes_to_write_)) {

[yzshen1, 2011/07/26 19:26:44]

In addition to the problem that I mentioned in another file about mixing
WillWrite and Write, it seems we have the same problem with a set of writes:

Consider two write ops: write_1 and write_2.
The callbacks from the user of PPB_FileIO_Impl are queued up in |callbacks_|: <
write_1_callback, write_2_callback >.

write_1 passes the check on line 78, so it goes to line 88.
However, write_2 fails to pass the check and goes to DidFail.

In this case, it is possible that write_2 is completed first, and calls the
first callback in |callbacks_|, that is, write_1_callback.

The key is, PPB_FileIO_Impl expects that the order of write completion is
exactly the same as the order it calls QuotaFileIO::Write. You need to either
change PPB_FileIO_Impl to not rely on that assumption or fix QuotaFileIO.

[michaeln, 2011/07/26 19:58:07]

Doh... right... my vote would be to make QuotaFileIO invoke the callbacks in
order so PPB_FileIO remains the same. This looks good besides the order in which
the callbacks are invoked. This class could defer calling the callbacks until
the 'batch' of pending ops initiated in DidQueryForQuotaCheck all completed, and
the call them in order.

[kinuko, 2011/07/27 09:33:14]

Done.  Thanks for your careful review.
Changed the code to have another queue for pending callbacks.

+      DidFail(base::PLATFORM_FILE_ERROR_NO_SPACE);
+      return;
+    }
+    if (is_will_operation_) {
+      // Assuming the write will succeed.
+      DidFinish(base::PLATFORM_FILE_OK, bytes_to_write_);
+      return;
+    }
+    DCHECK(buffer_.get());
+    if (!base::FileUtilProxy::Write(
+            quota_io_->instance_->delegate()->GetFileThreadMessageLoopProxy(),
+            quota_io_->file_, offset_, buffer_.get(), bytes_to_write_,
+            callback_factory_.NewCallback(&WriteOperation::DidFinish))) {
+      DidFail(base::PLATFORM_FILE_ERROR_FAILED);
+      return;
+    }
+  }
+
+  virtual void DidFail(PlatformFileError error) OVERRIDE {
+    DidFinish(error, 0);
+  }
+
+ private:
+  void DidFinish(PlatformFileError status, int bytes_written) {
+    quota_io_->DidWrite(offset_ + bytes_written);
+    quota_io_->UnregisterOperation(this);
+    DCHECK(callback_.get());
+    callback_->Run(status, bytes_written);
+    callback_.reset();
+    delete this;
+  }
+
+  const int64_t offset_;
+  scoped_array<char> buffer_;
+  const int32_t bytes_to_write_;
+  scoped_ptr<WriteCallback> callback_;
+  base::ScopedCallbackFactory<QuotaFileIO::WriteOperation> callback_factory_;
+};
+
+class QuotaFileIO::SetLengthOperation : public PendingOperationBase {
+ public:
+  SetLengthOperation(QuotaFileIO* quota_io,
+                     bool is_will_operation,
+                     int64_t length,
+                     StatusCallback* callback)
+      : PendingOperationBase(quota_io, is_will_operation),
+        length_(length),
+        callback_(callback),
+        callback_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)) {}
+
+  virtual ~SetLengthOperation() OVERRIDE {

[yzshen1, 2011/07/26 19:26:44]

I think it is not OVERRIDE, is it?

[kinuko, 2011/07/27 09:33:14]

Done.

+    if (callback_.get())
+      callback_->Run(base::PLATFORM_FILE_ERROR_ABORT);
+  }
+
+  virtual void Run() OVERRIDE {
+    DCHECK(quota_io_);
+    if (quota_io_->CheckIfExceedsQuota(length_)) {
+      DidFail(base::PLATFORM_FILE_ERROR_NO_SPACE);
+      return;
+    }
+    quota_io_->WillUpdate();
+    if (is_will_operation_) {
+      DidFinish(base::PLATFORM_FILE_OK);
+      return;
+    }
+    if (!base::FileUtilProxy::Truncate(
+            quota_io_->instance_->delegate()->GetFileThreadMessageLoopProxy(),
+            quota_io_->file_, length_,
+            callback_factory_.NewCallback(&SetLengthOperation::DidFinish))) {
+      DidFail(base::PLATFORM_FILE_ERROR_FAILED);
+      return;
+    }
+  }
+
+  virtual void DidFail(PlatformFileError error) OVERRIDE {
+    DidFinish(error);
+  }
+
+ private:
+  void DidFinish(PlatformFileError status) {
+    quota_io_->DidSetLength(status != base::PLATFORM_FILE_OK ? -1 : length_);
+    quota_io_->UnregisterOperation(this);
+    DCHECK(callback_.get());
+    callback_->Run(status);
+    callback_.reset();
+    delete this;
+  }
+
+  int64_t length_;
+  scoped_ptr<StatusCallback> callback_;
+  base::ScopedCallbackFactory<QuotaFileIO::SetLengthOperation>
+      callback_factory_;
+};
+
+// QuotaFileIO --------------------------------------------------------------
+
+QuotaFileIO::QuotaFileIO(
+    PluginInstance* instance,
+    PlatformFile file,
+    const GURL& file_url,
+    PP_FileSystemType type)
+    : instance_(instance),
+      file_(file),
+      file_url_(file_url),
+      storage_type_(PPFileSystemTypeToQuotaStorageType(type)),
+      cached_file_size_(0),
+      cached_available_space_(0),
+      outstanding_quota_queries_(0),
+      outstanding_errors_(0),
+      did_notify_will_update_(false),
+      max_written_offset_(0),
+      callback_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)) {
+  DCHECK(instance_);
+  DCHECK_NE(base::kInvalidPlatformFileValue, file_);
+  DCHECK_NE(quota::kStorageTypeUnknown, storage_type_);
+}
+
+QuotaFileIO::~QuotaFileIO() {
+  STLDeleteContainerPointers(registered_operations_.begin(),
+                             registered_operations_.end());
+}
+
+bool QuotaFileIO::Write(
+    int64_t offset, const char* buffer, int32_t bytes_to_write,
+    WriteCallback* callback) {
+  WriteOperation* op = new WriteOperation(
+      this, false, offset, buffer, bytes_to_write, callback);
+  return RegisterOperationForQuotaChecks(op);
+}
+
+bool QuotaFileIO::SetLength(int64_t length, StatusCallback* callback) {
+  DCHECK(pending_operations_.empty());
+  SetLengthOperation* op = new SetLengthOperation(
+      this, false, length, callback);
+  return RegisterOperationForQuotaChecks(op);
+}
+
+bool QuotaFileIO::WillWrite(
+    int64_t offset, int32_t bytes_to_write, WriteCallback* callback) {
+  WriteOperation* op = new WriteOperation(
+      this, true, offset, NULL, bytes_to_write, callback);
+  return RegisterOperationForQuotaChecks(op);
+}
+
+bool QuotaFileIO::WillSetLength(int64_t length, StatusCallback* callback) {
+  DCHECK(pending_operations_.empty());
+  SetLengthOperation* op = new SetLengthOperation(this, true, length, callback);
+  return RegisterOperationForQuotaChecks(op);
+}
+
+bool QuotaFileIO::RegisterOperationForQuotaChecks(
+    PendingOperationBase* op_ptr) {
+  scoped_ptr<PendingOperationBase> op(op_ptr);
+  if (pending_operations_.empty()) {
+    // This is the first pending quota check. Run querying the file size
+    // and available space.
+    outstanding_quota_queries_ = 0;
+    outstanding_errors_ = 0;
+
+    // Query the file size.
+    ++outstanding_quota_queries_;
+    if (!base::FileUtilProxy::GetFileInfoFromPlatformFile(
+            instance_->delegate()->GetFileThreadMessageLoopProxy(), file_,
+            callback_factory_.NewCallback(&QuotaFileIO::DidQueryInfoForQuota)))
+      return false;

[brettw, 2011/07/26 16:40:14]

Can you add a comment here that the op_ptr destructor will handle issuing the
callback with an error? I was initially confused about this.

[kinuko, 2011/07/27 09:33:14]

Changed to explicitly call DidFail.

+
+    // Query the current available space.
+    ++outstanding_quota_queries_;
+    instance_->delegate()->QueryAvailableSpace(
+        GURL(file_url_.path()).GetOrigin(), storage_type_,
+        callback_factory_.NewCallback(&QuotaFileIO::DidQueryAvailableSpace));
+  }
+  pending_operations_.push(op.release());
+  return true;
+}
+
+void QuotaFileIO::DidQueryInfoForQuota(
+    base::PlatformFileError error_code,
+    const base::PlatformFileInfo& file_info) {
+  if (error_code != base::PLATFORM_FILE_OK)
+    ++outstanding_errors_;
+  cached_file_size_ = file_info.size;
+  DCHECK_GT(outstanding_quota_queries_, 0);
+  if (--outstanding_quota_queries_ == 0)
+    DidQueryForQuotaCheck();
+}
+
+void QuotaFileIO::DidQueryAvailableSpace(int64_t avail_space) {
+  cached_available_space_ = avail_space;
+  DCHECK_GT(outstanding_quota_queries_, 0);
+  if (--outstanding_quota_queries_ == 0)
+    DidQueryForQuotaCheck();
+}
+
+void QuotaFileIO::DidQueryForQuotaCheck() {
+  DCHECK(!pending_operations_.empty());
+  while (!pending_operations_.empty()) {
+    PendingOperationBase* op = pending_operations_.front();
+    pending_operations_.pop();
+    if (outstanding_errors_ > 0) {
+      op->DidFail(base::PLATFORM_FILE_ERROR_FAILED);
+      continue;
+    }
+    op->Run();
+  }
+}
+
+bool QuotaFileIO::CheckIfExceedsQuota(int64_t new_file_size) const {
+  DCHECK_GE(cached_file_size_, 0);
+  DCHECK_GE(cached_available_space_, 0);
+  return new_file_size - cached_file_size_ > cached_available_space_;
+}
+
+void QuotaFileIO::WillUpdate() {
+  if (!did_notify_will_update_) {
+    instance_->delegate()->WillUpdateFile(file_url_);
+    did_notify_will_update_ = true;
+    DCHECK_EQ(0, max_written_offset_);
+  }
+}
+
+void QuotaFileIO::DidWrite(int64_t written_offset_end) {
+  max_written_offset_ = std::max(max_written_offset_, written_offset_end);
+  DCHECK(did_notify_will_update_);
+  // If we have no more pending writes, notify the browser that we did
+  // update the file.
+  if (registered_operations_.size() == 1) {
+    int64_t growth = max_written_offset_ - cached_file_size_;
+    growth = growth < 0 ? 0 : growth;
+    instance_->delegate()->DidUpdateFile(file_url_, growth);
+    did_notify_will_update_ = false;
+    max_written_offset_ = 0;
+  }
+}
+
+void QuotaFileIO::DidSetLength(int64_t new_file_size) {
+  DCHECK(did_notify_will_update_);
+  int64_t delta = new_file_size >= 0 ? new_file_size - cached_file_size_ : 0;
+  instance_->delegate()->DidUpdateFile(file_url_, delta);
+  did_notify_will_update_ = false;
+}
+
+void QuotaFileIO::RegisterOperation(PendingOperationBase* operation) {
+  registered_operations_.insert(operation);
+}
+
+void QuotaFileIO::UnregisterOperation(PendingOperationBase* op) {
+  DCHECK(registered_operations_.find(op) != registered_operations_.end());
+  registered_operations_.erase(op);
+}
+
+}  // namespace ppapi
+}  // namespace webkit