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

base/win/shortcut_unittest.cc

 // Copyright (c) 2012 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 "base/win/shortcut.h"
 
 #include <string>
 
 #include "base/file_util.h"
 #include "base/files/file_path.h"
 #include "base/files/scoped_temp_dir.h"
 #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 {
 namespace win {
 
 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:
   virtual void SetUp() OVERRIDE {
     ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
     ASSERT_TRUE(temp_dir_2_.CreateUniqueTempDir());
 
     link_file_ = temp_dir_.path().Append(L"My Link.lnk");
 
     // Shortcut 1's properties
@@ skipping 39 matching lines @@
 
   // Properties for the created shortcut.
   ShortcutProperties link_properties_;
 
   // Properties for the updated shortcut.
   ShortcutProperties link_properties_2_;
 };
 
 }  // 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);
 
   ASSERT_TRUE(CreateOrUpdateShortcutLink(
       link_file_, only_target_properties, SHORTCUT_CREATE_ALWAYS));
 
   FilePath resolved_name;
   EXPECT_TRUE(ResolveShortcut(link_file_, &resolved_name, NULL));
 
@@ skipping 171 matching lines @@
   ASSERT_TRUE(CreateOrUpdateShortcutLink(
       link_file_, link_properties_2_, SHORTCUT_REPLACE_EXISTING));
 
   ShortcutProperties expected_properties(link_properties_2_);
   expected_properties.set_arguments(link_properties_.arguments);
   ValidateShortcut(link_file_, expected_properties);
 }
 
 }  // namespace win
 }  // namespace base