Allow RequestOSFileHandle if an app has unlimited storage

content/browser/fileapi/fileapi_message_filter.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 "content/browser/fileapi/fileapi_message_filter.h"
 
 #include <string>
 #include <vector>
 
 #include "base/bind.h"
@@ skipping 17 matching lines @@
 #include "webkit/blob/blob_storage_controller.h"
 #include "webkit/blob/shareable_file_reference.h"
 #include "webkit/fileapi/file_observers.h"
 #include "webkit/fileapi/file_permission_policy.h"
 #include "webkit/fileapi/file_system_context.h"
 #include "webkit/fileapi/file_system_types.h"
 #include "webkit/fileapi/file_system_util.h"
 #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;
 using fileapi::FileSystemOperation;
 using fileapi::FileSystemURL;
 using fileapi::FileUpdateObserver;
 using fileapi::LocalFileSystemOperation;
 using fileapi::UpdateObserverList;
 using webkit_blob::BlobData;
 using webkit_blob::BlobStorageController;
@@ skipping 625 matching lines @@
     base::PlatformFileError result,
     const std::vector<base::FileUtilProxy::Entry>& entries,
     bool has_more) {
   if (result == base::PLATFORM_FILE_OK)
     Send(new FileSystemMsg_DidReadDirectory(request_id, entries, has_more));
   else
     Send(new FileSystemMsg_DidFail(request_id, result));
   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);
 }
 
 void FileAPIMessageFilter::DidWrite(int request_id,
                                     base::PlatformFileError result,
                                     int64 bytes,
                                     bool complete) {
   if (result == base::PLATFORM_FILE_OK) {
     Send(new FileSystemMsg_DidWrite(request_id, bytes, complete));
@@ skipping 171 matching lines @@
     Send(new FileSystemMsg_DidFail(request_id, error_code));
     return NULL;
   }
 
   DCHECK(operation);
   operations_.AddWithID(operation, request_id);
   return operation;
 }
 
 }  // namespace content