Experimentally isolate GetSaveFileName in a utility process.

Shell operations may cause 3rd-party shell extensions to be loaded into the calling process. Isolating them in a utility process protects the browser process from potential instability.

BUG=73098

Review URL: https://codereview.chromium.org/487453002

Cr-Commit-Position: refs/heads/master@{#291416}
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@291416 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/chrome_select_file_dialog_factory_win.cc

@@ -25,6 +25,29 @@
 
 namespace {
 
+bool CallMetroOPENFILENAMEMethod(const char* method_name, OPENFILENAME* ofn) {
+  typedef BOOL (*MetroOPENFILENAMEMethod)(OPENFILENAME*);
+  MetroOPENFILENAMEMethod metro_method = NULL;
+  HMODULE metro_module = base::win::GetMetroModule();
+
+  if (metro_module != NULL) {
+    metro_method = reinterpret_cast<MetroOPENFILENAMEMethod>(
+        ::GetProcAddress(metro_module, method_name));
+  }
+
+  if (metro_method != NULL)
+    return metro_method(ofn) == TRUE;
+
+  NOTREACHED();
+
+  return false;
+}
+
+bool ShouldIsolateShellOperations() {
+  return base::FieldTrialList::FindFullName("IsolateShellOperations") ==
+         "Enabled";
+}
+
 // Receives the GetOpenFileName result from the utility process.
 class GetOpenFileNameClient : public content::UtilityProcessHostClient {
  public:
@@ -117,7 +140,7 @@ void DoInvokeGetOpenFileName(
   utility_process_host->DisableSandbox();
   utility_process_host->Send(new ChromeUtilityMsg_GetOpenFileName(
       ofn->hwndOwner,
-      ofn->Flags,
+      ofn->Flags & ~OFN_ENABLEHOOK,  // We can't send a hook function over IPC.
       ui::win::OpenFileName::GetFilters(ofn),
       base::FilePath(ofn->lpstrInitialDir ? ofn->lpstrInitialDir
                                           : base::string16()),
@@ -149,28 +172,152 @@ bool GetOpenFileNameInUtilityProcess(
 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::win::IsMetroProcess())
+    return CallMetroOPENFILENAMEMethod("MetroGetOpenFileName", ofn);
 
-  if (base::FieldTrialList::FindFullName("IsolateShellOperations") ==
-      "Enabled") {
+  if (ShouldIsolateShellOperations())
     return GetOpenFileNameInUtilityProcess(blocking_task_runner, ofn);
-  }
 
   return ::GetOpenFileName(ofn) == TRUE;
 }
 
+class GetSaveFileNameClient : public content::UtilityProcessHostClient {
+ public:
+  GetSaveFileNameClient();
+
+  // Blocks until the GetSaveFileName result is received (including failure to
+  // launch or a crash of the utility process).
+  void WaitForCompletion();
+
+  // Returns the selected path.
+  const base::FilePath& path() const { return path_; }
+
+  // Returns the index of the user-selected filter.
+  int one_based_filter_index() const { return one_based_filter_index_; }
+
+  // UtilityProcessHostClient implementation
+  virtual void OnProcessCrashed(int exit_code) OVERRIDE;
+  virtual void OnProcessLaunchFailed() OVERRIDE;
+  virtual bool OnMessageReceived(const IPC::Message& message) OVERRIDE;
+
+ protected:
+  virtual ~GetSaveFileNameClient();
+
+ private:
+  void OnResult(const base::FilePath& path, int one_based_filter_index);
+  void OnFailure();
+
+  base::FilePath path_;
+  int one_based_filter_index_;
+  base::WaitableEvent event_;
+
+  DISALLOW_COPY_AND_ASSIGN(GetSaveFileNameClient);
+};
+
+GetSaveFileNameClient::GetSaveFileNameClient()
+    : event_(true, false), one_based_filter_index_(0) {
+}
+
+void GetSaveFileNameClient::WaitForCompletion() {
+  event_.Wait();
+}
+
+void GetSaveFileNameClient::OnProcessCrashed(int exit_code) {
+  event_.Signal();
+}
+
+void GetSaveFileNameClient::OnProcessLaunchFailed() {
+  event_.Signal();
+}
+
+bool GetSaveFileNameClient::OnMessageReceived(const IPC::Message& message) {
+  bool handled = true;
+  IPC_BEGIN_MESSAGE_MAP(GetSaveFileNameClient, message)
+    IPC_MESSAGE_HANDLER(ChromeUtilityHostMsg_GetSaveFileName_Failed,
+                        OnFailure)
+    IPC_MESSAGE_HANDLER(ChromeUtilityHostMsg_GetSaveFileName_Result,
+                        OnResult)
+    IPC_MESSAGE_UNHANDLED(handled = false)
+  IPC_END_MESSAGE_MAP()
+  return handled;
+}
+
+GetSaveFileNameClient::~GetSaveFileNameClient() {}
+
+void GetSaveFileNameClient::OnResult(const base::FilePath& path,
+                                     int one_based_filter_index) {
+  path_ = path;
+  one_based_filter_index_ = one_based_filter_index;
+  event_.Signal();
+}
+
+void GetSaveFileNameClient::OnFailure() {
+  event_.Signal();
+}
+
+// Initiates IPC with a new utility process using |client|. Instructs the
+// utility process to call GetSaveFileName with |ofn|. |current_task_runner|
+// must be the currently executing task runner.
+void DoInvokeGetSaveFileName(
+    OPENFILENAME* ofn,
+    scoped_refptr<GetSaveFileNameClient> 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();
+  ChromeUtilityMsg_GetSaveFileName_Params params;
+  params.owner = ofn->hwndOwner;
+  // We can't pass the hook function over IPC.
+  params.flags = ofn->Flags & ~OFN_ENABLEHOOK;
+  params.filters = ui::win::OpenFileName::GetFilters(ofn);
+  params.one_based_filter_index = ofn->nFilterIndex;
+  params.suggested_filename = base::FilePath(ofn->lpstrFile);
+  params.initial_directory = base::FilePath(
+      ofn->lpstrInitialDir ? ofn->lpstrInitialDir : base::string16());
+  params.default_extension =
+      ofn->lpstrDefExt ? base::string16(ofn->lpstrDefExt) : base::string16();
+
+  utility_process_host->Send(new ChromeUtilityMsg_GetSaveFileName(params));
+}
+
+// Invokes GetSaveFileName in a utility process. Blocks until the result is
+// received. Uses |blocking_task_runner| for IPC.
+bool GetSaveFileNameInUtilityProcess(
+    const scoped_refptr<base::SequencedTaskRunner>& blocking_task_runner,
+    OPENFILENAME* ofn) {
+  scoped_refptr<GetSaveFileNameClient> client(new GetSaveFileNameClient);
+  blocking_task_runner->PostTask(
+      FROM_HERE,
+      base::Bind(&DoInvokeGetSaveFileName,
+                 base::Unretained(ofn), client, blocking_task_runner));
+  client->WaitForCompletion();
+
+  if (client->path().empty())
+    return false;
+
+  base::wcslcpy(ofn->lpstrFile, client->path().value().c_str(), ofn->nMaxFile);
+  ofn->nFilterIndex = client->one_based_filter_index();
+
+  return true;
+}
+
+// Implements GetSaveFileName for CreateWinSelectFileDialog by delegating either
+// to Metro or a utility process.
+bool GetSaveFileNameImpl(
+    const scoped_refptr<base::SequencedTaskRunner>& blocking_task_runner,
+    OPENFILENAME* ofn) {
+  if (base::win::IsMetroProcess())
+    return CallMetroOPENFILENAMEMethod("MetroGetSaveFileName", ofn);
+
+  if (ShouldIsolateShellOperations())
+    return GetSaveFileNameInUtilityProcess(blocking_task_runner, ofn);
+
+  return ::GetSaveFileName(ofn) == TRUE;
+}
+
 }  // namespace
 
 ChromeSelectFileDialogFactory::ChromeSelectFileDialogFactory(
@@ -186,5 +333,6 @@ ui::SelectFileDialog* ChromeSelectFileDialogFactory::Create(
   return ui::CreateWinSelectFileDialog(
       listener,
       policy,
-      base::Bind(GetOpenFileNameImpl, blocking_task_runner_));
+      base::Bind(GetOpenFileNameImpl, blocking_task_runner_),
+      base::Bind(GetSaveFileNameImpl, blocking_task_runner_));
 }

chrome/common/chrome_utility_messages.h

@@ -36,6 +36,25 @@ IPC_STRUCT_TRAITS_BEGIN(safe_browsing::zip_analyzer::Results)
   IPC_STRUCT_TRAITS_MEMBER(has_archive)
 IPC_STRUCT_TRAITS_END()
 
+#if defined(OS_WIN)
+
+// A vector of filters, each being a Tuple2 containing a display string (i.e.
+// "Text Files") and a filter pattern (i.e. "*.txt").
+typedef std::vector<Tuple2<base::string16, base::string16> >
+    GetOpenFileNameFilter;
+
+IPC_STRUCT_BEGIN(ChromeUtilityMsg_GetSaveFileName_Params)
+  IPC_STRUCT_MEMBER(HWND, owner)
+  IPC_STRUCT_MEMBER(DWORD, flags)
+  IPC_STRUCT_MEMBER(GetOpenFileNameFilter, filters)
+  IPC_STRUCT_MEMBER(int, one_based_filter_index)
+  IPC_STRUCT_MEMBER(base::FilePath, suggested_filename)
+  IPC_STRUCT_MEMBER(base::FilePath, initial_directory)
+  IPC_STRUCT_MEMBER(base::string16, default_extension)
+IPC_STRUCT_END()
+
+#endif  // OS_WIN
+
 //------------------------------------------------------------------------------
 // Utility process messages:
 // These are messages from the browser to the utility process.
@@ -94,11 +113,6 @@ IPC_MESSAGE_CONTROL1(ChromeUtilityMsg_AnalyzeZipFileForDownloadProtection,
 IPC_MESSAGE_CONTROL1(ChromeUtilityMsg_OpenItemViaShell,
                      base::FilePath /* full_path */)
 
-// 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
@@ -113,6 +127,8 @@ IPC_MESSAGE_CONTROL5(ChromeUtilityMsg_GetOpenFileName,
                      GetOpenFileNameFilter /* filter */,
                      base::FilePath /* initial_directory */,
                      base::FilePath /* filename */)
+IPC_MESSAGE_CONTROL1(ChromeUtilityMsg_GetSaveFileName,
+                     ChromeUtilityMsg_GetSaveFileName_Params /* params */)
 #endif  // defined(OS_WIN)
 
 //------------------------------------------------------------------------------
@@ -163,4 +179,8 @@ IPC_MESSAGE_CONTROL0(ChromeUtilityHostMsg_GetOpenFileName_Failed)
 IPC_MESSAGE_CONTROL2(ChromeUtilityHostMsg_GetOpenFileName_Result,
                      base::FilePath /* directory */,
                      std::vector<base::FilePath> /* filenames */)
+IPC_MESSAGE_CONTROL0(ChromeUtilityHostMsg_GetSaveFileName_Failed)
+IPC_MESSAGE_CONTROL2(ChromeUtilityHostMsg_GetSaveFileName_Result,
+                     base::FilePath /* path */,
+                     int /* one_based_filter_index  */)
 #endif  // defined(OS_WIN)

chrome/utility/shell_handler_win.cc

@@ -23,6 +23,8 @@ bool ShellHandler::OnMessageReceived(const IPC::Message& message) {
                         OnOpenItemViaShell)
     IPC_MESSAGE_HANDLER(ChromeUtilityMsg_GetOpenFileName,
                         OnGetOpenFileName)
+    IPC_MESSAGE_HANDLER(ChromeUtilityMsg_GetSaveFileName,
+                        OnGetSaveFileName)
     IPC_MESSAGE_UNHANDLED(handled = false)
   IPC_END_MESSAGE_MAP()
   return handled;
@@ -56,3 +58,32 @@ void ShellHandler::OnGetOpenFileName(
         new ChromeUtilityHostMsg_GetOpenFileName_Failed());
   }
 }
+
+void ShellHandler::OnGetSaveFileName(
+    const ChromeUtilityMsg_GetSaveFileName_Params& params) {
+  ui::win::OpenFileName open_file_name(params.owner, params.flags);
+  open_file_name.SetInitialSelection(params.initial_directory,
+                                     params.suggested_filename);
+  open_file_name.GetOPENFILENAME()->nFilterIndex =
+      params.one_based_filter_index;
+  open_file_name.GetOPENFILENAME()->lpstrDefExt =
+      params.default_extension.c_str();
+
+  open_file_name.MaybeInstallWindowPositionHookForSaveAsOnXP();
+
+  if (::GetSaveFileName(open_file_name.GetOPENFILENAME())) {
+    content::UtilityThread::Get()->Send(
+        new ChromeUtilityHostMsg_GetSaveFileName_Result(
+            base::FilePath(open_file_name.GetOPENFILENAME()->lpstrFile),
+            open_file_name.GetOPENFILENAME()->nFilterIndex));
+    return;
+  }
+
+  // Zero means the dialog was closed, otherwise we had an error.
+  DWORD error_code = ::CommDlgExtendedError();
+  if (error_code != 0)
+    NOTREACHED() << "GetSaveFileName failed with code: " << error_code;
+
+  content::UtilityThread::Get()->Send(
+      new ChromeUtilityHostMsg_GetSaveFileName_Failed());
+}

chrome/utility/shell_handler_win.h

@@ -22,6 +22,8 @@ class FilePath;
 typedef std::vector<Tuple2<base::string16, base::string16> >
     GetOpenFileNameFilter;
 
+struct ChromeUtilityMsg_GetSaveFileName_Params;
+
 // 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.
@@ -35,6 +37,7 @@ class ShellHandler : public UtilityMessageHandler {
 
  private:
   void OnOpenItemViaShell(const base::FilePath& full_path);
+
   void OnGetOpenFileName(
     HWND owner,
     DWORD flags,
@@ -42,6 +45,8 @@ class ShellHandler : public UtilityMessageHandler {
     const base::FilePath& initial_directory,
     const base::FilePath& filename);
 
+  void OnGetSaveFileName(const ChromeUtilityMsg_GetSaveFileName_Params& params);
+
   DISALLOW_COPY_AND_ASSIGN(ShellHandler);
 };
 

ui/base/win/open_file_name_win.cc

@@ -6,10 +6,67 @@
 
 #include "base/files/file_path.h"
 #include "base/strings/string_util.h"
+#include "base/win/windows_version.h"
 
 namespace ui {
 namespace win {
 
+namespace {
+
+// Ensures that the Save As dialog is on-screen.
+UINT_PTR CALLBACK SaveAsDialogHook(HWND dialog, UINT message,
+                                   WPARAM wparam, LPARAM lparam) {
+  static const UINT kPrivateMessage = 0x2F3F;
+  switch (message) {
+    case WM_INITDIALOG: {
+      // Do nothing here. Just post a message to defer actual processing.
+      ::PostMessage(dialog, kPrivateMessage, 0, 0);
+      return TRUE;
+    }
+    case kPrivateMessage: {
+      // The dialog box is the parent of the current handle.
+      HWND real_dialog = ::GetParent(dialog);
+
+      // Retrieve the final size.
+      RECT dialog_rect;
+      ::GetWindowRect(real_dialog, &dialog_rect);
+
+      // Verify that the upper left corner is visible.
+      POINT point = { dialog_rect.left, dialog_rect.top };
+      HMONITOR monitor1 = ::MonitorFromPoint(point, MONITOR_DEFAULTTONULL);
+      point.x = dialog_rect.right;
+      point.y = dialog_rect.bottom;
+
+      // Verify that the lower right corner is visible.
+      HMONITOR monitor2 = ::MonitorFromPoint(point, MONITOR_DEFAULTTONULL);
+      if (monitor1 && monitor2)
+        return 0;
+
+      // Some part of the dialog box is not visible, fix it by moving is to the
+      // client rect position of the browser window.
+      HWND parent_window = ::GetParent(real_dialog);
+      if (!parent_window)
+        return 0;
+      WINDOWINFO parent_info;
+      parent_info.cbSize = sizeof(WINDOWINFO);
+      ::GetWindowInfo(parent_window, &parent_info);
+      ::SetWindowPos(
+          real_dialog,
+          NULL,
+          parent_info.rcClient.left,
+          parent_info.rcClient.top,
+          0,
+          0,  // Size.
+          SWP_NOACTIVATE | SWP_NOOWNERZORDER | SWP_NOSIZE | SWP_NOZORDER);
+
+      return 0;
+    }
+  }
+  return 0;
+}
+
+}  // namespace
+
 OpenFileName::OpenFileName(HWND parent_window, DWORD flags) {
   ::ZeroMemory(&openfilename_, sizeof(openfilename_));
   openfilename_.lStructSize = sizeof(openfilename_);
@@ -72,6 +129,15 @@ void OpenFileName::SetInitialSelection(const base::FilePath& initial_directory,
                 arraysize(filename_buffer_));
 }
 
+void OpenFileName::MaybeInstallWindowPositionHookForSaveAsOnXP() {
+  if (base::win::GetVersion() >= base::win::VERSION_VISTA)
+    return;
+
+  openfilename_.Flags |= OFN_ENABLEHOOK;
+  DCHECK(!openfilename_.lpfnHook);
+  openfilename_.lpfnHook = &SaveAsDialogHook;
+}
+
 base::FilePath OpenFileName::GetSingleResult() {
   base::FilePath directory;
   std::vector<base::FilePath> filenames;

ui/base/win/open_file_name_win.h

@@ -40,6 +40,12 @@ class UI_BASE_EXPORT OpenFileName {
   void SetInitialSelection(const base::FilePath& initial_directory,
                            const base::FilePath& initial_filename);
 
+  // The save as dialog on Windows XP remembers its last position, and if the
+  // screen resolution has changed it may be off screen. This method will check
+  // if we are running on XP and if so install a hook to reposition the dialog
+  // if necessary.
+  void MaybeInstallWindowPositionHookForSaveAsOnXP();
+
   // Returns the single selected file, or an empty path if there are more or
   // less than one results.
   base::FilePath GetSingleResult();

ui/shell_dialogs/select_file_dialog_win.h

@@ -25,7 +25,8 @@ SHELL_DIALOGS_EXPORT std::wstring AppendExtensionIfNeeded(
 SHELL_DIALOGS_EXPORT SelectFileDialog* CreateWinSelectFileDialog(
     SelectFileDialog::Listener* listener,
     SelectFilePolicy* policy,
-    const base::Callback<bool(OPENFILENAME* ofn)>& get_open_file_name_impl);
+    const base::Callback<bool(OPENFILENAME* ofn)>& get_open_file_name_impl,
+    const base::Callback<bool(OPENFILENAME* ofn)>& get_save_file_name_impl);
 
 SelectFileDialog* CreateDefaultWinSelectFileDialog(
     SelectFileDialog::Listener* listener,