Allow RequestOSFileHandle if an app has unlimited storage

content/browser/fileapi/fileapi_message_filter.cc

Index: content/browser/fileapi/fileapi_message_filter.cc
diff --git a/content/browser/fileapi/fileapi_message_filter.cc b/content/browser/fileapi/fileapi_message_filter.cc
index 7472115213db042bb44b6e3fc10a9ee4b1b1d499..1736bdcdb5551c999f5299b3c9c4b3ec9f06f89f 100644
--- a/content/browser/fileapi/fileapi_message_filter.cc
+++ b/content/browser/fileapi/fileapi_message_filter.cc
@@ -35,6 +35,7 @@
 #include "webkit/fileapi/isolated_context.h"
 #include "webkit/fileapi/local_file_system_operation.h"
 #include "webkit/fileapi/sandbox_mount_point_provider.h"
+#include "webkit/quota/quota_manager.h"
 
 using fileapi::FileSystemFileUtil;
 using fileapi::FileSystemMountPointProvider;
@@ -680,20 +681,47 @@ void FileAPIMessageFilter::DidReadDirectory(
   UnregisterOperation(request_id);
 }
 
-void FileAPIMessageFilter::DidOpenFile(int request_id,
-                                       const GURL& path,
-                                       base::PlatformFileError result,
-                                       base::PlatformFile file,
-                                       base::ProcessHandle peer_handle) {
+void FileAPIMessageFilter::DidOpenFile(
+    int request_id,
+    const GURL& path,
+    base::PlatformFileError result,
+    base::PlatformFile file,
+    base::ProcessHandle peer_handle) {

[dmichael (off chromium), 2013/04/04 17:25:09]

nit: Why did you move these? If they still fit, it seems like local style might
be to keep them aligned after the paren (see DidWrite below)

[hamaji, 2013/04/04 22:27:22]

Oops, I modified here because I was adding one more argument in the early
version of this patch. Fixed.

On 2013/04/04 17:25:09, dmichael wrote:

   if (result == base::PLATFORM_FILE_OK) {
     IPC::PlatformFileForTransit file_for_transit =
         file != base::kInvalidPlatformFileValue ?
             IPC::GetFileHandleForProcess(file, peer_handle, true) :
             IPC::InvalidPlatformFileForTransit();
     open_filesystem_urls_.insert(path);
-    Send(new FileSystemMsg_DidOpenFile(request_id, file_for_transit));
+
+    quota::QuotaPolicy quota_policy = quota::kQuotaPolicyUnknown;
+    quota::QuotaManagerProxy* quota_manager_proxy =
+        context_->quota_manager_proxy();
+    if (quota_manager_proxy) {

[dmichael (off chromium), 2013/04/04 17:25:09]

It looks like if there is no quota_manager_proxy, you'll succeed but provide
kQuotaPolicyUnknown. Is that what you want? I don't know this area, but I would
have guessed you'd send DidFail instead.

[hamaji, 2013/04/04 22:27:22]

webkit/fileapi/local_file_system_operation.cc seems to allow
!quota_manager_proxy case. Its comment says:

// If we don't have the quota manager or the requested filesystem type
// does not support quota, we should be able to let it go.                    

Kinuko-san, could you check if this code is OK?

On 2013/04/04 17:25:09, dmichael wrote:

[kinuko, 2013/04/05 02:37:11]

Actually in the content/ layer I think we must always have non-null
quota_manager_proxy, so I guess we can just have DCHECK(!quota_manager_proxy)
here.   (Trybots will tell if I'm wrong...)

[hamaji, 2013/04/05 03:33:19]

I see, thanks! I added DCHECK and removed this if.

On 2013/04/05 02:37:11, kinuko wrote:

+      FileSystemURL url = context_->CrackURL(path);
+      if (quota_manager_proxy->quota_manager()->IsStorageUnlimited(
+              url.origin(), FileSystemTypeToQuotaStorageType(url.type()))) {
+        // TODO(hamaji): We are planning to introduce a special
+        // permission which is stronger than unlimitedStorage.
+        // We'll return kQuotaPolicyCanBypassCheck for apps with
+        // unlimitedStorage for now, but ideally, we should return
+        // kQuotaPolicyCanBypassCheck only when the app has the
+        // special permission.
+        // TODO(hamaji): Return kQuotaPolicyCanBypassCheck only for
+        // dev channels for the meantime.
+        // http://crbug.com/220029
+        quota_policy = quota::kQuotaPolicyCanBypassCheck;
+      } else {
+        quota_policy = quota::kQuotaPolicyCheckRequired;
+      }
+    }
+
+    Send(new FileSystemMsg_DidOpenFile(request_id,
+                                       file_for_transit,
+                                       quota_policy));
   } else {
-    Send(new FileSystemMsg_DidFail(request_id, result));
+    Send(new FileSystemMsg_DidFail(request_id,
+                                   result));
   }
   UnregisterOperation(request_id);
 }