Suppress app shortcut deletion for profile deletion tests.

Deleting app shortcuts results in time out on Windows 7 when profile
deletion tests are finished. A suppressor is added to disable them.

Bug: 1073451
Change-Id: Ic8711b6a6821da8b4581613282dd04e80a2f1f4c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2264434Reviewed-by: Esmael Elmoslimany <aee@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
Commit-Queue: Ramin Halavati <rhalavati@chromium.org>
Cr-Commit-Position: refs/heads/master@{#786627}

chrome/browser/apps/platform_apps/shortcut_manager.cc

@@ -60,8 +60,16 @@ void CreateShortcutsForApp(Profile* profile, const Extension* app) {
                            creation_locations, profile, app, base::DoNothing());
 }
 
+// Used to disable shortcut deletion syscall to prevent tests from flaking.
+bool kSuppressDeleteAllShortcutsForTesting = false;
+
 }  // namespace
 
+// static
+void AppShortcutManager::SuppressDeleteAllShortcutsForTesting() {
+  kSuppressDeleteAllShortcutsForTesting = true;
+}
+
 // static
 void AppShortcutManager::RegisterProfilePrefs(
     user_prefs::PrefRegistrySyncable* registry) {
@@ -137,8 +145,11 @@ void AppShortcutManager::OnExtensionUninstalled(
 
 void AppShortcutManager::OnProfileWillBeRemoved(
     const base::FilePath& profile_path) {
-  if (profile_path != profile_->GetPath())
+  if (profile_path != profile_->GetPath() ||
+      kSuppressDeleteAllShortcutsForTesting) {
     return;
+  }
+
   web_app::internals::GetShortcutIOTaskRunner()->PostTask(
       FROM_HERE,
       base::BindOnce(&web_app::internals::DeleteAllShortcutsForProfile,

chrome/browser/apps/platform_apps/shortcut_manager.h

@@ -51,6 +51,8 @@ class AppShortcutManager : public KeyedService,
   // ProfileAttributesStorage::Observer.
   void OnProfileWillBeRemoved(const base::FilePath& profile_path) override;
 
+  static void SuppressDeleteAllShortcutsForTesting();
+
  private:
   void UpdateShortcutsForAllAppsNow();
   void SetCurrentAppShortcutsVersion();

chrome/browser/profiles/profile_manager_browsertest.cc

@@ -14,6 +14,7 @@
 #include "base/strings/utf_string_conversions.h"
 #include "base/test/bind_test_util.h"
 #include "build/build_config.h"
+#include "chrome/browser/apps/platform_apps/shortcut_manager.h"
 #include "chrome/browser/password_manager/password_store_factory.h"
 #include "chrome/browser/profiles/profile_attributes_entry.h"
 #include "chrome/browser/profiles/profile_attributes_storage.h"
@@ -225,11 +226,16 @@ base::FilePath GetFirstNonSigninNonLockScreenAppProfile(
 
 // This file contains tests for the ProfileManager that require a heavyweight
 // InProcessBrowserTest.  These include tests involving profile deletion.
-
-// TODO(jeremy): crbug.com/103355 - These tests should be enabled on all
-// platforms.
 class ProfileManagerBrowserTest : public InProcessBrowserTest {
  protected:
+  void SetUp() override {
+    // Shortcut deletion delays tests shutdown on Win-7 and results in time out.
+    // See crbug.com/1073451.
+#if defined(OS_WIN)
+    AppShortcutManager::SuppressDeleteAllShortcutsForTesting();
+#endif
+    InProcessBrowserTest::SetUp();
+  }
   void SetUpCommandLine(base::CommandLine* command_line) override {
 #if defined(OS_CHROMEOS)
     command_line->AppendSwitch(
@@ -241,15 +247,7 @@ class ProfileManagerBrowserTest : public InProcessBrowserTest {
 // CrOS multi-profiles implementation is too different for these tests.
 #if !defined(OS_CHROMEOS)
 
-// Delete single profile and make sure a new one is created.
-// TODO(https://crbug.com/1073451) flaky on windows bots
-#if defined(OS_WIN)
-#define MAYBE_DeleteSingletonProfile DISABLED_DeleteSingletonProfile
-#else
-#define MAYBE_DeleteSingletonProfile DeleteSingletonProfile
-#endif
-IN_PROC_BROWSER_TEST_F(ProfileManagerBrowserTest,
-                       MAYBE_DeleteSingletonProfile) {
+IN_PROC_BROWSER_TEST_F(ProfileManagerBrowserTest, DeleteSingletonProfile) {
   ProfileManager* profile_manager = g_browser_process->profile_manager();
   ProfileAttributesStorage& storage =
       profile_manager->GetProfileAttributesStorage();
@@ -320,15 +318,7 @@ IN_PROC_BROWSER_TEST_F(ProfileManagerBrowserTest, DeleteInactiveProfile) {
   EXPECT_EQ(current_profile_path, last_used->GetPath());
 }
 
-// Delete current profile in a multi profile setup and make sure an existing one
-// is loaded.
-// TODO(https://crbug.com/1073451) flaky on windows.
-#if defined(OS_WIN)
-#define MAYBE_DeleteCurrentProfile DISABLED_DeleteCurrentProfile
-#else
-#define MAYBE_DeleteCurrentProfile DeleteCurrentProfile
-#endif
-IN_PROC_BROWSER_TEST_F(ProfileManagerBrowserTest, MAYBE_DeleteCurrentProfile) {
+IN_PROC_BROWSER_TEST_F(ProfileManagerBrowserTest, DeleteCurrentProfile) {
   ProfileManager* profile_manager = g_browser_process->profile_manager();
   ProfileAttributesStorage& storage =
       profile_manager->GetProfileAttributesStorage();
@@ -358,15 +348,7 @@ IN_PROC_BROWSER_TEST_F(ProfileManagerBrowserTest, MAYBE_DeleteCurrentProfile) {
   EXPECT_EQ(new_path, last_used->GetPath());
 }
 
-// Delete all profiles in a multi profile setup and make sure a new one is
-// created.
-// TODO(https://crbug.com/1073451) flaky on windows bots
-#if defined(OS_WIN)
-#define MAYBE_DeleteAllProfiles DISABLED_DeleteAllProfiles
-#else
-#define MAYBE_DeleteAllProfiles DeleteAllProfiles
-#endif
-IN_PROC_BROWSER_TEST_F(ProfileManagerBrowserTest, MAYBE_DeleteAllProfiles) {
+IN_PROC_BROWSER_TEST_F(ProfileManagerBrowserTest, DeleteAllProfiles) {
   ProfileManager* profile_manager = g_browser_process->profile_manager();
   ProfileAttributesStorage& storage =
       profile_manager->GetProfileAttributesStorage();

chrome/browser/ui/webui/profile_helper_browsertest.cc

@@ -5,6 +5,7 @@
 #include "base/bind.h"
 #include "base/run_loop.h"
 #include "build/build_config.h"
+#include "chrome/browser/apps/platform_apps/shortcut_manager.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/profiles/profile_attributes_storage.h"
 #include "chrome/browser/profiles/profile_manager.h"
@@ -97,7 +98,20 @@ class BrowserAddedObserver : public BrowserListObserver {
 
 }  // namespace
 
-using ProfileHelperTest = InProcessBrowserTest;
+class ProfileHelperTest : public InProcessBrowserTest {
+ public:
+  ProfileHelperTest() = default;
+
+ protected:
+  void SetUp() override {
+    // Shortcut deletion delays tests shutdown on Win-7 and results in time out.
+    // See crbug.com/1073451.
+#if defined(OS_WIN)
+    AppShortcutManager::SuppressDeleteAllShortcutsForTesting();
+#endif
+    InProcessBrowserTest::SetUp();
+  }
+};
 
 IN_PROC_BROWSER_TEST_F(ProfileHelperTest, OpenNewWindowForProfile) {
   BrowserList* browser_list = BrowserList::GetInstance();
@@ -140,13 +154,7 @@ IN_PROC_BROWSER_TEST_F(ProfileHelperTest, OpenNewWindowForProfile) {
 #endif
 }
 
-// TODO(https://crbug.com/1073451) flaky on windows bots
-#if defined(OS_WIN)
-#define MAYBE_DeleteSoleProfile DISABLED_DeleteSoleProfile
-#else
-#define MAYBE_DeleteSoleProfile DeleteSoleProfile
-#endif
-IN_PROC_BROWSER_TEST_F(ProfileHelperTest, MAYBE_DeleteSoleProfile) {
+IN_PROC_BROWSER_TEST_F(ProfileHelperTest, DeleteSoleProfile) {
   content::TestWebUI web_ui;
   Browser* original_browser = browser();
   ProfileAttributesStorage& storage =
@@ -170,13 +178,7 @@ IN_PROC_BROWSER_TEST_F(ProfileHelperTest, MAYBE_DeleteSoleProfile) {
   EXPECT_EQ(1u, storage.GetNumberOfProfiles());
 }
 
-// TODO(https://crbug.com/1073451) flaky on windows bots
-#if defined(OS_WIN)
-#define MAYBE_DeleteActiveProfile DISABLED_DeleteActiveProfile
-#else
-#define MAYBE_DeleteActiveProfile DeleteActiveProfile
-#endif
-IN_PROC_BROWSER_TEST_F(ProfileHelperTest, MAYBE_DeleteActiveProfile) {
+IN_PROC_BROWSER_TEST_F(ProfileHelperTest, DeleteActiveProfile) {
   content::TestWebUI web_ui;
   Browser* original_browser = browser();
   ProfileAttributesStorage& storage =