Chrome updates should not change shortcut icon if there is an index specified in prefs file.

BUG=none
TEST=Specify a chrome with alternative icon (using master prefs), install an update and make sure the icon doesnt change.

Review URL: http://codereview.chromium.org/455041

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@33732 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 176 insertions, 10 deletions

diff --git a/chrome/installer/installer.gyp b/chrome/installer/installer.gyp
index 41a056a..3c82765 100644
--- a/chrome/installer/installer.gyp
+++ b/chrome/installer/installer.gyp
@@ -165,6 +165,7 @@
             'util/move_tree_work_item_unittest.cc',
             'util/run_all_unittests.cc',
             'util/set_reg_value_work_item_unittest.cc',
+            'util/shell_util_unittest.cc',
             'util/work_item_list_unittest.cc',
             'util/version_unittest.cc',
           ],
diff --git a/chrome/installer/util/run_all_unittests.cc b/chrome/installer/util/run_all_unittests.cc
index b559d23..79a2e4b 100644
--- a/chrome/installer/util/run_all_unittests.cc
+++ b/chrome/installer/util/run_all_unittests.cc
@@ -2,6 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include <shlobj.h>
+
 #include "base/test/test_suite.h"
 #include "chrome/common/chrome_paths.h"
 
@@ -11,5 +13,10 @@ int main(int argc, char** argv) {
   // Register Chrome Path provider so that we can get test data dir.
   chrome::RegisterPathProvider();
 
-  return test_suite.Run();
+  if (CoInitializeEx(NULL, COINIT_APARTMENTTHREADED) != S_OK)
+    return -1;
+
+  int ret = test_suite.Run();
+  CoUninitialize();
+  return ret;
 }
