platform_util::OpenItem fixes

chrome/browser/platform_util_chromeos.cc

Index: chrome/browser/platform_util_chromeos.cc
===================================================================
--- chrome/browser/platform_util_chromeos.cc	(revision 108523)
+++ chrome/browser/platform_util_chromeos.cc	(working copy)
@@ -4,6 +4,7 @@
 
 #include "chrome/browser/platform_util.h"
 
+#include "base/bind.h"
 #include "base/file_util.h"
 #include "base/process_util.h"
 #include "base/task.h"
@@ -20,6 +21,20 @@
 
 class Profile;
 
+namespace {
+
+void OpenItemOnFileThread(const FilePath& full_path) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
+  base::Closure callback;
+  if (file_util::DirectoryExists(full_path))
+    callback = base::Bind(&FileManagerUtil::ViewFolder, full_path);
+  else
+    callback = base::Bind(&FileManagerUtil::ViewItem, full_path, false);
+  BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, callback);
+}
+
+}  // namespace
+
 namespace platform_util {
 
 static const std::string kGmailComposeUrl =
@@ -48,6 +63,7 @@
 }
 
 void ShowItemInFolder(const FilePath& full_path) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
   FilePath dir = full_path.DirName();
   if (!file_util::DirectoryExists(dir))
     return;
@@ -55,14 +71,17 @@
   if (BrowserThread::CurrentlyOn(BrowserThread::UI)) {
     OpenFileBrowserOnUIThread(dir);
   } else {
-    BrowserThread::PostTask(
-        BrowserThread::UI, FROM_HERE,
-        NewRunnableFunction(&OpenFileBrowserOnUIThread, dir));
+    BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
+                            base::Bind(&OpenFileBrowserOnUIThread, dir));
   }
 }
 
 void OpenItem(const FilePath& full_path) {
-  FileManagerUtil::ViewItem(full_path, false);
+  if (BrowserThread::CurrentlyOn(BrowserThread::FILE))
+    OpenItemOnFileThread(full_path);
+  else
+    BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE,
+                            base::Bind(&OpenItemOnFileThread, full_path));

[asanka, 2011/11/08 15:29:29]

Would it make sense to have an OpenItemOnUIThread() instead and post a task
there if the call isn't already on the UI thread?  That will cut down on thread
hopping since FMU::ViewFolder and FMU::ViewItem want to be on the UI thread.

OpenItem() is currently called from the UI thread.

[achuithb, 2011/11/08 20:49:57]

file_util::DirectoryExists needs to be called on the FILE thread, so we have to
make 1 trip to the file thread before deciding to call FMU::ViewFolder or
FMU::ViewItem on the UI thread.

Am I missing something?

[asanka, 2011/11/08 20:59:33]

You're correct. Sorry.

 }
 
 static void OpenURL(const std::string& url) {
@@ -74,8 +93,8 @@
   if (url.SchemeIs("mailto")) {
     std::string string_url = kGmailComposeUrl;
     string_url.append(url.spec());
-    BrowserThread::PostTask(
-        BrowserThread::UI, FROM_HERE, NewRunnableFunction(OpenURL, string_url));
+    BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
+                            base::Bind(OpenURL, string_url));
   }
 }