ProfileShortcutManagerWin: Don't create user level shortcut when system level one exists.

chrome/browser/profiles/profile_shortcut_manager_unittest_win.cc

 // Copyright (c) 2012 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 <objbase.h>  // For CoInitialize().
 
+#include "base/base_paths.h"
 #include "base/file_util.h"
 #include "base/location.h"
 #include "base/message_loop.h"
 #include "base/path_service.h"
 #include "base/test/scoped_path_override.h"
 #include "base/string16.h"
 #include "base/test/test_shortcut_win.h"
 #include "base/win/shortcut.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_manager.h"
@@ skipping 11 matching lines @@
 using content::BrowserThread;
 
 class ProfileShortcutManagerTest : public testing::Test {
  protected:
   ProfileShortcutManagerTest()
       : distribution_(NULL),
         ui_thread_(BrowserThread::UI, &message_loop_),
         file_thread_(BrowserThread::FILE, &message_loop_),
         profile_shortcut_manager_(NULL),
         profile_info_cache_(NULL),
-        fake_user_desktop_(base::DIR_USER_DESKTOP) {
+        fake_user_desktop_(base::DIR_USER_DESKTOP),
+        fake_system_desktop_(base::DIR_COMMON_DESKTOP) {
   }
 
   virtual void SetUp() OVERRIDE {
     CoInitialize(NULL);
     distribution_ = BrowserDistribution::GetDistribution();
     ASSERT_TRUE(distribution_ != NULL);
 
     TestingBrowserProcess* browser_process =
         TestingBrowserProcess::GetGlobal();
     profile_manager_.reset(new TestingProfileManager(browser_process));
     ASSERT_TRUE(profile_manager_->SetUp());
     profile_info_cache_ = profile_manager_->profile_info_cache();
     profile_shortcut_manager_.reset(
         ProfileShortcutManager::Create(profile_manager_->profile_manager()));
 
     ASSERT_TRUE(PathService::Get(base::FILE_EXE, &exe_path_));
     ASSERT_TRUE(ShellUtil::GetShortcutPath(ShellUtil::SHORTCUT_LOCATION_DESKTOP,
                                            distribution_,
                                            ShellUtil::CURRENT_USER,
                                            &shortcuts_directory_));
-
+    ASSERT_TRUE(ShellUtil::GetShortcutPath(ShellUtil::SHORTCUT_LOCATION_DESKTOP,
+                                           distribution_,
+                                           ShellUtil::SYSTEM_LEVEL,
+                                           &system_shortcuts_directory_));
     profile_1_name_ = L"My profile";
     profile_1_path_ = CreateProfileDirectory(profile_1_name_);
     profile_2_name_ = L"My profile 2";
     profile_2_path_ = CreateProfileDirectory(profile_2_name_);
     profile_3_name_ = L"My profile 3";
     profile_3_path_ = CreateProfileDirectory(profile_3_name_);
   }
 
   virtual void TearDown() OVERRIDE {
     message_loop_.RunUntilIdle();
@@ skipping 139 matching lines @@
     product.AddDefaultShortcutProperties(exe_path_, &properties);
     properties.set_shortcut_name(shortcut_name);
     EXPECT_TRUE(ShellUtil::CreateOrUpdateShortcut(
         ShellUtil::SHORTCUT_LOCATION_DESKTOP, distribution_, properties,
         ShellUtil::SHELL_SHORTCUT_CREATE_ALWAYS)) << location.ToString();
     EXPECT_TRUE(file_util::PathExists(shortcut_path)) << location.ToString();
 
     return shortcut_path;
   }
 
+  FilePath CreateRegularSystemLevelShortcut(
+      const tracked_objects::Location& location) {
+    installer::Product product(distribution_);
+    ShellUtil::ShortcutProperties properties(ShellUtil::SYSTEM_LEVEL);
+    product.AddDefaultShortcutProperties(exe_path_, &properties);
+    EXPECT_TRUE(ShellUtil::CreateOrUpdateShortcut(
+        ShellUtil::SHORTCUT_LOCATION_DESKTOP, distribution_, properties,
+        ShellUtil::SHELL_SHORTCUT_CREATE_ALWAYS)) << location.ToString();
+    const FilePath system_level_shortcut_path =
+        system_shortcuts_directory_.Append(distribution_->GetAppShortCutName() +
+                                           installer::kLnkExt);
+    EXPECT_TRUE(file_util::PathExists(system_level_shortcut_path))
+        << location.ToString();
+    return system_level_shortcut_path;
+  }
+
   void RenameProfile(const tracked_objects::Location& location,
                      const FilePath& profile_path,
                      const string16& new_profile_name) {
     const size_t profile_index =
         profile_info_cache_->GetIndexOfProfileWithPath(profile_2_path_);
     ASSERT_NE(std::string::npos, profile_index);
     ASSERT_NE(profile_info_cache_->GetNameOfProfileAtIndex(profile_index),
               new_profile_name);
     profile_info_cache_->SetNameOfProfileAtIndex(profile_index,
                                                  new_profile_name);
     RunPendingTasks();
   }
 
   BrowserDistribution* distribution_;
   MessageLoopForUI message_loop_;
   content::TestBrowserThread ui_thread_;
   content::TestBrowserThread file_thread_;
   scoped_ptr<TestingProfileManager> profile_manager_;
   scoped_ptr<ProfileShortcutManager> profile_shortcut_manager_;
   ProfileInfoCache* profile_info_cache_;
+  FilePath exe_path_;
   base::ScopedPathOverride fake_user_desktop_;
-  FilePath exe_path_;
   FilePath shortcuts_directory_;
+  base::ScopedPathOverride fake_system_desktop_;
+  FilePath system_shortcuts_directory_;
   string16 profile_1_name_;
   FilePath profile_1_path_;
   string16 profile_2_name_;
   FilePath profile_2_path_;
   string16 profile_3_name_;
   FilePath profile_3_path_;
 };
 
 TEST_F(ProfileShortcutManagerTest, ShortcutFilename) {
   const string16 kProfileName = L"Harry";
@@ skipping 130 matching lines @@
   profile_info_cache_->DeleteProfileFromCache(profile_2_path_);
   RunPendingTasks();
 
   // Verify that the remaining shortcut does not have a profile name.
   ValidateNonProfileShortcut(FROM_HERE);
   // Verify that shortcuts with profile names do not exist.
   EXPECT_FALSE(file_util::PathExists(profile_1_shortcut_path));
   EXPECT_FALSE(file_util::PathExists(profile_2_shortcut_path));
 }
 
 TEST_F(ProfileShortcutManagerTest, DeleteOnlyProfileWithShortcuts) {

[gab, 2013/01/15 23:09:14]

Is this really the desired outcome (i.e. that if the only profile with a
shortcut is deleted we end up with no shortcut?) If so then:
1) This is weird, perhaps...
2) The last test tests the same thing, but in a scenario where there is also a
system-level shortcut present (but I don't see why that would matter since we
never recreate shortcuts apparently...); hence the last test wouldn't add more
test coverage.

