Isolate shell operations to the utility process.

Shell operations can cause 3rd-party shell extensions to be
loaded into the caller's process. The utility process protects the browser process from potential instability.

Also convert the IPC mechanism to Mojo.

BUG=73098

Review-Url: https://codereview.chromium.org/2122303002
Cr-Commit-Position: refs/heads/master@{#405270}

chrome/browser/chrome_browser_main.cc

@@ -1455,8 +1455,7 @@ int ChromeBrowserMainParts::PreMainMessageLoopRunImpl() {
         parsed_command_line_);
   }
 
-  ui::SelectFileDialog::SetFactory(new ChromeSelectFileDialogFactory(
-      BrowserThread::GetTaskRunnerForThread(BrowserThread::IO)));
+  ui::SelectFileDialog::SetFactory(new ChromeSelectFileDialogFactory());
 #endif  // defined(OS_WIN)
 
   if (parsed_command_line().HasSwitch(switches::kMakeDefaultBrowser)) {

chrome/browser/win/chrome_select_file_dialog_factory.h

@@ -5,24 +5,16 @@
 #ifndef CHROME_BROWSER_WIN_CHROME_SELECT_FILE_DIALOG_FACTORY_H_
 #define CHROME_BROWSER_WIN_CHROME_SELECT_FILE_DIALOG_FACTORY_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);
+  ChromeSelectFileDialogFactory();
   ~ChromeSelectFileDialogFactory() override;
 
   // ui::SelectFileDialogFactory implementation
@@ -30,8 +22,6 @@ class ChromeSelectFileDialogFactory : public ui::SelectFileDialogFactory {
                                ui::SelectFilePolicy* policy) override;
 
  private:
-  scoped_refptr<base::SequencedTaskRunner> blocking_task_runner_;
-
   DISALLOW_COPY_AND_ASSIGN(ChromeSelectFileDialogFactory);
 };
 

chrome/chrome_common.gypi

@@ -658,6 +658,7 @@
         ],
       },
       'dependencies': [
+        '../mojo/mojo_base.gyp:mojo_common_custom_types_mojom',
         '../mojo/mojo_public.gyp:mojo_cpp_bindings',
         '../skia/skia.gyp:skia_mojo',
       ],

chrome/chrome_utility.gypi

@@ -15,8 +15,6 @@
       'utility/cloud_print/pwg_encoder.h',
       'utility/image_decoder_impl.cc',
       'utility/image_decoder_impl.h',
-      'utility/ipc_shell_handler_win.cc',
-      'utility/ipc_shell_handler_win.h',
       'utility/printing_handler.cc',
       'utility/printing_handler.h',
       'utility/shell_handler_impl_win.cc',

chrome/common/BUILD.gn

@@ -434,4 +434,7 @@ mojom("mojo_bindings") {
   public_deps = [
     "//skia/public/interfaces",
   ]
+  deps = [
+    "//mojo/common:common_custom_types",
+  ]
 }

chrome/common/chrome_utility_messages.h

@@ -4,16 +4,10 @@
 
 // Multiply-included message file, so no include guard.
 
-#if defined(OS_WIN)
-#include <Windows.h>
-#endif  // defined(OS_WIN)
-
 #include <string>
-#include <tuple>
 #include <vector>
 
 #include "base/files/file_path.h"
-#include "base/strings/string16.h"
 #include "base/values.h"
 #include "build/build_config.h"
 #include "ipc/ipc_message_macros.h"
@@ -31,13 +25,6 @@
 #ifndef CHROME_COMMON_CHROME_UTILITY_MESSAGES_H_
 #define CHROME_COMMON_CHROME_UTILITY_MESSAGES_H_
 
-#if defined(OS_WIN)
-// A vector of filters, each being a tuple containing a display string (i.e.
-// "Text Files") and a filter pattern (i.e. "*.txt").
-typedef std::vector<std::tuple<base::string16, base::string16>>
-    GetOpenFileNameFilter;
-#endif  // OS_WIN
-
 #endif  // CHROME_COMMON_CHROME_UTILITY_MESSAGES_H_
 
 #define IPC_MESSAGE_START ChromeUtilityMsgStart
