Fix the path of shortcuts with an icon in the current install dir.

chrome/installer/setup/install_unittest.cc

Index: chrome/installer/setup/install_unittest.cc
diff --git a/chrome/installer/setup/install_unittest.cc b/chrome/installer/setup/install_unittest.cc
index 44ad12126b18b25e92a14a675cd2a289d1956935..ba7bdf7f7d92d855cb75d9e843334c64d425e663 100644
--- a/chrome/installer/setup/install_unittest.cc
+++ b/chrome/installer/setup/install_unittest.cc
@@ -13,6 +13,7 @@
 #include "base/files/scoped_temp_dir.h"
 #include "base/macros.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/path_service.h"
 #include "base/strings/string16.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/utf_string_conversions.h"
@@ -35,6 +36,12 @@
 
 namespace {
 
+base::FilePath GetNormalizedFilePath(const base::FilePath& path) {
+  base::FilePath normalized_path;
+  ASSERT_TRUE(base::NormalizeFilePath(path, &normalized_path));
+  return normalized_path;
+}
+
 class CreateVisualElementsManifestTest : public testing::Test {
  protected:
   void SetUp() override {
@@ -70,6 +77,12 @@ class CreateVisualElementsManifestTest : public testing::Test {
 
 class InstallShortcutTest : public testing::Test {
  protected:
+  struct UpdateShortcutsTestCase {
+    const base::FilePath::CharType* target_path;
+    const base::FilePath::CharType* icon_path;

[gab, 2016/03/18 16:21:24]

Comments on each member's meaning (relative path, etc.) and whether it's
optional (can be null)

[fdoray, 2016/03/18 17:01:01]

Done.

+    bool should_update;
+  };
+
   void SetUp() override {
     EXPECT_EQ(S_OK, CoInitialize(NULL));
 
@@ -176,6 +189,63 @@ class InstallShortcutTest : public testing::Test {
     return new installer::MasterPreferences(master_prefs);
   }
 
+  // Creates the shortcuts defined by |test_cases|. Tries to update the target
+  // path of these shortcuts to |new_target_path| using

[gab, 2016/03/18 16:21:24]

|new_target_path_relative|

[fdoray, 2016/03/18 17:01:01]

Done.

+  // UpdatePerUserShortcutsInLocation(). Verifies that the right shortcuts have
+  // been updated.
+  void TestUpdateShortcuts(const UpdateShortcutsTestCase* test_cases,
+                           size_t num_test_cases,
+                           const base::FilePath& new_target_path_relative) {
+    // Create shortcuts.
+    for (size_t i = 0; i < num_test_cases; ++i) {
+      // Make sure that the target exists.
+      const base::FilePath target_path =
+          temp_dir_.path().Append(test_cases[i].target_path);
+      ASSERT_TRUE(base::CreateDirectory(target_path.DirName()));
+      base::File file(target_path,
+                      base::File::FLAG_CREATE_ALWAYS | base::File::FLAG_WRITE);
+      ASSERT_TRUE(file.IsValid());
+      static const char kDummyData[] = "dummy";
+      ASSERT_EQ(arraysize(kDummyData),
+                file.WriteAtCurrentPos(kDummyData, arraysize(kDummyData)));

[gab, 2016/03/18 16:21:25]

Wrap lines 204-210 in
if (!base::PathExists(target_path)) {}

[fdoray, 2016/03/18 17:01:01]

Done.

+
+      // Create the shortcut.
+      base::win::ShortcutProperties properties;
+      properties.set_target(target_path);
+      properties.set_icon(temp_dir_.path().Append(test_cases[i].icon_path), 1);
+      ASSERT_TRUE(base::win::CreateOrUpdateShortcutLink(
+          user_desktop_shortcut_.InsertBeforeExtension(
+              base::SizeTToString16(i)),
+          properties, base::win::SHORTCUT_CREATE_ALWAYS));
+    }
+
+    // Update shortcuts.
+    const base::FilePath new_target_path =
+        temp_dir_.path().Append(new_target_path_relative);
+    installer::UpdatePerUserShortcutsInLocation(
+        ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_,
+        new_target_path.DirName().DirName(), new_target_path.BaseName(),
+        new_target_path);
+
+    // Verify that shortcuts were updated correctly.
+    for (size_t i = 0; i < num_test_cases; ++i) {
+      base::FilePath target_path;
+      ASSERT_TRUE(base::win::ResolveShortcut(
+          user_desktop_shortcut_.InsertBeforeExtension(
+              base::SizeTToString16(i)),
+          &target_path, nullptr));
+
+      if (test_cases[i].should_update) {
+        EXPECT_EQ(GetNormalizedFilePath(new_target_path),
+                  GetNormalizedFilePath(target_path));
+      } else {
+        EXPECT_EQ(GetNormalizedFilePath(
+                      temp_dir_.path().Append(test_cases[i].target_path)),
+                  GetNormalizedFilePath(target_path));
+      }
+    }
+  }
+
   base::win::ShortcutProperties expected_properties_;
   base::win::ShortcutProperties expected_start_menu_properties_;
 
@@ -205,12 +275,6 @@ class InstallShortcutTest : public testing::Test {
   base::FilePath system_start_menu_subdir_shortcut_;
 };
 
-base::FilePath GetNormalizedFilePath(const base::FilePath& path) {
-  base::FilePath normalized_path;
-  base::NormalizeFilePath(path, &normalized_path);
-  return normalized_path;
-}
-
 }  // namespace
 
 // Test that VisualElementsManifest.xml is not created when VisualElements are
@@ -467,151 +531,129 @@ TEST_F(InstallShortcutTest, CreateIfNoSystemLevelSomeSystemShortcutsExist) {
 }
 
 TEST_F(InstallShortcutTest, UpdatePerUserShortcuts) {

[gab, 2016/03/18 16:21:25]

UpdatePerUserChromeShortcuts

[fdoray, 2016/03/18 17:01:01]

Done.

-  static const struct TestCase {
-    const base::FilePath::CharType* relative_target_path;
-    bool should_update;
-  } kTargetPathsToUpdate[] = {
+  static const UpdateShortcutsTestCase kTestCases[] = {
+      // Target in the Chrome Canary install directory. No icon.

[gab, 2016/03/18 16:21:25]

s/Target/Shortcut target/ (same below)

(there are multiple "targets" in this test so being specific is easier to parse)

[fdoray, 2016/03/18 17:01:01]

Done.

       {FILE_PATH_LITERAL("AppData\\Local\\Google\\Chrome "
                          "SxS\\Temp\\scoped_dir\\new_chrome.exe"),
-       false},
+       nullptr, false},
       {FILE_PATH_LITERAL(
            "AppData\\Local\\Google\\Chrome SxS\\Temp\\scoped_dir\\chrome.exe"),
-       false},
+       nullptr, false},
       {FILE_PATH_LITERAL(
            "AppData\\Local\\Google\\Chrome SxS\\Application\\chrome.exe"),
-       false},
+       nullptr, false},
       {FILE_PATH_LITERAL("AppData\\Local\\Google\\Chrome "
                          "SxS\\Application\\something_else.exe"),
-       false},
+       nullptr, false},
+
+      // Target in the Chrome install directory. No icon.
       {FILE_PATH_LITERAL(
            "AppData\\Local\\Google\\Chrome\\Temp\\scoped_dir\\new_chrome.exe"),
-       true},
+       nullptr, true},
       {FILE_PATH_LITERAL(
            "AppData\\Local\\Google\\Chrome\\Temp\\scoped_dir\\chrome.exe"),
-       true},
+       nullptr, true},
       {FILE_PATH_LITERAL(
            "AppData\\Local\\Google\\Chrome\\Application\\chrome.exe"),
-       true},
+       nullptr, true},
       {FILE_PATH_LITERAL(
            "AppData\\Local\\Google\\Chrome\\Application\\something_else.exe"),
+       nullptr, false},
+

[gab, 2016/03/18 16:21:24]

Also add a section pointing to system-level Chrome the set of test cases (here
and in the other tests).

And also add a new test whose target is system-level Chrome.

[fdoray, 2016/03/18 17:01:01]

Done.

+      // Dummy target. Icon in the Chrome Canary install directory.
+      {FILE_PATH_LITERAL("dummy.exe"),
+       FILE_PATH_LITERAL(
+           "AppData\\Local\\Google\\Chrome SxS\\Application\\chrome.exe"),
+       false},
+      {FILE_PATH_LITERAL("dummy.exe"),
+       FILE_PATH_LITERAL("AppData\\Local\\Google\\Chrome "
+                         "SxS\\Application\\User Data\\Profile 1\\Google "
+                         "Profile.ico"),
        false},
-      {FILE_PATH_LITERAL("something_else.exe"), false},
-  };
 
