Be explicit about target type in platform_util::OpenItem()

chrome/browser/chromeos/file_manager/open_util.cc

Index: chrome/browser/chromeos/file_manager/open_util.cc
diff --git a/chrome/browser/chromeos/file_manager/open_util.cc b/chrome/browser/chromeos/file_manager/open_util.cc
index dad853242d6b4dd7ad68aacec4cdd91ebf68d9c0..568177afa977e1ebc1a264aac61470630832528a 100644
--- a/chrome/browser/chromeos/file_manager/open_util.cc
+++ b/chrome/browser/chromeos/file_manager/open_util.cc
@@ -127,16 +127,21 @@ void OpenFileWithMimeType(Profile* profile,
 // Opens the file specified by |url| by finding and executing a file task for
 // the file. In case of success, calls |callback| with true. Otherwise the
 // returned value is false.
-void OpenFile(Profile* profile,
-              const base::FilePath& path,
-              const GURL& url,
-              const base::Callback<void(bool)>& callback) {
+void OpenFileInternal(Profile* profile,
+                      const base::FilePath& path,
+                      const GURL& url,
+                      const base::Callback<void(bool)>& callback) {
   extensions::app_file_handler_util::GetMimeTypeForLocalPath(
       profile,
       path,
       base::Bind(&OpenFileWithMimeType, profile, path, url, callback));
 }
 
+enum ExpectedOpenTarget {
+  OPEN_FILE,
+  OPEN_FOLDER
+};
+
 // Called when execution of ContinueOpenItem() is completed.
 void OnContinueOpenItemCompleted(Profile* profile,
                                  const base::FilePath& file_path,
@@ -154,23 +159,35 @@ void OnContinueOpenItemCompleted(Profile* profile,
   }
 }
 
-// Used to implement OpenItem().
+// Used to implement OpenFile()/OpenFolder().
 void ContinueOpenItem(Profile* profile,

[hashimoto, 2014/09/25 02:34:05]

I think it's better to split this function into two.
One for opening files (this can be merged with OpenFileInternal) and another for
opening directories.

[asanka, 2014/09/25 22:24:20]

Done. Good call.

+                      ExpectedOpenTarget expected_target,
                       const base::FilePath& file_path,
                       const GURL& url,
-                      base::File::Error error) {
+                      base::File::Error check_directory_result) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
-
-  if (error == base::File::FILE_OK) {
-    // A directory exists at |url|. Open it with the file manager.
-    OpenFileManagerWithInternalActionId(profile, url, "open");
-  } else {
-    // |url| should be a file. Open it.
-    OpenFile(profile,
-             file_path,
-             url,
-             base::Bind(&OnContinueOpenItemCompleted, profile, file_path));
+  const bool target_is_a_directory =
+      (check_directory_result == base::File::FILE_OK);
+
+  switch (expected_target) {
+    case OPEN_FILE:
+      if (target_is_a_directory)
+        break;
+      OpenFileInternal(
+          profile,
+          file_path,
+          url,
+          base::Bind(&OnContinueOpenItemCompleted, profile, file_path));
+      return;
+
+    case OPEN_FOLDER:
+      if (target_is_a_directory) {
+        OpenFileManagerWithInternalActionId(profile, url, "open");
+        return;
+      }
+      break;
   }
+  OnContinueOpenItemCompleted(profile, file_path, false);
 }
 
 // Converts the |given_path| passed from external callers to the form that the
@@ -210,7 +227,21 @@ void OpenRemovableDrive(Profile* profile, const base::FilePath& file_path) {
   OpenFileManagerWithInternalActionId(profile, url, "auto-open");
 }
 
-void OpenItem(Profile* profile, const base::FilePath& file_path) {
+void OpenFile(Profile* profile, const base::FilePath& file_path) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+
+  base::FilePath converted_path;
+  GURL url;
+  if (!ConvertPath(profile, file_path, &converted_path, &url))
+    return;
+
+  CheckIfDirectoryExists(
+      GetFileSystemContextForExtensionId(profile, kFileManagerAppId),
+      url,
+      base::Bind(&ContinueOpenItem, profile, OPEN_FILE, converted_path, url));
+}
+
+void OpenFolder(Profile* profile, const base::FilePath& file_path) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
   base::FilePath converted_path;
@@ -221,7 +252,7 @@ void OpenItem(Profile* profile, const base::FilePath& file_path) {
   CheckIfDirectoryExists(
       GetFileSystemContextForExtensionId(profile, kFileManagerAppId),
       url,
-      base::Bind(&ContinueOpenItem, profile, converted_path, url));
+      base::Bind(&ContinueOpenItem, profile, OPEN_FOLDER, converted_path, url));
 }
 
 void ShowItemInFolder(Profile* profile, const base::FilePath& file_path) {