[notification_helper.exe] Launch Chrome to handle toast activation

This CL implements NotificationActivator::Activate() which launches Chrome
to handle native notification activation, using a command line with the
--notification-launch-id switch.

Bug: 734095
Change-Id: I90c67ced93de1f4cc377188e0f414f2d2a9da2a5
Reviewed-on: https://chromium-review.googlesource.com/917821
Commit-Queue: Xi Cheng <chengx@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Reviewed-by: Finnur Thorarinsson <finnur@chromium.org>
Reviewed-by: Peter Beverloo <peter@chromium.org>
Reviewed-by: Greg Thompson <grt@chromium.org>
Cr-Commit-Position: refs/heads/master@{#538985}

chrome/browser/notifications/notification_display_service_impl.cc

@@ -33,7 +33,7 @@
 
 #if defined(OS_WIN)
 #include "base/strings/utf_string_conversions.h"
-#include "base/win/windows_version.h"
+#include "chrome/browser/notifications/notification_platform_bridge_win.h"
 #endif
 
 namespace {
@@ -68,10 +68,8 @@ NotificationPlatformBridge* GetNativeNotificationPlatformBridge() {
   DCHECK(base::FeatureList::IsEnabled(features::kNativeNotifications));
   return g_browser_process->notification_platform_bridge();
 #elif defined(OS_WIN)
-  if (base::win::GetVersion() >= base::win::VERSION_WIN10_RS1 &&
-      base::FeatureList::IsEnabled(features::kNativeNotifications)) {
+  if (NotificationPlatformBridgeWin::NativeNotificationEnabled())
     return g_browser_process->notification_platform_bridge();
-  }
 #else
   if (base::FeatureList::IsEnabled(features::kNativeNotifications) &&
       g_browser_process->notification_platform_bridge()) {

chrome/browser/notifications/notification_platform_bridge_win.cc

@@ -15,6 +15,7 @@
 
 #include "base/bind.h"
 #include "base/compiler_specific.h"
+#include "base/feature_list.h"
 #include "base/hash.h"
 #include "base/logging.h"
 #include "base/strings/string16.h"
@@ -26,6 +27,7 @@
 #include "base/win/core_winrt_util.h"
 #include "base/win/scoped_hstring.h"
 #include "base/win/scoped_winrt_initializer.h"
+#include "base/win/windows_version.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/notifications/notification_common.h"
 #include "chrome/browser/notifications/notification_display_service_impl.h"
@@ -34,6 +36,7 @@
 #include "chrome/browser/notifications/notification_launch_id.h"
 #include "chrome/browser/notifications/notification_template_builder.h"
 #include "chrome/browser/profiles/profile_manager.h"
+#include "chrome/common/chrome_features.h"
 #include "chrome/installer/util/install_util.h"
 #include "chrome/installer/util/shell_util.h"
 #include "components/version_info/channel.h"
@@ -56,9 +59,6 @@ namespace {
 // applying Creators Update (build 15063).
 const wchar_t kGroup[] = L"Notifications";
 
-typedef winfoundtn::ITypedEventHandler<winui::Notifications::ToastNotification*,
-                                       IInspectable*>
-    ToastActivatedHandler;
 typedef winfoundtn::ITypedEventHandler<
     winui::Notifications::ToastNotification*,
     winui::Notifications::ToastDismissedEventArgs*>
@@ -94,7 +94,7 @@ void ForwardNotificationOperationOnUiThread(
       profile_id, incognito,
       base::Bind(&NotificationDisplayServiceImpl::ProfileLoadedCallback,
                  operation, notification_type, origin, notification_id,
-                 action_index, base::nullopt /*reply*/, by_user));
+                 action_index, /*reply=*/base::nullopt, by_user));
 }
 
 }  // namespace
@@ -291,14 +291,8 @@ class NotificationPlatformBridgeWinImpl
       return;
     }
 
-    auto activated_handler = mswr::Callback<ToastActivatedHandler>(
-        this, &NotificationPlatformBridgeWinImpl::OnActivated);
-    EventRegistrationToken activated_token;
-    hr = toast->add_Activated(activated_handler.Get(), &activated_token);
-    if (FAILED(hr)) {
-      LOG(ERROR) << "Unable to add toast activated event handler";
-      return;
-    }
+    // Activation via user interaction with the toast is handled in
+    // HandleActivation() by way of the notification_helper.
 
     auto dismissed_handler = mswr::Callback<ToastDismissedHandler>(
         this, &NotificationPlatformBridgeWinImpl::OnDismissed);