[Alexei Svitkine (slow), 2013/01/15 23:34:43]

If there's *three* (or more) profiles and only one of them has a shortcut and
you delete the one that does, its shortcut should be deleted and no other
shortcuts should be re-created. This is what the test is testing.

This is different when you have *two* profiles to start with, because then
you're deleting back to a single-profile mode. This is what other tests are
testing.
No it doesn't, the last test involves there being two shortcuts initially,
deleting down to single-profile mode, which should result in a nonprofile
shortcut existing if either profile had a shortcut.

See DeleteSecondToLastProfileWithoutShortcut and
DeleteSecondToLastProfileWithShortcut tests.

[gab, 2013/01/16 17:44:00]

Ah, awesome :)!

   SetupAndCreateTwoShortcuts(FROM_HERE);
   CreateProfileWithShortcut(FROM_HERE, profile_3_name_, profile_3_path_);
 
   const FilePath non_profile_shortcut_path =
       GetDefaultShortcutPathForProfile(string16());
   const FilePath profile_1_shortcut_path =
       GetDefaultShortcutPathForProfile(profile_1_name_);
   const FilePath profile_2_shortcut_path =
       GetDefaultShortcutPathForProfile(profile_2_name_);
   const FilePath profile_3_shortcut_path =
@@ skipping 219 matching lines @@
 
   // Delete the shortcut and check that the function returns false.
   const FilePath profile_2_shortcut_path =
       GetDefaultShortcutPathForProfile(profile_2_name_);
   ASSERT_TRUE(file_util::Delete(profile_2_shortcut_path, false));
   EXPECT_FALSE(file_util::PathExists(profile_2_shortcut_path));
   profile_shortcut_manager_->HasProfileShortcuts(profile_2_path_, callback);
   RunPendingTasks();
   EXPECT_FALSE(result.has_shortcuts);
 }