-  // Create shortcuts.
-  for (size_t i = 0; i < arraysize(kTargetPathsToUpdate); ++i) {
-    const base::FilePath target_path =
-        temp_dir_.path().Append(kTargetPathsToUpdate[i].relative_target_path);
-    ASSERT_TRUE(base::CreateDirectory(target_path.DirName()));
-    base::File file(target_path, base::File::FLAG_CREATE);
-    ASSERT_TRUE(file.IsValid());
-
-    base::win::ShortcutProperties properties;
-    properties.set_target(target_path);
-    ASSERT_TRUE(base::win::CreateOrUpdateShortcutLink(
-        user_desktop_shortcut_.InsertBeforeExtension(base::SizeTToString16(i)),
-        properties, base::win::SHORTCUT_CREATE_ALWAYS));
-  }
+      // Dummy target. Icon in the Chrome install directory.
+      {FILE_PATH_LITERAL("dummy.exe"),
+       FILE_PATH_LITERAL(
+           "AppData\\Local\\Google\\Chrome\\Application\\chrome.exe"),
+       true},
+      {FILE_PATH_LITERAL("dummy.exe"),
+       FILE_PATH_LITERAL("AppData\\Local\\Google\\Chrome\\Application\\User "
+                         "Data\\Profile 1\\Google Profile.ico"),
+       true},
 
