Reland "Reland "Adds a way to create app shortcuts in different folder on Mac/Win/Linux.""

This is a reland of 8e311e7

Original change's description:
> Reland "Adds a way to create app shortcuts in different folder on Mac/Win/Linux."
>
> This is a reland of 479f569
>
> Original change's description:
> > Adds a way to create app shortcuts in different folder on Mac/Win/Linux.
> >
> > Change-Id: I3c06104c86333cbbd0857085cf7d7dd1fca63616
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2906422
> > Commit-Queue: Clifford Cheng <cliffordcheng@chromium.org>
> > Reviewed-by: Daniel Murphy <dmurph@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#892494}
>
> Change-Id: I3d003f1cb644402ffafaa2553d225da42c230081
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2965263
> Reviewed-by: Daniel Murphy <dmurph@chromium.org>
> Commit-Queue: Clifford Cheng <cliffordcheng@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#895652}

Change-Id: I1291946931f5bc1dab76e0a1339e99b5cd5a2b8a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2987632
Commit-Queue: Clifford Cheng <cliffordcheng@chromium.org>
Reviewed-by: Daniel Murphy <dmurph@chromium.org>
Cr-Commit-Position: refs/heads/master@{#896707}

Author: Clifford Cheng

chrome/browser/ui/web_applications/web_app_browsertest.cc

@@ -2,12 +2,15 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include <codecvt>
 #include <string>
 
 #include "base/feature_list.h"
+#include "base/files/file_util.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/test/bind.h"
 #include "base/test/metrics/histogram_tester.h"
 #include "base/test/metrics/user_action_tester.h"
 #include "base/test/scoped_feature_list.h"
@@ -22,6 +25,7 @@
 #include "chrome/browser/banners/app_banner_manager_desktop.h"
 #include "chrome/browser/browser_features.h"
 #include "chrome/browser/browser_process.h"
+#include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/devtools/protocol/browser_handler.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/sessions/tab_restore_service_factory.h"
@@ -941,6 +945,94 @@ IN_PROC_BROWSER_TEST_F(WebAppBrowserTest, ReparentWebAppForSecureActiveTab) {
   ASSERT_EQ(app_browser->app_controller()->GetAppId(), app_id);
 }
 
+#if defined(OS_MAC) || defined(OS_WIN) || defined(OS_LINUX)
+IN_PROC_BROWSER_TEST_F(WebAppBrowserTest, WebAppCreateAndDeleteShortcut) {
+  os_hooks_suppress_.reset();
+
+  ShortcutOverrideForTesting shortcut_override;
+  base::ScopedAllowBlockingForTesting allow_blocking;
+  base::ScopedTempDir desktop_temp_dir;
+  EXPECT_TRUE(desktop_temp_dir.CreateUniqueTempDir());
+  base::FilePath desktop_dir = desktop_temp_dir.GetPath();
+
+  base::ScopedTempDir application_temp_dir;
+  EXPECT_TRUE(application_temp_dir.CreateUniqueTempDir());
+  base::FilePath application_dir = application_temp_dir.GetPath();
+
+#if defined(OS_MAC)
+  shortcut_override.chrome_apps_folder = application_dir;
+#endif
+#if defined(OS_WIN)
+  shortcut_override.application_menu = application_dir;
+#endif
+#if !defined(OS_MAC)
+  shortcut_override.desktop = desktop_dir;
+#endif
+  SetShortcutOverrideForTesting(shortcut_override);
+
+  auto* provider = WebAppProviderBase::GetProviderBase(profile());
+
+  NavigateToURLAndWait(browser(), GetInstallableAppURL());
+
+  // Wait for OS hooks and installation to complete and the app to launch.
+  base::RunLoop run_loop_install;
+  WebAppInstallObserver observer(profile());
+  observer.SetWebAppInstalledWithOsHooksDelegate(base::BindLambdaForTesting(
+      [&](const AppId& installed_app_id) { run_loop_install.Quit(); }));
+  content::WindowedNotificationObserver app_loaded_observer(
+      content::NOTIFICATION_LOAD_COMPLETED_MAIN_FRAME,
+      content::NotificationService::AllSources());
+  const AppId app_id = InstallPwaForCurrentUrl();
+  run_loop_install.Run();
+  app_loaded_observer.Wait();
+
+  EXPECT_TRUE(provider->registrar().IsInstalled(app_id));
+  EXPECT_EQ(provider->registrar().GetAppShortName(app_id),
+            GetInstallableAppName());
+
+#if defined(OS_WIN)
+  std::wstring_convert<std::codecvt_utf8<wchar_t>> converter;
+  std::wstring shortcut_filename = converter.from_bytes(
+      provider->registrar().GetAppShortName(app_id) + ".lnk");
+  base::FilePath desktop_shortcut_path = desktop_dir.Append(shortcut_filename);
+  base::FilePath app_menu_shortcut_path =
+      application_dir.Append(shortcut_filename);
+  EXPECT_TRUE(base::PathExists(desktop_shortcut_path));
+  EXPECT_TRUE(base::PathExists(app_menu_shortcut_path));
+#elif defined(OS_MAC)
+  std::string shortcut_filename =
+      provider->registrar().GetAppShortName(app_id) + ".app";
+  base::FilePath app_shortcut_path = application_dir.Append(shortcut_filename);
+  EXPECT_TRUE(base::PathExists(app_shortcut_path));
+#elif defined(OS_LINUX)
+  std::string shortcut_filename = "chrome-" + app_id + "-Default.desktop";
+  base::FilePath desktop_shortcut_path = desktop_dir.Append(shortcut_filename);
+  base::FilePath app_menu_shortcut_path =
+      application_dir.Append("applications").Append(shortcut_filename);
+  EXPECT_TRUE(base::PathExists(desktop_shortcut_path));
+#endif
+
+  // Unistall the web app
+  base::RunLoop run_loop_uninstall;
+  provider->install_finalizer().UninstallWebApp(
+      app_id, webapps::WebappUninstallSource::kAppMenu,
+      base::BindLambdaForTesting([&](bool uninstalled) {
+        DCHECK(uninstalled);
+        run_loop_uninstall.Quit();
+      }));
+  run_loop_uninstall.Run();
+
+#if defined(OS_WIN)
+  EXPECT_FALSE(base::PathExists(desktop_shortcut_path));
+  EXPECT_FALSE(base::PathExists(app_menu_shortcut_path));
+#elif defined(OS_MAC)
+  EXPECT_FALSE(base::PathExists(app_shortcut_path));
+#elif defined(OS_LINUX)
+  EXPECT_FALSE(base::PathExists(desktop_shortcut_path));
+#endif
+}  // namespace web_app
+#endif
+
 // Tests that reparenting the last browser tab doesn't close the browser window.
 IN_PROC_BROWSER_TEST_F(WebAppBrowserTest, ReparentLastBrowserTab) {
   const GURL app_url = GetSecureAppURL();

chrome/browser/ui/web_applications/web_app_controller_browsertest.h

@@ -83,6 +83,8 @@ class WebAppControllerBrowserTest : public WebAppControllerBrowserTestBase {
   ~WebAppControllerBrowserTest() override = 0;
 
  protected:
+  ScopedOsHooksSuppress os_hooks_suppress_;
+
   content::WebContents* OpenApplication(const AppId&);
 
   net::EmbeddedTestServer* https_server() { return &https_server_; }
@@ -105,8 +107,6 @@ class WebAppControllerBrowserTest : public WebAppControllerBrowserTestBase {
   // used by the NetworkService.
   content::ContentMockCertVerifier cert_verifier_;
 
-  ScopedOsHooksSuppress os_hooks_suppress_;
-
   DISALLOW_COPY_AND_ASSIGN(WebAppControllerBrowserTest);
 };
 

chrome/browser/web_applications/components/web_app_run_on_os_login_mac_unittest.mm

@@ -25,6 +25,7 @@
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #import "testing/gtest_mac.h"
+#include "third_party/abseil-cpp/absl/types/optional.h"
 
 namespace web_app {
 
@@ -96,7 +97,9 @@ void SetUp() override {
     user_data_dir_ = base::MakeAbsoluteFilePath(user_data_dir_);
     app_data_dir_ = base::MakeAbsoluteFilePath(app_data_dir_);
 
-    SetChromeAppsFolderForTesting(destination_dir_);
+    ShortcutOverrideForTesting shortcut_override;
+    shortcut_override.chrome_apps_folder = destination_dir_;
+    web_app::SetShortcutOverrideForTesting(shortcut_override);
 
     info_ = GetShortcutInfo();
     base::FilePath shim_base_name =
@@ -109,7 +112,7 @@ void SetUp() override {
 
   void TearDown() override {
     WebAppAutoLoginUtil::SetInstanceForTesting(nullptr);
-    SetChromeAppsFolderForTesting(base::FilePath());
+    web_app::SetShortcutOverrideForTesting(absl::nullopt);
     WebAppShortcutCreator::ResetHaveLocalizedAppDirNameForTesting();
     WebAppTest::TearDown();
   }

chrome/browser/web_applications/components/web_app_shortcut.cc

@@ -90,6 +90,22 @@ void DeleteMultiProfileShortcutsForAppAndPostCallback(
 
 }  // namespace
 
+ShortcutOverrideForTesting::ShortcutOverrideForTesting() = default;
+ShortcutOverrideForTesting::ShortcutOverrideForTesting(
+    const ShortcutOverrideForTesting& other) = default;
+ShortcutOverrideForTesting::~ShortcutOverrideForTesting() = default;
+
+absl::optional<ShortcutOverrideForTesting>& GetShortcutOverrideForTesting() {
+  static base::NoDestructor<absl::optional<ShortcutOverrideForTesting>>
+      g_shortcut_override;
+  return *g_shortcut_override;
+}
+
+void SetShortcutOverrideForTesting(
+    absl::optional<ShortcutOverrideForTesting> shortcut_override_for_testing) {
+  GetShortcutOverrideForTesting() = shortcut_override_for_testing;
+}
+
 ShortcutInfo::ShortcutInfo() = default;
 
 ShortcutInfo::~ShortcutInfo() {

chrome/browser/web_applications/components/web_app_shortcut.h

@@ -12,7 +12,10 @@
 #include "base/callback_forward.h"
 #include "base/containers/span.h"
 #include "base/files/file_path.h"
+#include "base/no_destructor.h"
 #include "base/sequence_checker.h"
+#include "build/build_config.h"
+#include "third_party/abseil-cpp/absl/types/optional.h"
 #include "ui/gfx/image/image_family.h"
 #include "url/gurl.h"
 
@@ -26,6 +29,27 @@ class ImageSkia;
 
 namespace web_app {
 
+struct ShortcutOverrideForTesting {
+  ShortcutOverrideForTesting(const ShortcutOverrideForTesting& other);
+  ShortcutOverrideForTesting();
+  ~ShortcutOverrideForTesting();
+#if defined(OS_WIN)
+  base::FilePath desktop;
+  base::FilePath application_menu;
+  base::FilePath quick_launch;
+  base::FilePath startup;
+#elif defined(OS_MAC)
+  base::FilePath chrome_apps_folder;
+#elif defined(OS_LINUX)
+  base::FilePath desktop;
+#else
+#endif
+};
+
+absl::optional<ShortcutOverrideForTesting>& GetShortcutOverrideForTesting();
+void SetShortcutOverrideForTesting(
+    absl::optional<ShortcutOverrideForTesting> shortcut_override_for_testing);
+
 // Represents the info required to create a shortcut for an app.
 struct ShortcutInfo {
   ShortcutInfo();

chrome/browser/web_applications/components/web_app_shortcut_linux.cc

@@ -48,6 +48,14 @@ web_app::LaunchXdgUtilityForTesting& GetInstalledLaunchXdgUtilityForTesting() {
   return *instance;
 }
 
+base::FilePath GetDesktopPath() {
+  base::FilePath desktop_path;
+  if (web_app::GetShortcutOverrideForTesting())
+    return web_app::GetShortcutOverrideForTesting()->desktop;
+  base::PathService::Get(base::DIR_USER_DESKTOP, &desktop_path);
+  return desktop_path;
+}
+
 // Result of creating app shortcut icon.
 // Success is recorded for each icon image, but the first two errors
 // are per app, so the success/error ratio might not be very meaningful.
@@ -201,8 +209,8 @@ bool CreateShortcutAtLocation(const base::FilePath location_path,
 
 bool CreateShortcutOnDesktop(const base::FilePath& shortcut_filename,
                              const std::string& contents) {
-  base::FilePath desktop_path;
-  if (!base::PathService::Get(base::DIR_USER_DESKTOP, &desktop_path)) {
+  base::FilePath desktop_path = GetDesktopPath();
+  if (desktop_path.empty()) {
     RecordCreateShortcut(CreateShortcutResult::kFailToGetDesktopPath);
     return false;
   }
@@ -213,6 +221,7 @@ bool CreateShortcutOnDesktop(const base::FilePath& shortcut_filename,
 bool CreateShortcutInAutoStart(base::Environment* env,
                                const base::FilePath& shortcut_filename,
                                const std::string& contents) {
+  DCHECK(!web_app::GetShortcutOverrideForTesting());
   base::FilePath autostart_path = AutoStart::GetAutostartDirectory(env);
   if (!base::DirectoryExists(autostart_path) &&
       !base::CreateDirectory(autostart_path)) {
@@ -231,6 +240,7 @@ bool CreateShortcutInApplicationsMenu(base::Environment* env,
                                       const std::string& contents,
                                       const base::FilePath& directory_filename,
                                       const std::string& directory_contents) {
+  DCHECK(!web_app::GetShortcutOverrideForTesting());
   base::ScopedTempDir temp_dir;
   if (!temp_dir.CreateUniqueTempDir()) {
     RecordCreateShortcut(CreateShortcutResult::kFailToCreateTempDir);
@@ -321,9 +331,9 @@ base::FilePath GetAppShortcutFilename(const base::FilePath& profile_path,
 }
 
 bool DeleteShortcutOnDesktop(const base::FilePath& shortcut_filename) {
-  base::FilePath desktop_path;
+  base::FilePath desktop_path = GetDesktopPath();
   bool result = false;
-  if (base::PathService::Get(base::DIR_USER_DESKTOP, &desktop_path))
+  if (!desktop_path.empty())
     result = base::DeleteFile(desktop_path.Append(shortcut_filename));
   return result;
 }
@@ -362,6 +372,21 @@ bool CreateDesktopShortcut(base::Environment* env,
   base::ScopedBlockingCall scoped_blocking_call(FROM_HERE,
                                                 base::BlockingType::MAY_BLOCK);
 
+  bool create_shortcut_in_startup = creation_locations.in_startup;
+  // Do not create the shortcuts in startup directory when testing because
+  // xdg-utility (which creates this shortcut) doesn't work well with temp
+  // directories.
+  if (web_app::GetShortcutOverrideForTesting())
+    create_shortcut_in_startup = false;
+
+  ApplicationsMenuLocation applications_menu_location =
+      creation_locations.applications_menu_location;
+  // Do not create the shortcuts in startup directory when testing because
+  // xdg-utility (which creates this shortcut) doesn't work well with temp
+  // directories.
+  if (web_app::GetShortcutOverrideForTesting())
+    applications_menu_location = APP_MENU_LOCATION_NONE;
+
   base::FilePath shortcut_filename;
   if (!shortcut_info.extension_id.empty()) {
     shortcut_filename = GetAppShortcutFilename(shortcut_info.profile_path,
@@ -371,11 +396,11 @@ bool CreateDesktopShortcut(base::Environment* env,
     if (creation_locations.on_desktop)
       DeleteShortcutOnDesktop(shortcut_filename);
 
-    if (creation_locations.in_startup)
+    if (create_shortcut_in_startup)
+      // if (creation_locations.in_startup)
       DeleteShortcutInAutoStart(env, shortcut_filename);
 
-    if (creation_locations.applications_menu_location !=
-        APP_MENU_LOCATION_NONE) {
+    if (applications_menu_location != APP_MENU_LOCATION_NONE) {
       DeleteShortcutInApplicationsMenu(shortcut_filename, base::FilePath());
     }
   } else {
@@ -410,7 +435,8 @@ bool CreateDesktopShortcut(base::Environment* env,
     success = CreateShortcutOnDesktop(shortcut_filename, contents);
   }
 
-  if (creation_locations.in_startup) {
+  if (create_shortcut_in_startup) {
+    // if (creation_locations.in_startup) {
     std::string contents = shell_integration_linux::GetDesktopFileContents(
         chrome_exe_path, app_name, shortcut_info.url,
         shortcut_info.extension_id, shortcut_info.title, icon_name,
@@ -419,7 +445,7 @@ bool CreateDesktopShortcut(base::Environment* env,
         CreateShortcutInAutoStart(env, shortcut_filename, contents) && success;
   }
 
-  if (creation_locations.applications_menu_location == APP_MENU_LOCATION_NONE) {
+  if (applications_menu_location == APP_MENU_LOCATION_NONE) {
     return success;
   }
 
@@ -479,9 +505,8 @@ ShortcutLocations GetExistingShortcutLocations(
   ShortcutLocations locations;
 
   // Determine whether there is a shortcut on desktop.
-  base::FilePath desktop_path;
-  // If Get returns false, just leave desktop_path empty.
-  base::PathService::Get(base::DIR_USER_DESKTOP, &desktop_path);
+  base::FilePath desktop_path = GetDesktopPath();
+
   if (!desktop_path.empty()) {
     locations.on_desktop =
         base::PathExists(desktop_path.Append(shortcut_filename));
@@ -542,8 +567,8 @@ bool DeleteAllDesktopShortcuts(base::Environment* env,
 
   bool result = true;
   // Delete shortcuts from Desktop.
-  base::FilePath desktop_path;
-  if (base::PathService::Get(base::DIR_USER_DESKTOP, &desktop_path)) {
+  base::FilePath desktop_path = GetDesktopPath();
+  if (!desktop_path.empty()) {
     std::vector<base::FilePath> shortcut_filenames_desktop =
         shell_integration_linux::GetExistingProfileShortcutFilenames(
             profile_path, desktop_path);
@@ -610,9 +635,7 @@ std::vector<base::FilePath> GetShortcutLocations(
   DCHECK(!shortcut_filename.empty());
 
   if (locations.on_desktop) {
-    base::FilePath desktop_path;
-    // If Get returns false, just leave |desktop_path| empty.
-    base::PathService::Get(base::DIR_USER_DESKTOP, &desktop_path);
+    base::FilePath desktop_path = GetDesktopPath();
     if (!desktop_path.empty()) {
       base::FilePath desktop_shortcut_path =
           desktop_path.Append(shortcut_filename);

chrome/browser/web_applications/components/web_app_shortcut_mac.h

@@ -57,9 +57,6 @@ bool AppShimLaunchDisabled();
 // Returns a path to the Chrome Apps folder in ~/Applications.
 base::FilePath GetChromeAppsFolder();
 
-// Testing method to override calls to GetChromeAppsFolder.
-void SetChromeAppsFolderForTesting(const base::FilePath& path);
-
 // Remove the specified app from the OS login item list.
 void RemoveAppShimFromLoginItems(const std::string& app_id);