Update profile desktop shortcuts

chrome/browser/profiles/profile_shortcut_manager_unittest_win.cc

Index: chrome/browser/profiles/profile_shortcut_manager_unittest_win.cc
===================================================================
--- chrome/browser/profiles/profile_shortcut_manager_unittest_win.cc	(revision 152316)
+++ chrome/browser/profiles/profile_shortcut_manager_unittest_win.cc	(working copy)
@@ -8,10 +8,13 @@
 #include "base/scoped_temp_dir.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/test/base/testing_profile_manager.h"
 #include "chrome/browser/profiles/profile_shortcut_manager.h"
 #include "chrome/installer/util/browser_distribution.h"
 #include "chrome/installer/util/shell_util.h"
 #include "chrome/test/base/testing_pref_service.h"
+#include "chrome/test/base/testing_profile.h"
+#include "chrome/test/base/testing_browser_process.h"
 #include "content/public/test/test_browser_thread.h"
 #include "grit/theme_resources.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -22,7 +25,8 @@
 class ProfileShortcutManagerTest : public testing::Test {
  protected:
   ProfileShortcutManagerTest()
-      : file_thread_(BrowserThread::FILE, &message_loop_) {
+      : ui_thread_(BrowserThread::UI, &message_loop_),
+        file_thread_(BrowserThread::FILE, &message_loop_) {
   }
 
   virtual void SetUp() {
@@ -38,13 +42,20 @@
   ScopedTempDir temp_dir_;
 
   MessageLoopForUI message_loop_;
+  content::TestBrowserThread ui_thread_;
   content::TestBrowserThread file_thread_;
 };
 
 TEST_F(ProfileShortcutManagerTest, DesktopShortcutsIconExists) {
+  TestingBrowserProcess* browser_process =
+      static_cast<TestingBrowserProcess*>(g_browser_process);
+  TestingProfileManager profile_manager(browser_process);
+  ASSERT_TRUE(profile_manager.SetUp());
+  ProfileInfoCache* cache = profile_manager.profile_info_cache();
+
   // Profile shortcut manager will be NULL for non-windows platforms
   ProfileShortcutManager* profile_shortcut_manager =
-      ProfileShortcutManager::Create();
+      ProfileShortcutManager::Create(*cache);
 
   if (!profile_shortcut_manager)
     return;
@@ -58,22 +69,53 @@
   profile_shortcut_manager->CreateChromeDesktopShortcut(dest_path,
                                                         profile_name, avatar);
 
+  MessageLoop::current()->PostTask(FROM_HERE, MessageLoop::QuitClosure());
+  MessageLoop::current()->Run();
+
   ASSERT_TRUE(file_util::PathExists(dest_path.Append(
       (FILE_PATH_LITERAL("Google Profile.ico")))));
 
-  profile_shortcut_manager->DeleteChromeDesktopShortcut(dest_path);
+  profile_shortcut_manager->DeleteChromeDesktopShortcut(

[sail, 2012/08/23 18:46:44]

how about moving everything from this line and below to a separate
DeleteShortcut test?

[Halli, 2012/08/24 02:56:49]

Done.

+     dest_path, profile_name);
 
-  // TODO(hallielaine): Verify shortcut deletion
+  MessageLoop::current()->PostTask(FROM_HERE, MessageLoop::QuitClosure());
+  MessageLoop::current()->Run();
+
+  FilePath exe_path;
+  ASSERT_TRUE(PathService::Get(base::FILE_EXE, &exe_path));
+
+  FilePath shortcut;
+  string16 shortcut_name;
+  BrowserDistribution* dist = BrowserDistribution::GetDistribution();
+
+  // Get the desktop path of the current user
+  ShellUtil::GetDesktopPath(false, &shortcut);
+  // Get the name of the shortcut with profile attached
+  ShellUtil::GetChromeShortcutName(dist, false, profile_name,
+      &shortcut_name);
+  shortcut = shortcut.Append(shortcut_name);
+  // Verify that the shortcut cannot be found on the desktop
+  EXPECT_EQ(ShellUtil::VERIFY_SHORTCUT_FAILURE_UNEXPECTED,

[sail, 2012/08/23 18:46:44]

after creating the shortcut you check that the .ico file exists but after
deleting it you check that the shortcut file doesn't exist.

maybe it should be something like this:
  verify .ico and shortcut do not exist
  create_shortcut
  verify .ico and shortcut do exist
  delete shortcut
  verify .ico and shortcut do not exist

[Halli, 2012/08/24 02:56:49]

Done.

+      ShellUtil::VerifyChromeShortcut(exe_path.value(),
+          shortcut.value(), dist->GetAppDescription(), 0));
 }
 
 TEST_F(ProfileShortcutManagerTest, DesktopShortcutsLnk) {
+  TestingBrowserProcess* browser_process =
+      static_cast<TestingBrowserProcess*>(g_browser_process);
+  TestingProfileManager profile_manager(browser_process);
+  ASSERT_TRUE(profile_manager.SetUp());
+  ProfileInfoCache* cache = profile_manager.profile_info_cache();
+
   // Profile shortcut manager will be NULL for non-windows platforms
   ProfileShortcutManager* profile_shortcut_manager =
-      ProfileShortcutManager::Create();
+      ProfileShortcutManager::Create(*cache);
 
   if (!profile_shortcut_manager)
     return;
 
+  string16 profile_name = ASCIIToUTF16("My Profile");
+
   FilePath dest_path = temp_dir_.path();
   dest_path = dest_path.Append(FILE_PATH_LITERAL("New Profile 1"));
 
@@ -81,8 +123,11 @@
       GetNativeImageNamed(IDR_PROFILE_AVATAR_0);
 
   profile_shortcut_manager->CreateChromeDesktopShortcut(dest_path,
-      ASCIIToUTF16("My Profile"), avatar);
+      profile_name, avatar);
 
+  MessageLoop::current()->PostTask(FROM_HERE, MessageLoop::QuitClosure());
+  MessageLoop::current()->Run();
+
   FilePath exe_path;
   ASSERT_TRUE(PathService::Get(base::FILE_EXE, &exe_path));
 
@@ -93,7 +138,7 @@
   // Get the desktop path of the current user
   ShellUtil::GetDesktopPath(false, &shortcut);
   // Get the name of the shortcut with profile attached
-  ShellUtil::GetChromeShortcutName(dist, false, ASCIIToUTF16("My Profile"),
+  ShellUtil::GetChromeShortcutName(dist, false, profile_name,
       &shortcut_name);
   shortcut = shortcut.Append(shortcut_name);
 
@@ -101,5 +146,68 @@
       ShellUtil::VerifyChromeShortcut(exe_path.value(),
           shortcut.value(), dist->GetAppDescription(), 0));
 
-  profile_shortcut_manager->DeleteChromeDesktopShortcut(dest_path);
+  profile_shortcut_manager->DeleteChromeDesktopShortcut(

[sail, 2012/08/23 18:46:44]

should check that this succeeds 

[Halli, 2012/08/24 02:56:49]

Done.

+      dest_path, profile_name);
 }
+
+TEST_F(ProfileShortcutManagerTest, DesktopShortcutsUpdate) {
+  TestingBrowserProcess* browser_process =
+      static_cast<TestingBrowserProcess*>(g_browser_process);
+  TestingProfileManager profile_manager(browser_process);
+  ASSERT_TRUE(profile_manager.SetUp());
+  ProfileInfoCache* cache = profile_manager.profile_info_cache();
+
+  // Profile shortcut manager will be NULL for non-windows platforms
+  ProfileShortcutManager* profile_shortcut_manager =

[sail, 2012/08/23 18:46:44]

all this code is repeated for each test. this should probably be moved to
ProfileShortcutManagerTest::SetUp()

[Halli, 2012/08/24 02:56:49]

Moving this code to SetUp() causes a problem in
ProfileInfoCache::AddProfileToCache on the DictionaryPrefUpdate, it fails the
thread DCHECK, suggestions? (:

+      ProfileShortcutManager::Create(*cache);
+
+  if (!profile_shortcut_manager)
+    return;
+
+  string16 profile_name = ASCIIToUTF16("My Profile");
+  string16 new_profile_name = ASCIIToUTF16("My New Profile Name");
+
+  FilePath dest_path = cache->GetUserDataDir();
+  dest_path = dest_path.Append(FILE_PATH_LITERAL("New Profile 1"));
+
+  cache->AddProfileToCache(dest_path, profile_name, string16(), 0);
+
+  gfx::Image& avatar = ResourceBundle::GetSharedInstance().
+      GetNativeImageNamed(IDR_PROFILE_AVATAR_0);
+
+  profile_shortcut_manager->CreateChromeDesktopShortcut(dest_path,
+      profile_name, avatar);
+
+  MessageLoop::current()->PostTask(FROM_HERE, MessageLoop::QuitClosure());
+  MessageLoop::current()->Run();
+
+  // Cause an update in ProfileShortcutManager by modifying the profile info
+  // cache
+  cache->SetNameOfProfileAtIndex(
+      cache->GetIndexOfProfileWithPath(dest_path), new_profile_name);
+
+  MessageLoop::current()->PostTask(FROM_HERE, MessageLoop::QuitClosure());
+  MessageLoop::current()->Run();
+
+  FilePath exe_path;
+  ASSERT_TRUE(PathService::Get(base::FILE_EXE, &exe_path));
+
+  FilePath shortcut;

[sail, 2012/08/23 18:46:44]

name is not descriptive, declaration should be move to first use

[Halli, 2012/08/24 02:56:49]

Done.

+  string16 shortcut_name;

[sail, 2012/08/23 18:46:44]

same, declaration should be move to first use

[Halli, 2012/08/24 02:56:49]

Done.

+  BrowserDistribution* dist = BrowserDistribution::GetDistribution();
+
+  // Get the desktop path of the current user
+  ShellUtil::GetDesktopPath(false, &shortcut);
+  // Get the name of the shortcut with profile attached
+  ShellUtil::GetChromeShortcutName(dist, false, new_profile_name,
+      &shortcut_name);
+  shortcut = shortcut.Append(shortcut_name);
+
+  EXPECT_EQ(ShellUtil::VERIFY_SHORTCUT_SUCCESS,

[sail, 2012/08/23 18:46:44]

currently this test looks like this:
  <SetUp>create shortcut
  <Invoke function>change profile name
  <Test Post condition> verify shortcut has new profile name
normally a test should look like this
  <SetUp>
  <Test Precondition>
  <Invoke function>
  <Test Post condition>

Also, when your test gets to be longer than 10 lines it a good idea to refactor
so that all of the above 4 steps are clear 

[Halli, 2012/08/24 02:56:49]

Done.

+      ShellUtil::VerifyChromeShortcut(exe_path.value(),
+          shortcut.value(), dist->GetAppDescription(), 0));
+
+  profile_shortcut_manager->DeleteChromeDesktopShortcut(

[sail, 2012/08/23 18:46:44]

same as above, should check that this succeeds 

[Halli, 2012/08/24 02:56:49]

Done.

+      dest_path, new_profile_name);
+}
+