Add best-effort/allow rollback flags on WorkItem.

chrome/installer/util/delete_tree_work_item_unittest.cc

Index: chrome/installer/util/delete_tree_work_item_unittest.cc
diff --git a/chrome/installer/util/delete_tree_work_item_unittest.cc b/chrome/installer/util/delete_tree_work_item_unittest.cc
index cc7ae2d1a2a78e53a334bf4051381108ec61b800..7828a20bde0eae9614d17932137ffece00c83f3a 100644
--- a/chrome/installer/util/delete_tree_work_item_unittest.cc
+++ b/chrome/installer/util/delete_tree_work_item_unittest.cc
@@ -4,230 +4,199 @@
 
 #include "chrome/installer/util/delete_tree_work_item.h"
 
-#include <windows.h>
-
-#include <fstream>
 #include <memory>
 
-#include "base/base_paths.h"
+#include "base/files/file.h"
+#include "base/files/file_path.h"
 #include "base/files/file_util.h"
 #include "base/files/scoped_temp_dir.h"
-#include "base/logging.h"
-#include "base/strings/string_util.h"
+#include "base/macros.h"
 #include "chrome/installer/util/work_item.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace {
 
+const char kTextContent[] = "delete me";

[grt (UTC plus 2), 2016/05/06 18:39:13]

nit: constexpr

[fdoray, 2016/05/10 18:35:28]

Done.

+
+class ScopedFileInUse {
+ public:
+  explicit ScopedFileInUse(const base::FilePath path)

[grt (UTC plus 2), 2016/05/06 18:39:13]

FilePath&

[fdoray, 2016/05/10 18:35:29]

n/a with new code

+      : file_in_use_(path, base::File::FLAG_OPEN | base::File::FLAG_READ) {
+    EXPECT_TRUE(file_in_use_.IsValid());
+  }
+
+ private:
+  const base::File file_in_use_;
+
+  DISALLOW_COPY_AND_ASSIGN(ScopedFileInUse);
+};
+
 class DeleteTreeWorkItemTest : public testing::Test {
  protected:
-  void SetUp() override { ASSERT_TRUE(temp_dir_.CreateUniqueTempDir()); }
+  DeleteTreeWorkItemTest() = default;
+
+  void SetUp() override {
+    ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
+
+    dir_name_ = temp_dir_.path().AppendASCII("to_be_deleted");

[grt (UTC plus 2), 2016/05/06 18:39:13]

nit: slightly better to use Append(FILE_PATH_LITERAL("to_be_deleted")) here and
below

[fdoray, 2016/05/10 18:35:28]

Done.

+    ASSERT_TRUE(base::CreateDirectory(dir_name_));
+    ASSERT_TRUE(base::PathExists(dir_name_));
+
+    dir_name_1_ = dir_name_.AppendASCII("1");
+    ASSERT_TRUE(base::CreateDirectory(dir_name_1_));
+    ASSERT_TRUE(base::PathExists(dir_name_1_));
+
+    dir_name_2_ = dir_name_.AppendASCII("2");
+    ASSERT_TRUE(base::CreateDirectory(dir_name_2_));
+    ASSERT_TRUE(base::PathExists(dir_name_2_));
+
+    file_name_1_ = dir_name_1_.AppendASCII("File_1.txt");
+    ASSERT_TRUE(
+        base::WriteFile(file_name_1_, kTextContent, sizeof(kTextContent)));
+    ASSERT_TRUE(base::PathExists(file_name_1_));
+
+    file_name_2_ = dir_name_2_.AppendASCII("File_2.txt");
+    ASSERT_TRUE(
+        base::WriteFile(file_name_2_, kTextContent, sizeof(kTextContent)));
+    ASSERT_TRUE(base::PathExists(file_name_2_));
+  }
+
+  void ExpectAllFilesExist() {
+    EXPECT_TRUE(base::PathExists(dir_name_));
+    EXPECT_TRUE(base::PathExists(dir_name_1_));
+    EXPECT_TRUE(base::PathExists(dir_name_2_));
+    EXPECT_TRUE(base::PathExists(file_name_1_));
+    EXPECT_TRUE(base::PathExists(file_name_2_));
+  }
+
+  void ExpectAllFilesDeleted() {
+    EXPECT_FALSE(base::PathExists(dir_name_));
+    EXPECT_FALSE(base::PathExists(dir_name_1_));
+    EXPECT_FALSE(base::PathExists(dir_name_2_));
+    EXPECT_FALSE(base::PathExists(file_name_1_));
+    EXPECT_FALSE(base::PathExists(file_name_2_));
+  }
 
   // The temporary directory used to contain the test operations.
   base::ScopedTempDir temp_dir_;
-};
 
