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..ac81220e319ef79db300da9f4a555b4fc07d4e28 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;
@@ -691,9 +692,26 @@ void FileAPIMessageFilter::DidOpenFile(int request_id,
             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);
 }