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 636 matching lines @@
                                        const GURL& path,
                                        base::PlatformFileError result,
                                        base::PlatformFile file,
                                        base::ProcessHandle peer_handle) {
   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::QuotaLimitType quota_policy = quota::kQuotaLimitTypeUnknown;
+    quota::QuotaManagerProxy* quota_manager_proxy =
+        context_->quota_manager_proxy();
+    CHECK(quota_manager_proxy);
+    CHECK(quota_manager_proxy->quota_manager());
+    FileSystemURL url = context_->CrackURL(path);
+    if (quota_manager_proxy->quota_manager()->IsStorageUnlimited(
+            url.origin(), FileSystemTypeToQuotaStorageType(url.type()))) {

[kinuko, 2013/04/10 03:35:19]

From the discussion, probably we should also check if it's installed app?
(hosted app can also get unlimited quota)  You can check it by calling
QuotaManager::IsInstalledApp(url.origin())

[hamaji, 2013/04/10 14:49:18]

Hmm, I'm not sure, but if we add this check, we end up needing the whiltelist?
We have the plugin mode as well as the extension mode and I think the plugin
mode is considered as a hosted app which requires unlimitedStorage. If I'm
correct, we might want to do something like

if (IsStorageUnlimited && (IsInstalledApp || HasSwitch(kUnlimitedStorage)))

This might work for us.

Anyway, I feel IsStorageUnlimited&&IsInstalledApp is a bit confusing for web
developers. Do you think we need IsInstalledApp forever? Or, will it stay until
we have some extra protections?

[dmichael (off chromium), 2013/04/10 16:02:42]

I'm not sure I understand. IsInstalledApp(path) looks like it would be true for
(legacy) Packaged apps, Platform apps, and Hosted apps:
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/common/exte...
(it looks to me like this is what is ultimately called to determine whether the
URL goes in to "installed_apps_":
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ext...)

That sounds like a good check to me, though it might be redundant (i.e., can any
URL have unlimited storage without having been installed? I think probably not).
Kinuko, it sounded like you were thinking we _don't_ want to support this for
hosted apps? Why not?

[kinuko, 2013/04/11 02:32:45]

Chatted /w hamaji. Currently the only unlimited&&!installed case I could think
of is specifying command line flag to force unlimitedStorage, and looks like
we'd want to return FD for that case too (for development).  Sorry for having
brought further confusion, I think it'd be enough for checking
IsStorageUnlimited() only.

+      quota_policy = quota::kQuotaLimitTypeUnlimited;
+    } else {
+      quota_policy = quota::kQuotaLimitTypeLimited;
+    }
+
+    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