Servicifying ShellHandlerWin.

As part of the effort to deprecate UtilityProcessMojoClient, changing
ShellHandlerWin to a service and consumers to bind the service through
the service manager.
Renamed the interface from ShellHandlerWin to WinShellUtil.

Bug: 777032
Change-Id: I71a45a38a5fd0d5d02a8a0bccabf6a7e140acbd7
Reviewed-on: https://chromium-review.googlesource.com/736147
Commit-Queue: Jay Civelli <jcivelli@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Patrick Monette <pmonette@chromium.org>
Cr-Commit-Position: refs/heads/master@{#512682}

chrome/app/BUILD.gn

@@ -406,6 +406,10 @@ if (is_chromeos) {
       [ "//chrome/browser/chromeos:ash_pref_connector_manifest" ]
 }
 
+if (is_win) {
+  chrome_packaged_services += [ "//chrome/services/util_win:manifest" ]
+}
+
 if (!is_android) {
   chrome_packaged_services += [ "//chrome/utility:profile_import_manifest" ]
 }

chrome/app/generated_resources.grd

@@ -11068,8 +11068,8 @@ read aloud to screenreader users to announce that a completion is available.">
     </message>
 
     <if expr="is_win">
-      <message name="IDS_UTILITY_PROCESS_SHELL_HANDLER_NAME" desc="The name of the utility process used to handle shell operations.">
-        Shell Handler
+      <message name="IDS_UTILITY_PROCESS_UTILITY_WIN_NAME" desc="The name of the utility process used to handle Windows utility operations.">
+        Windows Utilities
       </message>
     </if>
 

chrome/browser/BUILD.gn

@@ -2794,6 +2794,7 @@ split_static_library("browser") {
       "//chrome/common:metrics_constants_util_win",
       "//chrome/common:version_header",
       "//chrome/install_static:install_static_util",
+      "//chrome/services/util_win/public/interfaces",
       "//chrome_elf:blacklist",
       "//chrome_elf:constants",
       "//chrome_elf:dll_hash",

chrome/browser/DEPS

@@ -6,6 +6,7 @@ include_rules = [
   "+chrome/grit",
   "+chrome/install_static",
   "+chrome/installer/util",
+  "+chrome/services/util_win/public/interfaces",
   "+chrome_elf/blacklist",
   "+chrome_elf/chrome_elf_constants.h",
   "+chrome_elf/dll_hash",

chrome/browser/chrome_content_browser_client.cc

@@ -243,6 +243,7 @@
 #include "chrome/browser/chrome_browser_main_win.h"
 #include "chrome/browser/conflicts/module_database_win.h"
 #include "chrome/browser/conflicts/module_event_sink_impl_win.h"
+#include "chrome/services/util_win/public/interfaces/constants.mojom.h"
 #include "sandbox/win/src/sandbox_policy.h"
 #elif defined(OS_MACOSX)
 #include "chrome/browser/chrome_browser_main_mac.h"
@@ -3100,6 +3101,11 @@ void ChromeContentBrowserClient::RegisterOutOfProcessServices(
       l10n_util::GetStringUTF16(IDS_UTILITY_PROCESS_PROXY_RESOLVER_NAME);
 #endif
 
+#if defined(OS_WIN)
+  (*services)[chrome::mojom::kUtilWinServiceName] =
+      l10n_util::GetStringUTF16(IDS_UTILITY_PROCESS_UTILITY_WIN_NAME);
+#endif
+
 #if BUILDFLAG(ENABLE_PACKAGE_MASH_SERVICES)
   if (base::CommandLine::ForCurrentProcess()->HasSwitch(switches::kMash))
     mash_service_registry::RegisterOutOfProcessServices(services);

chrome/browser/chrome_content_browser_manifest_overlay.json

@@ -53,7 +53,8 @@
           "ime_registrar",
           "input_device_controller",
           "window_manager"
-        ]
+        ],
+        "util_win" : [ "shell_util_win" ]
       }
     },
     "navigation:frame": {

chrome/browser/metrics/chrome_browser_main_extra_parts_metrics.cc

@@ -25,6 +25,8 @@
 #include "chrome/browser/shell_integration.h"
 #include "components/flags_ui/pref_service_flags_storage.h"
 #include "content/public/common/content_switches.h"
+#include "content/public/common/service_manager_connection.h"
+#include "services/service_manager/public/cpp/connector.h"
 #include "ui/base/touch/touch_device.h"
 #include "ui/base/ui_base_switches.h"
 #include "ui/display/screen.h"
@@ -463,9 +465,10 @@ void OnIsPinnedToTaskbarResult(bool succeeded, bool is_pinned_to_taskbar) {
 // Records the pinned state of the current executable into a histogram. Should
 // be called on a background thread, with low priority, to avoid slowing down
 // startup.
-void RecordIsPinnedToTaskbarHistogram() {
+void RecordIsPinnedToTaskbarHistogram(
+    std::unique_ptr<service_manager::Connector> connector) {
   shell_integration::win::GetIsPinnedToTaskbarState(
-      base::Bind(&OnShellHandlerConnectionError),
+      std::move(connector), base::Bind(&OnShellHandlerConnectionError),
       base::Bind(&OnIsPinnedToTaskbarResult));
 }
 #endif  // defined(OS_WIN)
@@ -548,10 +551,14 @@ void ChromeBrowserMainExtraPartsMetrics::PostBrowserStart() {
   // TODO(isherman): The delay below is currently needed to avoid (flakily)
   // breaking some tests, including all of the ProcessMemoryMetricsEmitterTest
   // tests. Figure out why there is a dependency and fix the tests.
+  service_manager::Connector* connector =
+      content::ServiceManagerConnection::GetForProcess()->GetConnector();
+
   base::CreateSequencedTaskRunnerWithTraits(background_task_traits)
-      ->PostDelayedTask(FROM_HERE,
-                        base::BindOnce(&RecordIsPinnedToTaskbarHistogram),
-                        base::TimeDelta::FromSeconds(45));
+      ->PostDelayedTask(
+          FROM_HERE,
+          base::BindOnce(&RecordIsPinnedToTaskbarHistogram, connector->Clone()),
+          base::TimeDelta::FromSeconds(45));
 #endif  // defined(OS_WIN)
 
   display_count_ = display::Screen::GetScreen()->GetNumDisplays();

chrome/browser/metrics/chrome_browser_main_extra_parts_metrics_unittest.cc

@@ -9,7 +9,8 @@
 #include "base/macros.h"
 #include "base/message_loop/message_loop.h"
 #include "base/test/histogram_tester.h"
-#include "base/test/scoped_task_environment.h"
+#include "content/public/test/test_browser_thread_bundle.h"
+#include "content/public/test/test_service_manager_context.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/display/screen.h"
 #include "ui/display/test/test_screen.h"
@@ -34,16 +35,17 @@ class ChromeBrowserMainExtraPartsMetricsTest : public testing::Test {
 
  private:
   // Provides a message loop and allows the use of the task scheduler
-  base::test::ScopedTaskEnvironment scoped_task_environment_;
+  content::TestBrowserThreadBundle thread_bundle_;
+  content::TestServiceManagerContext service_manager_context_;
 
   // Dummy screen required by a ChromeBrowserMainExtraPartsMetrics test target.
   display::test::TestScreen test_screen_;
 
   DISALLOW_COPY_AND_ASSIGN(ChromeBrowserMainExtraPartsMetricsTest);
 };
 
-ChromeBrowserMainExtraPartsMetricsTest::ChromeBrowserMainExtraPartsMetricsTest()
-    : device_data_manager_test_api_() {
+ChromeBrowserMainExtraPartsMetricsTest::
+    ChromeBrowserMainExtraPartsMetricsTest() {
   display::Screen::SetScreenInstance(&test_screen_);
 }
 

chrome/browser/shell_integration_win.cc

@@ -54,8 +54,11 @@
 #include "chrome/installer/util/install_util.h"
 #include "chrome/installer/util/scoped_user_protocol_entry.h"
 #include "chrome/installer/util/shell_util.h"
+#include "chrome/services/util_win/public/interfaces/constants.mojom.h"
+#include "chrome/services/util_win/public/interfaces/shell_util_win.mojom.h"
 #include "components/variations/variations_associated_data.h"
-#include "content/public/browser/utility_process_mojo_client.h"
+#include "content/public/common/service_manager_connection.h"
+#include "services/service_manager/public/cpp/connector.h"
 #include "ui/base/l10n/l10n_util.h"
 
 namespace shell_integration {
@@ -451,22 +454,29 @@ class OpenSystemSettingsHelper {
 OpenSystemSettingsHelper* OpenSystemSettingsHelper::instance_ = nullptr;
 
 // Helper class to determine if Chrome is pinned to the taskbar. Hides the
-// complexity of managing the lifetime of a UtilityProcessMojoClient.
+// complexity of managing the lifetime of the connection to the ChromeWinUtil
+// service.
 class IsPinnedToTaskbarHelper {
  public:
   using ResultCallback = win::IsPinnedToTaskbarCallback;
   using ErrorCallback = win::ConnectionErrorCallback;
-  static void GetState(const ErrorCallback& error_callback,
+  static void GetState(std::unique_ptr<service_manager::Connector> connector,
+                       const ErrorCallback& error_callback,
                        const ResultCallback& result_callback);
 
  private:
-  IsPinnedToTaskbarHelper(const ErrorCallback& error_callback,
+  IsPinnedToTaskbarHelper(std::unique_ptr<service_manager::Connector> connector,
+                          const ErrorCallback& error_callback,
                           const ResultCallback& result_callback);
 
   void OnConnectionError();
   void OnIsPinnedToTaskbarResult(bool succeeded, bool is_pinned_to_taskbar);
 
-  content::UtilityProcessMojoClient<chrome::mojom::ShellHandler> shell_handler_;
+  chrome::mojom::ShellUtilWinPtr shell_util_win_ptr_;
+  // The connector used to retrieve the Patch service. We can't simply use
+  // content::ServiceManagerConnection::GetForProcess()->GetConnector() as this
+  // is called on a background thread.
+  std::unique_ptr<service_manager::Connector> connector_;
 
   ErrorCallback error_callback_;
   ResultCallback result_callback_;
@@ -477,37 +487,38 @@ class IsPinnedToTaskbarHelper {
 };
 
 // static
-void IsPinnedToTaskbarHelper::GetState(const ErrorCallback& error_callback,
-                                       const ResultCallback& result_callback) {
+void IsPinnedToTaskbarHelper::GetState(
+    std::unique_ptr<service_manager::Connector> connector,
+    const ErrorCallback& error_callback,
+    const ResultCallback& result_callback) {
   // Self-deleting when the ShellHandler completes.
-  new IsPinnedToTaskbarHelper(error_callback, result_callback);
+  new IsPinnedToTaskbarHelper(std::move(connector), error_callback,
+                              result_callback);
 }
 
 IsPinnedToTaskbarHelper::IsPinnedToTaskbarHelper(
+    std::unique_ptr<service_manager::Connector> connector,
     const ErrorCallback& error_callback,
     const ResultCallback& result_callback)
-    : shell_handler_(
-          l10n_util::GetStringUTF16(IDS_UTILITY_PROCESS_SHELL_HANDLER_NAME)),
+    : connector_(std::move(connector)),
       error_callback_(error_callback),
       result_callback_(result_callback) {
   DCHECK(error_callback_);
   DCHECK(result_callback_);
 
-  // |shell_handler_| owns the callbacks and is guaranteed to be destroyed
+  connector_->BindInterface(chrome::mojom::kUtilWinServiceName,
+                            &shell_util_win_ptr_);
+  // |shell_util_win_ptr_| owns the callbacks and is guaranteed to be destroyed
   // before |this|, therefore making base::Unretained() safe to use.
-  shell_handler_.set_error_callback(base::Bind(
+  shell_util_win_ptr_.set_connection_error_handler(base::Bind(
       &IsPinnedToTaskbarHelper::OnConnectionError, base::Unretained(this)));
-  shell_handler_.set_disable_sandbox();
-  shell_handler_.Start();
-
-  shell_handler_.service()->IsPinnedToTaskbar(
+  shell_util_win_ptr_->IsPinnedToTaskbar(
       base::Bind(&IsPinnedToTaskbarHelper::OnIsPinnedToTaskbarResult,
                  base::Unretained(this)));
 }
 
 void IsPinnedToTaskbarHelper::OnConnectionError() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-
   error_callback_.Run();
   delete this;
 }
@@ -744,9 +755,11 @@ void MigrateTaskbarPins() {
 }
 
 void GetIsPinnedToTaskbarState(
+    std::unique_ptr<service_manager::Connector> connector,
     const ConnectionErrorCallback& on_error_callback,
     const IsPinnedToTaskbarCallback& result_callback) {
-  IsPinnedToTaskbarHelper::GetState(on_error_callback, result_callback);
+  IsPinnedToTaskbarHelper::GetState(std::move(connector), on_error_callback,
+                                    result_callback);
 }
 
 int MigrateShortcutsInPathInternal(const base::FilePath& chrome_exe,

chrome/browser/shell_integration_win.h

@@ -5,12 +5,17 @@
 #ifndef CHROME_BROWSER_SHELL_INTEGRATION_WIN_H_
 #define CHROME_BROWSER_SHELL_INTEGRATION_WIN_H_
 
+#include <memory>
 #include <string>
 
 #include "base/callback_forward.h"
 #include "base/files/file_path.h"
 #include "base/strings/string16.h"
 
+namespace service_manager {
+class Connector;
+}
+
 namespace shell_integration {
 namespace win {
 
@@ -62,9 +67,11 @@ base::string16 GetChromiumModelIdForProfile(const base::FilePath& profile_path);
 // is true if Chrome is pinned to the taskbar.
 // The ConnectionErrorCallback is called instead if something wrong happened
 // with the connection to the remote process.
+// |connector| should be a fresh connector unbound to any thread.
 using ConnectionErrorCallback = base::Closure;
 using IsPinnedToTaskbarCallback = base::Callback<void(bool, bool)>;
 void GetIsPinnedToTaskbarState(
+    std::unique_ptr<service_manager::Connector> connector,
     const ConnectionErrorCallback& on_error_callback,
     const IsPinnedToTaskbarCallback& result_callback);
 

chrome/browser/ui/webui/welcome_win10_handler.cc

@@ -15,6 +15,8 @@
 #include "chrome/common/url_constants.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/web_contents.h"
+#include "content/public/common/service_manager_connection.h"
+#include "services/service_manager/public/cpp/connector.h"
 #include "url/gurl.h"
 
 namespace {
@@ -43,6 +45,13 @@ void RecordPinnedResult(const std::string& histogram_suffix,
       is_pinned);
 }
 
+// Returns a new Connector that can be used on a different thread.
+std::unique_ptr<service_manager::Connector> GetClonedConnector() {
+  return content::ServiceManagerConnection::GetForProcess()
+      ->GetConnector()
+      ->Clone();
+}
+
 }  // namespace
 
 WelcomeWin10Handler::WelcomeWin10Handler(bool inline_style_variant)
@@ -72,7 +81,8 @@ WelcomeWin10Handler::~WelcomeWin10Handler() {
     base::Closure error_callback =
         base::Bind(&RecordPinnedResult, histogram_suffix, false, false);
     shell_integration::win::GetIsPinnedToTaskbarState(
-        error_callback, base::Bind(&RecordPinnedResult, histogram_suffix));
+        GetClonedConnector(), error_callback,
+        base::Bind(&RecordPinnedResult, histogram_suffix));
   }
 }
 
@@ -139,7 +149,7 @@ void WelcomeWin10Handler::StartIsPinnedToTaskbarCheck() {
                  weak_ptr_factory_.GetWeakPtr(), false, true);
 
   shell_integration::win::GetIsPinnedToTaskbarState(
-      error_callback,
+      GetClonedConnector(), error_callback,
       base::Bind(&WelcomeWin10Handler::OnIsPinnedToTaskbarResult,
                  weak_ptr_factory_.GetWeakPtr()));
 }