diff options
author | erikwright@chromium.org <erikwright@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-08-14 17:14:50 +0000 |
---|---|---|
committer | erikwright@chromium.org <erikwright@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-08-14 17:14:50 +0000 |
commit | b041ed5038d77ca7a8daddafc9f6600c0f3e05fb (patch) | |
tree | 3e1f5c5a916af11a7fcb017d1e1912e9c5b9d636 | |
parent | 1e113d96b0f5b674f63e313c412dc1ee40bb8eba (diff) | |
download | chromium_src-b041ed5038d77ca7a8daddafc9f6600c0f3e05fb.zip chromium_src-b041ed5038d77ca7a8daddafc9f6600c0f3e05fb.tar.gz chromium_src-b041ed5038d77ca7a8daddafc9f6600c0f3e05fb.tar.bz2 |
Experimentally isolate GetOpenFileName in a utility process.
GetOpenFileName can cause 3rd-party shell extensions to be loaded into the caller's process. By putting it in a separate process we protect the browser process from potential instability.
BUG=73098
Review URL: https://codereview.chromium.org/419523006
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@289625 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r-- | chrome/browser/chrome_browser_main.cc | 5 | ||||
-rw-r--r-- | chrome/browser/chrome_select_file_dialog_factory_win.cc | 190 | ||||
-rw-r--r-- | chrome/browser/chrome_select_file_dialog_factory_win.h | 38 | ||||
-rw-r--r-- | chrome/chrome_browser.gypi | 2 | ||||
-rw-r--r-- | chrome/chrome_utility.gypi | 2 | ||||
-rw-r--r-- | chrome/common/chrome_utility_messages.h | 33 | ||||
-rw-r--r-- | chrome/utility/chrome_content_utility_client.cc | 8 | ||||
-rw-r--r-- | chrome/utility/shell_handler_win.cc | 51 | ||||
-rw-r--r-- | chrome/utility/shell_handler_win.h | 47 | ||||
-rw-r--r-- | ui/base/win/open_file_name_win.cc | 72 | ||||
-rw-r--r-- | ui/base/win/open_file_name_win.h | 19 | ||||
-rw-r--r-- | ui/base/win/open_file_name_win_unittest.cc | 119 | ||||
-rw-r--r-- | ui/shell_dialogs/select_file_dialog.cc | 2 | ||||
-rw-r--r-- | ui/shell_dialogs/select_file_dialog_win.cc | 57 | ||||
-rw-r--r-- | ui/shell_dialogs/select_file_dialog_win.h | 13 |
15 files changed, 615 insertions, 43 deletions
diff --git a/chrome/browser/chrome_browser_main.cc b/chrome/browser/chrome_browser_main.cc index 1b51179..5f49c00 100644 --- a/chrome/browser/chrome_browser_main.cc +++ b/chrome/browser/chrome_browser_main.cc @@ -165,6 +165,7 @@ #include "base/win/windows_version.h" #include "chrome/browser/browser_util_win.h" #include "chrome/browser/chrome_browser_main_win.h" +#include "chrome/browser/chrome_select_file_dialog_factory_win.h" #include "chrome/browser/component_updater/sw_reporter_installer_win.h" #include "chrome/browser/first_run/try_chrome_dialog_view.h" #include "chrome/browser/first_run/upgrade_util_win.h" @@ -175,6 +176,7 @@ #include "net/base/net_util.h" #include "ui/base/l10n/l10n_util_win.h" #include "ui/gfx/win/dpi.h" +#include "ui/shell_dialogs/select_file_dialog.h" #endif // defined(OS_WIN) #if defined(OS_MACOSX) @@ -1101,6 +1103,9 @@ int ChromeBrowserMainParts::PreMainMessageLoopRunImpl() { return ChromeBrowserMainPartsWin::HandleIconsCommands( parsed_command_line_); } + + ui::SelectFileDialog::SetFactory(new ChromeSelectFileDialogFactory( + BrowserThread::GetMessageLoopProxyForThread(BrowserThread::IO))); #endif if (parsed_command_line().HasSwitch(switches::kMakeDefaultBrowser)) { diff --git a/chrome/browser/chrome_select_file_dialog_factory_win.cc b/chrome/browser/chrome_select_file_dialog_factory_win.cc new file mode 100644 index 0000000..3522358 --- /dev/null +++ b/chrome/browser/chrome_select_file_dialog_factory_win.cc @@ -0,0 +1,190 @@ +// 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/chrome_select_file_dialog_factory_win.h" + +#include <Windows.h> +#include <commdlg.h> + +#include "base/bind.h" +#include "base/bind_helpers.h" +#include "base/callback.h" +#include "base/location.h" +#include "base/logging.h" +#include "base/metrics/field_trial.h" +#include "base/strings/string16.h" +#include "base/synchronization/waitable_event.h" +#include "base/win/metro.h" +#include "chrome/common/chrome_utility_messages.h" +#include "content/public/browser/utility_process_host.h" +#include "content/public/browser/utility_process_host_client.h" +#include "ipc/ipc_message_macros.h" +#include "ui/base/win/open_file_name_win.h" +#include "ui/shell_dialogs/select_file_dialog_win.h" + +namespace { + +// Receives the GetOpenFileName result from the utility process. +class GetOpenFileNameClient : public content::UtilityProcessHostClient { + public: + GetOpenFileNameClient(); + + // Blocks until the GetOpenFileName result is received (including failure to + // launch or a crash of the utility process). + void WaitForCompletion(); + + // Returns the selected directory. + const base::FilePath& directory() const { return directory_; } + + // Returns the list of selected filenames. Each should be interpreted as a + // child of directory(). + const std::vector<base::FilePath>& filenames() const { return filenames_; } + + // UtilityProcessHostClient implementation + virtual void OnProcessCrashed(int exit_code) OVERRIDE; + virtual void OnProcessLaunchFailed() OVERRIDE; + virtual bool OnMessageReceived(const IPC::Message& message) OVERRIDE; + + protected: + virtual ~GetOpenFileNameClient(); + + private: + void OnResult(const base::FilePath& directory, + const std::vector<base::FilePath>& filenames); + void OnFailure(); + + base::FilePath directory_; + std::vector<base::FilePath> filenames_; + base::WaitableEvent event_; + + DISALLOW_COPY_AND_ASSIGN(GetOpenFileNameClient); +}; + +GetOpenFileNameClient::GetOpenFileNameClient() : event_(true, false) { +} + +void GetOpenFileNameClient::WaitForCompletion() { + event_.Wait(); +} + +void GetOpenFileNameClient::OnProcessCrashed(int exit_code) { + event_.Signal(); +} + +void GetOpenFileNameClient::OnProcessLaunchFailed() { + event_.Signal(); +} + +bool GetOpenFileNameClient::OnMessageReceived(const IPC::Message& message) { + bool handled = true; + IPC_BEGIN_MESSAGE_MAP(GetOpenFileNameClient, message) + IPC_MESSAGE_HANDLER(ChromeUtilityHostMsg_GetOpenFileName_Failed, + OnFailure) + IPC_MESSAGE_HANDLER(ChromeUtilityHostMsg_GetOpenFileName_Result, + OnResult) + IPC_MESSAGE_UNHANDLED(handled = false) + IPC_END_MESSAGE_MAP() + return handled; +} + +GetOpenFileNameClient::~GetOpenFileNameClient() {} + +void GetOpenFileNameClient::OnResult( + const base::FilePath& directory, + const std::vector<base::FilePath>& filenames) { + directory_ = directory; + filenames_ = filenames; + event_.Signal(); +} + +void GetOpenFileNameClient::OnFailure() { + event_.Signal(); +} + +// Initiates IPC with a new utility process using |client|. Instructs the +// utility process to call GetOpenFileName with |ofn|. |current_task_runner| +// must be the currently executing task runner. +void DoInvokeGetOpenFileName( + OPENFILENAME* ofn, + scoped_refptr<GetOpenFileNameClient> client, + const scoped_refptr<base::SequencedTaskRunner>& current_task_runner) { + DCHECK(current_task_runner->RunsTasksOnCurrentThread()); + + base::WeakPtr<content::UtilityProcessHost> utility_process_host( + content::UtilityProcessHost::Create(client, current_task_runner) + ->AsWeakPtr()); + utility_process_host->DisableSandbox(); + utility_process_host->Send(new ChromeUtilityMsg_GetOpenFileName( + ofn->hwndOwner, + ofn->Flags, + ui::win::OpenFileName::GetFilters(ofn), + base::FilePath(ofn->lpstrInitialDir ? ofn->lpstrInitialDir + : base::string16()), + base::FilePath(ofn->lpstrFile))); +} + +// Invokes GetOpenFileName in a utility process. Blocks until the result is +// received. Uses |blocking_task_runner| for IPC. +bool GetOpenFileNameInUtilityProcess( + const scoped_refptr<base::SequencedTaskRunner>& blocking_task_runner, + OPENFILENAME* ofn) { + scoped_refptr<GetOpenFileNameClient> client(new GetOpenFileNameClient); + blocking_task_runner->PostTask( + FROM_HERE, + base::Bind(&DoInvokeGetOpenFileName, + base::Unretained(ofn), client, blocking_task_runner)); + client->WaitForCompletion(); + + if (!client->filenames().size()) + return false; + + ui::win::OpenFileName::SetResult( + client->directory(), client->filenames(), ofn); + return true; +} + +// Implements GetOpenFileName for CreateWinSelectFileDialog by delegating either +// to Metro or a utility process. +bool GetOpenFileNameImpl( + const scoped_refptr<base::SequencedTaskRunner>& blocking_task_runner, + OPENFILENAME* ofn) { + HMODULE metro_module = base::win::GetMetroModule(); + if (metro_module != NULL) { + typedef BOOL (*MetroGetOpenFileName)(OPENFILENAME*); + MetroGetOpenFileName metro_get_open_file_name = + reinterpret_cast<MetroGetOpenFileName>( + ::GetProcAddress(metro_module, "MetroGetOpenFileName")); + if (metro_get_open_file_name == NULL) { + NOTREACHED(); + return false; + } + + return metro_get_open_file_name(ofn) == TRUE; + } + + if (base::FieldTrialList::FindFullName("IsolateShellOperations") == + "Enabled") { + return GetOpenFileNameInUtilityProcess(blocking_task_runner, ofn); + } + + return ::GetOpenFileName(ofn) == TRUE; +} + +} // namespace + +ChromeSelectFileDialogFactory::ChromeSelectFileDialogFactory( + const scoped_refptr<base::SequencedTaskRunner>& blocking_task_runner) + : blocking_task_runner_(blocking_task_runner) { +} + +ChromeSelectFileDialogFactory::~ChromeSelectFileDialogFactory() {} + +ui::SelectFileDialog* ChromeSelectFileDialogFactory::Create( + ui::SelectFileDialog::Listener* listener, + ui::SelectFilePolicy* policy) { + return ui::CreateWinSelectFileDialog( + listener, + policy, + base::Bind(GetOpenFileNameImpl, blocking_task_runner_)); +} diff --git a/chrome/browser/chrome_select_file_dialog_factory_win.h b/chrome/browser/chrome_select_file_dialog_factory_win.h new file mode 100644 index 0000000..bcbeaba --- /dev/null +++ b/chrome/browser/chrome_select_file_dialog_factory_win.h @@ -0,0 +1,38 @@ +// 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. + +#ifndef CHROME_BROWSER_CHROME_SELECT_FILE_DIALOG_FACTORY_WIN_H_ +#define CHROME_BROWSER_CHROME_SELECT_FILE_DIALOG_FACTORY_WIN_H_ + +#include "base/compiler_specific.h" +#include "base/macros.h" +#include "base/memory/ref_counted.h" +#include "ui/shell_dialogs/select_file_dialog_factory.h" + +namespace base { +class SequencedTaskRunner; +} // namespace base + +// Implements a Select File dialog that delegates to a Metro file picker on +// Metro and to a utility process otherwise. The utility process is used in +// order to isolate the Chrome browser process from potential instability +// caused by Shell extension modules loaded by GetOpenFileName. +class ChromeSelectFileDialogFactory : public ui::SelectFileDialogFactory { + public: + // Uses |blocking_task_runner| to perform IPC with the utility process. + explicit ChromeSelectFileDialogFactory( + const scoped_refptr<base::SequencedTaskRunner>& blocking_task_runner); + virtual ~ChromeSelectFileDialogFactory(); + + // ui::SelectFileDialogFactory implementation + virtual ui::SelectFileDialog* Create(ui::SelectFileDialog::Listener* listener, + ui::SelectFilePolicy* policy) OVERRIDE; + + private: + scoped_refptr<base::SequencedTaskRunner> blocking_task_runner_; + + DISALLOW_COPY_AND_ASSIGN(ChromeSelectFileDialogFactory); +}; + +#endif // CHROME_BROWSER_CHROME_SELECT_FILE_DIALOG_FACTORY_WIN_H_ diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi index c68e650..0921841f 100644 --- a/chrome/chrome_browser.gypi +++ b/chrome/chrome_browser.gypi @@ -260,6 +260,8 @@ 'browser/chrome_page_zoom.h', 'browser/chrome_page_zoom_constants.cc', 'browser/chrome_page_zoom_constants.h', + 'browser/chrome_select_file_dialog_factory_win.cc', + 'browser/chrome_select_file_dialog_factory_win.h', 'browser/chrome_quota_permission_context.cc', 'browser/chrome_quota_permission_context.h', 'browser/command_observer.h', diff --git a/chrome/chrome_utility.gypi b/chrome/chrome_utility.gypi index fddb0df..bedb1fa 100644 --- a/chrome/chrome_utility.gypi +++ b/chrome/chrome_utility.gypi @@ -17,6 +17,8 @@ 'utility/local_discovery/service_discovery_message_handler.h', 'utility/printing_handler.cc', 'utility/printing_handler.h', + 'utility/shell_handler_win.cc', + 'utility/shell_handler_win.h', 'utility/utility_message_handler.h', 'utility/web_resource_unpacker.cc', 'utility/web_resource_unpacker.h', diff --git a/chrome/common/chrome_utility_messages.h b/chrome/common/chrome_utility_messages.h index 9b66589..b2216c3 100644 --- a/chrome/common/chrome_utility_messages.h +++ b/chrome/common/chrome_utility_messages.h @@ -4,6 +4,10 @@ // Multiply-included message file, so no include guard. +#if defined(OS_WIN) +#include <Windows.h> +#endif // defined(OS_WIN) + #include <string> #include <vector> @@ -86,6 +90,28 @@ IPC_MESSAGE_CONTROL1(ChromeUtilityMsg_AnalyzeZipFileForDownloadProtection, IPC::PlatformFileForTransit /* zip_file */) #endif +#if defined(OS_WIN) +// A vector of filters, each being a Tuple2a display string (i.e. "Text Files") +// and a filter pattern (i.e. "*.txt").. +typedef std::vector<Tuple2<base::string16, base::string16> > + GetOpenFileNameFilter; + +// Instructs the utility process to invoke GetOpenFileName. |owner| is the +// parent of the modal dialog, |flags| are OFN_* flags. |filter| constrains the +// user's file choices. |initial_directory| and |filename| select the directory +// to be displayed and the file to be initially selected. +// +// Either ChromeUtilityHostMsg_GetOpenFileName_Failed or +// ChromeUtilityHostMsg_GetOpenFileName_Result will be returned when the +// operation completes whether due to error or user action. +IPC_MESSAGE_CONTROL5(ChromeUtilityMsg_GetOpenFileName, + HWND /* owner */, + DWORD /* flags */, + GetOpenFileNameFilter /* filter */, + base::FilePath /* initial_directory */, + base::FilePath /* filename */) +#endif // defined(OS_WIN) + //------------------------------------------------------------------------------ // Utility process host messages: // These are messages from the utility process to the browser. @@ -128,3 +154,10 @@ IPC_MESSAGE_CONTROL1( ChromeUtilityHostMsg_AnalyzeZipFileForDownloadProtection_Finished, safe_browsing::zip_analyzer::Results) #endif + +#if defined(OS_WIN) +IPC_MESSAGE_CONTROL0(ChromeUtilityHostMsg_GetOpenFileName_Failed) +IPC_MESSAGE_CONTROL2(ChromeUtilityHostMsg_GetOpenFileName_Result, + base::FilePath /* directory */, + std::vector<base::FilePath> /* filenames */) +#endif // defined(OS_WIN) diff --git a/chrome/utility/chrome_content_utility_client.cc b/chrome/utility/chrome_content_utility_client.cc index 6225143..e2f9c45 100644 --- a/chrome/utility/chrome_content_utility_client.cc +++ b/chrome/utility/chrome_content_utility_client.cc @@ -27,6 +27,10 @@ #include "chrome/utility/profile_import_handler.h" #endif +#if defined(OS_WIN) +#include "chrome/utility/shell_handler_win.h" +#endif + #if defined(ENABLE_EXTENSIONS) #include "chrome/common/extensions/chrome_utility_extensions_messages.h" #include "chrome/utility/extensions/extensions_handler.h" @@ -87,6 +91,10 @@ ChromeContentUtilityClient::ChromeContentUtilityClient() handlers_.push_back(new local_discovery::ServiceDiscoveryMessageHandler()); } #endif + +#if defined(OS_WIN) + handlers_.push_back(new ShellHandler()); +#endif } ChromeContentUtilityClient::~ChromeContentUtilityClient() { diff --git a/chrome/utility/shell_handler_win.cc b/chrome/utility/shell_handler_win.cc new file mode 100644 index 0000000..7337f9a --- /dev/null +++ b/chrome/utility/shell_handler_win.cc @@ -0,0 +1,51 @@ +// 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/utility/shell_handler_win.h" + +#include <commdlg.h> + +#include "base/files/file_path.h" +#include "base/memory/scoped_ptr.h" +#include "chrome/common/chrome_utility_messages.h" +#include "content/public/utility/utility_thread.h" +#include "ui/base/win/open_file_name_win.h" + +ShellHandler::ShellHandler() {} +ShellHandler::~ShellHandler() {} + +bool ShellHandler::OnMessageReceived(const IPC::Message& message) { + bool handled = true; + IPC_BEGIN_MESSAGE_MAP(ShellHandler, message) + IPC_MESSAGE_HANDLER(ChromeUtilityMsg_GetOpenFileName, + OnGetOpenFileName) + IPC_MESSAGE_UNHANDLED(handled = false) + IPC_END_MESSAGE_MAP() + return handled; +} + +void ShellHandler::OnGetOpenFileName( + HWND owner, + DWORD flags, + const GetOpenFileNameFilter& filter, + const base::FilePath& initial_directory, + const base::FilePath& filename) { + ui::win::OpenFileName open_file_name(owner, flags); + open_file_name.SetInitialSelection(initial_directory, filename); + open_file_name.SetFilters(filter); + + base::FilePath directory; + std::vector<base::FilePath> filenames; + + if (::GetOpenFileName(open_file_name.GetOPENFILENAME())) + open_file_name.GetResult(&directory, &filenames); + + if (filenames.size()) { + content::UtilityThread::Get()->Send( + new ChromeUtilityHostMsg_GetOpenFileName_Result(directory, filenames)); + } else { + content::UtilityThread::Get()->Send( + new ChromeUtilityHostMsg_GetOpenFileName_Failed()); + } +} diff --git a/chrome/utility/shell_handler_win.h b/chrome/utility/shell_handler_win.h new file mode 100644 index 0000000..f91124c --- /dev/null +++ b/chrome/utility/shell_handler_win.h @@ -0,0 +1,47 @@ +// 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. + +#ifndef CHROME_UTILITY_SHELL_HANDLER_WIN_H_ +#define CHROME_UTILITY_SHELL_HANDLER_WIN_H_ + +#include <Windows.h> + +#include <vector> + +#include "base/compiler_specific.h" +#include "base/macros.h" +#include "base/strings/string16.h" +#include "base/tuple.h" +#include "chrome/utility/utility_message_handler.h" + +namespace base { +class FilePath; +} // namespace base + +typedef std::vector<Tuple2<base::string16, base::string16> > + GetOpenFileNameFilter; + +// Handles requests to execute shell operations. Used to protect the browser +// process from instability due to 3rd-party shell extensions. Must be invoked +// in a non-sandboxed utility process. +class ShellHandler : public UtilityMessageHandler { + public: + ShellHandler(); + virtual ~ShellHandler(); + + // IPC::Listener implementation + virtual bool OnMessageReceived(const IPC::Message& message) OVERRIDE; + + private: + void OnGetOpenFileName( + HWND owner, + DWORD flags, + const GetOpenFileNameFilter& filter, + const base::FilePath& initial_directory, + const base::FilePath& filename); + + DISALLOW_COPY_AND_ASSIGN(ShellHandler); +}; + +#endif // CHROME_UTILITY_SHELL_HANDLER_WIN_H_ diff --git a/ui/base/win/open_file_name_win.cc b/ui/base/win/open_file_name_win.cc index 2d67b1f..b63eb47 100644 --- a/ui/base/win/open_file_name_win.cc +++ b/ui/base/win/open_file_name_win.cc @@ -27,6 +27,25 @@ OpenFileName::OpenFileName(HWND parent_window, DWORD flags) { OpenFileName::~OpenFileName() { } +void OpenFileName::SetFilters( + const std::vector<Tuple2<base::string16, base::string16> >& filters) { + openfilename_.lpstrFilter = NULL; + filter_buffer_.clear(); + if (filters.empty()) + return; + for (std::vector<Tuple2<base::string16, base::string16> >::const_iterator + it = filters.begin(); + it != filters.end(); + ++it) { + filter_buffer_.append(it->a); + filter_buffer_.push_back(0); + filter_buffer_.append(it->b); + filter_buffer_.push_back(0); + } + filter_buffer_.push_back(0); + openfilename_.lpstrFilter = filter_buffer_.c_str(); +} + void OpenFileName::SetInitialSelection(const base::FilePath& initial_directory, const base::FilePath& initial_filename) { // First reset to the default case. @@ -83,5 +102,58 @@ void OpenFileName::GetResult(base::FilePath* directory, } } +// static +void OpenFileName::SetResult(const base::FilePath& directory, + const std::vector<base::FilePath>& filenames, + OPENFILENAME* openfilename) { + base::string16 filename_value; + if (filenames.size() == 1) { + filename_value = directory.Append(filenames[0]).value(); + } else { + filename_value = directory.value(); + filename_value.push_back(0); + for (std::vector<base::FilePath>::const_iterator it = filenames.begin(); + it != filenames.end(); + ++it) { + filename_value.append(it->value()); + filename_value.push_back(0); + } + } + if (filename_value.size() + 1 < openfilename->nMaxFile) { + // Because the result has embedded nulls, we must memcpy. + memcpy(openfilename->lpstrFile, + filename_value.c_str(), + (filename_value.size() + 1) * sizeof(filename_value[0])); + } else if (openfilename->nMaxFile) { + openfilename->lpstrFile[0] = 0; + } +} + +// static +std::vector<Tuple2<base::string16, base::string16> > OpenFileName::GetFilters( + const OPENFILENAME* openfilename) { + std::vector<Tuple2<base::string16, base::string16> > filters; + + const base::char16* display_string = openfilename->lpstrFilter; + if (!display_string) + return filters; + + while (*display_string) { + const base::char16* display_string_end = display_string; + while (*display_string_end) + ++display_string_end; + const base::char16* pattern = display_string_end + 1; + const base::char16* pattern_end = pattern; + while (*pattern_end) + ++pattern_end; + filters.push_back( + MakeTuple(base::string16(display_string, display_string_end), + base::string16(pattern, pattern_end))); + display_string = pattern_end + 1; + } + + return filters; +} + } // namespace win } // namespace ui diff --git a/ui/base/win/open_file_name_win.h b/ui/base/win/open_file_name_win.h index 4787c83..79a7185 100644 --- a/ui/base/win/open_file_name_win.h +++ b/ui/base/win/open_file_name_win.h @@ -32,6 +32,10 @@ class UI_BASE_EXPORT OpenFileName { OpenFileName(HWND parent_window, DWORD flags); ~OpenFileName(); + // Initializes |lpstrFilter| from the label/pattern pairs in |filters|. + void SetFilters( + const std::vector<Tuple2<base::string16, base::string16> >& filters); + // Sets |lpstrInitialDir| and |lpstrFile|. void SetInitialSelection(const base::FilePath& initial_directory, const base::FilePath& initial_filename); @@ -47,10 +51,25 @@ class UI_BASE_EXPORT OpenFileName { // Returns the OPENFILENAME structure. OPENFILENAME* GetOPENFILENAME() { return &openfilename_; } + // Returns the OPENFILENAME structure. + const OPENFILENAME* GetOPENFILENAME() const { return &openfilename_; } + + // Stores directory and filenames in the buffer pointed to by + // |openfilename->lpstrFile| and sized |openfilename->nMaxFile|. + static void SetResult(const base::FilePath& directory, + const std::vector<base::FilePath>& filenames, + OPENFILENAME* openfilename); + + // Returns a vector of label/pattern pairs built from + // |openfilename->lpstrFilter|. + static std::vector<Tuple2<base::string16, base::string16> > GetFilters( + const OPENFILENAME* openfilename); + private: OPENFILENAME openfilename_; base::string16 initial_directory_buffer_; wchar_t filename_buffer_[UNICODE_STRING_MAX_CHARS]; + base::string16 filter_buffer_; DISALLOW_COPY_AND_ASSIGN(OpenFileName); }; diff --git a/ui/base/win/open_file_name_win_unittest.cc b/ui/base/win/open_file_name_win_unittest.cc index a38e4d6..f69ec97 100644 --- a/ui/base/win/open_file_name_win_unittest.cc +++ b/ui/base/win/open_file_name_win_unittest.cc @@ -12,16 +12,64 @@ const HWND kHwnd = reinterpret_cast<HWND>(0xDEADBEEF); const DWORD kFlags = OFN_OVERWRITEPROMPT | OFN_EXPLORER | OFN_ENABLESIZING; void SetResult(const base::string16& result, ui::win::OpenFileName* ofn) { - EXPECT_GT(ofn->GetOPENFILENAME()->nMaxFile, result.size()); - if (ofn->GetOPENFILENAME()->nMaxFile > result.size()) { - if (!result.size()) { - ofn->GetOPENFILENAME()->lpstrFile[0] = 0; - } else { - // Because the result has embedded nulls, we must memcpy. - memcpy(ofn->GetOPENFILENAME()->lpstrFile, - result.c_str(), - (result.size() + 1) * sizeof(result[0])); - } + if (ofn->GetOPENFILENAME()->nMaxFile <= result.size()) { + ADD_FAILURE() << "filename buffer insufficient."; + return; + } + if (!result.size()) { + ofn->GetOPENFILENAME()->lpstrFile[0] = 0; + } else { + // Because the result has embedded nulls, we must memcpy. + memcpy(ofn->GetOPENFILENAME()->lpstrFile, + result.c_str(), + (result.size() + 1) * sizeof(result[0])); + } +} + +void CheckFilters( + const std::vector<Tuple2<base::string16, base::string16> >& expected, + const std::vector<Tuple2<base::string16, base::string16> >& actual) { + if (expected.size() != actual.size()) { + ADD_FAILURE() << "filter count mismatch. Got " << actual.size() + << " expected " << expected.size() << "."; + return; + } + + for (size_t i = 0; i < expected.size(); ++i) { + EXPECT_EQ(expected[i].a, actual[i].a) << "Mismatch at index " << i; + EXPECT_EQ(expected[i].b, actual[i].b) << "Mismatch at index " << i; + } +} + +void CheckFilterString(const base::string16& expected, + const ui::win::OpenFileName& ofn) { + if (!ofn.GetOPENFILENAME()->lpstrFilter) { + ADD_FAILURE() << "Filter string is NULL."; + return; + } + if (expected.size() == 0) { + EXPECT_EQ(0, ofn.GetOPENFILENAME()->lpstrFilter[0]); + } else { + EXPECT_EQ(0, + memcmp(expected.c_str(), + ofn.GetOPENFILENAME()->lpstrFilter, + expected.size() + 1 * sizeof(expected[0]))); + } +} + +void CheckResult(const base::string16& expected, + const ui::win::OpenFileName& ofn) { + if (!ofn.GetOPENFILENAME()->lpstrFile) { + ADD_FAILURE() << "File string is NULL."; + return; + } + if (expected.size() == 0) { + EXPECT_EQ(0, ofn.GetOPENFILENAME()->lpstrFile[0]); + } else { + EXPECT_EQ(0, + memcmp(expected.c_str(), + ofn.GetOPENFILENAME()->lpstrFile, + expected.size() + 1 * sizeof(expected[0]))); } } @@ -135,3 +183,54 @@ TEST(OpenFileNameTest, GetResult) { EXPECT_EQ(base::FilePath(), directory); ASSERT_EQ(0u, filenames.size()); } + +TEST(OpenFileNameTest, SetAndGetFilters) { + const base::string16 kNull(L"\0", 1); + + ui::win::OpenFileName ofn(kHwnd, kFlags); + std::vector<Tuple2<base::string16, base::string16> > filters; + ofn.SetFilters(filters); + EXPECT_FALSE(ofn.GetOPENFILENAME()->lpstrFilter); + CheckFilters(filters, + ui::win::OpenFileName::GetFilters(ofn.GetOPENFILENAME())); + + filters.push_back(MakeTuple(base::string16(L"a"), base::string16(L"b"))); + ofn.SetFilters(filters); + CheckFilterString(L"a" + kNull + L"b" + kNull, ofn); + CheckFilters(filters, + ui::win::OpenFileName::GetFilters(ofn.GetOPENFILENAME())); + + filters.push_back(MakeTuple(base::string16(L"X"), base::string16(L"Y"))); + ofn.SetFilters(filters); + CheckFilterString(L"a" + kNull + L"b" + kNull + L"X" + kNull + L"Y" + kNull, + ofn); + CheckFilters(filters, + ui::win::OpenFileName::GetFilters(ofn.GetOPENFILENAME())); +} + +TEST(OpenFileNameTest, SetResult) { + const base::string16 kNull(L"\0", 1); + + ui::win::OpenFileName ofn(kHwnd, kFlags); + base::FilePath directory; + std::vector<base::FilePath> filenames; + + ui::win::OpenFileName::SetResult(directory, filenames, ofn.GetOPENFILENAME()); + CheckResult(L"", ofn); + + directory = base::FilePath(L"C:\\dir"); + filenames.push_back(base::FilePath(L"file")); + ui::win::OpenFileName::SetResult(directory, filenames, ofn.GetOPENFILENAME()); + CheckResult(L"C:\\dir\\file" + kNull, ofn); + + filenames.push_back(base::FilePath(L"otherfile")); + ui::win::OpenFileName::SetResult(directory, filenames, ofn.GetOPENFILENAME()); + CheckResult(L"C:\\dir" + kNull + L"file" + kNull + L"otherfile" + kNull, ofn); + + base::char16 short_buffer[10] = L""; + + ofn.GetOPENFILENAME()->lpstrFile = short_buffer; + ofn.GetOPENFILENAME()->nMaxFile = arraysize(short_buffer); + ui::win::OpenFileName::SetResult(directory, filenames, ofn.GetOPENFILENAME()); + CheckResult(L"", ofn); +} diff --git a/ui/shell_dialogs/select_file_dialog.cc b/ui/shell_dialogs/select_file_dialog.cc index 13d3ba9..e5abd07 100644 --- a/ui/shell_dialogs/select_file_dialog.cc +++ b/ui/shell_dialogs/select_file_dialog.cc @@ -84,7 +84,7 @@ scoped_refptr<SelectFileDialog> SelectFileDialog::Create( #if defined(OS_WIN) // TODO(ananta) // Fix this for Chrome ASH on Windows. - return CreateWinSelectFileDialog(listener, policy); + return CreateDefaultWinSelectFileDialog(listener, policy); #elif defined(OS_MACOSX) && !defined(USE_AURA) return CreateMacSelectFileDialog(listener, policy); #elif defined(OS_ANDROID) diff --git a/ui/shell_dialogs/select_file_dialog_win.cc b/ui/shell_dialogs/select_file_dialog_win.cc index 73aacf6..0fccc78 100644 --- a/ui/shell_dialogs/select_file_dialog_win.cc +++ b/ui/shell_dialogs/select_file_dialog_win.cc @@ -4,8 +4,6 @@ #include "ui/shell_dialogs/select_file_dialog_win.h" -#include <windows.h> -#include <commdlg.h> #include <shlobj.h> #include <algorithm> @@ -38,6 +36,10 @@ namespace { +bool CallBuiltinGetOpenFileName(OPENFILENAME* ofn) { + return ::GetOpenFileName(ofn) == TRUE; +} + // Given |extension|, if it's not empty, then remove the leading dot. std::wstring GetExtensionWithoutLeadingDot(const std::wstring& extension) { DCHECK(extension.empty() || extension[0] == L'.'); @@ -45,25 +47,6 @@ std::wstring GetExtensionWithoutLeadingDot(const std::wstring& extension) { } // Diverts to a metro-specific implementation as appropriate. -bool CallGetOpenFileName(OPENFILENAME* ofn) { - HMODULE metro_module = base::win::GetMetroModule(); - if (metro_module != NULL) { - typedef BOOL (*MetroGetOpenFileName)(OPENFILENAME*); - MetroGetOpenFileName metro_get_open_file_name = - reinterpret_cast<MetroGetOpenFileName>( - ::GetProcAddress(metro_module, "MetroGetOpenFileName")); - if (metro_get_open_file_name == NULL) { - NOTREACHED(); - return false; - } - - return metro_get_open_file_name(ofn) == TRUE; - } else { - return GetOpenFileName(ofn) == TRUE; - } -} - -// Diverts to a metro-specific implementation as appropriate. bool CallGetSaveFileName(OPENFILENAME* ofn) { HMODULE metro_module = base::win::GetMetroModule(); if (metro_module != NULL) { @@ -410,8 +393,10 @@ bool SaveFileAs(HWND owner, class SelectFileDialogImpl : public ui::SelectFileDialog, public ui::BaseShellDialogImpl { public: - explicit SelectFileDialogImpl(Listener* listener, - ui::SelectFilePolicy* policy); + SelectFileDialogImpl( + Listener* listener, + ui::SelectFilePolicy* policy, + const base::Callback<bool(OPENFILENAME*)>& get_open_file_name_impl); // BaseShellDialog implementation: virtual bool IsRunning(gfx::NativeWindow owning_window) const OVERRIDE; @@ -520,15 +505,19 @@ class SelectFileDialogImpl : public ui::SelectFileDialog, base::string16 GetFilterForFileTypes(const FileTypeInfo* file_types); bool has_multiple_file_type_choices_; + base::Callback<bool(OPENFILENAME*)> get_open_file_name_impl_; DISALLOW_COPY_AND_ASSIGN(SelectFileDialogImpl); }; -SelectFileDialogImpl::SelectFileDialogImpl(Listener* listener, - ui::SelectFilePolicy* policy) +SelectFileDialogImpl::SelectFileDialogImpl( + Listener* listener, + ui::SelectFilePolicy* policy, + const base::Callback<bool(OPENFILENAME*)>& get_open_file_name_impl) : SelectFileDialog(listener, policy), BaseShellDialogImpl(), - has_multiple_file_type_choices_(false) { + has_multiple_file_type_choices_(false), + get_open_file_name_impl_(get_open_file_name_impl) { } SelectFileDialogImpl::~SelectFileDialogImpl() { @@ -786,7 +775,8 @@ bool SelectFileDialogImpl::RunOpenFileDialog( if (!filter.empty()) ofn.GetOPENFILENAME()->lpstrFilter = filter.c_str(); - bool success = CallGetOpenFileName(ofn.GetOPENFILENAME()); + + bool success = get_open_file_name_impl_.Run(ofn.GetOPENFILENAME()); DisableOwner(owner); if (success) *path = ofn.GetSingleResult(); @@ -811,8 +801,9 @@ bool SelectFileDialogImpl::RunOpenMultiFileDialog( base::FilePath directory; std::vector<base::FilePath> filenames; - if (CallGetOpenFileName(ofn.GetOPENFILENAME())) + if (get_open_file_name_impl_.Run(ofn.GetOPENFILENAME())) ofn.GetResult(&directory, &filenames); + DisableOwner(owner); for (std::vector<base::FilePath>::iterator it = filenames.begin(); @@ -894,8 +885,16 @@ std::wstring AppendExtensionIfNeeded( SelectFileDialog* CreateWinSelectFileDialog( SelectFileDialog::Listener* listener, + SelectFilePolicy* policy, + const base::Callback<bool(OPENFILENAME* ofn)>& get_open_file_name_impl) { + return new SelectFileDialogImpl(listener, policy, get_open_file_name_impl); +} + +SelectFileDialog* CreateDefaultWinSelectFileDialog( + SelectFileDialog::Listener* listener, SelectFilePolicy* policy) { - return new SelectFileDialogImpl(listener, policy); + return CreateWinSelectFileDialog( + listener, policy, base::Bind(&CallBuiltinGetOpenFileName)); } } // namespace ui diff --git a/ui/shell_dialogs/select_file_dialog_win.h b/ui/shell_dialogs/select_file_dialog_win.h index 04025c9..d8035c5 100644 --- a/ui/shell_dialogs/select_file_dialog_win.h +++ b/ui/shell_dialogs/select_file_dialog_win.h @@ -5,6 +5,10 @@ #ifndef UI_SHELL_DIALOGS_SELECT_FILE_DIALOG_WIN_H_ #define UI_SHELL_DIALOGS_SELECT_FILE_DIALOG_WIN_H_ +#include <Windows.h> +#include <commdlg.h> + +#include "base/callback_forward.h" #include "ui/gfx/native_widget_types.h" #include "ui/shell_dialogs/select_file_dialog.h" #include "ui/shell_dialogs/shell_dialogs_export.h" @@ -18,9 +22,12 @@ SHELL_DIALOGS_EXPORT std::wstring AppendExtensionIfNeeded( const std::wstring& filter_selected, const std::wstring& suggested_ext); -// Intentionally not exported. Implementation detail of SelectFileDialog. Use -// SelectFileDialog::Create() instead. -SelectFileDialog* CreateWinSelectFileDialog( +SHELL_DIALOGS_EXPORT SelectFileDialog* CreateWinSelectFileDialog( + SelectFileDialog::Listener* listener, + SelectFilePolicy* policy, + const base::Callback<bool(OPENFILENAME* ofn)>& get_open_file_name_impl); + +SelectFileDialog* CreateDefaultWinSelectFileDialog( SelectFileDialog::Listener* listener, SelectFilePolicy* policy); |