-// Simple function to dump some text into a new file.
-void CreateTextFile(const std::wstring& filename,
-                    const std::wstring& contents) {
-  std::ofstream file;
-  file.open(filename.c_str());
-  ASSERT_TRUE(file.is_open());
-  file << contents;
-  file.close();
-}
+  base::FilePath dir_name_;
+  base::FilePath dir_name_1_;
+  base::FilePath dir_name_2_;
+  base::FilePath file_name_1_;
+  base::FilePath file_name_2_;
 
-const wchar_t text_content_1[] = L"delete me";
+  DISALLOW_COPY_AND_ASSIGN(DeleteTreeWorkItemTest);
+};
 
 }  // namespace
 
-// Delete a tree without key path. Everything should be deleted.
-TEST_F(DeleteTreeWorkItemTest, DeleteTreeNoKeyPath) {
-  // Create tree to be deleted.
-  base::FilePath dir_name_delete(temp_dir_.path());
-  dir_name_delete = dir_name_delete.AppendASCII("to_be_delete");
-  base::CreateDirectory(dir_name_delete);
-  ASSERT_TRUE(base::PathExists(dir_name_delete));
-
-  base::FilePath dir_name_delete_1(dir_name_delete);
-  dir_name_delete_1 = dir_name_delete_1.AppendASCII("1");
-  base::CreateDirectory(dir_name_delete_1);
-  ASSERT_TRUE(base::PathExists(dir_name_delete_1));
-
-  base::FilePath dir_name_delete_2(dir_name_delete);
-  dir_name_delete_2 = dir_name_delete_2.AppendASCII("2");
-  base::CreateDirectory(dir_name_delete_2);
-  ASSERT_TRUE(base::PathExists(dir_name_delete_2));
-
-  base::FilePath file_name_delete_1(dir_name_delete_1);
-  file_name_delete_1 = file_name_delete_1.AppendASCII("File_1.txt");
-  CreateTextFile(file_name_delete_1.value(), text_content_1);
-  ASSERT_TRUE(base::PathExists(file_name_delete_1));
-
-  base::FilePath file_name_delete_2(dir_name_delete_2);
-  file_name_delete_2 = file_name_delete_2.AppendASCII("File_2.txt");
-  CreateTextFile(file_name_delete_2.value(), text_content_1);
-  ASSERT_TRUE(base::PathExists(file_name_delete_2));
-
-  // Test Do().
+// Delete a tree without key path. Do() should delete everything and Rollback()
+// should bring back everything.
+TEST_F(DeleteTreeWorkItemTest, NoKeyPath) {
   base::ScopedTempDir temp_dir;
   ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
 
-  std::vector<base::FilePath> key_files;
+  std::vector<base::FilePath> key_paths;
   std::unique_ptr<DeleteTreeWorkItem> work_item(
-      WorkItem::CreateDeleteTreeWorkItem(dir_name_delete, temp_dir.path(),
-                                         key_files));
-  EXPECT_TRUE(work_item->Do());
+      WorkItem::CreateDeleteTreeWorkItem(dir_name_, temp_dir.path(),
+                                         key_paths));
 
-  // everything should be gone
-  EXPECT_FALSE(base::PathExists(file_name_delete_1));
-  EXPECT_FALSE(base::PathExists(file_name_delete_2));
-  EXPECT_FALSE(base::PathExists(dir_name_delete));
+  EXPECT_TRUE(work_item->Do());
+  ExpectAllFilesDeleted();
 
   work_item->Rollback();
-  // everything should come back
-  EXPECT_TRUE(base::PathExists(file_name_delete_1));
-  EXPECT_TRUE(base::PathExists(file_name_delete_2));
-  EXPECT_TRUE(base::PathExists(dir_name_delete));
+  ExpectAllFilesExist();
 }
 
