Linux: call xdg-open on downloaded files to open them.

BUG=12299
TEST=1) right click/save as on some image. Clicking on the download item's filename area should launch it in some image viewer (assuming xdg-open works for you---as it happens, ubuntu broke xdg-open for desktops that are not gnome, kde, or xfce). 2) Download a large file. Click on the download item before it is finished. The text should change to "opening in..." and the checkbox in the dropdown menu should show as checked. 3) completed downloads should have the "open when finished" menu item replaced by the "open" menu item. Selecting that should also open the download.
Review URL: http://codereview.chromium.org/112064

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@17106 0039d316-1c4b-4281-b951-d872f2087c98

12 files changed, 71 insertions, 40 deletions

diff --git a/app/win_util.cc b/app/win_util.cc
index 4f4f14f..70e6ecc 100644
--- a/app/win_util.cc
+++ b/app/win_util.cc
@@ -162,7 +162,7 @@ bool ShouldUseVistaFrame() {
 
 // Open an item via a shell execute command. Error code checking and casting
 // explanation: http://msdn2.microsoft.com/en-us/library/ms647732.aspx
-bool OpenItemViaShell(const FilePath& full_path, bool ask_for_app) {
+bool OpenItemViaShell(const FilePath& full_path) {
   HINSTANCE h = ::ShellExecuteW(
       NULL, NULL, full_path.value().c_str(), NULL,
       full_path.DirName().value().c_str(), SW_SHOWNORMAL);
@@ -171,14 +171,13 @@ bool OpenItemViaShell(const FilePath& full_path, bool ask_for_app) {
   if (error > 32)
     return true;
 
-  if ((error == SE_ERR_NOASSOC) && ask_for_app)
+  if ((error == SE_ERR_NOASSOC))
     return OpenItemWithExternalApp(full_path.value());
 
   return false;
 }
 
-bool OpenItemViaShellNoZoneCheck(const FilePath& full_path,
-                                 bool ask_for_app) {
+bool OpenItemViaShellNoZoneCheck(const FilePath& full_path) {
   SHELLEXECUTEINFO sei = { sizeof(sei) };
   sei.fMask = SEE_MASK_NOZONECHECKS | SEE_MASK_FLAG_DDEWAIT;
   sei.nShow = SW_SHOWNORMAL;
@@ -187,7 +186,7 @@ bool OpenItemViaShellNoZoneCheck(const FilePath& full_path,
   if (::ShellExecuteExW(&sei))
     return true;
   LONG_PTR error = reinterpret_cast<LONG_PTR>(sei.hInstApp);
-  if ((error == SE_ERR_NOASSOC) && ask_for_app)
+  if ((error == SE_ERR_NOASSOC))
     return OpenItemWithExternalApp(full_path.value());
   return false;
 }
diff --git a/app/win_util.h b/app/win_util.h
index 19bbba7..be5f325 100644
--- a/app/win_util.h
+++ b/app/win_util.h
@@ -117,16 +117,14 @@ bool ShouldUseVistaFrame();
 
 // Open or run a file via the Windows shell. In the event that there is no
 // default application registered for the file specified by 'full_path',
-// ask the user, via the Windows "Open With" dialog, for an application to use
-// if 'ask_for_app' is true.
+// ask the user, via the Windows "Open With" dialog.
 // Returns 'true' on successful open, 'false' otherwise.
-bool OpenItemViaShell(const FilePath& full_path, bool ask_for_app);
+bool OpenItemViaShell(const FilePath& full_path);
 
 // The download manager now writes the alternate data stream with the
 // zone on all downloads. This function is equivalent to OpenItemViaShell
 // without showing the zone warning dialog.
-bool OpenItemViaShellNoZoneCheck(const FilePath& full_path,
-                                 bool ask_for_app);
+bool OpenItemViaShellNoZoneCheck(const FilePath& full_path);
 
 // Ask the user, via the Windows "Open With" dialog, for an application to use
 // to open the file specified by 'full_path'.
diff --git a/chrome/browser/download/download_file.cc b/chrome/browser/download/download_file.cc
index 069fc58..b871540 100644
--- a/chrome/browser/download/download_file.cc
+++ b/chrome/browser/download/download_file.cc
@@ -530,24 +530,23 @@ void DownloadFileManager::OnShowDownloadInShell(const FilePath& full_path) {
   platform_util::ShowItemInFolder(full_path);
 }
 
-// Launches the selected download using ShellExecute 'open' verb. If there is
-// a valid parent window, the 'safer' version will be used which can
+// Launches the selected download using ShellExecute 'open' verb. For windows,
+// if there is a valid parent window, the 'safer' version will be used which can
 // display a modal dialog asking for user consent on dangerous files.
 void DownloadFileManager::OnOpenDownloadInShell(const FilePath& full_path,
                                                 const GURL& url,
                                                 gfx::NativeView parent_window) {
-#if defined(OS_WIN)
   DCHECK(MessageLoop::current() == file_loop_);
+
+#if defined(OS_WIN)
   if (NULL != parent_window) {
     win_util::SaferOpenItemViaShell(parent_window, L"", full_path,
-                                    UTF8ToWide(url.spec()), true);
-  } else {
-    win_util::OpenItemViaShell(full_path, true);
+                                    UTF8ToWide(url.spec()));
+    return;
   }
-#else
-  // TODO(port) implement me.
-  NOTREACHED();
 #endif
+
+  platform_util::OpenItem(full_path);
 }
 
 // The DownloadManager in the UI thread has provided a final name for the
diff --git a/chrome/browser/download/download_shelf.cc b/chrome/browser/download/download_shelf.cc
index b71caee..6962a3e 100644
--- a/chrome/browser/download/download_shelf.cc
+++ b/chrome/browser/download/download_shelf.cc
@@ -14,10 +14,9 @@
 #include "chrome/common/url_constants.h"
 #include "grit/generated_resources.h"
 
-#if defined(OS_WIN)
+#if defined(OS_WIN) || defined(OS_LINUX)
+// TODO(port): port this for mac. See two uses below.
 #include "chrome/browser/download/download_util.h"
-#elif defined(OS_POSIX)
-#include "chrome/common/temp_scaffolding_stubs.h"
 #endif
 
 // DownloadShelf ---------------------------------------------------------------
@@ -88,10 +87,9 @@ bool DownloadShelfContextMenu::IsItemCommandEnabled(int id) const {
     case OPEN_WHEN_COMPLETE:
       return download_->state() != DownloadItem::CANCELLED;
     case ALWAYS_OPEN_TYPE:
-#if defined(OS_WIN)
+#if defined(OS_WIN) || defined(OS_LINUX)
       return download_util::CanOpenDownload(download_);
-#elif defined(OS_LINUX)
-      // Need to implement dangerous download stuff: http://crbug.com/11780
+#else
       return false;
 #endif
     case CANCEL:
@@ -107,11 +105,8 @@ void DownloadShelfContextMenu::ExecuteItemCommand(int id) {
       download_->manager()->ShowDownloadInShell(download_);
       break;
     case OPEN_WHEN_COMPLETE:
-#if defined(OS_WIN)
+#if defined(OS_WIN) || defined(OS_LINUX)
       download_util::OpenDownload(download_);
-#else
-      // TODO(port): port download_util
-      NOTIMPLEMENTED();
 #endif
       break;
     case ALWAYS_OPEN_TYPE: {
diff --git a/chrome/browser/gtk/download_item_gtk.cc b/chrome/browser/gtk/download_item_gtk.cc
index 580db74..6b1ff4e 100644
--- a/chrome/browser/gtk/download_item_gtk.cc
+++ b/chrome/browser/gtk/download_item_gtk.cc
@@ -181,6 +181,8 @@ DownloadItemGtk::DownloadItemGtk(DownloadShelfGtk* parent_shelf,
   gtk_widget_set_app_paintable(body_, TRUE);
   g_signal_connect(body_, "expose-event",
                    G_CALLBACK(OnExpose), this);
+  g_signal_connect(body_, "clicked",
+                   G_CALLBACK(OnClick), this);
   GTK_WIDGET_UNSET_FLAGS(body_, GTK_CAN_FOCUS);
   // Remove internal padding on the button.
   GtkRcStyle* no_padding_style = gtk_rc_style_new();
@@ -565,6 +567,19 @@ gboolean DownloadItemGtk::OnExpose(GtkWidget* widget, GdkEventExpose* e,
 }
 
 // static
+void DownloadItemGtk::OnClick(GtkWidget* widget, DownloadItemGtk* item) {
+  DownloadItem* download = item->get_download();
+
+  // TODO(estade): add clickjacking histogram stuff.
+  if (download->state() == DownloadItem::IN_PROGRESS) {
+    download->set_open_when_complete(
+        !download->open_when_complete());
+  } else if (download->state() == DownloadItem::COMPLETE) {
+    download_util::OpenDownload(download);
+  }
+}
+
+// static
 gboolean DownloadItemGtk::OnProgressAreaExpose(GtkWidget* widget,
     GdkEventExpose* event, DownloadItemGtk* download_item) {
   // Create a transparent canvas.
diff --git a/chrome/browser/gtk/download_item_gtk.h b/chrome/browser/gtk/download_item_gtk.h
index fba3a8a..c9b1ae0 100644
--- a/chrome/browser/gtk/download_item_gtk.h
+++ b/chrome/browser/gtk/download_item_gtk.h
@@ -68,6 +68,9 @@ class DownloadItemGtk : public DownloadItem::Observer,
   static gboolean OnExpose(GtkWidget* widget, GdkEventExpose* e,
                            DownloadItemGtk* download_item);
 
+  // Called when |body_| is clicked.
+  static void OnClick(GtkWidget* widget, DownloadItemGtk* item);
+
   // Used for the download icon.
   static gboolean OnProgressAreaExpose(GtkWidget* widget,
                                        GdkEventExpose* e,
diff --git a/chrome/common/platform_util.h b/chrome/common/platform_util.h
index c4a46b7..54e7487 100644
--- a/chrome/common/platform_util.h
+++ b/chrome/common/platform_util.h
@@ -15,6 +15,9 @@ namespace platform_util {
 // Show the given file in a file manager. If possible, select the file.
 void ShowItemInFolder(const FilePath& full_path);
 
+// Open the given file in the desktop's default manner.
+void OpenItem(const FilePath& full_path);
+
 // Get the top level window for the native view. This can return NULL.
 gfx::NativeWindow GetTopLevel(gfx::NativeView view);
 
diff --git a/chrome/common/platform_util_linux.cc b/chrome/common/platform_util_linux.cc
index 8d3a5a1..988de92 100644
--- a/chrome/common/platform_util_linux.cc
+++ b/chrome/common/platform_util_linux.cc
@@ -11,6 +11,18 @@
 #include "base/process_util.h"
 #include "base/string_util.h"
 
+namespace {
+
+void XDGOpen(const FilePath& path) {
+  std::vector<std::string> argv;
+  argv.push_back("xdg-open");
+  argv.push_back(path.value());
+  base::file_handle_mapping_vector no_files;
+  base::LaunchApp(argv, no_files, false, NULL);
+}
+
+}  // namespace
+
 namespace platform_util {
 
 // TODO(estade): It would be nice to be able to select the file in the file
@@ -21,11 +33,11 @@ void ShowItemInFolder(const FilePath& full_path) {
   if (!file_util::DirectoryExists(dir))
     return;
 
-  std::vector<std::string> argv;
-  argv.push_back("xdg-open");
-  argv.push_back(dir.value());
-  base::file_handle_mapping_vector no_files;
-  base::LaunchApp(argv, no_files, false, NULL);
+  XDGOpen(dir);
+}
+
+void OpenItem(const FilePath& full_path) {
+  XDGOpen(full_path);
 }
 
 gfx::NativeWindow GetTopLevel(gfx::NativeView view) {
diff --git a/chrome/common/platform_util_mac.mm b/chrome/common/platform_util_mac.mm
index 2586d1f..c3a4ab7 100644
--- a/chrome/common/platform_util_mac.mm
+++ b/chrome/common/platform_util_mac.mm
@@ -21,6 +21,10 @@ void ShowItemInFolder(const FilePath& full_path) {
                    inFileViewerRootedAtPath:nil];
 }
 
+void OpenItem(const FilePath& full_path) {
+  NOTIMPLEMENTED();
+}
+
 gfx::NativeWindow GetTopLevel(gfx::NativeView view) {
   return [view window];
 }
diff --git a/chrome/common/platform_util_win.cc b/chrome/common/platform_util_win.cc
index f1af709..e3cd5db 100644
--- a/chrome/common/platform_util_win.cc
+++ b/chrome/common/platform_util_win.cc
@@ -82,6 +82,10 @@ void ShowItemInFolder(const FilePath& full_path) {
                                      highlight, NULL);
 }
 
+void OpenItem(const FilePath& full_path) {
+  win_util::OpenItemViaShell(full_path);
+}
+
 gfx::NativeWindow GetTopLevel(gfx::NativeView view) {
   return GetAncestor(view, GA_ROOT);
 }
diff --git a/chrome/common/win_safe_util.cc b/chrome/common/win_safe_util.cc
index 2187d30..8afe8c5 100644
--- a/chrome/common/win_safe_util.cc
+++ b/chrome/common/win_safe_util.cc
@@ -23,8 +23,7 @@ namespace win_util {
 // http://msdn2.microsoft.com/en-us/library/ms647048.aspx
 bool SaferOpenItemViaShell(HWND hwnd, const std::wstring& window_title,
                            const FilePath& full_path,
-                           const std::wstring& source_url,
-                           bool ask_for_app) {
+                           const std::wstring& source_url) {
   ATL::CComPtr<IAttachmentExecute> attachment_services;
   HRESULT hr = attachment_services.CoCreateInstance(CLSID_AttachmentServices);
   if (FAILED(hr)) {
@@ -34,7 +33,7 @@ bool SaferOpenItemViaShell(HWND hwnd, const std::wstring& window_title,
       NOTREACHED();
       return false;
     }
-    return OpenItemViaShell(full_path, ask_for_app);
+    return OpenItemViaShell(full_path);
   }
 
   // This GUID is associated with any 'don't ask me again' settings that the
@@ -92,7 +91,7 @@ bool SaferOpenItemViaShell(HWND hwnd, const std::wstring& window_title,
       return false;
     }
   }
-  return OpenItemViaShellNoZoneCheck(full_path, ask_for_app);
+  return OpenItemViaShellNoZoneCheck(full_path);
 }
 
 bool SetInternetZoneIdentifier(const FilePath& full_path) {
diff --git a/chrome/common/win_safe_util.h b/chrome/common/win_safe_util.h
index 9b8ac1b..8705d8a 100644
--- a/chrome/common/win_safe_util.h
+++ b/chrome/common/win_safe_util.h
@@ -34,11 +34,11 @@ namespace win_util {
 //
 // In the event that there is no default application registered for the file
 // specified by 'full_path' it ask the user, via the Windows "Open With"
-// dialog, for an application to use if 'ask_for_app' is true.
+// dialog.
 // Returns 'true' on successful open, 'false' otherwise.
 bool SaferOpenItemViaShell(HWND hwnd, const std::wstring& window_title,
                            const FilePath& full_path,
-                           const std::wstring& source_url, bool ask_for_app);
+                           const std::wstring& source_url);
 
 // Sets the Zone Identifier on the file to "Internet" (3). Returns true if the
 // function succeeds, false otherwise. A failure is expected on system where