ShellUtil: Add generic methods to add/delete Windows file associations.

chrome/installer/util/shell_util_unittest.cc

Index: chrome/installer/util/shell_util_unittest.cc
diff --git a/chrome/installer/util/shell_util_unittest.cc b/chrome/installer/util/shell_util_unittest.cc
index 8ebad4caa36efa4fd7a08d6c95f08e42eab18baa..8f00af39dc33423418deb01c216722c790fee39d 100644
--- a/chrome/installer/util/shell_util_unittest.cc
+++ b/chrome/installer/util/shell_util_unittest.cc
@@ -8,7 +8,9 @@
 
 #include "base/base_paths.h"
 #include "base/base_paths_win.h"
+#include "base/command_line.h"
 #include "base/files/file_enumerator.h"
+#include "base/files/file_path.h"
 #include "base/files/file_util.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/md5.h"
@@ -17,7 +19,9 @@
 #include "base/strings/string_util.h"
 #include "base/synchronization/cancellation_flag.h"
 #include "base/test/scoped_path_override.h"
+#include "base/test/test_reg_util_win.h"
 #include "base/test/test_shortcut_win.h"
+#include "base/win/registry.h"
 #include "base/win/shortcut.h"
 #include "base/win/windows_version.h"
 #include "chrome/installer/util/browser_distribution.h"
@@ -31,6 +35,16 @@ const wchar_t kManganeseExe[] = L"manganese.exe";
 const wchar_t kIronExe[] = L"iron.exe";
 const wchar_t kOtherIco[] = L"other.ico";
 
+// For registry tests.
+const wchar_t kTestProgid[] = L"TestApp";
+const wchar_t kTestOpenCommand[] = L"C:\\test.exe";
+const wchar_t kTestFileTypeName[] = L"Test File Type";
+const wchar_t kTestIconPath[] = L"D:\\test.ico";
+const wchar_t* kTestFileExtensions[] = {
+    L"test1",
+    L"test2",
+};
+
 // TODO(huangs): Separate this into generic shortcut tests and Chrome-specific
 // tests. Specifically, we should not overly rely on getting shortcut properties
 // from product_->AddDefaultShortcutProperties().
@@ -792,6 +806,128 @@ TEST_F(ShellUtilShortcutTest, DontRemoveChromeShortcutIfPointsToAnotherChrome) {
   ASSERT_TRUE(base::PathExists(shortcut_path.DirName()));
 }
 