-  // Update shortcuts.
-  const base::FilePath new_target_path =
-      temp_dir_.path().Append(FILE_PATH_LITERAL(
-          "AppData\\Local\\Google\\Chrome\\Application\\chrome.exe"));
-  installer::UpdatePerUserShortcutsInLocation(
-      ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_,
-      new_target_path.DirName().DirName(), new_target_path.BaseName(),
-      new_target_path);
-
-  // Verify that shortcuts were updated correctly.
-  for (size_t i = 0; i < arraysize(kTargetPathsToUpdate); ++i) {
-    base::FilePath target_path;
-    ASSERT_TRUE(base::win::ResolveShortcut(
-        user_desktop_shortcut_.InsertBeforeExtension(base::SizeTToString16(i)),
-        &target_path, nullptr));
-
-    if (kTargetPathsToUpdate[i].should_update) {
-      EXPECT_EQ(GetNormalizedFilePath(new_target_path),
-                GetNormalizedFilePath(target_path));
-    } else {
-      EXPECT_EQ(GetNormalizedFilePath(temp_dir_.path().Append(
-                    kTargetPathsToUpdate[i].relative_target_path)),
-                GetNormalizedFilePath(target_path));
-    }
-  }
+      // Shortcuts that don't belong to Chrome.
+      {FILE_PATH_LITERAL("something_else.exe"), nullptr, false},
+      {FILE_PATH_LITERAL("something_else.exe"),
+       FILE_PATH_LITERAL("AppData\\Local\\Google\\Something Else.ico"), false},
+  };
+
+  TestUpdateShortcuts(
+      kTestCases, arraysize(kTestCases),
+      base::FilePath(FILE_PATH_LITERAL(
+          "AppData\\Local\\Google\\Chrome\\Application\\chrome.exe")));
 }
 
 TEST_F(InstallShortcutTest, UpdatePerUserShortcutsCanary) {

[gab, 2016/03/18 16:21:25]

UpdatePerUserCanaryShortcuts

[fdoray, 2016/03/18 17:01:01]

Done.

-  static const struct TestCase {
-    const base::FilePath::CharType* relative_target_path;
-    bool should_update;
-  } kTargetPathsToUpdate[] = {
+  static const UpdateShortcutsTestCase kTestCases[] = {
+      // Target in the Chrome Canary install directory. No icon.
       {FILE_PATH_LITERAL("AppData\\Local\\Google\\Chrome "
                          "SxS\\Temp\\scoped_dir\\new_chrome.exe"),
-       true},
+       nullptr, true},
       {FILE_PATH_LITERAL(
            "AppData\\Local\\Google\\Chrome SxS\\Temp\\scoped_dir\\chrome.exe"),
-       true},
+       nullptr, true},
       {FILE_PATH_LITERAL(
            "AppData\\Local\\Google\\Chrome SxS\\Application\\chrome.exe"),
-       true},
+       nullptr, true},
       {FILE_PATH_LITERAL("AppData\\Local\\Google\\Chrome "
                          "SxS\\Application\\something_else.exe"),
-       false},
+       nullptr, false},
+
+      // Target in the Chrome install directory. No icon.
       {FILE_PATH_LITERAL(
            "AppData\\Local\\Google\\Chrome\\Temp\\scoped_dir\\new_chrome.exe"),
-       false},
+       nullptr, false},
       {FILE_PATH_LITERAL(
            "AppData\\Local\\Google\\Chrome\\Temp\\scoped_dir\\chrome.exe"),
-       false},
+       nullptr, false},
       {FILE_PATH_LITERAL(
            "AppData\\Local\\Google\\Chrome\\Application\\chrome.exe"),
-       false},
+       nullptr, false},
       {FILE_PATH_LITERAL(
            "AppData\\Local\\Google\\Chrome\\Application\\something_else.exe"),
+       nullptr, false},
+
+      // Dummy target. Icon in the Chrome Canary install directory.
+      {FILE_PATH_LITERAL("dummy.exe"),
+       FILE_PATH_LITERAL(
+           "AppData\\Local\\Google\\Chrome SxS\\Application\\chrome.exe"),
+       true},
+      {FILE_PATH_LITERAL("dummy.exe"),
+       FILE_PATH_LITERAL("AppData\\Local\\Google\\Chrome "
+                         "SxS\\Application\\User Data\\Profile 1\\Google "
+                         "Profile.ico"),
+       true},
+
+      // Dummy target. Icon in the Chrome install directory.
+      {FILE_PATH_LITERAL("dummy.exe"),
+       FILE_PATH_LITERAL(
+           "AppData\\Local\\Google\\Chrome\\Application\\chrome.exe"),
+       false},
+      {FILE_PATH_LITERAL("dummy.exe"),
+       FILE_PATH_LITERAL("AppData\\Local\\Google\\Chrome\\Application\\User "
+                         "Data\\Profile 1\\Google Profile.ico"),
        false},