@@ -124,18 +111,6 @@ IPC_STRUCT_TRAITS_BEGIN(safe_browsing::zip_analyzer::Results)
 IPC_STRUCT_TRAITS_END()
 #endif  // FULL_SAFE_BROWSING
 
-#if defined(OS_WIN)
-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.
@@ -180,25 +155,6 @@ IPC_MESSAGE_CONTROL1(ChromeUtilityMsg_AnalyzeDmgFileForDownloadProtection,
 #endif  // defined(OS_MACOSX)
 #endif  // defined(FULL_SAFE_BROWSING)
 
-#if defined(OS_WIN)
-// 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 */)
-IPC_MESSAGE_CONTROL1(ChromeUtilityMsg_GetSaveFileName,
-                     ChromeUtilityMsg_GetSaveFileName_Params /* params */)
-#endif  // defined(OS_WIN)
-
 //------------------------------------------------------------------------------
 // Utility process host messages:
 // These are messages from the utility process to the browser.
@@ -233,14 +189,3 @@ IPC_MESSAGE_CONTROL1(
     safe_browsing::zip_analyzer::Results)
 #endif  // defined(OS_MACOSX)
 #endif  // defined(FULL_SAFE_BROWSING)
-
-#if defined(OS_WIN)
-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/common/shell_handler_win.mojom

@@ -4,7 +4,52 @@
 
 module mojom;
 
+import "mojo/common/common_custom_types.mojom";
+
+[Native]
+struct FileExtensionFilters;
+
 interface ShellHandler {
   // Returns the pinned state of the current executable.
   IsPinnedToTaskbar() => (bool succeeded, bool is_pinned_to_taskbar);
-};
\ No newline at end of file
+
+  // Invokes GetOpenFileName.
+  // |owner| is the HWND to use as the parent of the modal dialog.
+  // |flags| is the OFN_* flags of OPENFILENAME.
+  // |filters| constrains the user's file choices.
+  // |initial_directory| is the directory to be displayed.
+  // |filename| is the file initially selected.
+  //
+  // Returns the list of selected |files|, which are all childs of |directory|.
+  // On cancelation or failure, |files| will be empty.
+  DoGetOpenFileName(uint32 owner,
+                    uint32 flags,
+                    FileExtensionFilters filters,
+                    mojo.common.mojom.FilePath initial_directory,
+                    mojo.common.mojom.FilePath filename) =>
+      (mojo.common.mojom.FilePath directory,
+       array<mojo.common.mojom.FilePath> files);
+
+  // Invokes GetSaveFileName.
+  // |owner| is the HWND to use as the parent of the modal dialog.
+  // |flags| is the OFN_* flags of OPENFILENAME.
+  // |filters| constrains the user's file choices.
+  // |one_based_filter_index| is The index of the currently selected filter in
+  //   |filters|.
+  // |initial_directory| is the directory to be displayed.
+  // |filename| is the file initially selected.
+  // |default_extension| is the extension to add to the file if the user doesn't
+  //   type one.
+  //
+  // Returns the selected |path| and the |one_based_filter_index| of the filter
+  // passed to the initial call to DoGetSaveFileName. On cancelation or failure,
+  // |path| will be empty.
+  DoGetSaveFileName(uint32 owner,
+                    uint32 flags,
+                    FileExtensionFilters filters,
+                    uint32 one_based_filter_index,
+                    mojo.common.mojom.FilePath initial_directory,
+                    mojo.common.mojom.FilePath suggested_filename,
+                    mojo.common.mojom.FilePath default_extension) =>
+      (mojo.common.mojom.FilePath path, uint32 one_based_filter_index);
+};

chrome/common/shell_handler_win.typemap

+# Copyright 2016 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.
+
+mojom = "//chrome/common/shell_handler_win.mojom"
+public_headers = [ "//base/strings/string16.h" ]
+deps = [
+  "//base",
+]
+type_mappings = [ "mojom.FileExtensionFilters=" +
+                  "std::vector<std::tuple<base::string16, base::string16>>" ]

chrome/common/typemaps.gni

+# Copyright 2016 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.
+
+typemaps = [ "//chrome/common/shell_handler_win.typemap" ]

chrome/utility/chrome_content_utility_client.cc

@@ -38,7 +38,6 @@
 #endif
 
 #if defined(OS_WIN)
