Migrate profile_shortcut_manager_win to use TaskScheduler.

This patch removes all references to BrowserThread::FILE and updates
the tests to handle the fact that the shortcut manager is now truly
multithreaded.

Bug: 689520
Change-Id: I31d145c6ac1685b150a57523d2268796a351f238
Reviewed-on: https://chromium-review.googlesource.com/582367Reviewed-by: Ryan Hamilton <rch@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Bret Sepulveda <bsep@chromium.org>
Cr-Commit-Position: refs/heads/master@{#491513}

chrome/browser/profiles/profile_shortcut_manager.h

@@ -47,6 +47,9 @@ class ProfileShortcutManager {
                                      base::string16* name,
                                      base::FilePath* icon_path) = 0;
 
+  // Any time a profile is created this class might do a lot of work in the
+  // background that's rarely important to unit tests.
+  static void DisableForUnitTests();
   static bool IsFeatureEnabled();
   static ProfileShortcutManager* Create(ProfileManager* manager);
 

chrome/browser/profiles/profile_shortcut_manager_stub.cc

@@ -4,6 +4,8 @@
 
 #include "chrome/browser/profiles/profile_shortcut_manager.h"
 
+void ProfileShortcutManager::DisableForUnitTests() {}
+
 // static
 bool ProfileShortcutManager::IsFeatureEnabled() {
   return false;

chrome/browser/profiles/profile_shortcut_manager_win.cc

@@ -22,6 +22,8 @@
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/task_runner_util.h"
+#include "base/task_scheduler/post_task.h"
 #include "base/win/shortcut.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chrome_notification_types.h"
@@ -173,13 +175,10 @@ base::FilePath CreateOrUpdateShortcutIconForProfile(
     const base::FilePath& profile_path,
     const SkBitmap& avatar_bitmap_1x,
     const SkBitmap& avatar_bitmap_2x) {
-  DCHECK_CURRENTLY_ON(BrowserThread::FILE);
+  base::ThreadRestrictions::AssertIOAllowed();
 
-  if (!base::PathExists(profile_path)) {
-    LOG(ERROR) << "Profile directory " << profile_path.value()
-               << " did not exist when trying to create profile icon";
+  if (!base::PathExists(profile_path))
     return base::FilePath();
-  }
 
   std::unique_ptr<gfx::ImageFamily> family = GetAppIconImageFamily();
   if (!family)
@@ -221,11 +220,8 @@ base::FilePath CreateOrUpdateShortcutIconForProfile(
   const bool had_icon = base::PathExists(icon_path);
 
   if (!IconUtil::CreateIconFileFromImageFamily(badged_bitmaps, icon_path)) {
-    // This can happen in theory if the profile directory is deleted between the
-    // beginning of this function and here; however this is extremely unlikely
-    // and this check will help catch any regression where this call would start
-    // failing constantly.
-    NOTREACHED();
+    // This can happen if the profile directory is deleted between the beginning
+    // of this function and here.
     return base::FilePath();
   }
 
@@ -283,7 +279,7 @@ base::FilePath ConvertToLongPath(const base::FilePath& path) {
 bool IsChromeShortcut(const base::FilePath& path,
                       const base::FilePath& chrome_exe,
                       base::string16* command_line) {
-  DCHECK_CURRENTLY_ON(BrowserThread::FILE);
+  base::ThreadRestrictions::AssertIOAllowed();
 
   if (path.Extension() != installer::kLnkExt)
     return false;
@@ -363,8 +359,7 @@ bool RenameDesktopShortcut(const base::FilePath& old_shortcut_path,
   return true;
 }
 
-// Renames an existing Chrome desktop profile shortcut. Must be called on the
-// FILE thread.
+// Renames an existing Chrome desktop profile shortcut.
 // |profile_shortcuts| are Chrome desktop shortcuts for the profile (there can
 // be several).
 // |desktop_contents| is the collection of all user desktop shortcuts
@@ -379,7 +374,7 @@ void RenameChromeDesktopShortcutForProfile(
     std::set<base::FilePath>* desktop_contents) {
   DCHECK(profile_shortcuts);
   DCHECK(desktop_contents);
-  DCHECK_CURRENTLY_ON(BrowserThread::FILE);
+  base::ThreadRestrictions::AssertIOAllowed();
 
   base::FilePath user_shortcuts_directory;
   base::FilePath system_shortcuts_directory;
@@ -480,11 +475,11 @@ struct CreateOrUpdateShortcutsParams {
 // Updates all desktop shortcuts for the given profile to have the specified
 // parameters. If |params.create_mode| is CREATE_WHEN_NONE_FOUND, a new shortcut
 // is created if no existing ones were found. Whether non-profile shortcuts
-// should be updated is specified by |params.action|. Must be called on the FILE
-// thread.
+// should be updated is specified by |params.action|. File and COM operations
+// must be allowed on the calling thread.
 void CreateOrUpdateDesktopShortcutsAndIconForProfile(
     const CreateOrUpdateShortcutsParams& params) {
-  DCHECK_CURRENTLY_ON(BrowserThread::FILE);
+  base::ThreadRestrictions::AssertIOAllowed();
 
   const base::FilePath shortcut_icon =
       CreateOrUpdateShortcutIconForProfile(params.profile_path,
@@ -585,10 +580,10 @@ bool ChromeDesktopShortcutsExist(const base::FilePath& chrome_exe) {
 // Deletes all desktop shortcuts for the specified profile. If
 // |ensure_shortcuts_remain| is true, then a regular non-profile shortcut will
 // be created if this function would otherwise delete the last Chrome desktop
-// shortcut(s). Must be called on the FILE thread.
+// shortcut(s). File and COM operations must be allowed on the calling thread.
 void DeleteDesktopShortcuts(const base::FilePath& profile_path,
                             bool ensure_shortcuts_remain) {
-  DCHECK_CURRENTLY_ON(BrowserThread::FILE);
+  base::ThreadRestrictions::AssertIOAllowed();
 
   base::FilePath chrome_exe;
   if (!PathService::Get(base::FILE_EXE, &chrome_exe)) {
@@ -632,9 +627,10 @@ void DeleteDesktopShortcuts(const base::FilePath& profile_path,
 }
 
 // Returns true if profile at |profile_path| has any shortcuts. Does not
-// consider non-profile shortcuts. Must be called on the FILE thread.
+// consider non-profile shortcuts. File and COM operations must be allowed on
+// the calling thread.
 bool HasAnyProfileShortcuts(const base::FilePath& profile_path) {
-  DCHECK_CURRENTLY_ON(BrowserThread::FILE);
+  base::ThreadRestrictions::AssertIOAllowed();
 
   base::FilePath chrome_exe;
   if (!PathService::Get(base::FILE_EXE, &chrome_exe)) {
@@ -778,8 +774,19 @@ base::string16 CreateProfileShortcutFlags(const base::FilePath& profile_path) {
 }  // namespace internal
 }  // namespace profiles
 
+namespace {
+bool disabled_for_unit_tests = false;
+}
+
+void ProfileShortcutManager::DisableForUnitTests() {
+  disabled_for_unit_tests = true;
+}
+
 // static
 bool ProfileShortcutManager::IsFeatureEnabled() {
+  if (disabled_for_unit_tests)
+    return false;
+
   base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
   if (command_line->HasSwitch(switches::kEnableProfileShortcutManager))
     return true;
@@ -830,17 +837,17 @@ void ProfileShortcutManagerWin::CreateProfileShortcut(
 
 void ProfileShortcutManagerWin::RemoveProfileShortcuts(
     const base::FilePath& profile_path) {
-  BrowserThread::PostTask(
-      BrowserThread::FILE, FROM_HERE,
-      base::Bind(&DeleteDesktopShortcuts, profile_path, false));
+  base::CreateCOMSTATaskRunnerWithTraits({base::MayBlock()})
+      ->PostTask(FROM_HERE,
+                 base::Bind(&DeleteDesktopShortcuts, profile_path, false));
 }
 
 void ProfileShortcutManagerWin::HasProfileShortcuts(
     const base::FilePath& profile_path,
     const base::Callback<void(bool)>& callback) {
-  BrowserThread::PostTaskAndReplyWithResult(
-      BrowserThread::FILE, FROM_HERE,
-      base::Bind(&HasAnyProfileShortcuts, profile_path), callback);
+  base::PostTaskAndReplyWithResult(
+      base::CreateCOMSTATaskRunnerWithTraits({base::MayBlock()}).get(),
+      FROM_HERE, base::Bind(&HasAnyProfileShortcuts, profile_path), callback);
 }
 
 void ProfileShortcutManagerWin::GetShortcutProperties(
@@ -905,10 +912,9 @@ void ProfileShortcutManagerWin::OnProfileWasRemoved(
         IGNORE_NON_PROFILE_SHORTCUTS);
   }
 
-  BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE,
-                          base::Bind(&DeleteDesktopShortcuts,
-                                     profile_path,
-                                     deleting_down_to_last_profile));
+  base::CreateCOMSTATaskRunnerWithTraits({base::MayBlock()})
+      ->PostTask(FROM_HERE, base::Bind(&DeleteDesktopShortcuts, profile_path,
+                                       deleting_down_to_last_profile));
 }
 
 void ProfileShortcutManagerWin::OnProfileNameChanged(
@@ -996,14 +1002,15 @@ void ProfileShortcutManagerWin::CreateOrUpdateShortcutsForProfileAtPath(
           profiles::GetDefaultAvatarIconResourceIDAtIndex(icon_index);
       const int resource_id_2x = kProfileAvatarIconResources2x[icon_index];
       // Make a copy of the SkBitmaps to ensure that we can safely use the image
-      // data on the FILE thread.
+      // data on the thread we post to.
       params.avatar_image_1x = GetImageResourceSkBitmapCopy(resource_id_1x);
       params.avatar_image_2x = GetImageResourceSkBitmapCopy(resource_id_2x);
     }
   }
-  BrowserThread::PostTask(
-      BrowserThread::FILE, FROM_HERE,
-      base::Bind(&CreateOrUpdateDesktopShortcutsAndIconForProfile, params));
+  base::CreateCOMSTATaskRunnerWithTraits({base::MayBlock()})
+      ->PostTask(
+          FROM_HERE,
+          base::Bind(&CreateOrUpdateDesktopShortcutsAndIconForProfile, params));
 
   entry->SetShortcutName(params.profile_name);
 }

chrome/test/base/chrome_unit_test_suite.cc

@@ -10,6 +10,7 @@
 #include "base/process/process_handle.h"
 #include "build/build_config.h"
 #include "chrome/browser/chrome_content_browser_client.h"
+#include "chrome/browser/profiles/profile_shortcut_manager.h"
 #include "chrome/browser/ui/webui/chrome_web_ui_controller_factory.h"
 #include "chrome/browser/update_client/chrome_update_query_params_delegate.h"
 #include "chrome/common/chrome_content_client.h"
@@ -112,6 +113,7 @@ void ChromeUnitTestSuite::Initialize() {
   InitializeResourceBundle();
 
   base::DiscardableMemoryAllocator::SetInstance(&discardable_memory_allocator_);
+  ProfileShortcutManager::DisableForUnitTests();
 }
 
 void ChromeUnitTestSuite::Shutdown() {