-      {FILE_PATH_LITERAL("something_else.exe"), false},
-  };
 
-  // Create shortcuts.
-  for (size_t i = 0; i < arraysize(kTargetPathsToUpdate); ++i) {
-    const base::FilePath target_path =
-        temp_dir_.path().Append(kTargetPathsToUpdate[i].relative_target_path);
-    ASSERT_TRUE(base::CreateDirectory(target_path.DirName()));
-    base::File file(target_path, base::File::FLAG_CREATE);
-    ASSERT_TRUE(file.IsValid());
-
-    base::win::ShortcutProperties properties;
-    properties.set_target(target_path);
-    ASSERT_TRUE(base::win::CreateOrUpdateShortcutLink(
-        user_desktop_shortcut_.InsertBeforeExtension(base::SizeTToString16(i)),
-        properties, base::win::SHORTCUT_CREATE_ALWAYS));
-  }
+      // Shortcuts that don't belong to Chrome.
+      {FILE_PATH_LITERAL("something_else.exe"), nullptr, false},
+      {FILE_PATH_LITERAL("something_else.exe"),
+       FILE_PATH_LITERAL("AppData\\Local\\Google\\Something Else.ico"), false},
+  };
 
-  // Update shortcuts.
-  const base::FilePath new_target_path =
-      temp_dir_.path().Append(FILE_PATH_LITERAL(
-          "AppData\\Local\\Google\\Chrome SxS\\Application\\chrome.exe"));
-  installer::UpdatePerUserShortcutsInLocation(
-      ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_,
-      new_target_path.DirName().DirName(), new_target_path.BaseName(),
-      new_target_path);
-
-  // Verify that shortcuts were updated correctly.
-  for (size_t i = 0; i < arraysize(kTargetPathsToUpdate); ++i) {
-    base::FilePath target_path;
-    ASSERT_TRUE(base::win::ResolveShortcut(
-        user_desktop_shortcut_.InsertBeforeExtension(base::SizeTToString16(i)),
-        &target_path, nullptr));
-
-    if (kTargetPathsToUpdate[i].should_update) {
-      EXPECT_EQ(GetNormalizedFilePath(new_target_path),
-                GetNormalizedFilePath(target_path));
-    } else {
-      EXPECT_EQ(GetNormalizedFilePath(temp_dir_.path().Append(
-                    kTargetPathsToUpdate[i].relative_target_path)),
-                GetNormalizedFilePath(target_path));
-    }
-  }
+  TestUpdateShortcuts(
+      kTestCases, arraysize(kTestCases),
+      base::FilePath(FILE_PATH_LITERAL(
+          "AppData\\Local\\Google\\Chrome SxS\\Application\\chrome.exe")));
 }
 
 TEST(EscapeXmlAttributeValueTest, EscapeCrazyValue) {