Pepper quota support

webkit/plugins/ppapi/ppb_file_io_impl.cc

Index: webkit/plugins/ppapi/ppb_file_io_impl.cc
diff --git a/webkit/plugins/ppapi/ppb_file_io_impl.cc b/webkit/plugins/ppapi/ppb_file_io_impl.cc
index 309067cd1b5cbf51783770aa46f8c63a8acff00a..6cc4e1f1533db69ff2a577e0643fbe71579e20a3 100644
--- a/webkit/plugins/ppapi/ppb_file_io_impl.cc
+++ b/webkit/plugins/ppapi/ppb_file_io_impl.cc
@@ -4,7 +4,6 @@
 
 #include "webkit/plugins/ppapi/ppb_file_io_impl.h"
 
-#include "base/callback.h"
 #include "base/file_util.h"
 #include "base/file_util_proxy.h"
 #include "base/message_loop_proxy.h"
@@ -18,12 +17,14 @@
 #include "ppapi/shared_impl/time_conversion.h"
 #include "ppapi/thunk/enter.h"
 #include "ppapi/thunk/ppb_file_ref_api.h"
+#include "webkit/fileapi/file_system_util.h"
 #include "webkit/plugins/ppapi/common.h"
 #include "webkit/plugins/ppapi/file_type_conversions.h"
 #include "webkit/plugins/ppapi/plugin_module.h"
 #include "webkit/plugins/ppapi/ppapi_plugin_instance.h"
 #include "webkit/plugins/ppapi/ppb_file_ref_impl.h"
 #include "webkit/plugins/ppapi/resource_tracker.h"
+#include "webkit/quota/quota_client.h"
 
 using ppapi::PPTimeToTime;
 using ppapi::TimeToPPTime;
