Added CopyRegKeyWorkItem in support of IE low rights policy fixes.

chrome/installer/util/copy_reg_key_work_item.cc

Index: chrome/installer/util/copy_reg_key_work_item.cc
diff --git a/chrome/installer/util/copy_reg_key_work_item.cc b/chrome/installer/util/copy_reg_key_work_item.cc
new file mode 100644
index 0000000000000000000000000000000000000000..ccb9d4d01caa750115e72fe9ed06b772b71fa19e
--- /dev/null
+++ b/chrome/installer/util/copy_reg_key_work_item.cc
@@ -0,0 +1,122 @@
+// Copyright (c) 2011 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 "chrome/installer/util/copy_reg_key_work_item.h"
+
+#include <shlwapi.h>
+
+#include "base/logging.h"
+#include "base/stringprintf.h"
+#include "base/win/registry.h"
+#include "chrome/installer/util/registry_key_backup.h"
+
+using base::win::RegKey;
+
+namespace {
+const REGSAM kKeyReadNoNotify = (KEY_READ) & ~(KEY_NOTIFY);
+}

[erikwright (departed), 2011/09/15 18:38:57]

}  // namespace

http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Namespaces

[grt (UTC plus 2), 2011/09/16 17:45:45]

Done.

+
+CopyRegKeyWorkItem::~CopyRegKeyWorkItem() {
+}
+
+CopyRegKeyWorkItem::CopyRegKeyWorkItem(HKEY predefined_root,
+                                       const std::wstring& source_key_path,
+                                       const std::wstring& dest_key_path)
+    : predefined_root_(predefined_root),
+      source_key_path_(source_key_path),
+      dest_key_path_(dest_key_path) {
+  // It's a safe bet that we don't want to copy or overwrite one of the root
+  // trees.
+  DCHECK(!source_key_path.empty());
+  DCHECK(!dest_key_path.empty());
+}
+
+bool CopyRegKeyWorkItem::Do() {
+  scoped_ptr<RegistryKeyBackup> backup;
+  RegKey dest_key;
+
+  // Only try to make a backup if we're not configured to ignore failures.
+  if (!ignore_failure_) {
+    // Does the key exist?
+    LONG result = dest_key.Open(predefined_root_, dest_key_path_.c_str(),

[erikwright (departed), 2011/09/15 18:38:57]

Any chance you can make this a little DRYer?

http://codesearch.google.com/codesearch#OAMlx_jo-ck/src/chrome/installer/util...

[grt (UTC plus 2), 2011/09/16 17:45:45]

That code was so damn sweet I had to do it twice, man!  Done.

+                                kKeyReadNoNotify);
+    if (result == ERROR_SUCCESS) {
+      backup.reset(new RegistryKeyBackup());
+      if (!backup->Initialize(dest_key)) {
+        LOG(ERROR) << "Failed to backup key at " << dest_key_path_;
+        return ignore_failure_;
+      }
+    } else if (result != ERROR_FILE_NOT_FOUND) {
+      LOG(ERROR) << "Failed to open key at " << dest_key_path_
+                 << " to create backup, result: " << result;
+      return ignore_failure_;
+    }
+    dest_key.Close();
+  }
+
+  // Delete the destination before attempting to copy.
+  LONG result = SHDeleteKey(predefined_root_, dest_key_path_.c_str());

[erikwright (departed), 2011/09/15 18:38:57]

Should you not, immediately after this delete succeeds, be doing
backup_.swap(backup)?

It looks as though, currently, a failure at lines 65 or 71 would leave the
registry messed up. I assume that the work item system invokes Rollback() on the
item that failed as well as previously successful items?

[grt (UTC plus 2), 2011/09/16 17:45:45]

Good catch.  Yes, the list will rollback the failed item, so this is definitely
the right thing to do.

+  if (result != ERROR_SUCCESS && result != ERROR_FILE_NOT_FOUND) {
+    LOG(ERROR) << "Failed to delete key at " << dest_key_path_ << ", result: "
+               << result;
+  } else {
+    // Make the copy.
+    result = dest_key.Create(predefined_root_, dest_key_path_.c_str(),
+                             KEY_WRITE);
+    if (result != ERROR_SUCCESS) {
+      LOG(ERROR) << "Failed to open destination key at " << dest_key_path_
+                 << ", result: " << result;
+    } else {
+      result = SHCopyKey(predefined_root_, source_key_path_.c_str(),
+                         dest_key.Handle(), 0);
+      switch (result) {
+        case ERROR_FILE_NOT_FOUND:
+          // The source didn't exist, so neither should the destination.
+          dest_key.Close();
+          SHDeleteKey(predefined_root_, dest_key_path_.c_str());
+          // Handle like a success.
+          result = ERROR_SUCCESS;
+          // -- FALL THROUGH TO SUCCESS CASE --
+        case ERROR_SUCCESS:
+          // We've succeeded, so remember any backup we may have made.
+          backup_.swap(backup);
+          break;
+        default:
+          LOG(ERROR) << "Failed to copy key from " << source_key_path_ << " to "
+                     << dest_key_path_ << ", result: " << result;
+          break;
+      }
+    }
+  }
+
+  return ignore_failure_ ? true : (result == ERROR_SUCCESS);
+}
+
+void CopyRegKeyWorkItem::Rollback() {
+  if (ignore_failure_)

[erikwright (departed), 2011/09/15 18:38:57]

Is it really up to each task to do this?

[grt (UTC plus 2), 2011/09/16 17:45:45]

Sadly, yes.  Overhauling WorkItemList is outside of the scope of this CL.

+    return;
+
+  // Delete anything in the key before restoring the backup in case someone else
+  // put new data in the key after Do().
+  LONG result = SHDeleteKey(predefined_root_, dest_key_path_.c_str());
+  if (result != ERROR_SUCCESS && result != ERROR_FILE_NOT_FOUND) {
+    LOG(ERROR) << "Failed to delete key at " << dest_key_path_
+               << " in rollback, result: " << result;

[erikwright (departed), 2011/09/15 18:38:57]

Is it intentional that we continue with the attempt to restore? I guess it's a
best-effort at that point?

[grt (UTC plus 2), 2011/09/16 17:45:45]

Yeah, I think it's consistent to go ahead and restore.  When rolling back a
WorkItemList, the failure of one item to rollback doesn't stop the rollback of
the list as a whole.

+  }
+
+  // Restore the old contents.  The restoration takes on its default security

[erikwright (departed), 2011/09/15 18:38:57]

This is also a nearly line-for-line duplicate of delete_reg_key_work_item.cc.
DRYer?

[grt (UTC plus 2), 2011/09/16 17:45:45]

Done.

+  // attributes; any custom attributes are lost.
+  if (backup_.get() != NULL) {
+    RegKey dest_key;
+    result = dest_key.Create(predefined_root_, dest_key_path_.c_str(),
+                             KEY_WRITE);
+    if (result != ERROR_SUCCESS) {
+      LOG(ERROR) << "Failed to create destination key at " << dest_key_path_
+                 << " in rollback, result: " << result;
+    } else {
+      if (!backup_->WriteTo(&dest_key))
+        LOG(ERROR) << "Failed to restore key in rollback.";
+    }
+  }
+}