+
+TEST_F(ProfileShortcutManagerTest, ProfileShortcutsWithSystemLevelShortcut) {
+  const FilePath system_level_shortcut_path =
+      CreateRegularSystemLevelShortcut(FROM_HERE);
+
+  // Create the initial profile.
+  profile_info_cache_->AddProfileToCache(profile_1_path_, profile_1_name_,
+                                         string16(), 0, false);
+  RunPendingTasks();
+  ASSERT_EQ(1U, profile_info_cache_->GetNumberOfProfiles());
+
+  // Ensure system level continues to exist and user level was not created.

[gab, 2013/01/15 23:09:14]

nit: We tend to write "system-level" with a dash around the installer codebase,
please keep the same syntax here (and at a few other places in this CL with a
quick search).

[Alexei Svitkine (slow), 2013/01/16 16:11:44]

Done.

+  EXPECT_TRUE(file_util::PathExists(system_level_shortcut_path));
+  EXPECT_FALSE(file_util::PathExists(
+                   GetDefaultShortcutPathForProfile(string16())));
+
+  // Create another profile with a shortcut and ensure both profiles receive
+  // user level profile shortcuts and the system level one still exists.
+  CreateProfileWithShortcut(FROM_HERE, profile_2_name_, profile_2_path_);
+  ValidateProfileShortcut(FROM_HERE, profile_1_name_, profile_1_path_);
+  ValidateProfileShortcut(FROM_HERE, profile_2_name_, profile_2_path_);
+  EXPECT_TRUE(file_util::PathExists(system_level_shortcut_path));
+}
+
+TEST_F(ProfileShortcutManagerTest,
+       DeleteSecondToLastProfileWithSystemLevelShortcut) {
+  SetupAndCreateTwoShortcuts(FROM_HERE);
+
+  const FilePath system_level_shortcut_path =
+      CreateRegularSystemLevelShortcut(FROM_HERE);
+
+  // Delete a profile and verify that only the system level shortcut still
+  // exists.
+  profile_info_cache_->DeleteProfileFromCache(profile_1_path_);
+  RunPendingTasks();
+
+  EXPECT_TRUE(file_util::PathExists(system_level_shortcut_path));
+  EXPECT_FALSE(ProfileShortcutExistsAtDefaultPath(string16()));
+  EXPECT_FALSE(ProfileShortcutExistsAtDefaultPath(profile_1_name_));
+  EXPECT_FALSE(ProfileShortcutExistsAtDefaultPath(profile_2_name_));
+}
+
+TEST_F(ProfileShortcutManagerTest,
+       DeleteSecondToLastProfileWithShortcutWhenSystemLevelShortcutExists) {
+  SetupAndCreateTwoShortcuts(FROM_HERE);
+
+  const FilePath profile_1_shortcut_path =
+      GetDefaultShortcutPathForProfile(profile_1_name_);
+  const FilePath profile_2_shortcut_path =
+      GetDefaultShortcutPathForProfile(profile_2_name_);
+
+  // Delete the shortcut for the first profile, but keep the one for the 2nd.
+  ASSERT_TRUE(file_util::Delete(profile_1_shortcut_path, false));
+  ASSERT_FALSE(file_util::PathExists(profile_1_shortcut_path));
+  ASSERT_TRUE(file_util::PathExists(profile_2_shortcut_path));
+
+  const FilePath system_level_shortcut_path =
+      CreateRegularSystemLevelShortcut(FROM_HERE);
+
+  // Delete the profile that has a shortcut, which will exercise the non-profile
+  // shortcut creation path in |DeleteDesktopShortcutsAndIconFile()|, which is
+  // not covered by the |DeleteSecondToLastProfileWithSystemLevelShortcut| test.
+  profile_info_cache_->DeleteProfileFromCache(profile_2_path_);
+  RunPendingTasks();
+
+  // Verify that only the system level shortcut still exists.
+  EXPECT_TRUE(file_util::PathExists(system_level_shortcut_path));
+  EXPECT_FALSE(file_util::PathExists(
+                   GetDefaultShortcutPathForProfile(string16())));
+  EXPECT_FALSE(file_util::PathExists(profile_1_shortcut_path));
+  EXPECT_FALSE(file_util::PathExists(profile_2_shortcut_path));
+}