-#include "chrome/utility/ipc_shell_handler_win.h"
 #include "chrome/utility/shell_handler_impl_win.h"
 #endif
 
@@ -126,10 +125,6 @@ ChromeContentUtilityClient::ChromeContentUtilityClient()
     (defined(ENABLE_BASIC_PRINTING) && defined(OS_WIN))
   handlers_.push_back(new printing::PrintingHandler());
 #endif
-
-#if defined(OS_WIN)
-  handlers_.push_back(new IPCShellHandler());
-#endif
 }
 
 ChromeContentUtilityClient::~ChromeContentUtilityClient() {

chrome/utility/ipc_shell_handler_win.cc

-// 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/ipc_shell_handler_win.h"
-
-#include <commdlg.h>
-
-#include <memory>
-
-#include "base/files/file_path.h"
-#include "chrome/common/chrome_utility_messages.h"
-#include "content/public/utility/utility_thread.h"
-#include "ui/base/win/open_file_name_win.h"
-
-IPCShellHandler::IPCShellHandler() {}
-IPCShellHandler::~IPCShellHandler() {}
-
-bool IPCShellHandler::OnMessageReceived(const IPC::Message& message) {
-  bool handled = true;
-  IPC_BEGIN_MESSAGE_MAP(IPCShellHandler, message)
-    IPC_MESSAGE_HANDLER(ChromeUtilityMsg_GetOpenFileName, OnGetOpenFileName)
-    IPC_MESSAGE_HANDLER(ChromeUtilityMsg_GetSaveFileName, OnGetSaveFileName)
-    IPC_MESSAGE_UNHANDLED(handled = false)
-  IPC_END_MESSAGE_MAP()
-  return handled;
-}
-
-void IPCShellHandler::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());
-  }
-}
-
-void IPCShellHandler::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.SetFilters(params.filters);
-  open_file_name.GetOPENFILENAME()->nFilterIndex =
-      params.one_based_filter_index;
-  open_file_name.GetOPENFILENAME()->lpstrDefExt =
-      params.default_extension.c_str();
-
-  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/ipc_shell_handler_win.h

-// 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_IPC_SHELL_HANDLER_WIN_H_
-#define CHROME_UTILITY_IPC_SHELL_HANDLER_WIN_H_
-
-#include <Windows.h>
-
-#include <tuple>
-#include <vector>
-
-#include "base/compiler_specific.h"
-#include "base/macros.h"
-#include "base/strings/string16.h"
-#include "chrome/utility/utility_message_handler.h"
-
-namespace base {
-class FilePath;
-}  // namespace base
-
-using GetOpenFileNameFilter =
-    std::vector<std::tuple<base::string16, base::string16>>;
-
-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.
-// Note: This class is deprecated in favor of the Mojo version.
-//       See chrome/common/shell_handler_win.mojom and
-//       chrome/utility/shell_handler_impl_win.h
-class IPCShellHandler : public UtilityMessageHandler {
- public:
-  IPCShellHandler();
-  ~IPCShellHandler() override;
-
-  // IPC::Listener implementation
-  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);
-
-  void OnGetSaveFileName(const ChromeUtilityMsg_GetSaveFileName_Params& params);
-
-  DISALLOW_COPY_AND_ASSIGN(IPCShellHandler);
-};
-
-#endif  // CHROME_UTILITY_IPC_SHELL_HANDLER_WIN_H_

chrome/utility/shell_handler_impl_win.cc

@@ -17,9 +17,14 @@
 #include "base/win/shortcut.h"
 #include "chrome/installer/util/install_util.h"
 #include "content/public/utility/utility_thread.h"
