Implement IsNonNativeLocalPathDirectory for supporting non-local non-Drive directory access in apps.

chrome/browser/chromeos/file_manager/filesystem_api_util.cc

Index: chrome/browser/chromeos/file_manager/filesystem_api_util.cc
diff --git a/chrome/browser/chromeos/file_manager/filesystem_api_util.cc b/chrome/browser/chromeos/file_manager/filesystem_api_util.cc
index 04dc1d5febe6aefae7fb5a84e25449f8ab380ac9..f5cac13874f43abc68543fc3720e3c00ecbcc0ad 100644
--- a/chrome/browser/chromeos/file_manager/filesystem_api_util.cc
+++ b/chrome/browser/chromeos/file_manager/filesystem_api_util.cc
@@ -36,11 +36,14 @@ void GetMimeTypeAfterGetResourceEntry(
   callback.Run(true, entry->file_specific_info().content_mime_type());
 }
 
-void CheckDirectoryAfterDriveCheck(const base::Callback<void(bool)>& callback,
-                                   drive::FileError error) {
+// Helper function to converts a callback that takes boolean value to that takes
+// File::Error, by regarding FILE_OK as the only successful value.
+void BoolCallbackAsFileErrorCallback(
+    const base::Callback<void(bool)>& callback,
+    base::File::Error error) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
-  return callback.Run(error == drive::FILE_ERROR_OK);
+  return callback.Run(error == base::File::FILE_OK);
 }
 
 void CheckWritableAfterDriveCheck(const base::Callback<void(bool)>& callback,
@@ -65,13 +68,9 @@ bool IsUnderNonNativeLocalPath(Profile* profile,
     return false;
   }
 
-  content::StoragePartition* partition =
-      content::BrowserContext::GetStoragePartitionForSite(
-          profile,
-          extensions::util::GetSiteForExtensionId(kFileManagerAppId, profile));
-  fileapi::FileSystemContext* context = partition->GetFileSystemContext();
-
-  fileapi::FileSystemURL filesystem_url = context->CrackURL(url);
+  fileapi::FileSystemURL filesystem_url =
+      GetFileSystemContextForExtensionId(profile,
+                                         kFileManagerAppId)->CrackURL(url);
   if (!filesystem_url.is_valid())
     return false;
 
@@ -123,12 +122,21 @@ void IsNonNativeLocalPathDirectory(
     const base::FilePath& path,
     const base::Callback<void(bool)>& callback) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  DCHECK(IsUnderNonNativeLocalPath(profile, path));
 
-  // TODO(kinaba): support other types of volumes besides Drive.
-  drive::util::CheckDirectoryExists(
-      profile,
-      path,
-      base::Bind(&CheckDirectoryAfterDriveCheck, callback));
+  GURL url;
+  if (!util::ConvertAbsoluteFilePathToFileSystemUrl(
+           profile, path, kFileManagerAppId, &url)) {
+    content::BrowserThread::PostTask(content::BrowserThread::UI,

[mtomasz, 2014/05/30 03:53:44]

This PostTask looks unnecessary, since we are already on UI.

[kinaba, 2014/05/30 04:07:01]

This is intentional to make all callbacks done asynchronously.

The current client code looks safe, but in some cases it sometimes causes
trouble if callback.Run() is synchronously called here before exiting the caller
function (unexpected stack overflow, early destruction of objects in the
callback before the caller losing the access to it, etc.)

[mtomasz, 2014/05/30 05:23:48]

Hm. Isn't it an overkill? Do we always do that? If so, shall we add a comment?

[kinaba, 2014/05/30 05:43:00]

We always do it at least in the Drive code (see, e.g., crbug.com/139446 the
"We only need to do this ..." paragraph), and conditional sync/async callback
had actually caused issues in the past (like codereview.chromium.org/18033003/)
in other places as well.

Added a comment for clarification.

[mtomasz, 2014/05/30 05:55:07]

Got it. I'll review File System Provider API code to follow the same pattern
then.

+                                     FROM_HERE,
+                                     base::Bind(callback, false));
+    return;
+  }
+
+  util::CheckIfDirectoryExists(
+      GetFileSystemContextForExtensionId(profile, kFileManagerAppId),
+      url,
+      base::Bind(&BoolCallbackAsFileErrorCallback, callback));
 }
 
 void PrepareNonNativeLocalPathWritableFile(