Installer: adding ResolveShortcutProperties(); updating shortcut icons during shortcut migration.

base/win/shortcut_unittest.cc

Index: base/win/shortcut_unittest.cc
diff --git a/base/win/shortcut_unittest.cc b/base/win/shortcut_unittest.cc
index eaf152eb9dcbd328716cf71a1aab5e63074c4d81..0126dc7b5030dcde7a767f8c61a9b9d34ab907cc 100644
--- a/base/win/shortcut_unittest.cc
+++ b/base/win/shortcut_unittest.cc
@@ -12,6 +12,7 @@
 #include "base/test/test_file_util.h"
 #include "base/test/test_shortcut_win.h"
 #include "base/win/scoped_com_initializer.h"
+#include "base/win/windows_version.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace base {
@@ -41,8 +42,10 @@ class ShortcutTest : public testing::Test {
       link_properties_.set_arguments(L"--magic --awesome");
       link_properties_.set_description(L"Chrome is awesome.");
       link_properties_.set_icon(link_properties_.target, 4);
-      link_properties_.set_app_id(L"Chrome");
-      link_properties_.set_dual_mode(false);
+      if (GetVersion() >= VERSION_WIN7) {

[gab, 2014/01/03 18:25:52]

I think it's good to set these unconditionally (to test that the implementation
correctly ignores them -- as opposed to trying to apply them and crash -- on
older Win versions).

[huangs, 2014/01/03 20:19:35]

Done.

+        link_properties_.set_app_id(L"Chrome");
+        link_properties_.set_dual_mode(false);
+      }
     }
 
     // Shortcut 2's properties (all different from properties of shortcut 1).
@@ -59,8 +62,10 @@ class ShortcutTest : public testing::Test {
       link_properties_2_.set_arguments(L"--super --crazy");
       link_properties_2_.set_description(L"The best in the west.");
       link_properties_2_.set_icon(icon_path_2, 0);
-      link_properties_2_.set_app_id(L"Chrome.UserLevelCrazySuffix");
-      link_properties_2_.set_dual_mode(true);
+      if (GetVersion() >= VERSION_WIN7) {

[gab, 2014/01/03 18:25:52]

Remove condition here too.

[huangs, 2014/01/03 20:19:35]

Done.

+        link_properties_2_.set_app_id(L"Chrome.UserLevelCrazySuffix");
+        link_properties_2_.set_dual_mode(true);
+      }
     }
   }
 
@@ -80,6 +85,47 @@ class ShortcutTest : public testing::Test {
 
 }  // namespace
 
+TEST_F(ShortcutTest, CreateAndResolveShortcutProperties) {
+  uint32 properties_use = ShortcutProperties::PROPERTIES_BASIC;

[gab, 2014/01/03 18:25:52]

s/properties_use/valid_properties

[huangs, 2014/01/03 20:19:35]

Done.

+  if (GetVersion() >= VERSION_WIN7)
+    properties_use |= ShortcutProperties::PROPERTIES_WIN7;
+
+  // Test all properties.
+  FilePath file_1(temp_dir_.path().Append(L"Link1.lnk"));
+  ASSERT_TRUE(CreateOrUpdateShortcutLink(
+      file_1, link_properties_, SHORTCUT_CREATE_ALWAYS));
+
+  ShortcutProperties properties_read_1;
+  ASSERT_TRUE(
+      ResolveShortcutProperties(file_1, properties_use, &properties_read_1));

[gab, 2014/01/03 18:25:52]

Still resolve on PROPERTIES_ALL and expect the method is able to skip those on
its own prior to Win7

[huangs, 2014/01/03 20:19:35]

Done.

+  EXPECT_EQ(link_properties_.options, properties_use);
+  EXPECT_EQ(link_properties_.target, properties_read_1.target);
+  EXPECT_EQ(link_properties_.working_dir, properties_read_1.working_dir);
+  EXPECT_EQ(link_properties_.arguments, properties_read_1.arguments);
+  EXPECT_EQ(link_properties_.description, properties_read_1.description);
+  EXPECT_EQ(link_properties_.icon, properties_read_1.icon);
+  EXPECT_EQ(link_properties_.icon_index, properties_read_1.icon_index);
+  if (GetVersion() >= VERSION_WIN7) {
+    EXPECT_EQ(link_properties_.app_id, properties_read_1.app_id);
+    EXPECT_EQ(link_properties_.dual_mode, properties_read_1.dual_mode);
+  }
+
+  // Test simple shortcut with no special properties set.
+  FilePath file_2(temp_dir_.path().Append(L"Link2.lnk"));
+  ShortcutProperties only_target_properties;
+  only_target_properties.set_target(link_properties_.target);
+  ASSERT_TRUE(CreateOrUpdateShortcutLink(
+      file_2, only_target_properties, SHORTCUT_CREATE_ALWAYS));
+
+  ShortcutProperties properties_read_2;
+  ASSERT_TRUE(
+      ResolveShortcutProperties(file_2, properties_use, &properties_read_2));

[gab, 2014/01/03 18:25:52]

PROPERTIES_ALL here too

[huangs, 2014/01/03 20:19:35]

Done.

+  EXPECT_EQ(properties_use, properties_read_2.options);
+  EXPECT_EQ(only_target_properties.target, properties_read_2.target);
+  EXPECT_EQ(L"", properties_read_2.arguments);
+  EXPECT_EQ(L"", properties_read_2.description);

[gab, 2014/01/03 18:25:52]

Why not test all properties?

This will be good in particular to test handling of VT_EMPTY for
dual_mode/app_id.

[huangs, 2014/01/03 20:19:35]

Test for (empty) defaults. Will need to watch try job carefully, in case
different versions of Windows do different things.

+}
+
 TEST_F(ShortcutTest, CreateAndResolveShortcut) {
   ShortcutProperties only_target_properties;
   only_target_properties.set_target(link_properties_.target);