+class ShellUtilRegistryTest : public testing::Test {

[gab, 2014/10/01 14:31:11]

I didn't re-review the expectation below in details, I assume the code below was
moved as-is from the unittest file in patch set 16.

[Matt Giuca, 2014/10/02 09:36:34]

Correct.

+ public:
+  ShellUtilRegistryTest() {}
+
+ protected:
+  virtual void SetUp() OVERRIDE {
+    registry_overrides_.OverrideRegistry(HKEY_CURRENT_USER);
+
+    // .test2 files already have a default application.
+    base::win::RegKey key;
+    ASSERT_EQ(
+        ERROR_SUCCESS,
+        key.Create(
+            HKEY_CURRENT_USER, L"Software\\Classes\\.test2", KEY_ALL_ACCESS));
+    EXPECT_EQ(ERROR_SUCCESS, key.WriteValue(L"", L"SomeOtherApp"));
+  }
+
+  static base::CommandLine OpenCommand() {
+    base::FilePath open_command_path(kTestOpenCommand);
+    return base::CommandLine(open_command_path);
+  }
+
+  static std::set<base::string16> FileExtensions() {
+    std::set<base::string16> file_extensions;
+    for (size_t i = 0; i < arraysize(kTestFileExtensions); ++i)
+      file_extensions.insert(kTestFileExtensions[i]);
+    return file_extensions;
+  }
+
+ private:
+  registry_util::RegistryOverrideManager registry_overrides_;
+
+  DISALLOW_COPY_AND_ASSIGN(ShellUtilRegistryTest);
+};
+
+TEST_F(ShellUtilRegistryTest, AddFileAssociations) {
+  // Create file associations.
+  EXPECT_TRUE(ShellUtil::AddFileAssociations(kTestProgid,
+                                             OpenCommand(),
+                                             kTestFileTypeName,
+                                             base::FilePath(kTestIconPath),
+                                             FileExtensions()));
+
+  // Ensure that the registry keys have been correctly set.
+  base::win::RegKey key;
+  std::wstring value;
+  ASSERT_EQ(
+      ERROR_SUCCESS,
+      key.Open(HKEY_CURRENT_USER, L"Software\\Classes\\TestApp", KEY_READ));
+  EXPECT_EQ(ERROR_SUCCESS, key.ReadValue(L"", &value));
+  EXPECT_EQ(L"Test File Type", value);
+  ASSERT_EQ(ERROR_SUCCESS,
+            key.Open(HKEY_CURRENT_USER,
+                     L"Software\\Classes\\TestApp\\DefaultIcon",
+                     KEY_READ));
+  EXPECT_EQ(ERROR_SUCCESS, key.ReadValue(L"", &value));
+  EXPECT_EQ(L"D:\\test.ico,0", value);
+  ASSERT_EQ(ERROR_SUCCESS,
+            key.Open(HKEY_CURRENT_USER,
+                     L"Software\\Classes\\TestApp\\shell\\open\\command",
+                     KEY_READ));
+  EXPECT_EQ(ERROR_SUCCESS, key.ReadValue(L"", &value));
+  EXPECT_EQ(L"\"C:\\test.exe\"", value);
+
+  // .test1 should be default-associated with our test app.
+  ASSERT_EQ(
+      ERROR_SUCCESS,
+      key.Open(HKEY_CURRENT_USER, L"Software\\Classes\\.test1", KEY_READ));
+  EXPECT_EQ(ERROR_SUCCESS, key.ReadValue(L"", &value));
+  EXPECT_EQ(L"TestApp", value);
+  ASSERT_EQ(ERROR_SUCCESS,
+            key.Open(HKEY_CURRENT_USER,
+                     L"Software\\Classes\\.test1\\OpenWithProgids",
+                     KEY_READ));
+  EXPECT_TRUE(key.HasValue(L"TestApp"));
+  EXPECT_EQ(ERROR_SUCCESS, key.ReadValue(L"TestApp", &value));
+  EXPECT_EQ(L"", value);
+
+  // .test2 should still be associated with the other app (should not have been
+  // overridden). But it should have our app in its Open With list.
+  ASSERT_EQ(
+      ERROR_SUCCESS,
+      key.Open(HKEY_CURRENT_USER, L"Software\\Classes\\.test2", KEY_READ));
+  EXPECT_EQ(ERROR_SUCCESS, key.ReadValue(L"", &value));
+  EXPECT_EQ(L"SomeOtherApp", value);
+  ASSERT_EQ(ERROR_SUCCESS,
+            key.Open(HKEY_CURRENT_USER,
+                     L"Software\\Classes\\.test2\\OpenWithProgids",
+                     KEY_READ));
+  EXPECT_TRUE(key.HasValue(L"TestApp"));
+  EXPECT_EQ(ERROR_SUCCESS, key.ReadValue(L"TestApp", &value));
+  EXPECT_EQ(L"", value);
+}
+
+TEST_F(ShellUtilRegistryTest, DeleteFileAssociations) {
+  // Create file associations.
+  EXPECT_TRUE(ShellUtil::AddFileAssociations(kTestProgid,
+                                             OpenCommand(),
+                                             kTestFileTypeName,
+                                             base::FilePath(kTestIconPath),
+                                             FileExtensions()));
+
+  // Delete them.
+  EXPECT_TRUE(ShellUtil::DeleteFileAssociations(kTestProgid));
+
+  // The class key should have been completely deleted.
+  base::win::RegKey key;
+  std::wstring value;
+  ASSERT_NE(
+      ERROR_SUCCESS,
+      key.Open(HKEY_CURRENT_USER, L"Software\\Classes\\TestApp", KEY_READ));
+
+  // We don't need to delete the associations with the particular extensions

[gab, 2014/10/01 14:31:11]

It's not true (as discussed) that we should not delete them.

Add TODO to delete them (i.e. add an expectation that they're not for now with a
TODO to flip that expectation when the TODO in the code is fixed).

[Matt Giuca, 2014/10/02 09:36:34]

Done.

+  // (Windows will ignore those). However, we must ensure that .test2 is still
+  // associated with the other app.
+  ASSERT_EQ(
+      ERROR_SUCCESS,
+      key.Open(HKEY_CURRENT_USER, L"Software\\Classes\\.test2", KEY_READ));
+  EXPECT_EQ(ERROR_SUCCESS, key.ReadValue(L"", &value));
+  EXPECT_EQ(L"SomeOtherApp", value);
+}
+
 TEST(ShellUtilTest, BuildAppModelIdBasic) {
   std::vector<base::string16> components;
   BrowserDistribution* dist = BrowserDistribution::GetDistribution();