+#include "ui/base/win/open_file_name_win.h"
 
 namespace {
 
+HWND Uint32ToHWND(uint32_t value) {
+  return reinterpret_cast<HWND>(value);
+}
+
 // This class checks if the current executable is pinned to the taskbar. It also
 // keeps track of the errors that occurs that prevents it from getting a result.
 class IsPinnedToTaskbarHelper {
@@ -222,3 +227,56 @@ void ShellHandlerImpl::IsPinnedToTaskbar(
   bool is_pinned_to_taskbar = helper.GetResult();
   callback.Run(!helper.error_occured(), is_pinned_to_taskbar);
 }
+
+void ShellHandlerImpl::DoGetOpenFileName(
+    uint32_t owner,
+    uint32_t flags,
+    const std::vector<std::tuple<base::string16, base::string16>>& filters,
+    const base::FilePath& initial_directory,
+    const base::FilePath& filename,
+    const DoGetOpenFileNameCallback& callback) {
+  ui::win::OpenFileName open_file_name(Uint32ToHWND(owner), flags);
+  open_file_name.SetInitialSelection(initial_directory, filename);
+  open_file_name.SetFilters(filters);
+
+  base::FilePath directory;
+  std::vector<base::FilePath> filenames;
+
+  if (::GetOpenFileName(open_file_name.GetOPENFILENAME()))
+    open_file_name.GetResult(&directory, &filenames);
+
+  if (!filenames.empty())
+    callback.Run(directory, filenames);
+  else
+    callback.Run(base::FilePath(), std::vector<base::FilePath>());
+}
+
+void ShellHandlerImpl::DoGetSaveFileName(
+    uint32_t owner,
+    uint32_t flags,
+    const std::vector<std::tuple<base::string16, base::string16>>& filters,
+    uint32_t one_based_filter_index,
+    const base::FilePath& initial_directory,
+    const base::FilePath& suggested_filename,
+    const base::FilePath& default_extension,
+    const DoGetSaveFileNameCallback& callback) {
+  ui::win::OpenFileName open_file_name(reinterpret_cast<HWND>(owner), flags);
+  open_file_name.SetInitialSelection(initial_directory, suggested_filename);
+  open_file_name.SetFilters(filters);
+  open_file_name.GetOPENFILENAME()->nFilterIndex = one_based_filter_index;
+  open_file_name.GetOPENFILENAME()->lpstrDefExt =
+      default_extension.value().c_str();
+
+  if (::GetSaveFileName(open_file_name.GetOPENFILENAME())) {
+    callback.Run(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;
+
+  callback.Run(base::FilePath(), 0);
+}

chrome/utility/shell_handler_impl_win.h

@@ -5,6 +5,9 @@
 #ifndef CHROME_UTILITY_SHELL_HANDLER_IMPL_WIN_H_
 #define CHROME_UTILITY_SHELL_HANDLER_IMPL_WIN_H_
 
+#include <tuple>
+#include <vector>
+
 #include "base/macros.h"
 #include "chrome/common/shell_handler_win.mojom.h"
 #include "mojo/public/cpp/bindings/strong_binding.h"
@@ -21,6 +24,24 @@ class ShellHandlerImpl : public mojom::ShellHandler {
   // mojom::ShellHandler:
   void IsPinnedToTaskbar(const IsPinnedToTaskbarCallback& callback) override;
 
+  void DoGetOpenFileName(
+      uint32_t owner,
+      uint32_t flags,
+      const std::vector<std::tuple<base::string16, base::string16>>& filters,
+      const base::FilePath& initial_directory,
+      const base::FilePath& filename,
+      const DoGetOpenFileNameCallback& callback) override;
+
+  void DoGetSaveFileName(
+      uint32_t owner,
+      uint32_t flags,
+      const std::vector<std::tuple<base::string16, base::string16>>& filters,
+      uint32_t one_based_filter_index,
+      const base::FilePath& initial_directory,
+      const base::FilePath& suggested_filename,
+      const base::FilePath& default_extension,
+      const DoGetSaveFileNameCallback& callback) override;
+
   mojo::StrongBinding<ShellHandler> binding_;
 
   DISALLOW_COPY_AND_ASSIGN(ShellHandlerImpl);

mojo/public/tools/bindings/chromium_bindings_configuration.gni

@@ -4,9 +4,10 @@
 
 _typemap_imports = [
   "//cc/ipc/typemaps.gni",
-  "//device/bluetooth/public/interfaces/typemaps.gni",
+  "//chrome/common/typemaps.gni",
   "//components/arc/common/typemaps.gni",
   "//components/typemaps.gni",
+  "//device/bluetooth/public/interfaces/typemaps.gni",
   "//gpu/ipc/common/typemaps.gni",
   "//media/mojo/interfaces/typemaps.gni",
   "//mojo/common/typemaps.gni",