@@ -464,7 +458,7 @@ class NotificationPlatformBridgeWinImpl
     content::BrowserThread::PostTask(
         content::BrowserThread::UI, FROM_HERE,
         base::BindOnce(callback, std::move(displayed_notifications),
-                       true /* supports_synchronization */));
+                       /*supports_synchronization=*/true));
   }
 
   void SetReadyCallback(
@@ -606,20 +600,6 @@ class NotificationPlatformBridgeWinImpl
     return NotificationLaunchId(value.GetAsUTF8());
   }
 
-  HRESULT OnActivated(winui::Notifications::IToastNotification* notification,
-                      IInspectable* inspectable) {
-    base::Optional<int> action_index;
-
-    winui::Notifications::IToastActivatedEventArgs* args = nullptr;
-    HRESULT hr = inspectable->QueryInterface(&args);
-    if (SUCCEEDED(hr))
-      action_index = ParseActionIndex(args);
-
-    HandleEvent(notification, NotificationCommon::CLICK, action_index,
-                /*by_user=*/base::nullopt);
-    return S_OK;
-  }
-
   HRESULT OnDismissed(
       winui::Notifications::IToastNotification* notification,
       winui::Notifications::IToastDismissedEventArgs* arguments) {
@@ -736,6 +716,37 @@ void NotificationPlatformBridgeWin::SetReadyCallback(
                      impl_, std::move(callback)));
 }
 
+// static
+bool NotificationPlatformBridgeWin::HandleActivation(
+    const std::string& launch_id_str) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+
+  NotificationLaunchId launch_id(launch_id_str);
+  if (!launch_id.is_valid())
+    return false;
+
+  ForwardNotificationOperationOnUiThread(
+      NotificationCommon::CLICK, launch_id.notification_type(),
+      launch_id.origin_url(), launch_id.notification_id(),
+      launch_id.profile_id(), launch_id.incognito(),
+      /*action_index=*/base::nullopt, /*by_user=*/true);
+
+  return true;
+}
+
+// static
+std::string NotificationPlatformBridgeWin::GetProfileIdFromLaunchId(
+    const std::string& launch_id_str) {
+  NotificationLaunchId launch_id(launch_id_str);
+  return launch_id.is_valid() ? launch_id.profile_id() : std::string();
+}
+
+// static
+bool NotificationPlatformBridgeWin::NativeNotificationEnabled() {
+  return base::win::GetVersion() >= base::win::VERSION_WIN10_RS1 &&
+         base::FeatureList::IsEnabled(features::kNativeNotifications);
+}
+
 void NotificationPlatformBridgeWin::PostTaskToTaskRunnerThread(
     base::OnceClosure closure) const {
   bool success = task_runner_->PostTask(FROM_HERE, std::move(closure));
@@ -766,5 +777,5 @@ HRESULT NotificationPlatformBridgeWin::GetToastNotificationForTesting(
     winui::Notifications::IToastNotification** toast_notification) {
   return impl_->GetToastNotification(
       notification, notification_template_builder, "UnusedValue",
-      false /* incognito */, toast_notification);
+      /*incognito=*/false, toast_notification);
 }

chrome/browser/notifications/notification_platform_bridge_win.h

@@ -38,6 +38,16 @@ class NotificationPlatformBridgeWin : public NotificationPlatformBridge {
       const GetDisplayedNotificationsCallback& callback) const override;
   void SetReadyCallback(NotificationBridgeReadyCallback callback) override;
 
+  // Handles notification activation given |launch_id_str| via the
+  // notification_helper process. Returns false if |launch_id_str| is invalid.
+  static bool HandleActivation(const std::string& launch_id_str);
+
+  // Extracts the profile ID from |launch_id_str|.
+  static std::string GetProfileIdFromLaunchId(const std::string& launch_id_str);
+
+  // Checks if native notification is enabled.
+  static bool NativeNotificationEnabled();
+
  private:
   friend class NotificationPlatformBridgeWinImpl;
   friend class NotificationPlatformBridgeWinTest;