diff --git a/chrome/installer/util/shell_util.cc b/chrome/installer/util/shell_util.cc
index d4b39cc..82d0fb5b 100644
--- a/chrome/installer/util/shell_util.cc
+++ b/chrome/installer/util/shell_util.cc
@@ -759,15 +759,15 @@ bool ShellUtil::UpdateChromeShortcut(const std::wstring& chrome_exe,
                                      bool create_new) {
   std::wstring chrome_path = file_util::GetDirectoryFromPath(chrome_exe);
 
+  FilePath prefs_path(chrome_path);
+  prefs_path = prefs_path.Append(installer_util::kDefaultMasterPrefs);
+  scoped_ptr<DictionaryValue> prefs(
+      installer_util::ParseDistributionPreferences(prefs_path));
+  int icon_index = 0;
+  installer_util::GetDistroIntegerPreference(prefs.get(),
+      installer_util::master_preferences::kChromeShortcutIconIndex,
+      &icon_index);
   if (create_new) {
-    FilePath prefs_path(chrome_path);
-    prefs_path = prefs_path.Append(installer_util::kDefaultMasterPrefs);
-    scoped_ptr<DictionaryValue> prefs(
-        installer_util::ParseDistributionPreferences(prefs_path));
-    int icon_index = 0;
-    installer_util::GetDistroIntegerPreference(prefs.get(),
-        installer_util::master_preferences::kChromeShortcutIconIndex,
-        &icon_index);
     return file_util::CreateShortcutLink(chrome_exe.c_str(),      // target
                                          shortcut.c_str(),        // shortcut
                                          chrome_path.c_str(),     // working dir
@@ -783,7 +783,7 @@ bool ShellUtil::UpdateChromeShortcut(const std::wstring& chrome_exe,
                                          NULL,                    // arguments
                                          description.c_str(),     // description
                                          chrome_exe.c_str(),      // icon file
-                                         0,                       // icon index
+                                         icon_index,              // icon index
                                          chrome::kBrowserAppID);  // app id
   }
 }
diff --git a/chrome/installer/util/shell_util_unittest.cc b/chrome/installer/util/shell_util_unittest.cc
new file mode 100644
index 0000000..9babd2d
--- /dev/null
+++ b/chrome/installer/util/shell_util_unittest.cc
@@ -0,0 +1,158 @@
+// Copyright (c) 2009 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 <windows.h>
+#include <shellapi.h>
+#include <shlobj.h>
+
+#include <fstream>
+#include <iostream>
+
+#include "base/file_util.h"
+#include "base/path_service.h"
+#include "base/scoped_comptr_win.h"
+#include "chrome/installer/util/master_preferences.h"
+#include "chrome/installer/util/shell_util.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace {
+bool VerifyChromeShortcut(const std::wstring& exe_path,
+                          const std::wstring& shortcut,
+                          const std::wstring& description,
+                          int icon_index) {
+  ScopedComPtr<IShellLink> i_shell_link;
+  ScopedComPtr<IPersistFile> i_persist_file;
+
+  // Get pointer to the IShellLink interface
+  bool failed = FAILED(i_shell_link.CreateInstance(CLSID_ShellLink, NULL,
+                                                   CLSCTX_INPROC_SERVER));
+  EXPECT_FALSE(failed) << "Failed to get IShellLink";
+  if (failed)
+    return false;
+
+  // Query IShellLink for the IPersistFile interface
+  failed = FAILED(i_persist_file.QueryFrom(i_shell_link));
+  EXPECT_FALSE(failed) << "Failed to get IPersistFile";
+  if (failed)
+    return false;
+
+  failed = FAILED(i_persist_file->Load(shortcut.c_str(), 0));
+  EXPECT_FALSE(failed) << "Failed to load shortcut " << shortcut.c_str();
+  if (failed)
+    return false;
+
+  wchar_t long_path[MAX_PATH] = {0};
+  wchar_t short_path[MAX_PATH] = {0};
+  failed = ((::GetLongPathName(exe_path.c_str(), long_path, MAX_PATH) == 0) ||
+            (::GetShortPathName(exe_path.c_str(), short_path, MAX_PATH) == 0));
+  EXPECT_FALSE(failed) << "Failed to get long and short path names for "
+                       << exe_path;
+  if (failed)
+    return false;
+
+  wchar_t file_path[MAX_PATH] = {0};
+  failed = ((FAILED(i_shell_link->GetPath(file_path, MAX_PATH, NULL,
+                                          SLGP_UNCPRIORITY))) ||
+            ((FilePath(file_path) != FilePath(long_path)) &&
+             (FilePath(file_path) != FilePath(short_path))));
+  EXPECT_FALSE(failed) << "File path " << file_path << " did not match with "
+                       << exe_path;
+  if (failed)
+    return false;
+
+  wchar_t desc[MAX_PATH] = {0};
+  failed = ((FAILED(i_shell_link->GetDescription(desc, MAX_PATH))) ||
+            (std::wstring(desc) != std::wstring(description)));
+  EXPECT_FALSE(failed) << "Description " << desc << " did not match with "
+                       << description;
+  if (failed)
+    return false;
+
+  wchar_t icon_path[MAX_PATH] = {0};
+  int index = 0;
+  failed = ((FAILED(i_shell_link->GetIconLocation(icon_path, MAX_PATH,
+                                                  &index))) ||
+            ((FilePath(file_path) != FilePath(long_path)) &&
+             (FilePath(file_path) != FilePath(short_path))) ||
+            (index != icon_index));
+  EXPECT_FALSE(failed);
+  if (failed)
+    return false;
+
+  return true;
+}
+
+class ShellUtilTest : public testing::Test {
+ protected:
+  virtual void SetUp() {
+    // Name a subdirectory of the user temp directory.
+    ASSERT_TRUE(PathService::Get(base::DIR_TEMP, &test_dir_));
+    test_dir_ = test_dir_.AppendASCII("ShellUtilTest");
+
+    // Create a fresh, empty copy of this test directory.
+    file_util::Delete(test_dir_, true);
+    file_util::CreateDirectoryW(test_dir_);
+    ASSERT_TRUE(file_util::PathExists(test_dir_));
+  }
+
+  virtual void TearDown() {
+    // Clean up test directory
+    ASSERT_TRUE(file_util::Delete(test_dir_, false));
+    ASSERT_FALSE(file_util::PathExists(test_dir_));
+  }
+
+  // The path to temporary directory used to contain the test operations.
+  FilePath test_dir_;
+};
+};
+
+// Test that we can open archives successfully.
+TEST_F(ShellUtilTest, UpdateChromeShortcutTest) {
+  // Create an executable in test path by copying ourself to it.
+  wchar_t exe_full_path_str[MAX_PATH];
+  EXPECT_FALSE(::GetModuleFileName(NULL, exe_full_path_str, MAX_PATH) == 0);
+  FilePath exe_full_path(exe_full_path_str);
+
+  FilePath exe_path = test_dir_.AppendASCII("setup.exe");
+  EXPECT_TRUE(file_util::CopyFile(exe_full_path, exe_path));
+
+  FilePath shortcut_path = test_dir_.AppendASCII("shortcut.lnk");
+  const std::wstring description(L"dummy description");
+  EXPECT_TRUE(ShellUtil::UpdateChromeShortcut(exe_path.value(),
+                                              shortcut_path.value(),
+                                              description, true));
+  EXPECT_TRUE(VerifyChromeShortcut(exe_path.value(),
+                                   shortcut_path.value(),
+                                   description, 0));
+
+  // Now specify an icon index in master prefs and make sure it works.
+  FilePath prefs_path = test_dir_.Append(installer_util::kDefaultMasterPrefs);
+  std::ofstream file;
+  file.open(prefs_path.value().c_str());
+  ASSERT_TRUE(file.is_open());
+  file <<
+"{"
+" \"distribution\":{"
+"   \"chrome_shortcut_icon_index\" : 1"
+" }"
+"}";
+  file.close();
+  ASSERT_TRUE(file_util::Delete(shortcut_path, false));
+  EXPECT_TRUE(ShellUtil::UpdateChromeShortcut(exe_path.value(),
+                                              shortcut_path.value(),
+                                              description, true));
+  EXPECT_TRUE(VerifyChromeShortcut(exe_path.value(),
+                                   shortcut_path.value(),
+                                   description, 1));
+
+  // Now change only description to update shortcut and make sure icon index
+  // doesn't change.
+  const std::wstring description2(L"dummy description 2");
+  EXPECT_TRUE(ShellUtil::UpdateChromeShortcut(exe_path.value(),
+                                              shortcut_path.value(),
+                                              description2, false));
+  EXPECT_TRUE(VerifyChromeShortcut(exe_path.value(),
+                                   shortcut_path.value(),
+                                   description2, 1));
+}