+// Delete a tree without key path and with rollback disabled. Do() should delete
+// everything and Rollback() shouldn't bring back anything.
+TEST_F(DeleteTreeWorkItemTest, NoKeyPathRollbackDisabled) {
+  base::ScopedTempDir temp_dir;
+  ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
+
+  std::vector<base::FilePath> key_paths;
+  std::unique_ptr<DeleteTreeWorkItem> work_item(
+      WorkItem::CreateDeleteTreeWorkItem(dir_name_, temp_dir.path(),
+                                         key_paths));
+  work_item->set_rollback_enabled(false);
 
-// Delete a tree with keypath but not in use. Everything should be gone.
-// Rollback should bring back everything
-TEST_F(DeleteTreeWorkItemTest, DeleteTree) {
-  // Create tree to be deleted
-  base::FilePath dir_name_delete(temp_dir_.path());
-  dir_name_delete = dir_name_delete.AppendASCII("to_be_delete");
-  base::CreateDirectory(dir_name_delete);
-  ASSERT_TRUE(base::PathExists(dir_name_delete));
-
-  base::FilePath dir_name_delete_1(dir_name_delete);
-  dir_name_delete_1 = dir_name_delete_1.AppendASCII("1");
-  base::CreateDirectory(dir_name_delete_1);
-  ASSERT_TRUE(base::PathExists(dir_name_delete_1));
-
-  base::FilePath dir_name_delete_2(dir_name_delete);
-  dir_name_delete_2 = dir_name_delete_2.AppendASCII("2");
-  base::CreateDirectory(dir_name_delete_2);
-  ASSERT_TRUE(base::PathExists(dir_name_delete_2));
-
-  base::FilePath file_name_delete_1(dir_name_delete_1);
-  file_name_delete_1 = file_name_delete_1.AppendASCII("File_1.txt");
-  CreateTextFile(file_name_delete_1.value(), text_content_1);
-  ASSERT_TRUE(base::PathExists(file_name_delete_1));
-
-  base::FilePath file_name_delete_2(dir_name_delete_2);
-  file_name_delete_2 = file_name_delete_2.AppendASCII("File_2.txt");
-  CreateTextFile(file_name_delete_2.value(), text_content_1);
-  ASSERT_TRUE(base::PathExists(file_name_delete_2));
-
-  // test Do()
+  EXPECT_TRUE(work_item->Do());
+  ExpectAllFilesDeleted();
+
+  work_item->Rollback();
+  ExpectAllFilesDeleted();
+}
+
+// Delete a tree with key path not in use. Do() should delete everything and
+// Rollback() should bring back everything.
+TEST_F(DeleteTreeWorkItemTest, KeyPathNotInUse) {
   base::ScopedTempDir temp_dir;
   ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
 
-  std::vector<base::FilePath> key_files(1, file_name_delete_1);
+  std::vector<base::FilePath> key_paths(1, file_name_1_);
   std::unique_ptr<DeleteTreeWorkItem> work_item(
-      WorkItem::CreateDeleteTreeWorkItem(dir_name_delete, temp_dir.path(),
-                                         key_files));
+      WorkItem::CreateDeleteTreeWorkItem(dir_name_, temp_dir.path(),
+                                         key_paths));
+
   EXPECT_TRUE(work_item->Do());
+  ExpectAllFilesDeleted();
+
+  work_item->Rollback();
+  ExpectAllFilesExist();
+}
+
+// Delete a tree with key path not in use and rollback disabled. Do() should
+// delete everything and Rollback() shouldn't bring back anything.
+TEST_F(DeleteTreeWorkItemTest, KeyPathNotInUseRollbackDisabled) {
+  base::ScopedTempDir temp_dir;
+  ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
+
+  std::vector<base::FilePath> key_paths(1, file_name_1_);
+  std::unique_ptr<DeleteTreeWorkItem> work_item(
+      WorkItem::CreateDeleteTreeWorkItem(dir_name_, temp_dir.path(),
+                                         key_paths));
+  work_item->set_rollback_enabled(false);
 
-  // everything should be gone
-  EXPECT_FALSE(base::PathExists(file_name_delete_1));
-  EXPECT_FALSE(base::PathExists(file_name_delete_2));
-  EXPECT_FALSE(base::PathExists(dir_name_delete));
+  EXPECT_TRUE(work_item->Do());
+  ExpectAllFilesDeleted();
 
   work_item->Rollback();
-  // everything should come back
-  EXPECT_TRUE(base::PathExists(file_name_delete_1));
-  EXPECT_TRUE(base::PathExists(file_name_delete_2));
-  EXPECT_TRUE(base::PathExists(dir_name_delete));
+  ExpectAllFilesDeleted();
 }
 