chrome/browser/notifications/notification_platform_bridge_win_interactive_uitest.cc

@@ -171,8 +171,9 @@ IN_PROC_BROWSER_TEST_F(NotificationPlatformBridgeWinUITest, HandleEvent) {
 
   // Simulate clicks on the toast.
   NotificationPlatformBridgeWin* bridge =
-      reinterpret_cast<NotificationPlatformBridgeWin*>(
+      static_cast<NotificationPlatformBridgeWin*>(
           g_browser_process->notification_platform_bridge());
+  ASSERT_TRUE(bridge);
   bridge->ForwardHandleEventForTesting(NotificationCommon::CLICK, &toast, &args,
                                        base::nullopt);
   run_loop.Run();
@@ -195,8 +196,9 @@ IN_PROC_BROWSER_TEST_F(NotificationPlatformBridgeWinUITest, GetDisplayed) {
     return;
 
   NotificationPlatformBridgeWin* bridge =
-      reinterpret_cast<NotificationPlatformBridgeWin*>(
+      static_cast<NotificationPlatformBridgeWin*>(
           g_browser_process->notification_platform_bridge());
+  ASSERT_TRUE(bridge);
 
   std::vector<winui::Notifications::IToastNotification*> notifications;
   bridge->SetDisplayedNotificationsForTesting(&notifications);

chrome/browser/ui/startup/startup_browser_creator.cc

@@ -90,7 +90,9 @@
 #endif
 
 #if defined(OS_WIN)
+#include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/metrics/jumplist_metrics_win.h"
+#include "chrome/browser/notifications/notification_platform_bridge_win.h"
 #include "chrome/browser/ui/webui/settings/reset_settings_handler.h"
 #endif
 
@@ -951,6 +953,19 @@ base::FilePath GetStartupProfilePath(const base::FilePath& user_data_dir,
         command_line.GetSwitchValuePath(switches::kProfileDirectory));
   }
 
+#if defined(OS_WIN)
+  if (command_line.HasSwitch(switches::kNotificationLaunchId) &&
+      NotificationPlatformBridgeWin::NativeNotificationEnabled()) {
+    std::string profile_id =
+        NotificationPlatformBridgeWin::GetProfileIdFromLaunchId(
+            command_line.GetSwitchValueASCII(switches::kNotificationLaunchId));
+    if (!profile_id.empty()) {
+      return user_data_dir.Append(
+          base::FilePath(base::UTF8ToUTF16(profile_id)));
+    }
+  }
+#endif  // defined(OS_WIN)
+
 #if BUILDFLAG(ENABLE_APP_LIST)
   // If we are showing the app list then chrome isn't shown so load the app
   // list's profile rather than chrome's.

chrome/browser/ui/startup/startup_browser_creator_impl.cc

@@ -85,6 +85,7 @@
 #if defined(OS_WIN)
 #include "base/win/windows_version.h"
 #include "chrome/browser/apps/app_launch_for_metro_restart_win.h"
+#include "chrome/browser/notifications/notification_platform_bridge_win.h"
 #include "chrome/browser/search_engines/template_url_service_factory.h"
 #include "chrome/browser/shell_integration_win.h"
 #endif
@@ -125,6 +126,8 @@ enum LaunchMode {
   LM_MAC_DOCK_STATUS_ERROR = 15,      // Error determining dock status.
   LM_MAC_DMG_STATUS_ERROR = 16,       // Error determining dmg status.
   LM_MAC_DOCK_DMG_STATUS_ERROR = 17,  // Error determining dock and dmg status.
+  LM_WIN_PLATFORM_NOTIFICATION = 18,  // Launched from toast notification
+                                      // activation on Windows.
 };
 
 #if defined(OS_WIN)
@@ -324,6 +327,22 @@ bool StartupBrowserCreatorImpl::Launch(Profile* profile,
     }
   }
 
+#if defined(OS_WIN)
+  // If the command line has the kNotificationLaunchId switch, then this
+  // Launch() call is from notification_helper.exe to process toast activation.
+  // Delegate to the notification system; do not open a browser window here.
+  if (command_line_.HasSwitch(switches::kNotificationLaunchId)) {
+    if (NotificationPlatformBridgeWin::NativeNotificationEnabled() &&
+        NotificationPlatformBridgeWin::HandleActivation(
+            command_line_.GetSwitchValueASCII(
+                switches::kNotificationLaunchId))) {
+      RecordLaunchModeHistogram(LM_WIN_PLATFORM_NOTIFICATION);
+      return true;
+    }
+    return false;
+  }
+#endif  // defined(OS_WIN)
+
   // Open the required browser windows and tabs. First, see if
   // we're being run as an application window. If so, the user
   // opened an app shortcut.  Don't restore tabs or open initial

chrome/common/chrome_switches.cc

@@ -939,6 +939,10 @@ const char kHideIcons[]                     = "hide-icons";
 // This flag is only relevant for Windows currently.
 const char kNoNetworkProfileWarning[]       = "no-network-profile-warning";
 
+// Used for launching Chrome when a toast displayed in the Windows Action Center
+// has been activated. Should contain the launch ID encoded by Chrome.
+const char kNotificationLaunchId[] = "notification-launch-id";
+
 // /prefetch:# arguments for the browser process launched in background mode and
 // for the watcher process. Use profiles 5, 6 and 7 as documented on
 // kPrefetchArgument* in content_switches.cc.

chrome/common/chrome_switches.h

@@ -294,6 +294,7 @@ extern const char kEnableCloudPrintXps[];
 extern const char kEnableProfileShortcutManager[];
 extern const char kHideIcons[];
 extern const char kNoNetworkProfileWarning[];
+extern const char kNotificationLaunchId[];
 extern const char kPrefetchArgumentBrowserBackground[];
 extern const char kPrefetchArgumentWatcher[];
 extern const char kShowIcons[];

notification_helper/BUILD.gn

@@ -39,6 +39,7 @@ source_set("lib") {
 
   deps = [
     "//base",
+    "//chrome/common:constants",
     "//chrome/install_static:install_static_util",
   ]
 }

notification_helper/DEPS

 include_rules = [
   "+base",
+  "+chrome/common/chrome_constants.h",
+  "+chrome/common/chrome_switches.h",
   "+chrome/install_static",
   "+chrome/installer/setup",
   "+chrome/installer/util",

notification_helper/notification_activator.cc

@@ -4,46 +4,97 @@
 
 #include "notification_helper/notification_activator.h"
 
+#include <shellapi.h>
+
+#include "base/command_line.h"
+#include "base/files/file_util.h"
+#include "base/path_service.h"
+#include "base/strings/string16.h"
+#include "base/win/windows_types.h"
+#include "chrome/common/chrome_constants.h"
+#include "chrome/common/chrome_switches.h"
+#include "notification_helper/trace_util.h"
+
+namespace {
+
+// Returns the file path of chrome.exe if found, or an empty file path if not.
+base::FilePath GetChromeExePath() {
+  // Look for chrome.exe one folder above notification_helper.exe (as expected
+  // in Chrome installs). Failing that, look for it alonside
+  // notification_helper.exe.
+  base::FilePath dir_exe;
+  if (!PathService::Get(base::DIR_EXE, &dir_exe))
+    return base::FilePath();
+
+  base::FilePath chrome_exe =
+      dir_exe.DirName().Append(chrome::kBrowserProcessExecutableName);
+
+  if (!base::PathExists(chrome_exe)) {
+    chrome_exe = dir_exe.Append(chrome::kBrowserProcessExecutableName);
+    if (!base::PathExists(chrome_exe))
+      return base::FilePath();
+  }
+  return chrome_exe;
+}
+
+}  // namespace
+
 namespace notification_helper {
 
 NotificationActivator::~NotificationActivator() = default;
 
-// |invoked_args| contains the template ID string encoded by Chrome via
-// NotificationPlatformBridgeWin::EncodeTemplateId.
-// Chrome adds it to the launch argument and gets it back via invoked_args here.
-// Chrome needs the data to be able to look up the notification on its end.
-//
-// |invoked_args| string constains the following structure:
-// "notification_type|profile_id|incognito|origin_url|notification_id"
+// Handles toast activation outside of the browser process lifecycle by
+// launching chrome.exe with --notification-launch-id. This new process may
+// rendezvous to an existing browser process or become a new one, as
+// appropriate.
 //
-// An example of input parameter value. The toast from which the activation
-// comes is generated from https://tests.peter.sh/notification-generator/ with
-// the default setting.
-// {
-//    app_user_model_id = "Chromium.KX56J2SJSCJTWGPH2RNH3MHAM4"
-//    invoked_args =
-//         "0|Default|0|https://tests.peter.sh/|p#https://tests.peter.sh/#01"
-//    data = nullptr
-//    count = 0
-// }
+// When this method is called, there are three possibilities depending on the
+// running state of Chrome.
+// 1) NOT_RUNNING: Chrome is not running.
+// 2) NEW_INSTANCE: Chrome is running, but it's NOT the same instance that sent
+//    the toast.
+// 3) SAME_INSTANCE : Chrome is running, and it _is_ the same instance that sent
+//    the toast.
 //
-// Then invoked_args we will be decoded into:
-//    |notification_type| = 0
-//           |profile_id| = Default
-//            |incognito| = 0
-//           |origin_url| = https://tests.peter.sh/
-//      |notification_id| = p#https://tests.peter.sh/#01
+// Chrome could attach an activatation event handler to the toast so that
+// Windows can call it directly to handle the activation. However, Windows makes
+// this function call only in case SAME_INSTANCE. For the other two cases,
+// Chrome needs to handle the activation on its own. Since there is no way to
+// differentiate cases SAME_INSTANCE and NEW_INSTANCE in this
+// notification_helper process,  Chrome doesn't attach an activatation event
+// handler to the teast and handles all three cases through the command line.
 HRESULT NotificationActivator::Activate(
     LPCWSTR app_user_model_id,
     LPCWSTR invoked_args,
     const NOTIFICATION_USER_INPUT_DATA* data,
     ULONG count) {
-  // When Chrome is running, Windows will call this API and
-  // NotificationPlatformBridgeWinImpl::OnActivated serially.
+  base::FilePath chrome_exe_path = GetChromeExePath();
+  if (chrome_exe_path.empty()) {
+    Trace(L"Failed to get chrome exe path\n");
+    return HRESULT_FROM_WIN32(ERROR_FILE_NOT_FOUND);
+  }
+
+  // |invoked_args| contains the launch ID string encoded by Chrome. Chrome adds
+  // it to the launch argument of the toast and gets it back via |invoked_args|
+  // here. Chrome needs the data to be able to look up the notification on its
+  // end.
+  base::CommandLine command_line(base::CommandLine::NO_PROGRAM);
+  command_line.AppendSwitchNative(switches::kNotificationLaunchId,
+                                  invoked_args);
+
+  SHELLEXECUTEINFO info;
+  memset(&info, 0, sizeof(info));
+  info.cbSize = sizeof(info);
+  info.fMask = SEE_MASK_NOASYNC | SEE_MASK_FLAG_LOG_USAGE;
+  info.lpFile = chrome_exe_path.value().c_str();
+  info.lpParameters = command_line.GetCommandLineString().c_str();
+  info.nShow = SW_SHOWNORMAL;
 
-  // TODO(chengx): Investigate the correct activate behavior (mainly when Chrome
-  // is not running) and implement it. For example, decode the useful data from
-  // the function input and launch Chrome with proper args.
+  if (!::ShellExecuteEx(&info)) {
+    DWORD error_code = ::GetLastError();
+    Trace(L"Unable to launch Chrome.exe; error: 0x%08X\n", error_code);
+    return HRESULT_FROM_WIN32(error_code);
+  }
 
   return S_OK;
 }

tools/metrics/histograms/enums.xml

@@ -24864,6 +24864,9 @@ Called by update_gpu_driver_bug_workaround_entries.py.-->
   <int value="17" label="LM_MAC_DOCK_DMG_STATUS_ERROR">
     Mac - error determining Chrome's Dock and disk image statuses
   </int>
+  <int value="18" label="LM_WIN_PLATFORM_NOTIFICATION">
+    Launched from toast notification activation (Win10-RS1 and higher)
+  </int>
 </enum>
 
 <enum name="LazyCSSParseUsage">