Experimentally isolate OpenItemViaShell 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/431343002

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

6 files changed, 50 insertions, 6 deletions

diff --git a/chrome/browser/platform_util_win.cc b/chrome/browser/platform_util_win.cc
index be651c4..7d18db6 100644
--- a/chrome/browser/platform_util_win.cc
+++ b/chrome/browser/platform_util_win.cc
@@ -13,6 +13,7 @@
 #include "base/bind_helpers.h"
 #include "base/files/file_path.h"
 #include "base/logging.h"
+#include "base/metrics/field_trial.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/win/registry.h"
@@ -21,7 +22,9 @@
 #include "base/win/windows_version.h"
 #include "chrome/browser/lifetime/application_lifetime.h"
 #include "chrome/browser/ui/host_desktop.h"
+#include "chrome/common/chrome_utility_messages.h"
 #include "content/public/browser/browser_thread.h"
+#include "content/public/browser/utility_process_host.h"
 #include "ui/base/win/shell.h"
 #include "ui/gfx/native_widget_types.h"
 #include "url/gurl.h"
@@ -155,6 +158,13 @@ void OpenExternalOnFileThread(const GURL& url) {
   }
 }
 
+void OpenItemViaShellInUtilityProcess(const base::FilePath& full_path) {
+  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 {
@@ -175,9 +185,18 @@ void OpenItem(Profile* profile, const base::FilePath& full_path) {
   if (chrome::GetActiveDesktop() == chrome::HOST_DESKTOP_TYPE_ASH)
     chrome::ActivateDesktopHelper(chrome::ASH_KEEP_RUNNING);
 
-  BrowserThread::PostTask(
-      BrowserThread::FILE, FROM_HERE,
-      base::Bind(base::IgnoreResult(&ui::win::OpenItemViaShell), full_path));
+  if (base::FieldTrialList::FindFullName("IsolateShellOperations") ==
+      "Enabled") {
+    BrowserThread::PostTask(
+        BrowserThread::IO,
+        FROM_HERE,
+        base::Bind(&OpenItemViaShellInUtilityProcess, full_path));
+  } else {
+    BrowserThread::PostTask(
+        BrowserThread::FILE,
+        FROM_HERE,
+        base::Bind(base::IgnoreResult(&ui::win::OpenItemViaShell), full_path));
+  }
 }
 
 void OpenExternal(Profile* profile, const GURL& url) {
diff --git a/chrome/common/chrome_utility_messages.h b/chrome/common/chrome_utility_messages.h
index b2216c3..352d12bf 100644
--- a/chrome/common/chrome_utility_messages.h
+++ b/chrome/common/chrome_utility_messages.h
@@ -91,6 +91,9 @@ IPC_MESSAGE_CONTROL1(ChromeUtilityMsg_AnalyzeZipFileForDownloadProtection,
 #endif
 
 #if defined(OS_WIN)
+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> >
diff --git a/chrome/utility/shell_handler_win.cc b/chrome/utility/shell_handler_win.cc
index 7337f9a..9edd042 100644
--- a/chrome/utility/shell_handler_win.cc
+++ b/chrome/utility/shell_handler_win.cc
@@ -11,6 +11,7 @@
 #include "chrome/common/chrome_utility_messages.h"
 #include "content/public/utility/utility_thread.h"
 #include "ui/base/win/open_file_name_win.h"
+#include "ui/base/win/shell.h"
 
 ShellHandler::ShellHandler() {}
 ShellHandler::~ShellHandler() {}
@@ -18,6 +19,8 @@ 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_GetOpenFileName,
                         OnGetOpenFileName)
     IPC_MESSAGE_UNHANDLED(handled = false)
@@ -25,6 +28,10 @@ bool ShellHandler::OnMessageReceived(const IPC::Message& message) {
   return handled;
 }
 
+void ShellHandler::OnOpenItemViaShell(const base::FilePath& full_path) {
+  ui::win::OpenItemViaShell(full_path);
+}
+
 void ShellHandler::OnGetOpenFileName(
     HWND owner,
     DWORD flags,
diff --git a/chrome/utility/shell_handler_win.h b/chrome/utility/shell_handler_win.h
index f91124c..10ffe72 100644
--- a/chrome/utility/shell_handler_win.h
+++ b/chrome/utility/shell_handler_win.h
@@ -34,6 +34,7 @@ class ShellHandler : public UtilityMessageHandler {
   virtual bool OnMessageReceived(const IPC::Message& message) OVERRIDE;
 
  private:
+  void OnOpenItemViaShell(const base::FilePath& full_path);
   void OnGetOpenFileName(
     HWND owner,
     DWORD flags,
diff --git a/content/browser/utility_process_host_impl.cc b/content/browser/utility_process_host_impl.cc
index 1a1b942..16a42d6 100644
--- a/content/browser/utility_process_host_impl.cc
+++ b/content/browser/utility_process_host_impl.cc
@@ -268,15 +268,23 @@ bool UtilityProcessHostImpl::StartProcess() {
 }
 
 bool UtilityProcessHostImpl::OnMessageReceived(const IPC::Message& message) {
+  if (!client_)
+    return true;
+
   client_task_runner_->PostTask(
       FROM_HERE,
-      base::Bind(base::IgnoreResult(
-          &UtilityProcessHostClient::OnMessageReceived), client_.get(),
+      base::Bind(
+          base::IgnoreResult(&UtilityProcessHostClient::OnMessageReceived),
+          client_.get(),
           message));
+
   return true;
 }
 
 void UtilityProcessHostImpl::OnProcessLaunchFailed() {
+  if (!client_)
+    return;
+
   client_task_runner_->PostTask(
       FROM_HERE,
       base::Bind(&UtilityProcessHostClient::OnProcessLaunchFailed,
@@ -284,6 +292,9 @@ void UtilityProcessHostImpl::OnProcessLaunchFailed() {
 }
 
 void UtilityProcessHostImpl::OnProcessCrashed(int exit_code) {
+  if (!client_)
+    return;
+
   client_task_runner_->PostTask(
       FROM_HERE,
       base::Bind(&UtilityProcessHostClient::OnProcessCrashed, client_.get(),
diff --git a/content/public/browser/utility_process_host.h b/content/public/browser/utility_process_host.h
index 756d50f..51a89fa 100644
--- a/content/public/browser/utility_process_host.h
+++ b/content/public/browser/utility_process_host.h
@@ -34,7 +34,10 @@ struct ChildProcessData;
 class UtilityProcessHost : public IPC::Sender,
                            public base::SupportsWeakPtr<UtilityProcessHost> {
  public:
-  // Used to create a utility process.
+  // Used to create a utility process. |client| is optional. If supplied it will
+  // be notified of incoming messages from the utility process.
+  // |client_task_runner| is required if |client| is supplied and is the task
+  // runner upon which |client| will be invoked.
   CONTENT_EXPORT static UtilityProcessHost* Create(
       UtilityProcessHostClient* client,
       base::SequencedTaskRunner* client_task_runner);