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..c49c5641facc50fbb7fcdeae90cfefda1f0c98ea 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 {
@@ -21,6 +22,13 @@ namespace {
 
 static const char kFileContents[] = "This is a target.";
 static const char kFileContents2[] = "This is another target.";
+const uint32 kPropertiesAll = ShortcutProperties::PROPERTIES_TARGET |

[gab, 2014/01/02 21:01:46]

I would say it's best to put this as the last value of the ShortcutProperties
enum itself;

e.g. PROPERTIES_ALL = PROPERTIES_TARGET | PROPERTIES_WORKING_DIR | ...;

This will make it more obvious to someone that adds a new property that they
should add it to PROPERTIES_ALL as well.

[huangs, 2014/01/02 23:22:37]

Going a step further, and defining specialized masks.

+    ShortcutProperties::PROPERTIES_WORKING_DIR |
+    ShortcutProperties::PROPERTIES_ARGUMENTS |
+    ShortcutProperties::PROPERTIES_DESCRIPTION |
+    ShortcutProperties::PROPERTIES_ICON |
+    ShortcutProperties::PROPERTIES_APP_ID |
+    ShortcutProperties::PROPERTIES_DUAL_MODE;
 
 class ShortcutTest : public testing::Test {
  protected:
@@ -80,6 +88,40 @@ class ShortcutTest : public testing::Test {
 
 }  // namespace
 
+TEST_F(ShortcutTest, CreateAndResolveShortcutProperties) {

[huangs, 2014/01/02 20:00:01]

This test takes 160 ms to 170 ms to run.

+  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, kPropertiesAll, &properties_read_1));
+  EXPECT_EQ(link_properties_.options, kPropertiesAll);

[gab, 2014/01/02 21:01:46]

This will fail before Win7.

[huangs, 2014/01/02 23:22:37]

Ah yes. Good catch!

+  EXPECT_EQ(link_properties_.target, properties_read_1.target);
+  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);

[gab, 2014/01/02 21:01:46]

Also test working_dir and icon_index.

Please put the checks in the same order as the properties above for easy visual
check that everything is being tested.

[huangs, 2014/01/02 23:22:37]

Done.

+  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);
+  }
+
+  FilePath file_2(temp_dir_.path().Append(L"Link2.lnk"));

[gab, 2014/01/02 21:01:46]

I don't think testing this twice has any added value. The purpose of the second
set of properties is to test update/replace, not double create.

[huangs, 2014/01/02 23:22:37]

Done; replacing this with a simpler shortcut.

+  ASSERT_TRUE(CreateOrUpdateShortcutLink(
+      file_2, link_properties_2_, SHORTCUT_CREATE_ALWAYS));
+  ShortcutProperties properties_read_2;
+  ASSERT_TRUE(
+      ResolveShortcutProperties(file_2, kPropertiesAll, &properties_read_2));
+  EXPECT_EQ(link_properties_2_.options, kPropertiesAll);
+  EXPECT_EQ(link_properties_2_.target, properties_read_2.target);
+  EXPECT_EQ(link_properties_2_.arguments, properties_read_2.arguments);
+  EXPECT_EQ(link_properties_2_.description, properties_read_2.description);
+  EXPECT_EQ(link_properties_2_.icon, properties_read_2.icon);
+  if (GetVersion() >= VERSION_WIN7) {
+    EXPECT_EQ(link_properties_2_.app_id, properties_read_2.app_id);
+    EXPECT_EQ(link_properties_2_.dual_mode, properties_read_2.dual_mode);
+  }
+}
+

[gab, 2014/01/02 21:01:46]

Please also add a test for resolving a shortcut with no special properties set.

[huangs, 2014/01/02 23:22:37]

Done.

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