@@ -34,6 +35,25 @@ using ppapi::thunk::PPB_FileRef_API;
 namespace webkit {
 namespace ppapi {
 
+namespace {
+quota::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();
+}
+
+bool DoesRequireQuotaCheck(PP_FileSystemType type) {
+  return (type == PP_FILESYSTEMTYPE_LOCALTEMPORARY ||
+          type == PP_FILESYSTEMTYPE_LOCALPERSISTENT);
+}
+}  // anonymous namespace
+
 PPB_FileIO_Impl::CallbackEntry::CallbackEntry()
     : read_buffer(NULL) {
 }
@@ -51,7 +71,8 @@ PPB_FileIO_Impl::PPB_FileIO_Impl(PluginInstance* instance)
       ALLOW_THIS_IN_INITIALIZER_LIST(callback_factory_(this)),
       file_(base::kInvalidPlatformFileValue),
       pending_op_(OPERATION_NONE),
-      info_(NULL) {
+      info_(NULL),
+      pending_setlength_length_(-1) {
 }
 
 PPB_FileIO_Impl::~PPB_FileIO_Impl() {
@@ -88,13 +109,16 @@ int32_t PPB_FileIO_Impl::Open(PP_Resource pp_file_ref,
         return PP_ERROR_FAILED;
       break;
     case PP_FILESYSTEMTYPE_LOCALPERSISTENT:
-    case PP_FILESYSTEMTYPE_LOCALTEMPORARY:
+    case PP_FILESYSTEMTYPE_LOCALTEMPORARY: {
+      GURL root_url = file_ref->GetFileSystemURL();
       if (!instance()->delegate()->AsyncOpenFileSystemURL(
-              file_ref->GetFileSystemURL(), flags,
+              root_url, flags,
               callback_factory_.NewCallback(
                   &PPB_FileIO_Impl::AsyncOpenFileCallback)))
         return PP_ERROR_FAILED;
+      origin_url_ = GURL(root_url.path()).GetOrigin();
       break;
+    }
     default:
       return PP_ERROR_FAILED;
   }
@@ -168,11 +192,31 @@ int32_t PPB_FileIO_Impl::Write(int64_t offset,
   if (rv != PP_OK)
     return rv;
 
-  if (!base::FileUtilProxy::Write(
-          instance()->delegate()->GetFileThreadMessageLoopProxy(),
-          file_, offset, buffer, bytes_to_write,
-          callback_factory_.NewCallback(&PPB_FileIO_Impl::WriteCallback)))
-    return PP_ERROR_FAILED;
+  PendingWrite write;
+  write.offset = offset;
+  write.buffer = buffer;
+  write.bytes_to_write = bytes_to_write;
+
+  int64 expected_growth = offset + bytes_to_write - file_size_;

[yzshen1, 2011/07/19 17:41:55]

It is possible that this method gets called for multiple times before we get
replies via WriteCallback / DidCheckQuotaForWrite.
In that case, file_size_ is not correct, right?

[michaeln, 2011/07/19 22:01:45]

I have these concerns too, keeping track of filesize looks fragile.

This is a usecase where having a cached 'space_available' value being updated by
the backend would be really helpful. Stopping the write to ask the browser and
them resuming the write is bumpy.

[kinuko, 2011/07/20 13:39:35]

Putting aside the fact that the current design is not really great, I think this
would be ok (if my assumptions are correct).  By writes the file_size_ can only
be extended but not be truncated, and every write comes with a proper offset--
therefore parallel writes won't screw up the file_size_.
I updated the patch to do some caching.

+  if (!DoesRequireQuotaCheck(file_system_type_) ||
+      expected_growth <= 0) {
+    // we're ok to go.
+    pending_write_callbacks_.push(write);
+    if (!base::FileUtilProxy::Write(
+            instance()->delegate()->GetFileThreadMessageLoopProxy(),
+            file_, offset, buffer, bytes_to_write,
+            callback_factory_.NewCallback(&PPB_FileIO_Impl::WriteCallback))) {
+      pending_write_callbacks_.pop();
+      return PP_ERROR_FAILED;
+    }
+  } else {
+    pending_writes_.push(write);
+    instance()->delegate()->CanWrite(
+        origin_url_,
+        PPFileSystemTypeToQuotaStorageType(file_system_type_),
+        expected_growth,
+        callback_factory_.NewCallback(&PPB_FileIO_Impl::DidCheckQuotaForWrite));
+  }
 
   RegisterCallback(OPERATION_WRITE, callback, NULL);
   return PP_OK_COMPLETIONPENDING;
@@ -184,11 +228,28 @@ int32_t PPB_FileIO_Impl::SetLength(int64_t length,
   if (rv != PP_OK)
     return rv;
 
-  if (!base::FileUtilProxy::Truncate(
-          instance()->delegate()->GetFileThreadMessageLoopProxy(),
-          file_, length,
-          callback_factory_.NewCallback(&PPB_FileIO_Impl::StatusCallback)))
-    return PP_ERROR_FAILED;
+  DCHECK_EQ(-1, pending_setlength_length_);

[michaeln, 2011/07/19 22:01:45]

What guarantees are there that there's only one SetLength() pending at a time...
just checking.

[kinuko, 2011/07/20 13:39:35]

Seems like EXCLUSIVE operations cannot run in parallel, if I understand
correctly.  (Otherwise I'll change this with queue)

[yzshen1, 2011/07/20 20:50:36]

Yes, that is correct.

On 2011/07/20 13:39:35, kinuko wrote:

+  pending_setlength_length_ = length;
+
+  int64 expected_growth = length - file_size_;
+  if (!DoesRequireQuotaCheck(file_system_type_) ||
+      expected_growth <= 0) {
+    if (!base::FileUtilProxy::Truncate(
+            instance()->delegate()->GetFileThreadMessageLoopProxy(),
+            file_, length,
+            callback_factory_.NewCallback(
+                &PPB_FileIO_Impl::SetLengthCallback))) {
+      pending_setlength_length_ = -1;
+      return PP_ERROR_FAILED;
+    }
+  } else {
+    instance()->delegate()->CanWrite(
+        origin_url_,
+        PPFileSystemTypeToQuotaStorageType(file_system_type_),
+        expected_growth,
+        callback_factory_.NewCallback(
+            &PPB_FileIO_Impl::DidCheckQuotaForSetLength));
+  }
 
   RegisterCallback(OPERATION_EXCLUSIVE, callback, NULL);
   return PP_OK_COMPLETIONPENDING;
@@ -304,7 +365,8 @@ void PPB_FileIO_Impl::StatusCallback(base::PlatformFileError error_code) {
 
 void PPB_FileIO_Impl::AsyncOpenFileCallback(
     base::PlatformFileError error_code,
-    base::PassPlatformFile file) {
+    base::PassPlatformFile file,
+    int64_t file_size) {
   if (pending_op_ != OPERATION_EXCLUSIVE || callbacks_.empty()) {
     NOTREACHED();
     return;
@@ -312,6 +374,7 @@ void PPB_FileIO_Impl::AsyncOpenFileCallback(
 
   DCHECK(file_ == base::kInvalidPlatformFileValue);
   file_ = file.ReleaseValue();
+  file_size_ = file_size;
   RunAndRemoveFirstPendingCallback(PlatformFileErrorToPepperError(error_code));
 }
 
@@ -369,13 +432,77 @@ void PPB_FileIO_Impl::WriteCallback(base::PlatformFileError error_code,
     return;
   }
 
+  DCHECK(!pending_write_callbacks_.empty());
+  PendingWrite write = pending_write_callbacks_.front();
+  pending_write_callbacks_.pop();
+
   if (error_code != base::PLATFORM_FILE_OK) {
     RunAndRemoveFirstPendingCallback(
         PlatformFileErrorToPepperError(error_code));
   } else {
+    int64 growth = write.offset + bytes_written - file_size_;
+    if (growth >= 0) {
+      if (DoesRequireQuotaCheck(file_system_type_)) {
+        instance()->delegate()->NotifyStorageModified(
+            quota::QuotaClient::kFileSystem,
+            origin_url_,
+            PPFileSystemTypeToQuotaStorageType(file_system_type_),
+            growth);
+      }
+      file_size_ += growth;
+    }
     RunAndRemoveFirstPendingCallback(bytes_written);
   }
 }
 
+void PPB_FileIO_Impl::SetLengthCallback(
+    base::PlatformFileError error_code) {
+  if (callbacks_.empty()) {
+    NOTREACHED();
+    return;
+  }
+
+  DCHECK_NE(-1, pending_setlength_length_);
+
+  if (base::PLATFORM_FILE_OK == error_code &&
+      DoesRequireQuotaCheck(file_system_type_)) {
+    int64 delta = pending_setlength_length_ - file_size_;
+    instance()->delegate()->NotifyStorageModified(
+        quota::QuotaClient::kFileSystem,
+        origin_url_,
+        PPFileSystemTypeToQuotaStorageType(file_system_type_),
+        delta);
+  }
+
+  RunAndRemoveFirstPendingCallback(PlatformFileErrorToPepperError(error_code));

[michaeln, 2011/07/19 22:01:45]

Now that some of these operations call out the the browser process instead of
just marshaling to the local file thread (via FileUtilProxy)... is it safe to
assume that the FirstPendingCallback is the callback associated with whatever
operation is being completed.

Seems like that assumption depends on all operations being serialized and
completing in the same order. True when everything is handled via a single hop
to and return from the local file thread... but now that some operations go thru
a different async code path that is not serialized with the FileUtilProxy
operations, is this assumption busted.

[yzshen1, 2011/07/19 23:14:14]

What is more, the order of multiple write operations may be changed.
(We only allow one pending SetLength at a time, so SetLength should be fine.)

On 2011/07/19 22:01:45, michaeln wrote:

[kinuko, 2011/07/20 13:39:35]

I splitted the callbacks queue into two, one for regular calls and the other for
the ones waiting a quota query callback.

+  file_size_ = pending_setlength_length_;

[yzshen1, 2011/07/19 17:41:55]

Even if it failed, we still set file_size_ to pending_setlength_length_?

[kinuko, 2011/07/20 13:39:35]

Good catch... fixed.

+  pending_setlength_length_ = -1;
+}
+
+void PPB_FileIO_Impl::DidCheckQuotaForWrite(bool success) {

[yzshen1, 2011/07/19 17:41:55]

This method and DidCheckQuotaForSetLength ignore |success| entirely. Is that
intended?

+  DCHECK(!pending_writes_.empty());
+  PendingWrite write = pending_writes_.front();
+  pending_writes_.pop();
+  pending_write_callbacks_.push(write);
+
+  if (!base::FileUtilProxy::Write(
+          instance()->delegate()->GetFileThreadMessageLoopProxy(),
+          file_, write.offset, write.buffer, write.bytes_to_write,
+          callback_factory_.NewCallback(&PPB_FileIO_Impl::WriteCallback))) {
+    RunAndRemoveFirstPendingCallback(PP_ERROR_FAILED);
+  }
+}
+
+void PPB_FileIO_Impl::DidCheckQuotaForSetLength(bool success) {
+  DCHECK(!pending_setlength_length_ != -1);
+  if (!base::FileUtilProxy::Truncate(
+          instance()->delegate()->GetFileThreadMessageLoopProxy(),
+          file_, pending_setlength_length_,
+          callback_factory_.NewCallback(
+              &PPB_FileIO_Impl::SetLengthCallback))) {
+    RunAndRemoveFirstPendingCallback(PP_ERROR_FAILED);
+  }
+}
+
 }  // namespace ppapi
 }  // namespace webkit