-// Delete a tree with key_path in use. Everything should still be there.
-TEST_F(DeleteTreeWorkItemTest, DeleteTreeInUse) {
-  // Create tree to be deleted
-  base::FilePath dir_name_delete(temp_dir_.path());
-  dir_name_delete = dir_name_delete.AppendASCII("to_be_delete");
-  base::CreateDirectory(dir_name_delete);
-  ASSERT_TRUE(base::PathExists(dir_name_delete));
-
-  base::FilePath dir_name_delete_1(dir_name_delete);
-  dir_name_delete_1 = dir_name_delete_1.AppendASCII("1");
-  base::CreateDirectory(dir_name_delete_1);
-  ASSERT_TRUE(base::PathExists(dir_name_delete_1));
-
-  base::FilePath dir_name_delete_2(dir_name_delete);
-  dir_name_delete_2 = dir_name_delete_2.AppendASCII("2");
-  base::CreateDirectory(dir_name_delete_2);
-  ASSERT_TRUE(base::PathExists(dir_name_delete_2));
-
-  base::FilePath file_name_delete_1(dir_name_delete_1);
-  file_name_delete_1 = file_name_delete_1.AppendASCII("File_1.txt");
-  CreateTextFile(file_name_delete_1.value(), text_content_1);
-  ASSERT_TRUE(base::PathExists(file_name_delete_1));
-
-  base::FilePath file_name_delete_2(dir_name_delete_2);
-  file_name_delete_2 = file_name_delete_2.AppendASCII("File_2.txt");
-  CreateTextFile(file_name_delete_2.value(), text_content_1);
-  ASSERT_TRUE(base::PathExists(file_name_delete_2));
-
-  // Create a key path file.
-  base::FilePath key_path(dir_name_delete);
-  key_path = key_path.AppendASCII("key_file.exe");
-
-  wchar_t exe_full_path_str[MAX_PATH];
-  ::GetModuleFileNameW(NULL, exe_full_path_str, MAX_PATH);
-  base::FilePath exe_full_path(exe_full_path_str);
-
-  base::CopyFile(exe_full_path, key_path);
-  ASSERT_TRUE(base::PathExists(key_path));
-
-  VLOG(1) << "copy ourself from " << exe_full_path.value()
-          << " to " << key_path.value();
-
-  // Run the key path file to keep it in use.
-  STARTUPINFOW si = {sizeof(si)};
-  PROCESS_INFORMATION pi = {0};
-  base::FilePath::StringType writable_key_path = key_path.value();
-  ASSERT_TRUE(
-      ::CreateProcessW(NULL, &writable_key_path[0],
-                       NULL, NULL, FALSE, CREATE_NO_WINDOW | CREATE_SUSPENDED,
-                       NULL, NULL, &si, &pi));
-
-  // test Do().
-  {
-    base::ScopedTempDir temp_dir;
-    ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
-
-    std::vector<base::FilePath> key_paths(1, key_path);
-    std::unique_ptr<DeleteTreeWorkItem> work_item(
-        WorkItem::CreateDeleteTreeWorkItem(dir_name_delete, temp_dir.path(),
-                                           key_paths));
-
-    // delete should fail as file in use.
-    EXPECT_FALSE(work_item->Do());
-  }
+// Delete a tree with key path in use. Do() shouldn't delete anything.
+TEST_F(DeleteTreeWorkItemTest, KeyPathInUse) {
+  ScopedFileInUse file_in_use(file_name_1_);
 
-  {
-    base::ScopedTempDir temp_dir;
-    ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
+  base::ScopedTempDir temp_dir;
+  ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
 
-    // No key paths, the deletion should succeed.
-    std::vector<base::FilePath> key_paths;
-    std::unique_ptr<DeleteTreeWorkItem> work_item(
-        WorkItem::CreateDeleteTreeWorkItem(dir_name_delete, temp_dir.path(),
-                                           key_paths));
+  std::vector<base::FilePath> key_paths(1, file_name_1_);
+  std::unique_ptr<DeleteTreeWorkItem> work_item(
+      WorkItem::CreateDeleteTreeWorkItem(dir_name_, temp_dir.path(),
+                                         key_paths));
 
-    EXPECT_TRUE(work_item->Do());
-    work_item->Rollback();
-  }
+  // Do() should fail as the key path is in use.
+  EXPECT_FALSE(work_item->Do());
+  ExpectAllFilesExist();
+}
+
+// Delete a tree with key path in use and rollback disabled. Do() shouldn't
+// delete anything.
+TEST_F(DeleteTreeWorkItemTest, KeyPathInUseRollbackDisabled) {
+  ScopedFileInUse file_in_use(file_name_1_);
+
+  base::ScopedTempDir temp_dir;
+  ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
 
-  // verify everything is still there.
-  EXPECT_TRUE(base::PathExists(key_path));
-  EXPECT_TRUE(base::PathExists(file_name_delete_1));
-  EXPECT_TRUE(base::PathExists(file_name_delete_2));
+  std::vector<base::FilePath> key_paths(1, file_name_1_);
+  std::unique_ptr<DeleteTreeWorkItem> work_item(
+      WorkItem::CreateDeleteTreeWorkItem(dir_name_, temp_dir.path(),
+                                         key_paths));
+  work_item->set_rollback_enabled(false);
 
-  TerminateProcess(pi.hProcess, 0);
-  // make sure the handle is closed.
-  WaitForSingleObject(pi.hProcess, INFINITE);
+  // Do() should fail as the key path is in use.
+  EXPECT_FALSE(work_item->Do());
+  ExpectAllFilesExist();
 }