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

chrome/browser/chromeos/file_manager/filesystem_api_util.cc

 // Copyright 2014 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 "chrome/browser/chromeos/file_manager/filesystem_api_util.h"
 
 #include "base/callback.h"
 #include "base/files/file_path.h"
 #include "base/memory/scoped_ptr.h"
 #include "chrome/browser/chromeos/drive/file_errors.h"
@@ skipping 18 matching lines @@
     scoped_ptr<drive::ResourceEntry> entry) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
   if (error != drive::FILE_ERROR_OK || !entry->has_file_specific_info()) {
     callback.Run(false, std::string());
     return;
   }
   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,
                                   drive::FileError error,
                                   const base::FilePath& local_path) {
   // This is called on the IO-allowed blocking pool. Call back to UI.
   content::BrowserThread::PostTask(
       content::BrowserThread::UI,
       FROM_HERE,
       base::Bind(callback, error == drive::FILE_ERROR_OK));
 }
 
 }  // namespace
 
 bool IsUnderNonNativeLocalPath(Profile* profile,
                         const base::FilePath& path) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
   GURL url;
   if (!util::ConvertAbsoluteFilePathToFileSystemUrl(
            profile, path, kFileManagerAppId, &url)) {
     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;
 
   switch (filesystem_url.type()) {
     case fileapi::kFileSystemTypeNativeLocal:
     case fileapi::kFileSystemTypeRestrictedNativeLocal:
       return false;
     default:
       // The path indeed corresponds to a mount point not associated with a
       // native local path.
@@ skipping 31 matching lines @@
   file_system->GetResourceEntry(
       drive::util::ExtractDrivePath(path),
       base::Bind(&GetMimeTypeAfterGetResourceEntry, callback));
 }
 
 void IsNonNativeLocalPathDirectory(
     Profile* profile,
     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(
     Profile* profile,
     const base::FilePath& path,
     const base::Callback<void(bool)>& callback) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
   // TODO(kinaba): support other types of volumes besides Drive.
   drive::util::PrepareWritableFileAndRun(
       profile,
       path,
       base::Bind(&CheckWritableAfterDriveCheck, callback));
 }
 
 }  // namespace util
 }  // namespace file_manager