diff options
25 files changed, 856 insertions, 204 deletions
diff --git a/chrome/browser/chromeos/file_manager/fileapi_util.cc b/chrome/browser/chromeos/file_manager/fileapi_util.cc index 15550c5..e7a1a4b 100644 --- a/chrome/browser/chromeos/file_manager/fileapi_util.cc +++ b/chrome/browser/chromeos/file_manager/fileapi_util.cc @@ -229,6 +229,18 @@ void CheckIfDirectoryExistsOnIOThread( file_system_url, callback); } +// Used by GetMetadataForPath +void GetMetadataOnIOThread( + scoped_refptr<storage::FileSystemContext> file_system_context, + const GURL& url, + const storage::FileSystemOperationRunner::GetMetadataCallback& callback) { + DCHECK_CURRENTLY_ON(BrowserThread::IO); + const storage::FileSystemURL file_system_url = + file_system_context->CrackURL(url); + file_system_context->operation_runner()->GetMetadata(file_system_url, + callback); +} + // Checks if the |file_path| points non-native location or not. bool IsUnderNonNativeLocalPath(const storage::FileSystemContext& context, const base::FilePath& file_path) { @@ -557,9 +569,24 @@ void CheckIfDirectoryExists( BrowserThread::PostTask( BrowserThread::IO, FROM_HERE, - base::Bind(&CheckIfDirectoryExistsOnIOThread, - file_system_context, - url, + base::Bind(&CheckIfDirectoryExistsOnIOThread, file_system_context, url, + google_apis::CreateRelayCallback(callback))); +} + +void GetMetadataForPath( + scoped_refptr<storage::FileSystemContext> file_system_context, + const GURL& url, + const storage::FileSystemOperationRunner::GetMetadataCallback& callback) { + DCHECK_CURRENTLY_ON(BrowserThread::UI); + + storage::ExternalFileSystemBackend* const backend = + file_system_context->external_backend(); + DCHECK(backend); + backend->GrantFullAccessToExtension(kFileManagerAppId); + + BrowserThread::PostTask( + BrowserThread::IO, FROM_HERE, + base::Bind(&GetMetadataOnIOThread, file_system_context, url, google_apis::CreateRelayCallback(callback))); } diff --git a/chrome/browser/chromeos/file_manager/fileapi_util.h b/chrome/browser/chromeos/file_manager/fileapi_util.h index 3756c7b..a6ae85e 100644 --- a/chrome/browser/chromeos/file_manager/fileapi_util.h +++ b/chrome/browser/chromeos/file_manager/fileapi_util.h @@ -151,6 +151,12 @@ void CheckIfDirectoryExists( const GURL& url, const storage::FileSystemOperationRunner::StatusCallback& callback); +// Get metadata for object at |url|. +void GetMetadataForPath( + scoped_refptr<storage::FileSystemContext> file_system_context, + const GURL& url, + const storage::FileSystemOperationRunner::GetMetadataCallback& callback); + // Obtains isolated file system URL from |virtual_path| pointing a file in the // external file system. storage::FileSystemURL CreateIsolatedURLFromVirtualPath( diff --git a/chrome/browser/chromeos/file_manager/open_util.cc b/chrome/browser/chromeos/file_manager/open_util.cc index 9cbc3a0..b21834e 100644 --- a/chrome/browser/chromeos/file_manager/open_util.cc +++ b/chrome/browser/chromeos/file_manager/open_util.cc @@ -15,18 +15,12 @@ #include "chrome/browser/chromeos/file_manager/path_util.h" #include "chrome/browser/chromeos/file_manager/url_util.h" #include "chrome/browser/extensions/api/file_handlers/mime_util.h" -#include "chrome/browser/ui/browser.h" -#include "chrome/browser/ui/browser_finder.h" -#include "chrome/browser/ui/browser_window.h" -#include "chrome/browser/ui/simple_message_box.h" -#include "chrome/grit/generated_resources.h" #include "content/public/browser/browser_thread.h" #include "content/public/browser/user_metrics.h" #include "storage/browser/fileapi/file_system_backend.h" #include "storage/browser/fileapi/file_system_context.h" #include "storage/browser/fileapi/file_system_operation_runner.h" #include "storage/browser/fileapi/file_system_url.h" -#include "ui/base/l10n/l10n_util.h" using content::BrowserThread; using storage::FileSystemURL; @@ -35,25 +29,14 @@ namespace file_manager { namespace util { namespace { -// Shows a warning message box saying that the file could not be opened. -void ShowWarningMessageBox(Profile* profile, - const base::FilePath& file_path, - int message_id) { - Browser* browser = chrome::FindTabbedBrowser( - profile, false, chrome::HOST_DESKTOP_TYPE_ASH); - chrome::ShowMessageBox( - browser ? browser->window()->GetNativeWindow() : NULL, - l10n_util::GetStringFUTF16( - IDS_FILE_BROWSER_ERROR_VIEWING_FILE_TITLE, - base::UTF8ToUTF16(file_path.BaseName().AsUTF8Unsafe())), - l10n_util::GetStringUTF16(message_id), - chrome::MESSAGE_BOX_TYPE_WARNING); -} +bool shell_operations_allowed = true; // Executes the |task| for the file specified by |url|. void ExecuteFileTaskForUrl(Profile* profile, const file_tasks::TaskDescriptor& task, const GURL& url) { + if (!shell_operations_allowed) + return; storage::FileSystemContext* file_system_context = GetFileSystemContextForExtensionId(profile, kFileManagerAppId); @@ -75,6 +58,8 @@ void OpenFileManagerWithInternalActionId(Profile* profile, const GURL& url, const std::string& action_id) { DCHECK(action_id == "open" || action_id == "select"); + if (!shell_operations_allowed) + return; content::RecordAction(base::UserMetricsAction("ShowFileBrowserFullTab")); file_tasks::TaskDescriptor task(kFileManagerAppId, @@ -87,7 +72,7 @@ void OpenFileManagerWithInternalActionId(Profile* profile, void OpenFileWithMimeType(Profile* profile, const base::FilePath& path, const GURL& url, - const base::Callback<void(bool)>& callback, + const platform_util::OpenOperationCallback& callback, const std::string& mime_type) { extensions::app_file_handler_util::PathAndMimeTypeSet path_mime_set; path_mime_set.insert(std::make_pair(path, mime_type)); @@ -115,100 +100,96 @@ void OpenFileWithMimeType(Profile* profile, } if (chosen_task != nullptr) { - ExecuteFileTaskForUrl(profile, chosen_task->task_descriptor(), url); - callback.Run(true); + if (shell_operations_allowed) + ExecuteFileTaskForUrl(profile, chosen_task->task_descriptor(), url); + callback.Run(platform_util::OPEN_SUCCEEDED); } else { - callback.Run(false); + callback.Run(platform_util::OPEN_FAILED_NO_HANLDER_FOR_FILE_TYPE); } } // 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. +// the file. Calls |callback| with the result. void OpenFile(Profile* profile, const base::FilePath& path, const GURL& url, - const base::Callback<void(bool)>& callback) { + const platform_util::OpenOperationCallback& callback) { extensions::app_file_handler_util::GetMimeTypeForLocalPath( - profile, - path, + profile, path, base::Bind(&OpenFileWithMimeType, profile, path, url, callback)); } -// Called when execution of ContinueOpenItem() is completed. -void OnContinueOpenItemCompleted(Profile* profile, - const base::FilePath& file_path, - bool result) { - if (!result) { - int message; - if (file_path.Extension() == FILE_PATH_LITERAL(".dmg")) - message = IDS_FILE_BROWSER_ERROR_VIEWING_FILE_FOR_DMG; - else if (file_path.Extension() == FILE_PATH_LITERAL(".exe") || - file_path.Extension() == FILE_PATH_LITERAL(".msi")) - message = IDS_FILE_BROWSER_ERROR_VIEWING_FILE_FOR_EXECUTABLE; - else - message = IDS_FILE_BROWSER_ERROR_VIEWING_FILE; - ShowWarningMessageBox(profile, file_path, message); - } -} - -// Used to implement OpenItem(). -void ContinueOpenItem(Profile* profile, - const base::FilePath& file_path, - const GURL& url, - base::File::Error error) { +void OpenItemWithMetadata(Profile* profile, + const base::FilePath& file_path, + const GURL& url, + platform_util::OpenItemType expected_type, + const platform_util::OpenOperationCallback& callback, + base::File::Error error, + const base::File::Info& file_info) { DCHECK_CURRENTLY_ON(BrowserThread::UI); + if (error != base::File::FILE_OK) { + callback.Run(error == base::File::FILE_ERROR_NOT_FOUND + ? platform_util::OPEN_FAILED_PATH_NOT_FOUND + : platform_util::OPEN_FAILED_FILE_ERROR); + return; + } - if (error == base::File::FILE_OK) { - // A directory exists at |url|. Open it with the file manager. + // Note that there exists a TOCTOU race between the time the metadata for + // |file_path| was determined and when it is opened based on the metadata. + if (expected_type == platform_util::OPEN_FOLDER && file_info.is_directory) { OpenFileManagerWithInternalActionId(profile, url, "open"); - } else { - // |url| should be a file. Open it. - OpenFile(profile, - file_path, - url, - base::Bind(&OnContinueOpenItemCompleted, profile, file_path)); + callback.Run(platform_util::OPEN_SUCCEEDED); + return; } -} -// Converts the |path| passed from external callers to the filesystem URL -// that the file manager can correctly handle. -// -// When conversion fails, it shows a warning dialog UI and returns false. -bool ConvertPath(Profile* profile, const base::FilePath& path, GURL* url) { - if (!ConvertAbsoluteFilePathToFileSystemUrl( - profile, path, kFileManagerAppId, url)) { - ShowWarningMessageBox(profile, path, - IDS_FILE_BROWSER_ERROR_UNRESOLVABLE_FILE); - return false; + if (expected_type == platform_util::OPEN_FILE && !file_info.is_directory) { + OpenFile(profile, file_path, url, callback); + return; } - return true; + + callback.Run(platform_util::OPEN_FAILED_INVALID_TYPE); } } // namespace -void OpenItem(Profile* profile, const base::FilePath& file_path) { +void OpenItem(Profile* profile, + const base::FilePath& file_path, + platform_util::OpenItemType expected_type, + const platform_util::OpenOperationCallback& callback) { DCHECK_CURRENTLY_ON(BrowserThread::UI); GURL url; - if (!ConvertPath(profile, file_path, &url)) + if (!ConvertAbsoluteFilePathToFileSystemUrl(profile, file_path, + kFileManagerAppId, &url)) { + callback.Run(platform_util::OPEN_FAILED_PATH_NOT_FOUND); return; + } - CheckIfDirectoryExists( - GetFileSystemContextForExtensionId(profile, kFileManagerAppId), - url, - base::Bind(&ContinueOpenItem, profile, file_path, url)); + GetMetadataForPath( + GetFileSystemContextForExtensionId(profile, kFileManagerAppId), url, + base::Bind(&OpenItemWithMetadata, profile, file_path, url, expected_type, + callback)); } -void ShowItemInFolder(Profile* profile, const base::FilePath& file_path) { +void ShowItemInFolder(Profile* profile, + const base::FilePath& file_path, + const platform_util::OpenOperationCallback& callback) { DCHECK_CURRENTLY_ON(BrowserThread::UI); GURL url; - if (!ConvertPath(profile, file_path, &url)) + if (!ConvertAbsoluteFilePathToFileSystemUrl(profile, file_path, + kFileManagerAppId, &url)) { + callback.Run(platform_util::OPEN_FAILED_PATH_NOT_FOUND); return; + } // This action changes the selection so we do not reuse existing tabs. OpenFileManagerWithInternalActionId(profile, url, "select"); + callback.Run(platform_util::OPEN_SUCCEEDED); +} + +void DisableShellOperationsForTesting() { + shell_operations_allowed = false; } } // namespace util diff --git a/chrome/browser/chromeos/file_manager/open_util.h b/chrome/browser/chromeos/file_manager/open_util.h index 898cd8b..668827a 100644 --- a/chrome/browser/chromeos/file_manager/open_util.h +++ b/chrome/browser/chromeos/file_manager/open_util.h @@ -8,6 +8,9 @@ #ifndef CHROME_BROWSER_CHROMEOS_FILE_MANAGER_OPEN_UTIL_H_ #define CHROME_BROWSER_CHROMEOS_FILE_MANAGER_OPEN_UTIL_H_ +#include "base/callback_forward.h" +#include "chrome/browser/platform_util.h" + class Profile; namespace base { @@ -17,16 +20,33 @@ class FilePath; namespace file_manager { namespace util { -// Opens an item (file or directory). If the target is a directory, the -// directory will be opened in the file manager. If the target is a file, the -// file will be opened using a file handler, a file browser handler, or the -// browser (open in a tab). The default handler has precedence over other -// handlers, if defined for the type of the target file. -void OpenItem(Profile* profile, const base::FilePath& file_path); +// If |item_type| is OPEN_FILE: Opens an item using a file handler, a file +// browser handler, or the browser (open in a tab). The default handler has +// precedence over other handlers, if defined for the type of the target file. +// +// If |item_type| is OPEN_FOLDER: Open the directory at |file_path| using the +// file browser. +// +// It is an error for |file_path| to not match the type implied by |item_type|. +// This error will be reported to |callback|. +// +// If |callback| is null, shows an error message to the user indicating the +// error if the operation is unsuccessful. No error messages will be displayed +// if |callback| is non-null. +void OpenItem(Profile* profile, + const base::FilePath& file_path, + platform_util::OpenItemType item_type, + const platform_util::OpenOperationCallback& callback); // Opens the file manager for the folder containing the item specified by // |file_path|, with the item selected. -void ShowItemInFolder(Profile* profile, const base::FilePath& file_path); +void ShowItemInFolder(Profile* profile, + const base::FilePath& file_path, + const platform_util::OpenOperationCallback& callback); + +// Change the behavior of the above functions to do everything except launch any +// extensions including a file browser. +void DisableShellOperationsForTesting(); } // namespace util } // namespace file_manager diff --git a/chrome/browser/download/chrome_download_manager_delegate.cc b/chrome/browser/download/chrome_download_manager_delegate.cc index 52a151b..ad2b94b 100644 --- a/chrome/browser/download/chrome_download_manager_delegate.cc +++ b/chrome/browser/download/chrome_download_manager_delegate.cc @@ -433,7 +433,8 @@ void ChromeDownloadManagerDelegate::OpenDownloadUsingPlatformHandler( base::FilePath platform_path( GetPlatformDownloadPath(profile_, download, PLATFORM_TARGET_PATH)); DCHECK(!platform_path.empty()); - platform_util::OpenItem(profile_, platform_path); + platform_util::OpenItem(profile_, platform_path, platform_util::OPEN_FILE, + platform_util::OpenOperationCallback()); } void ChromeDownloadManagerDelegate::OpenDownload(DownloadItem* download) { diff --git a/chrome/browser/extensions/api/downloads/downloads_api.cc b/chrome/browser/extensions/api/downloads/downloads_api.cc index ae841dc..268043e 100644 --- a/chrome/browser/extensions/api/downloads/downloads_api.cc +++ b/chrome/browser/extensions/api/downloads/downloads_api.cc @@ -1331,8 +1331,8 @@ bool DownloadsShowDefaultFolderFunction::RunAsync() { DownloadManager* incognito_manager = NULL; GetManagers(GetProfile(), include_incognito(), &manager, &incognito_manager); platform_util::OpenItem( - GetProfile(), - DownloadPrefs::FromDownloadManager(manager)->DownloadPath()); + GetProfile(), DownloadPrefs::FromDownloadManager(manager)->DownloadPath(), + platform_util::OPEN_FOLDER, platform_util::OpenOperationCallback()); RecordApiFunctions(DOWNLOADS_FUNCTION_SHOW_DEFAULT_FOLDER); return true; } diff --git a/chrome/browser/media_galleries/media_galleries_scan_result_controller.cc b/chrome/browser/media_galleries/media_galleries_scan_result_controller.cc index 3b39a44..1b791b2 100644 --- a/chrome/browser/media_galleries/media_galleries_scan_result_controller.cc +++ b/chrome/browser/media_galleries/media_galleries_scan_result_controller.cc @@ -186,7 +186,9 @@ void MediaGalleriesScanResultController::DidClickOpenFolderViewer( NOTREACHED(); return; } - platform_util::OpenItem(GetProfile(), entry->second.pref_info.AbsolutePath()); + platform_util::OpenItem(GetProfile(), entry->second.pref_info.AbsolutePath(), + platform_util::OPEN_FOLDER, + platform_util::OpenOperationCallback()); } void MediaGalleriesScanResultController::DidForgetEntry( diff --git a/chrome/browser/platform_util.cc b/chrome/browser/platform_util.cc new file mode 100644 index 0000000..5cd2e45 --- /dev/null +++ b/chrome/browser/platform_util.cc @@ -0,0 +1,65 @@ +// Copyright 2015 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/platform_util.h" + +#include "base/files/file.h" +#include "base/files/file_util.h" +#include "base/logging.h" +#include "chrome/browser/platform_util_internal.h" +#include "content/public/browser/browser_thread.h" + +using content::BrowserThread; + +namespace platform_util { + +namespace { + +bool shell_operations_allowed = true; + +void VerifyAndOpenItemOnBlockingThread(const base::FilePath& path, + OpenItemType type, + const OpenOperationCallback& callback) { + base::File target_item(path, base::File::FLAG_OPEN | base::File::FLAG_READ); + if (!base::PathExists(path)) { + if (!callback.is_null()) + BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, + base::Bind(callback, OPEN_FAILED_PATH_NOT_FOUND)); + return; + } + if (base::DirectoryExists(path) != (type == OPEN_FOLDER)) { + if (!callback.is_null()) + BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, + base::Bind(callback, OPEN_FAILED_INVALID_TYPE)); + return; + } + + if (shell_operations_allowed) + internal::PlatformOpenVerifiedItem(path, type); + if (!callback.is_null()) + BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, + base::Bind(callback, OPEN_SUCCEEDED)); +} + +} // namespace + +namespace internal { + +void DisableShellOperationsForTesting() { + shell_operations_allowed = false; +} + +} // namespace internal + +void OpenItem(Profile* profile, + const base::FilePath& full_path, + OpenItemType item_type, + const OpenOperationCallback& callback) { + DCHECK_CURRENTLY_ON(BrowserThread::UI); + BrowserThread::PostBlockingPoolTask( + FROM_HERE, base::Bind(&VerifyAndOpenItemOnBlockingThread, full_path, + item_type, callback)); +} + +} // namespace platform_util diff --git a/chrome/browser/platform_util.h b/chrome/browser/platform_util.h index 3e93469..822e89d 100644 --- a/chrome/browser/platform_util.h +++ b/chrome/browser/platform_util.h @@ -7,6 +7,7 @@ #include <string> +#include "base/callback_forward.h" #include "base/strings/string16.h" #include "ui/gfx/native_widget_types.h" @@ -19,16 +20,50 @@ class FilePath; namespace platform_util { -// Show the given file in a file manager. If possible, select the file. -// The |profile| is used to determine the running profile of file manager app -// in Chrome OS only. Not used in other platforms. -// Must be called from the UI thread. +// Result of calling OpenFile() or OpenFolder() passed into OpenOperationResult. +enum OpenOperationResult { + OPEN_SUCCEEDED, + OPEN_FAILED_PATH_NOT_FOUND, // Specified path does not exist. + OPEN_FAILED_INVALID_TYPE, // Type of object found at path did not match what + // was expected. I.e. OpenFile was called on a + // folder or OpenFolder called on a file. + OPEN_FAILED_NO_HANLDER_FOR_FILE_TYPE, // There was no file handler capable of + // opening file. Only returned on + // ChromeOS. + OPEN_FAILED_FILE_ERROR, // Open operation failed due to some other file + // error. +}; + +// Type of item that is the target of the OpenItem() call. +enum OpenItemType { OPEN_FILE, OPEN_FOLDER }; + +// Callback used with OpenFile and OpenFolder. +typedef base::Callback<void(OpenOperationResult)> OpenOperationCallback; + +// Opens the item specified by |full_path|, which is expected to be the type +// indicated by |item_type| in the desktop's default manner. +// |callback| will be invoked on the UI thread with the result of the open +// operation. +// +// It is an error if the object at |full_path| does not match the intended type +// specified in |item_type|. This error will be reported to |callback|. +// +// Note: On all platforms, the user may be shown additional UI if there is no +// suitable handler for |full_path|. On Chrome OS, all errors will result in +// visible error messages iff |callback| is not specified. +// Must be called on the UI thread. +void OpenItem(Profile* profile, + const base::FilePath& full_path, + OpenItemType item_type, + const OpenOperationCallback& callback); + +// Opens the folder containing the item specified by |full_path| in the +// desktop's default manner. If possible, the item will be selected. The +// |profile| is used to determine the running profile of file manager app in +// Chrome OS only. |profile| is not used in platforms other than Chrome OS. Must +// be called on the UI thread. void ShowItemInFolder(Profile* profile, const base::FilePath& full_path); -// Open the given file in the desktop's default manner. -// Must be called from the UI thread. -void OpenItem(Profile* profile, const base::FilePath& full_path); - // Open the given external protocol URL in the desktop's default manner. // (For example, mailto: URLs in the default mail user agent.) // Must be called from the UI thread. diff --git a/chrome/browser/platform_util_android.cc b/chrome/browser/platform_util_android.cc index b74b86d..8a14b58 100644 --- a/chrome/browser/platform_util_android.cc +++ b/chrome/browser/platform_util_android.cc @@ -14,7 +14,10 @@ void ShowItemInFolder(Profile* profile, const base::FilePath& full_path) { NOTIMPLEMENTED(); } -void OpenItem(Profile* profile, const base::FilePath& full_path) { +void OpenItem(Profile* profile, + const base::FilePath& full_path, + OpenItemType item_type, + const OpenOperationCallback& callback) { NOTIMPLEMENTED(); } diff --git a/chrome/browser/platform_util_chromeos.cc b/chrome/browser/platform_util_chromeos.cc index aad0b73..8d463bf 100644 --- a/chrome/browser/platform_util_chromeos.cc +++ b/chrome/browser/platform_util_chromeos.cc @@ -4,34 +4,99 @@ #include "chrome/browser/platform_util.h" +#include "base/bind.h" +#include "base/files/file_path.h" #include "chrome/browser/chromeos/file_manager/open_util.h" +#include "chrome/browser/platform_util_internal.h" +#include "chrome/browser/ui/browser.h" +#include "chrome/browser/ui/browser_finder.h" #include "chrome/browser/ui/browser_navigator.h" +#include "chrome/browser/ui/browser_window.h" +#include "chrome/browser/ui/simple_message_box.h" +#include "chrome/grit/generated_resources.h" #include "content/public/browser/browser_thread.h" +#include "ui/base/l10n/l10n_util.h" #include "url/gurl.h" using content::BrowserThread; +namespace platform_util { + namespace { const char kGmailComposeUrl[] = "https://mail.google.com/mail/?extsrc=mailto&url="; +void ShowWarningOnOpenOperationResult(Profile* profile, + const base::FilePath& path, + OpenOperationResult result) { + int message_id = IDS_FILE_BROWSER_ERROR_VIEWING_FILE; + switch (result) { + case OPEN_SUCCEEDED: + return; + + case OPEN_FAILED_PATH_NOT_FOUND: + message_id = IDS_FILE_BROWSER_ERROR_UNRESOLVABLE_FILE; + break; + + case OPEN_FAILED_INVALID_TYPE: + return; + + case OPEN_FAILED_NO_HANLDER_FOR_FILE_TYPE: + if (path.MatchesExtension(FILE_PATH_LITERAL(".dmg"))) + message_id = IDS_FILE_BROWSER_ERROR_VIEWING_FILE_FOR_DMG; + else if (path.MatchesExtension(FILE_PATH_LITERAL(".exe")) || + path.MatchesExtension(FILE_PATH_LITERAL(".msi"))) + message_id = IDS_FILE_BROWSER_ERROR_VIEWING_FILE_FOR_EXECUTABLE; + else + message_id = IDS_FILE_BROWSER_ERROR_VIEWING_FILE; + break; + + case OPEN_FAILED_FILE_ERROR: + message_id = IDS_FILE_BROWSER_ERROR_VIEWING_FILE; + break; + } + + Browser* browser = + chrome::FindTabbedBrowser(profile, false, chrome::HOST_DESKTOP_TYPE_ASH); + chrome::ShowMessageBox( + browser ? browser->window()->GetNativeWindow() : nullptr, + l10n_util::GetStringFUTF16(IDS_FILE_BROWSER_ERROR_VIEWING_FILE_TITLE, + path.BaseName().AsUTF16Unsafe()), + l10n_util::GetStringUTF16(message_id), chrome::MESSAGE_BOX_TYPE_WARNING); +} + } // namespace -namespace platform_util { +namespace internal { + +void DisableShellOperationsForTesting() { + file_manager::util::DisableShellOperationsForTesting(); +} + +} // namespace internal void ShowItemInFolder(Profile* profile, const base::FilePath& full_path) { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); - file_manager::util::ShowItemInFolder(profile, full_path); + file_manager::util::ShowItemInFolder( + profile, full_path, + base::Bind(&ShowWarningOnOpenOperationResult, profile, full_path)); } -void OpenItem(Profile* profile, const base::FilePath& full_path) { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); - file_manager::util::OpenItem(profile, full_path); +void OpenItem(Profile* profile, + const base::FilePath& full_path, + OpenItemType item_type, + const OpenOperationCallback& callback) { + DCHECK_CURRENTLY_ON(BrowserThread::UI); + file_manager::util::OpenItem( + profile, full_path, item_type, + callback.is_null() + ? base::Bind(&ShowWarningOnOpenOperationResult, profile, full_path) + : callback); } void OpenExternal(Profile* profile, const GURL& url) { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); + DCHECK_CURRENTLY_ON(BrowserThread::UI); // This code should be obsolete since we have default handlers in ChromeOS // which should handle this. However - there are two things which make it diff --git a/chrome/browser/platform_util_internal.h b/chrome/browser/platform_util_internal.h new file mode 100644 index 0000000..0b93b7d --- /dev/null +++ b/chrome/browser/platform_util_internal.h @@ -0,0 +1,29 @@ +// Copyright 2015 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. + +#ifndef CHROME_BROWSER_PLATFORM_UTIL_INTERNAL_H_ +#define CHROME_BROWSER_PLATFORM_UTIL_INTERNAL_H_ + +#include "chrome/browser/platform_util.h" + +namespace base { +class FilePath; +} + +namespace platform_util { +namespace internal { + +// Called by platform_util.cc on desktop platforms to invoke platform specific +// logic to open |path| using a suitable handler. |path| has been verified to be +// of type |type|. +// Always called on the blocking pool. +void PlatformOpenVerifiedItem(const base::FilePath& path, OpenItemType type); + +// Prevent shell or external applications from being invoked during testing. +void DisableShellOperationsForTesting(); + +} // namespace internal +} // namespace platform_util + +#endif // CHROME_BROWSER_PLATFORM_UTIL_INTERNAL_H_ diff --git a/chrome/browser/platform_util_linux.cc b/chrome/browser/platform_util_linux.cc index c94f8fe..9938451 100644 --- a/chrome/browser/platform_util_linux.cc +++ b/chrome/browser/platform_util_linux.cc @@ -9,19 +9,25 @@ #include "base/process/kill.h" #include "base/process/launch.h" #include "base/strings/utf_string_conversions.h" +#include "chrome/browser/platform_util_internal.h" #include "content/public/browser/browser_thread.h" #include "url/gurl.h" using content::BrowserThread; +namespace platform_util { + namespace { -void XDGUtil(const std::string& util, const std::string& arg) { +void XDGUtil(const std::string& util, + const base::FilePath& working_directory, + const std::string& arg) { std::vector<std::string> argv; argv.push_back(util); argv.push_back(arg); base::LaunchOptions options; + options.current_directory = working_directory; options.allow_new_privs = true; // xdg-open can fall back on mailcap which eventually might plumb through // to a command that needs a terminal. Set the environment variable telling @@ -42,39 +48,43 @@ void XDGUtil(const std::string& util, const std::string& arg) { base::EnsureProcessGetsReaped(process.Pid()); } -void XDGOpen(const std::string& path) { - XDGUtil("xdg-open", path); +void XDGOpen(const base::FilePath& working_directory, const std::string& path) { + XDGUtil("xdg-open", working_directory, path); } void XDGEmail(const std::string& email) { - XDGUtil("xdg-email", email); -} - -// TODO(estade): It would be nice to be able to select the file in the file -// manager, but that probably requires extending xdg-open. For now just -// show the folder. -void ShowItemInFolderOnFileThread(const base::FilePath& full_path) { - base::FilePath dir = full_path.DirName(); - if (!base::DirectoryExists(dir)) - return; - - XDGOpen(dir.value()); + XDGUtil("xdg-email", base::FilePath(), email); } } // namespace -namespace platform_util { - -void ShowItemInFolder(Profile* profile, const base::FilePath& full_path) { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); - BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE, - base::Bind(&ShowItemInFolderOnFileThread, full_path)); +namespace internal { + +void PlatformOpenVerifiedItem(const base::FilePath& path, OpenItemType type) { + switch (type) { + case OPEN_FILE: + XDGOpen(path.DirName(), path.value()); + break; + case OPEN_FOLDER: + // The utility process checks the working directory prior to the + // invocation of xdg-open by changing the current directory into it. This + // operation only succeeds if |path| is a directory. Opening "." from + // there ensures that the target of the operation is a directory. Note + // that there remains a TOCTOU race where the directory could be unlinked + // between the time the utility process changes into the directory and the + // time the application invoked by xdg-open inspects the path by name. + XDGOpen(path, "."); + break; + } } +} // namespace internal -void OpenItem(Profile* profile, const base::FilePath& full_path) { +void ShowItemInFolder(Profile* profile, const base::FilePath& full_path) { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); - BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE, - base::Bind(&XDGOpen, full_path.value())); + // TODO(estade): It would be nice to be able to select the file in the file + // manager, but that probably requires extending xdg-open. For now just show + // the folder. + OpenItem(profile, full_path.DirName(), OPEN_FOLDER, OpenOperationCallback()); } void OpenExternal(Profile* profile, const GURL& url) { @@ -82,7 +92,7 @@ void OpenExternal(Profile* profile, const GURL& url) { if (url.SchemeIs("mailto")) XDGEmail(url.spec()); else - XDGOpen(url.spec()); + XDGOpen(base::FilePath(), url.spec()); } } // namespace platform_util diff --git a/chrome/browser/platform_util_mac.mm b/chrome/browser/platform_util_mac.mm index 15622f1..8ce07be 100644 --- a/chrome/browser/platform_util_mac.mm +++ b/chrome/browser/platform_util_mac.mm @@ -8,6 +8,8 @@ #import <Cocoa/Cocoa.h> #include <CoreServices/CoreServices.h> +#include "base/bind.h" +#include "base/files/file_util.h" #include "base/files/file_path.h" #include "base/logging.h" #include "base/mac/mac_logging.h" @@ -15,6 +17,8 @@ #import "base/mac/sdk_forward_declarations.h" #include "base/mac/scoped_aedesc.h" #include "base/strings/sys_string_conversions.h" +#include "chrome/browser/platform_util_internal.h" +#include "content/public/browser/browser_thread.h" #include "url/gurl.h" namespace platform_util { @@ -27,7 +31,7 @@ void ShowItemInFolder(Profile* profile, const base::FilePath& full_path) { LOG(WARNING) << "NSWorkspace failed to select file " << full_path.value(); } -void OpenItem(Profile* profile, const base::FilePath& full_path) { +void OpenFileOnMainThread(const base::FilePath& full_path) { DCHECK([NSThread isMainThread]); NSString* path_string = base::SysUTF8ToNSString(full_path.value()); if (!path_string) @@ -64,7 +68,7 @@ void OpenItem(Profile* profile, const base::FilePath& full_path) { sizeof(finderCreatorCode), // dataSize address.OutPointer()); // result if (status != noErr) { - OSSTATUS_LOG(WARNING, status) << "Could not create OpenItem() AE target"; + OSSTATUS_LOG(WARNING, status) << "Could not create OpenFile() AE target"; return; } @@ -77,7 +81,7 @@ void OpenItem(Profile* profile, const base::FilePath& full_path) { kAnyTransactionID, // transactionID theEvent.OutPointer()); // result if (status != noErr) { - OSSTATUS_LOG(WARNING, status) << "Could not create OpenItem() AE event"; + OSSTATUS_LOG(WARNING, status) << "Could not create OpenFile() AE event"; return; } @@ -88,7 +92,7 @@ void OpenItem(Profile* profile, const base::FilePath& full_path) { false, // isRecord fileList.OutPointer()); // resultList if (status != noErr) { - OSSTATUS_LOG(WARNING, status) << "Could not create OpenItem() AE file list"; + OSSTATUS_LOG(WARNING, status) << "Could not create OpenFile() AE file list"; return; } @@ -104,11 +108,11 @@ void OpenItem(Profile* profile, const base::FilePath& full_path) { sizeof(pathRef)); // dataSize if (status != noErr) { OSSTATUS_LOG(WARNING, status) - << "Could not add file path to AE list in OpenItem()"; + << "Could not add file path to AE list in OpenFile()"; return; } } else { - LOG(WARNING) << "Could not get FSRef for path URL in OpenItem()"; + LOG(WARNING) << "Could not get FSRef for path URL in OpenFile()"; return; } @@ -118,7 +122,7 @@ void OpenItem(Profile* profile, const base::FilePath& full_path) { fileList); // theAEDesc if (status != noErr) { OSSTATUS_LOG(WARNING, status) - << "Could not put the AE file list the path in OpenItem()"; + << "Could not put the AE file list the path in OpenFile()"; return; } @@ -133,10 +137,32 @@ void OpenItem(Profile* profile, const base::FilePath& full_path) { NULL); // filterProc if (status != noErr) { OSSTATUS_LOG(WARNING, status) - << "Could not send AE to Finder in OpenItem()"; + << "Could not send AE to Finder in OpenFile()"; } } +namespace internal { + +void PlatformOpenVerifiedItem(const base::FilePath& path, OpenItemType type) { + switch (type) { + case OPEN_FILE: + content::BrowserThread::PostTask(content::BrowserThread::UI, FROM_HERE, + base::Bind(&OpenFileOnMainThread, path)); + return; + case OPEN_FOLDER: + NSString* path_string = base::SysUTF8ToNSString(path.value()); + if (!path_string) + return; + // Note that there exists a TOCTOU race between the time that |path| was + // verified as being a directory and when NSWorkspace invokes Finder (or + // alternative) to open |path_string|. + [[NSWorkspace sharedWorkspace] openFile:path_string]; + return; + } +} + +} // namespace internal + void OpenExternal(Profile* profile, const GURL& url) { DCHECK([NSThread isMainThread]); NSString* url_string = base::SysUTF8ToNSString(url.spec()); diff --git a/chrome/browser/platform_util_unittest.cc b/chrome/browser/platform_util_unittest.cc new file mode 100644 index 0000000..16a5edc --- /dev/null +++ b/chrome/browser/platform_util_unittest.cc @@ -0,0 +1,300 @@ +// Copyright 2015 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/platform_util.h" + +#include "base/bind.h" +#include "base/callback.h" +#include "base/files/file_util.h" +#include "base/files/scoped_temp_dir.h" +#include "base/memory/scoped_ptr.h" +#include "base/run_loop.h" +#include "chrome/browser/platform_util_internal.h" +#include "testing/gtest/include/gtest/gtest.h" + +#if defined(OS_CHROMEOS) +#include "base/json/json_string_value_serializer.h" +#include "base/values.h" +#include "chrome/browser/chrome_content_browser_client.h" +#include "chrome/browser/chromeos/file_manager/app_id.h" +#include "chrome/browser/chromeos/fileapi/file_system_backend.h" +#include "chrome/browser/extensions/extension_special_storage_policy.h" +#include "chrome/test/base/browser_with_test_window_test.h" +#include "content/public/browser/browser_context.h" +#include "content/public/common/content_client.h" +#include "content/public/test/mock_special_storage_policy.h" +#include "extensions/browser/extension_registry.h" +#include "extensions/common/extension.h" +#include "storage/browser/fileapi/external_mount_points.h" +#include "storage/common/fileapi/file_system_types.h" +#else +#include "content/public/test/test_browser_thread_bundle.h" +#endif + +namespace platform_util { + +namespace { + +#if defined(OS_CHROMEOS) + +// ChromeContentBrowserClient subclass that sets up a custom file system backend +// that allows the test to grant file access to the file manager extension ID +// without having to install the extension. +class PlatformUtilTestContentBrowserClient + : public chrome::ChromeContentBrowserClient { + public: + void GetAdditionalFileSystemBackends( + content::BrowserContext* browser_context, + const base::FilePath& storage_partition_path, + ScopedVector<storage::FileSystemBackend>* additional_backends) override { + storage::ExternalMountPoints* external_mount_points = + content::BrowserContext::GetMountPoints(browser_context); + scoped_refptr<content::MockSpecialStoragePolicy> special_storage_policy = + new content::MockSpecialStoragePolicy(); + + // New FileSystemBackend that uses our MockSpecialStoragePolicy. + chromeos::FileSystemBackend* backend = new chromeos::FileSystemBackend( + nullptr, nullptr, nullptr, special_storage_policy, + external_mount_points, + storage::ExternalMountPoints::GetSystemInstance()); + additional_backends->push_back(backend); + + // The only change we need to make is to add kFileManagerAppId as a file + // handler. + special_storage_policy->AddFileHandler(file_manager::kFileManagerAppId); + } +}; + +// Base test fixture class to be used on Chrome OS. +class PlatformUtilTestBase : public BrowserWithTestWindowTest { + protected: + void SetUpPlatformFixture(const base::FilePath& test_directory) { + content_browser_client_.reset(new PlatformUtilTestContentBrowserClient()); + old_content_browser_client_ = + content::SetBrowserClientForTesting(content_browser_client_.get()); + + // The test_directory needs to be mounted for it to be accessible. + content::BrowserContext::GetMountPoints(GetProfile()) + ->RegisterFileSystem("test", storage::kFileSystemTypeNativeLocal, + storage::FileSystemMountOption(), test_directory); + + // To test opening a file, we are going to register a mock extension that + // handles .txt files. The extension doesn't actually need to exist due to + // the DisableShellOperationsForTesting() call which prevents the extension + // from being invoked. + std::string error; + int error_code = 0; + + std::string json_manifest = + "{" + " \"manifest_version\": 2," + " \"name\": \"Test extension\"," + " \"version\": \"0\"," + " \"app\": { \"background\": { \"scripts\": [\"main.js\"] }}," + " \"file_handlers\": {" + " \"text\": {" + " \"extensions\": [ \"txt\" ]," + " \"title\": \"Text\"" + " }" + " }" + "}"; + JSONStringValueDeserializer json_string_deserializer(json_manifest); + scoped_ptr<base::Value> manifest( + json_string_deserializer.Deserialize(&error_code, &error)); + base::DictionaryValue* manifest_dictionary; + + manifest->GetAsDictionary(&manifest_dictionary); + ASSERT_TRUE(manifest_dictionary); + + scoped_refptr<extensions::Extension> extension = + extensions::Extension::Create( + test_directory.AppendASCII("invalid-extension"), + extensions::Manifest::INVALID_LOCATION, *manifest_dictionary, + extensions::Extension::NO_FLAGS, &error); + ASSERT_TRUE(error.empty()) << error; + extensions::ExtensionRegistry::Get(GetProfile())->AddEnabled(extension); + } + + void TearDown() override { + content::ContentBrowserClient* content_browser_client = + content::SetBrowserClientForTesting(old_content_browser_client_); + old_content_browser_client_ = nullptr; + DCHECK_EQ(static_cast<content::ContentBrowserClient*>( + content_browser_client_.get()), + content_browser_client) + << "ContentBrowserClient changed during test."; + BrowserWithTestWindowTest::TearDown(); + } + + private: + scoped_ptr<content::ContentBrowserClient> content_browser_client_; + content::ContentBrowserClient* old_content_browser_client_ = nullptr; +}; + +#else + +// Test fixture used by all desktop platforms other than Chrome OS. +class PlatformUtilTestBase : public testing::Test { + protected: + Profile* GetProfile() { return nullptr; } + void SetUpPlatformFixture(const base::FilePath&) {} + + private: + content::TestBrowserThreadBundle thread_bundle_; +}; + +#endif + +class PlatformUtilTest : public PlatformUtilTestBase { + public: + void SetUp() override { + ASSERT_NO_FATAL_FAILURE(PlatformUtilTestBase::SetUp()); + + static const char kTestFileData[] = "Cow says moo!"; + const int kTestFileDataLength = arraysize(kTestFileData) - 1; + + // This prevents platfrom_util from invoking any shell or external APIs + // during tests. Doing so may result in external applications being launched + // and intefering with tests. + internal::DisableShellOperationsForTesting(); + + ASSERT_TRUE(directory_.CreateUniqueTempDir()); + + // A valid file. + existing_file_ = directory_.path().AppendASCII("test_file.txt"); + ASSERT_EQ( + kTestFileDataLength, + base::WriteFile(existing_file_, kTestFileData, kTestFileDataLength)); + + // A valid folder. + existing_folder_ = directory_.path().AppendASCII("test_folder"); + ASSERT_TRUE(base::CreateDirectory(existing_folder_)); + + // A non-existent path. + nowhere_ = directory_.path().AppendASCII("nowhere"); + + SetUpPlatformFixture(directory_.path()); + } + + OpenOperationResult CallOpenItem(const base::FilePath& path, + OpenItemType item_type) { + base::RunLoop run_loop; + OpenOperationResult result = OPEN_SUCCEEDED; + OpenOperationCallback callback = + base::Bind(&OnOpenOperationDone, run_loop.QuitClosure(), &result); + OpenItem(GetProfile(), path, item_type, callback); + run_loop.Run(); + return result; + } + + base::FilePath existing_file_; + base::FilePath existing_folder_; + base::FilePath nowhere_; + + protected: + base::ScopedTempDir directory_; + + private: + scoped_ptr<base::RunLoop> run_loop_; + + static void OnOpenOperationDone(const base::Closure& closure, + OpenOperationResult* store_result, + OpenOperationResult result) { + *store_result = result; + closure.Run(); + } +}; + +} // namespace + +TEST_F(PlatformUtilTest, OpenFile) { + EXPECT_EQ(OPEN_SUCCEEDED, CallOpenItem(existing_file_, OPEN_FILE)); + EXPECT_EQ(OPEN_FAILED_INVALID_TYPE, + CallOpenItem(existing_folder_, OPEN_FILE)); + EXPECT_EQ(OPEN_FAILED_PATH_NOT_FOUND, CallOpenItem(nowhere_, OPEN_FILE)); +} + +TEST_F(PlatformUtilTest, OpenFolder) { + EXPECT_EQ(OPEN_SUCCEEDED, CallOpenItem(existing_folder_, OPEN_FOLDER)); + EXPECT_EQ(OPEN_FAILED_INVALID_TYPE, + CallOpenItem(existing_file_, OPEN_FOLDER)); + EXPECT_EQ(OPEN_FAILED_PATH_NOT_FOUND, CallOpenItem(nowhere_, OPEN_FOLDER)); +} + +#if defined(OS_POSIX) +// Symbolic links are currently only supported on Posix. Windows technically +// supports it as well, but not on Windows XP. +class PlatformUtilPosixTest : public PlatformUtilTest { + public: + void SetUp() override { + ASSERT_NO_FATAL_FAILURE(PlatformUtilTest::SetUp()); + + symlink_to_file_ = directory_.path().AppendASCII("l_file.txt"); + ASSERT_TRUE(base::CreateSymbolicLink(existing_file_, symlink_to_file_)); + symlink_to_folder_ = directory_.path().AppendASCII("l_folder"); + ASSERT_TRUE(base::CreateSymbolicLink(existing_folder_, symlink_to_folder_)); + symlink_to_nowhere_ = directory_.path().AppendASCII("l_nowhere"); + ASSERT_TRUE(base::CreateSymbolicLink(nowhere_, symlink_to_nowhere_)); + } + + protected: + base::FilePath symlink_to_file_; + base::FilePath symlink_to_folder_; + base::FilePath symlink_to_nowhere_; +}; +#endif // OS_POSIX + +#if defined(OS_CHROMEOS) +// ChromeOS doesn't follow symbolic links in sandboxed filesystems. So all the +// symbolic link tests should return PATH_NOT_FOUND. + +TEST_F(PlatformUtilPosixTest, OpenFileWithPosixSymlinksChromeOS) { + EXPECT_EQ(OPEN_FAILED_PATH_NOT_FOUND, + CallOpenItem(symlink_to_file_, OPEN_FILE)); + EXPECT_EQ(OPEN_FAILED_PATH_NOT_FOUND, + CallOpenItem(symlink_to_folder_, OPEN_FILE)); + EXPECT_EQ(OPEN_FAILED_PATH_NOT_FOUND, + CallOpenItem(symlink_to_nowhere_, OPEN_FILE)); +} + +TEST_F(PlatformUtilPosixTest, OpenFolderWithPosixSymlinksChromeOS) { + EXPECT_EQ(OPEN_FAILED_PATH_NOT_FOUND, + CallOpenItem(symlink_to_folder_, OPEN_FOLDER)); + EXPECT_EQ(OPEN_FAILED_PATH_NOT_FOUND, + CallOpenItem(symlink_to_file_, OPEN_FOLDER)); + EXPECT_EQ(OPEN_FAILED_PATH_NOT_FOUND, + CallOpenItem(symlink_to_nowhere_, OPEN_FOLDER)); +} + +TEST_F(PlatformUtilTest, OpenFileWithUnhandledFileType) { + base::FilePath unhandled_file = + directory_.path().AppendASCII("myfile.filetype"); + ASSERT_EQ(3, base::WriteFile(unhandled_file, "cat", 3)); + EXPECT_EQ(OPEN_FAILED_NO_HANLDER_FOR_FILE_TYPE, + CallOpenItem(unhandled_file, OPEN_FILE)); +} +#endif // OS_CHROMEOS + +#if defined(OS_POSIX) && !defined(OS_CHROMEOS) +// On all other Posix platforms, the symbolic link tests should work as +// expected. + +TEST_F(PlatformUtilPosixTest, OpenFileWithPosixSymlinks) { + EXPECT_EQ(OPEN_SUCCEEDED, CallOpenItem(symlink_to_file_, OPEN_FILE)); + EXPECT_EQ(OPEN_FAILED_INVALID_TYPE, + CallOpenItem(symlink_to_folder_, OPEN_FILE)); + EXPECT_EQ(OPEN_FAILED_PATH_NOT_FOUND, + CallOpenItem(symlink_to_nowhere_, OPEN_FILE)); +} + +TEST_F(PlatformUtilPosixTest, OpenFolderWithPosixSymlinks) { + EXPECT_EQ(OPEN_SUCCEEDED, CallOpenItem(symlink_to_folder_, OPEN_FOLDER)); + EXPECT_EQ(OPEN_FAILED_INVALID_TYPE, + CallOpenItem(symlink_to_file_, OPEN_FOLDER)); + EXPECT_EQ(OPEN_FAILED_PATH_NOT_FOUND, + CallOpenItem(symlink_to_nowhere_, OPEN_FOLDER)); +} +#endif // OS_POSIX && !OS_CHROMEOS + +} // namespace platform_util diff --git a/chrome/browser/platform_util_win.cc b/chrome/browser/platform_util_win.cc index 65aee57..a905b2f 100644 --- a/chrome/browser/platform_util_win.cc +++ b/chrome/browser/platform_util_win.cc @@ -12,6 +12,7 @@ #include "base/bind.h" #include "base/bind_helpers.h" #include "base/files/file_path.h" +#include "base/files/file_util.h" #include "base/logging.h" #include "base/metrics/field_trial.h" #include "base/strings/string_util.h" @@ -21,6 +22,7 @@ #include "base/win/scoped_comptr.h" #include "base/win/windows_version.h" #include "chrome/browser/lifetime/application_lifetime.h" +#include "chrome/browser/platform_util_internal.h" #include "chrome/browser/ui/host_desktop.h" #include "chrome/common/chrome_utility_messages.h" #include "content/public/browser/browser_thread.h" @@ -32,8 +34,12 @@ using content::BrowserThread; +namespace platform_util { + namespace { +// TODO(asanka): Move this to ui/base/win/shell.{h,cc} and invoke it from the +// utility process. void ShowItemInFolderOnFileThread(const base::FilePath& full_path) { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE)); base::FilePath dir = full_path.DirName().AsEndingWithSeparator(); @@ -102,10 +108,9 @@ void ShowItemInFolderOnFileThread(const base::FilePath& full_path) { if (hr == ERROR_FILE_NOT_FOUND) { ShellExecute(NULL, L"open", dir.value().c_str(), NULL, NULL, SW_SHOW); } else { - LOG(WARNING) << " " << __FUNCTION__ - << "(): Can't open full_path = \"" + LOG(WARNING) << " " << __FUNCTION__ << "(): Can't open full_path = \"" << full_path.value() << "\"" - << " hr = " << logging::SystemErrorCodeToString(hr); + << " hr = " << logging::SystemErrorCodeToString(hr); } } } @@ -159,47 +164,64 @@ void OpenExternalOnFileThread(const GURL& url) { } } -void OpenItemViaShellInUtilityProcess(const base::FilePath& full_path) { +void OpenItemViaShellInUtilityProcess(const base::FilePath& full_path, + OpenItemType type) { base::WeakPtr<content::UtilityProcessHost> utility_process_host( content::UtilityProcessHost::Create(NULL, NULL)->AsWeakPtr()); utility_process_host->DisableSandbox(); - utility_process_host->Send(new ChromeUtilityMsg_OpenItemViaShell(full_path)); -} - -} // namespace - -namespace platform_util { + switch (type) { + case OPEN_FILE: + utility_process_host->Send( + new ChromeUtilityMsg_OpenFileViaShell(full_path)); + return; -void ShowItemInFolder(Profile* profile, const base::FilePath& full_path) { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); + case OPEN_FOLDER: + utility_process_host->Send( + new ChromeUtilityMsg_OpenFolderViaShell(full_path)); + return; + } +} +void ActivateDesktopIfNecessary() { + DCHECK_CURRENTLY_ON(BrowserThread::UI); if (chrome::GetActiveDesktop() == chrome::HOST_DESKTOP_TYPE_ASH) chrome::ActivateDesktopHelper(chrome::ASH_KEEP_RUNNING); +} + +} // namespace +void ShowItemInFolder(Profile* profile, const base::FilePath& full_path) { + ActivateDesktopIfNecessary(); BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE, base::Bind(&ShowItemInFolderOnFileThread, full_path)); } -void OpenItem(Profile* profile, const base::FilePath& full_path) { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); +namespace internal { - if (chrome::GetActiveDesktop() == chrome::HOST_DESKTOP_TYPE_ASH) - chrome::ActivateDesktopHelper(chrome::ASH_KEEP_RUNNING); +void PlatformOpenVerifiedItem(const base::FilePath& path, OpenItemType type) { + BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, + base::Bind(&ActivateDesktopIfNecessary)); if (base::FieldTrialList::FindFullName("IsolateShellOperations") == "Enabled") { BrowserThread::PostTask( - BrowserThread::IO, - FROM_HERE, - base::Bind(&OpenItemViaShellInUtilityProcess, full_path)); + BrowserThread::IO, FROM_HERE, + base::Bind(&OpenItemViaShellInUtilityProcess, path, type)); } else { - BrowserThread::PostTask( - BrowserThread::FILE, - FROM_HERE, - base::Bind(base::IgnoreResult(&ui::win::OpenItemViaShell), full_path)); + switch (type) { + case OPEN_FILE: + ui::win::OpenFileViaShell(path); + break; + + case OPEN_FOLDER: + ui::win::OpenFolderViaShell(path); + break; + } } } +} // namespace internal + void OpenExternal(Profile* profile, const GURL& url) { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); diff --git a/chrome/browser/ui/ash/chrome_screenshot_grabber.cc b/chrome/browser/ui/ash/chrome_screenshot_grabber.cc index 36b873a..9671d32 100644 --- a/chrome/browser/ui/ash/chrome_screenshot_grabber.cc +++ b/chrome/browser/ui/ash/chrome_screenshot_grabber.cc @@ -17,6 +17,7 @@ #include "chrome/browser/browser_process.h" #include "chrome/browser/download/download_prefs.h" #include "chrome/browser/notifications/notification_ui_manager.h" +#include "chrome/browser/platform_util.h" #include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile_manager.h" #include "chrome/common/pref_names.h" @@ -114,11 +115,7 @@ class ScreenshotGrabberNotificationDelegate : public NotificationDelegate { void Click() override { if (!success_) return; -#if defined(OS_CHROMEOS) - file_manager::util::ShowItemInFolder(profile_, screenshot_path_); -#else -// TODO(sschmitz): perhaps add similar action for Windows. -#endif + platform_util::ShowItemInFolder(profile_, screenshot_path_); } void ButtonClick(int button_index) override { DCHECK(success_ && button_index == 0); diff --git a/chrome/browser/ui/webui/downloads_dom_handler.cc b/chrome/browser/ui/webui/downloads_dom_handler.cc index 1a1dbc9..60bdcf5 100644 --- a/chrome/browser/ui/webui/downloads_dom_handler.cc +++ b/chrome/browser/ui/webui/downloads_dom_handler.cc @@ -522,7 +522,8 @@ void DownloadsDOMHandler::HandleOpenDownloadsFolder( if (manager) { platform_util::OpenItem( Profile::FromBrowserContext(manager->GetBrowserContext()), - DownloadPrefs::FromDownloadManager(manager)->DownloadPath()); + DownloadPrefs::FromDownloadManager(manager)->DownloadPath(), + platform_util::OPEN_FOLDER, platform_util::OpenOperationCallback()); } } diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi index 0f1b79d..be4f0d3 100644 --- a/chrome/chrome_browser.gypi +++ b/chrome/chrome_browser.gypi @@ -521,6 +521,7 @@ 'browser/platform_util.h', 'browser/platform_util_android.cc', 'browser/platform_util_chromeos.cc', + 'browser/platform_util_internal.h', 'browser/platform_util_mac.mm', 'browser/platform_util_win.cc', 'browser/precache/most_visited_urls_provider.cc', @@ -777,6 +778,7 @@ ], # Everything but Android, iOS, and CrOS. 'chrome_browser_desktop_sources': [ + 'browser/platform_util.cc', 'browser/profiles/avatar_menu_actions_desktop.cc', 'browser/profiles/avatar_menu_actions_desktop.h', 'browser/profiles/avatar_menu_desktop.cc', diff --git a/chrome/chrome_tests_unit.gypi b/chrome/chrome_tests_unit.gypi index 360aa35..3977471 100644 --- a/chrome/chrome_tests_unit.gypi +++ b/chrome/chrome_tests_unit.gypi @@ -1434,6 +1434,7 @@ 'browser/media_galleries/win/mtp_device_delegate_impl_win_unittest.cc', 'browser/media_galleries/win/mtp_device_object_enumerator_unittest.cc', 'browser/net/firefox_proxy_settings_unittest.cc', + 'browser/platform_util_unittest.cc', 'browser/power/process_power_collector_unittest.cc', 'browser/process_singleton_posix_unittest.cc', 'browser/profile_resetter/profile_resetter_unittest.cc', diff --git a/chrome/common/chrome_utility_messages.h b/chrome/common/chrome_utility_messages.h index 5f07459..1e50ef1 100644 --- a/chrome/common/chrome_utility_messages.h +++ b/chrome/common/chrome_utility_messages.h @@ -111,7 +111,12 @@ IPC_MESSAGE_CONTROL1(ChromeUtilityMsg_AnalyzeZipFileForDownloadProtection, #endif #if defined(OS_WIN) -IPC_MESSAGE_CONTROL1(ChromeUtilityMsg_OpenItemViaShell, +// Invokes ui::base::win::OpenFileViaShell from the utility process. +IPC_MESSAGE_CONTROL1(ChromeUtilityMsg_OpenFileViaShell, + base::FilePath /* full_path */) + +// Invokes ui::base::win::OpenFolderViaShell from the utility process. +IPC_MESSAGE_CONTROL1(ChromeUtilityMsg_OpenFolderViaShell, base::FilePath /* full_path */) // Instructs the utility process to invoke GetOpenFileName. |owner| is the diff --git a/chrome/utility/shell_handler_win.cc b/chrome/utility/shell_handler_win.cc index e7b3c09..96cce54 100644 --- a/chrome/utility/shell_handler_win.cc +++ b/chrome/utility/shell_handler_win.cc @@ -19,8 +19,10 @@ ShellHandler::~ShellHandler() {} bool ShellHandler::OnMessageReceived(const IPC::Message& message) { bool handled = true; IPC_BEGIN_MESSAGE_MAP(ShellHandler, message) - IPC_MESSAGE_HANDLER(ChromeUtilityMsg_OpenItemViaShell, - OnOpenItemViaShell) + IPC_MESSAGE_HANDLER(ChromeUtilityMsg_OpenFileViaShell, + OnOpenFileViaShell) + IPC_MESSAGE_HANDLER(ChromeUtilityMsg_OpenFolderViaShell, + OnOpenFolderViaShell) IPC_MESSAGE_HANDLER(ChromeUtilityMsg_GetOpenFileName, OnGetOpenFileName) IPC_MESSAGE_HANDLER(ChromeUtilityMsg_GetSaveFileName, @@ -30,8 +32,12 @@ bool ShellHandler::OnMessageReceived(const IPC::Message& message) { return handled; } -void ShellHandler::OnOpenItemViaShell(const base::FilePath& full_path) { - ui::win::OpenItemViaShell(full_path); +void ShellHandler::OnOpenFileViaShell(const base::FilePath& full_path) { + ui::win::OpenFileViaShell(full_path); +} + +void ShellHandler::OnOpenFolderViaShell(const base::FilePath& full_path) { + ui::win::OpenFolderViaShell(full_path); } void ShellHandler::OnGetOpenFileName( diff --git a/chrome/utility/shell_handler_win.h b/chrome/utility/shell_handler_win.h index fdbed1b..729f111 100644 --- a/chrome/utility/shell_handler_win.h +++ b/chrome/utility/shell_handler_win.h @@ -36,7 +36,8 @@ class ShellHandler : public UtilityMessageHandler { virtual bool OnMessageReceived(const IPC::Message& message) override; private: - void OnOpenItemViaShell(const base::FilePath& full_path); + void OnOpenFileViaShell(const base::FilePath& full_path); + void OnOpenFolderViaShell(const base::FilePath& full_path); void OnGetOpenFileName( HWND owner, diff --git a/ui/base/win/shell.cc b/ui/base/win/shell.cc index 9e47f8c..2a5bd29 100644 --- a/ui/base/win/shell.cc +++ b/ui/base/win/shell.cc @@ -11,9 +11,11 @@ #include "base/command_line.h" #include "base/debug/alias.h" +#include "base/files/file.h" #include "base/files/file_path.h" #include "base/native_library.h" #include "base/strings/string_util.h" +#include "base/threading/thread_restrictions.h" #include "base/win/metro.h" #include "base/win/scoped_comptr.h" #include "base/win/win_util.h" @@ -25,15 +27,39 @@ namespace win { namespace { -// Show the Windows "Open With" dialog box to ask the user to pick an app to -// open the file with. -bool OpenItemWithExternalApp(const base::string16& full_path) { - SHELLEXECUTEINFO sei = { sizeof(sei) }; - sei.fMask = SEE_MASK_FLAG_DDEWAIT; +// Default ShellExecuteEx flags used with the "openas" verb. +// +// SEE_MASK_NOASYNC is specified so that ShellExecuteEx can be invoked from a +// thread whose message loop may not wait around long enough for the +// asynchronous tasks initiated by ShellExecuteEx to complete. Using this flag +// causes ShellExecuteEx() to block until these tasks complete. +const DWORD kDefaultOpenAsFlags = SEE_MASK_NOASYNC; + +// Default ShellExecuteEx flags used with the "explore", "open" or default verb. +// +// See kDefaultOpenFlags for description SEE_MASK_NOASYNC flag. +// SEE_MASK_FLAG_NO_UI is used to suppress any error message boxes that might be +// displayed if there is an error in opening the file. Failure in invoking the +// "open" actions result in invocation of the "saveas" verb, making the error +// dialog superfluous. +const DWORD kDefaultOpenFlags = SEE_MASK_NOASYNC | SEE_MASK_FLAG_NO_UI; + +// Invokes ShellExecuteExW() with the given parameters. +DWORD InvokeShellExecute(const base::string16 path, + const base::string16 working_directory, + const base::string16 args, + const base::string16 verb, + DWORD mask) { + base::ThreadRestrictions::AssertIOAllowed(); + SHELLEXECUTEINFO sei = {sizeof(sei)}; + sei.fMask = mask; sei.nShow = SW_SHOWNORMAL; - sei.lpVerb = L"openas"; - sei.lpFile = full_path.c_str(); - return (TRUE == ::ShellExecuteExW(&sei)); + sei.lpVerb = (verb.empty() ? nullptr : verb.c_str()); + sei.lpFile = path.c_str(); + sei.lpDirectory = + (working_directory.empty() ? nullptr : working_directory.c_str()); + sei.lpParameters = (args.empty() ? nullptr : args.c_str()); + return ::ShellExecuteExW(&sei) ? ERROR_SUCCESS : ::GetLastError(); } } // namespace @@ -42,24 +68,32 @@ bool OpenAnyViaShell(const base::string16& full_path, const base::string16& directory, const base::string16& args, DWORD mask) { - SHELLEXECUTEINFO sei = { sizeof(sei) }; - sei.fMask = mask; - sei.nShow = SW_SHOWNORMAL; - sei.lpFile = full_path.c_str(); - sei.lpDirectory = directory.c_str(); - if (!args.empty()) - sei.lpParameters = args.c_str(); - - if (::ShellExecuteExW(&sei)) + DWORD open_result = + InvokeShellExecute(full_path, directory, args, base::string16(), mask); + if (open_result == ERROR_SUCCESS) return true; - if (::GetLastError() == ERROR_NO_ASSOCIATION) - return OpenItemWithExternalApp(full_path); + // Show the Windows "Open With" dialog box to ask the user to pick an app to + // open the file with. Note that we are not forwarding |args| for the "openas" + // call since the target application is nolonger known at this point. + if (open_result == ERROR_NO_ASSOCIATION) + return InvokeShellExecute(full_path, directory, base::string16(), L"openas", + kDefaultOpenAsFlags) == ERROR_SUCCESS; return false; } -bool OpenItemViaShell(const base::FilePath& full_path) { +bool OpenFileViaShell(const base::FilePath& full_path) { return OpenAnyViaShell(full_path.value(), full_path.DirName().value(), - base::string16(), 0); + base::string16(), kDefaultOpenFlags); +} + +bool OpenFolderViaShell(const base::FilePath& full_path) { + // The "explore" verb causes the folder at |full_path| to be displayed in a + // file browser. This will fail if |full_path| is not a directory. The + // resulting error does not cause UI due to the SEE_MASK_FLAG_NO_UI flag in + // kDefaultOpenFlags. + return InvokeShellExecute(full_path.value(), full_path.value(), + base::string16(), L"explore", + kDefaultOpenFlags) == ERROR_SUCCESS; } bool PreventWindowFromPinning(HWND hwnd) { diff --git a/ui/base/win/shell.h b/ui/base/win/shell.h index 3b64ce7..4031de6 100644 --- a/ui/base/win/shell.h +++ b/ui/base/win/shell.h @@ -17,16 +17,29 @@ class FilePath; namespace ui { namespace win { -// 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. -// Returns 'true' on successful open, 'false' otherwise. -UI_BASE_EXPORT bool OpenItemViaShell(const base::FilePath& full_path); +// Open the folder at |full_path| via the Windows shell. Does nothing if +// |full_path| is not a folder. +// +// Note: Must be called on a thread that allows blocking. +UI_BASE_EXPORT bool OpenFolderViaShell(const base::FilePath& full_path); + +// Invokes the default verb on the file specified by |full_path| via the Windows +// shell. Usually, the default verb is "open" unless specified otherwise for the +// file type. +// +// In the event that there is no default application registered for the +// specified file, asks the user via the Windows "Open With" dialog. Returns +// |true| on success. +// +// Note: Must be called on a thread that allows blocking. +UI_BASE_EXPORT bool OpenFileViaShell(const base::FilePath& full_path); // Lower level function that allows opening of non-files like urls or GUIDs // don't use it if one of the above will do. |mask| is a valid combination -// of SEE_MASK_FLAG_XXX as stated in msdn. If there is no default application -// registered for the item, it behaves the same as OpenItemViaShell. +// of SEE_MASK_XXX as stated in MSDN. If there is no default application +// registered for the item, it behaves the same as OpenFileViaShell. +// +// Note: Must be called on a thread that allows blocking. UI_BASE_EXPORT bool OpenAnyViaShell(const base::string16& full_path, const base::string16& directory, const base::string16& args, |