[remoting host][win] Show prompt when fallback browser unknown or inc…

…orrect

Per discussion offline, we are not going to support the case where the
user changes the default browser to the URL forwarder on the settings
app, so it's unlikely that the fallback browser settings will be
missing or configured incorrectly, unless the user has uninstalled the
fallback browser. This CL simply makes the app show a prompt telling
the user to change the browser to something else and reconfigure the
URL forwarder in that case.

Screenshot: https://screenshot.googleplex.com/6fc6xvW32fP9ax5
Bug: b:183135000
Change-Id: I72f0c908e9e45f1322296ebf4b6587d405b43526
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3139553
Commit-Queue: Yuwei Huang <yuweih@chromium.org>
Reviewed-by: Joe Downing <joedow@chromium.org>
Cr-Commit-Position: refs/heads/main@{#917933}

remoting/host/BUILD.gn

@@ -191,7 +191,7 @@ source_set("clipboard") {
   }
 }
 
-# A source set to be shared by the host binary and the remote_open_url binary.
+# A source set to be shared by the :common and remoting/host/win.
 source_set("remote_open_url_support") {
   sources = [
     "remote_open_url_constants.cc",

remoting/host/installer/win/chromoting.wxs

@@ -28,7 +28,7 @@
   <?if $(var.OfficialBuild) != 0 ?>
     <?define ChromotingKeyPath = "Google\Chrome Remote Desktop" ?>
     <?define UrlForwarderName = "Chrome Remote Desktop URL Forwarder" ?>
-    <?define UrlForwarderProgId = "CrdUrlForwarder" ?>
+    <?define UrlForwarderProgId = "ChromeRemoteDesktopUrlForwarder" ?>
   <?else?>
     <?define ChromotingKeyPath = "Chromoting" ?>
     <?define UrlForwarderName = "Chromoting URL Forwarder" ?>

remoting/host/remote_open_url_client_delegate_win.cc

@@ -16,9 +16,14 @@
 #include "base/process/launch.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/win/current_module.h"
 #include "remoting/base/logging.h"
+#include "remoting/host/remote_open_url_constants.h"
 #include "remoting/host/user_setting_keys.h"
 #include "remoting/host/user_settings.h"
+#include "remoting/host/win/core_resource.h"
+#include "remoting/host/win/default_apps_util.h"
+#include "remoting/host/win/simple_task_dialog.h"
 
 namespace remoting {
 
@@ -43,6 +48,19 @@ std::wstring GetLaunchBrowserCommand(const std::wstring& browser_prog_id,
   return open_command;
 }
 
+void ShowIncorrectConfigurationPrompt() {
+  SimpleTaskDialog task_dialog(CURRENT_MODULE());
+  task_dialog.SetTitleTextWithStringId(IDS_URL_FORWARDER_NAME);
+  task_dialog.SetMessageTextWithStringId(
+      IDS_URL_FORWARDER_INCORRECTLY_CONFIGURED);
+  task_dialog.AppendButtonWithStringId(IDOK,
+                                       IDS_OPEN_DEFAULT_APPS_SETTINGS_BUTTON);
+  task_dialog.set_default_button(IDOK);
+  absl::optional<int> result = task_dialog.Show();
+  DCHECK_EQ(IDOK, *result);
+  LaunchDefaultAppsSettingsModernDialog();
+}
+
 }  // namespace
 
 RemoteOpenUrlClientDelegateWin::RemoteOpenUrlClientDelegateWin() = default;
@@ -58,9 +76,9 @@ void RemoteOpenUrlClientDelegateWin::OpenUrlOnFallbackBrowser(const GURL& url) {
   std::wstring fallback_browser_prog_id =
       base::UTF8ToWide(UserSettings::GetInstance()->GetString(
           kWinPreviousDefaultWebBrowserProgId));
-  if (fallback_browser_prog_id.empty()) {
-    // TODO(b/183135000): Implement some sort of fallback browser chooser.
-    LOG(ERROR) << "Cannot determine the fallback browser.";
+  if (fallback_browser_prog_id.empty() ||
+      fallback_browser_prog_id == kUrlForwarderProgId) {
+    ShowIncorrectConfigurationPrompt();
     return;
   }
 
@@ -74,6 +92,7 @@ void RemoteOpenUrlClientDelegateWin::OpenUrlOnFallbackBrowser(const GURL& url) {
   std::wstring launch_command =
       GetLaunchBrowserCommand(fallback_browser_prog_id, url);
   if (launch_command.empty()) {
+    ShowIncorrectConfigurationPrompt();
     return;
   }
   HOST_LOG << "Opening URL with fallback browser: " << fallback_browser_prog_id;

remoting/host/remote_open_url_client_delegate_win.h

@@ -5,6 +5,7 @@
 #ifndef REMOTING_HOST_REMOTE_OPEN_URL_CLIENT_DELEGATE_WIN_H_
 #define REMOTING_HOST_REMOTE_OPEN_URL_CLIENT_DELEGATE_WIN_H_
 
+#include "base/win/scoped_com_initializer.h"
 #include "remoting/host/remote_open_url_client.h"
 
 namespace remoting {
@@ -19,6 +20,9 @@ class RemoteOpenUrlClientDelegateWin final
   bool IsInRemoteDesktopSession() override;
   void OpenUrlOnFallbackBrowser(const GURL& url) override;
   void ShowOpenUrlError(const GURL& url) override;
+
+ private:
+  base::win::ScopedCOMInitializer scoped_com_initializer_;
 };
 
 }  // namespace remoting

remoting/host/remote_open_url_constants.cc

@@ -36,4 +36,14 @@ const mojo::NamedPlatformChannel::ServerName& GetRemoteOpenUrlIpcChannelName() {
   return *server_name;
 }
 
+#if defined(OS_WIN)
+
+#if defined(OFFICIAL_BUILD)
+const wchar_t kUrlForwarderProgId[] = L"ChromeRemoteDesktopUrlForwarder";
+#else
+const wchar_t kUrlForwarderProgId[] = L"ChromotingUrlForwarder";
+#endif
+
+#endif  // defined (OS_WIN)
+
 }  // namespace remoting

remoting/host/remote_open_url_constants.h

@@ -5,6 +5,7 @@
 #ifndef REMOTING_HOST_REMOTE_OPEN_URL_CONSTANTS_H_
 #define REMOTING_HOST_REMOTE_OPEN_URL_CONSTANTS_H_
 
+#include "build/build_config.h"
 #include "mojo/public/cpp/platform/named_platform_channel.h"
 
 namespace remoting {
@@ -13,6 +14,13 @@ extern const char kRemoteOpenUrlDataChannelName[];
 
 const mojo::NamedPlatformChannel::ServerName& GetRemoteOpenUrlIpcChannelName();
 
+#if defined(OS_WIN)
+
+// The ProgID of the URL forwarder.
+extern const wchar_t kUrlForwarderProgId[];
+
+#endif  // defined (OS_WIN)
+
 }  // namespace remoting
 
 #endif  // REMOTING_HOST_REMOTE_OPEN_URL_CONSTANTS_H_

remoting/host/win/BUILD.gn

@@ -99,6 +99,8 @@ source_set("win") {
     "com_security.cc",
     "com_security.h",
     "core_resource.h",
+    "default_apps_util.cc",
+    "default_apps_util.h",
     "default_audio_device_change_detector.cc",
     "default_audio_device_change_detector.h",
     "etw_trace_consumer.h",
@@ -455,6 +457,7 @@ shared_library("remoting_core") {
     "//remoting/host:host_main_headers",
     "//remoting/host:host_settings",
     "//remoting/host:logging",
+    "//remoting/host:remote_open_url_support",
     "//remoting/host:remoting_me2me_host_static",
     "//remoting/host/it2me:common",
     "//remoting/host/native_messaging",

remoting/host/win/core.rc.jinja2

@@ -41,6 +41,10 @@ BEGIN
             format(url_forwarder_name)|
             replace('"', '""') }}"
   IDS_CANCEL "{% trans %}CANCEL{% endtrans %}"
+  IDS_URL_FORWARDER_INCORRECTLY_CONFIGURED "{{
+          gettext("URL_FORWARDER_INCORRECTLY_CONFIGURED")|
+            format(url_forwarder_name)|
+            replace('"', '""') }}"
 END
 
 IDI_CHROME_REMOTE_DESKTOP      ICON    "remoting/resources/chromoting.ico"

remoting/host/win/core_resource.h

@@ -31,6 +31,7 @@
 #define IDS_SET_UP_URL_FORWARDER_MESSAGE 121
 #define IDS_OPEN_DEFAULT_APPS_SETTINGS_BUTTON 122
 #define IDS_CANCEL 123
+#define IDS_URL_FORWARDER_INCORRECTLY_CONFIGURED 124
 
 #define IDC_DISCONNECT                  1001
 #define IDC_DISCONNECT_SHARINGWITH      1002

remoting/host/win/default_apps_util.cc

@@ -0,0 +1,41 @@
+// Copyright 2021 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 "remoting/host/win/default_apps_util.h"
+
+#include <shobjidl.h>
+#include <wrl/client.h>
+
+#include "base/logging.h"
+
+namespace remoting {
+
+// The appModelId looks arbitrary but it is the same in Win8 and Win10. There is
+// no easy way to retrieve the appModelId from the registry.
+bool LaunchDefaultAppsSettingsModernDialog() {
+  static const wchar_t kControlPanelAppModelId[] =
+      L"windows.immersivecontrolpanel_cw5n1h2txyewy"
+      L"!microsoft.windows.immersivecontrolpanel";
+
+  Microsoft::WRL::ComPtr<IApplicationActivationManager> activator;
+  HRESULT hr = ::CoCreateInstance(CLSID_ApplicationActivationManager, nullptr,
+                                  CLSCTX_ALL, IID_PPV_ARGS(&activator));
+  if (FAILED(hr)) {
+    LOG(ERROR) << "Failed to create IApplicationActivationManager: "
+               << logging::SystemErrorCodeToString(hr);
+    return false;
+  }
+  DWORD pid = 0;
+  CoAllowSetForegroundWindow(activator.Get(), nullptr);
+  hr = activator->ActivateApplication(
+      kControlPanelAppModelId, L"page=SettingsPageAppsDefaults", AO_NONE, &pid);
+  if (FAILED(hr)) {
+    LOG(ERROR) << "Failed to activate application: "
+               << logging::SystemErrorCodeToString(hr);
+    return false;
+  }
+  return true;
+}
+
+}  // namespace remoting

remoting/host/win/default_apps_util.h

@@ -0,0 +1,18 @@
+// Copyright 2021 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 REMOTING_HOST_WIN_DEFAULT_APPS_UTIL_H_
+#define REMOTING_HOST_WIN_DEFAULT_APPS_UTIL_H_
+
+namespace remoting {
+
+// Launches the Windows 'settings' modern app with the 'default apps' view
+// focused. This only works for Windows 8 and Windows 10.
+// Returns a boolean indicating whether the default apps view is successfully
+// launched.
+bool LaunchDefaultAppsSettingsModernDialog();
+
+}  // namespace remoting
+
+#endif  // REMOTING_HOST_WIN_DEFAULT_APPS_UTIL_H_

remoting/host/win/url_forwarder_configurator_main.cc

@@ -27,22 +27,18 @@
 #include "base/win/scoped_com_initializer.h"
 #include "base/win/windows_types.h"
 #include "remoting/base/logging.h"
+#include "remoting/host/remote_open_url_constants.h"
 #include "remoting/host/switches.h"
 #include "remoting/host/user_setting_keys.h"
 #include "remoting/host/user_settings.h"
 #include "remoting/host/win/core_resource.h"
+#include "remoting/host/win/default_apps_util.h"
 #include "remoting/host/win/simple_task_dialog.h"
 
 namespace remoting {
 
 namespace {
 
-#if defined(OFFICIAL_BUILD)
-constexpr wchar_t kUrlForwarderProgId[] = L"CrdUrlForwarder";
-#else
-constexpr wchar_t kUrlForwarderProgId[] = L"ChromotingUrlForwarder";
-#endif
-
 constexpr wchar_t kProtocolToTestSetup[] = L"http";
 
 constexpr base::TimeDelta kPollingInterval =
@@ -89,37 +85,6 @@ bool IsUrlForwarderSetUp(bool log_current_default_app = false) {
   return true;
 }
 
-// Launches the Windows 'settings' modern app with the 'default apps' view
-// focused. This only works for Windows 8 and Windows 10. The appModelId
-// looks arbitrary but it is the same in Win8 and Win10. There is no easy way to
-// retrieve the appModelId from the registry.
-// Returns a boolean indicating whether the default apps view is successfully
-// launched.
-bool LaunchDefaultAppsSettingsModernDialog() {
-  // This method is modified from chrome/installer/util/shell_util.cc
-
-  static const wchar_t kControlPanelAppModelId[] =
-      L"windows.immersivecontrolpanel_cw5n1h2txyewy"
-      L"!microsoft.windows.immersivecontrolpanel";
-
-  Microsoft::WRL::ComPtr<IApplicationActivationManager> activator;
-  HRESULT hr = ::CoCreateInstance(CLSID_ApplicationActivationManager, nullptr,
-                                  CLSCTX_ALL, IID_PPV_ARGS(&activator));
-  if (FAILED(hr)) {
-    PLOG(ERROR) << "Failed to create IApplicationActivationManager";
-    return false;
-  }
-  DWORD pid = 0;
-  CoAllowSetForegroundWindow(activator.Get(), nullptr);
-  hr = activator->ActivateApplication(
-      kControlPanelAppModelId, L"page=SettingsPageAppsDefaults", AO_NONE, &pid);
-  if (FAILED(hr)) {
-    PLOG(ERROR) << "Failed to activate application";
-    return false;
-  }
-  return true;
-}
-
 bool ShowSetUpUrlForwarderDialog() {
   // |resource_module| does not need to be freed as GetModuleHandle() does not
   // increment the refcount for the module.  This DLL is not unloaded until the

remoting/resources/remoting_strings.grd

@@ -918,6 +918,9 @@ If '<ph name="SERVICE_SCRIPT_NAME">$3<ex>org.chromium.chromoting.me2me.sh</ex></
       <message name="IDS_BROWSER_IS_INVALID" desc="Message shown when the user has chosen an invalid app as the browser to open URLs locally.">
         The chosen browser cannot be used to open URLs on the local machine.
       </message>
+      <message name="IDS_URL_FORWARDER_INCORRECTLY_CONFIGURED" desc="Message shown when the Chrome Remote Desktop URL forwarder is incorrectly configured.">
+        <ph name="URL_FORWARDER_NAME">%s<ex>Chrome Remote Desktop URL Forwarder</ex></ph> is not configured correctly. Please choose a different default web browser and then enable URL forwarding again.
+      </message>
       <if expr="is_linux">
         <message name="IDS_SESSION_DIALOG_MESSAGE" desc="The message to show at the top of the session-selection dialog.">
           Select a session to launch within your Chrome Remote Desktop environment. (Note that some session types may not support running within Chrome Remote Desktop and on the local console simultaneously.)

remoting/resources/remoting_strings_grd/IDS_URL_FORWARDER_INCORRECTLY_CONFIGURED.png.sha1

@@ -0,0 +1 @@
+658450bb1e1608cfa439f67f136cb6ef20ec5ca1