Regkey functions return error code instead of bool

chrome_frame/chrome_tab.cc

Index: chrome_frame/chrome_tab.cc
===================================================================
--- chrome_frame/chrome_tab.cc	(revision 70414)
+++ chrome_frame/chrome_tab.cc	(working copy)
@@ -320,15 +320,15 @@
       }
 
       RegKey run_once;
-      if (run_once.Create(hive, kRunOnce, KEY_READ | KEY_WRITE)) {
+      LONG ret = run_once.Create(hive, kRunOnce, KEY_READ | KEY_WRITE);
+      if (ret == ERROR_SUCCESS) {
         CommandLine run_once_cmd(chrome_launcher::GetChromeExecutablePath());
         run_once_cmd.AppendSwitchASCII(switches::kAutomationClientChannelID,
                                        "0");
         run_once_cmd.AppendSwitch(switches::kChromeFrame);
-        if (run_once.WriteValue(L"A",
-                                run_once_cmd.command_line_string().c_str())) {
-          result = S_OK;
-        }
+        ret = run_once.WriteValue(L"A",
+                                  run_once_cmd.command_line_string().c_str());
+        result = HRESULT_FROM_WIN32(ret);
       }
     } else {
       result = S_FALSE;
@@ -430,7 +430,8 @@
   HKEY parent_hive = is_system ? HKEY_LOCAL_MACHINE : HKEY_CURRENT_USER;
 
   RegKey ua_key;
-  if (ua_key.Create(parent_hive, kPostPlatformUAKey, KEY_WRITE)) {
+  LONG result = ua_key.Create(parent_hive, kPostPlatformUAKey, KEY_WRITE);
+  if (result == ERROR_SUCCESS) {
     std::wstring chrome_frame_ua_value_name = kChromeFramePrefix;
     chrome_frame_ua_value_name += GetCurrentModuleVersion();
     if (value) {
@@ -628,7 +629,7 @@
     RegKey backup_key(HKEY_LOCAL_MACHINE, backup_key_name_.c_str(),
                       KEY_READ | KEY_WRITE);
     if (backup_key.Valid()) {
-      return backup_key.WriteValue(NULL, str.GetString());
+      return backup_key.WriteValue(NULL, str.GetString()) == ERROR_SUCCESS;
     }
 
     return false;
@@ -670,15 +671,15 @@
 
     DWORD len = 0;
     DWORD reg_type = REG_NONE;
-    if (!backup_key.ReadValue(NULL, NULL, &len, &reg_type))
+    if (backup_key.ReadValue(NULL, NULL, &len, &reg_type) != ERROR_SUCCESS)
       return false;
 
     if (reg_type != REG_SZ)
       return false;
 
     size_t wchar_count = 1 + len / sizeof(wchar_t);
-    if (!backup_key.ReadValue(NULL, WriteInto(sddl, wchar_count), &len,
-                              &reg_type)) {
+    if (backup_key.ReadValue(NULL, WriteInto(sddl, wchar_count), &len,
+                             &reg_type) != ERROR_SUCCESS) {
       return false;
     }
 
@@ -734,16 +735,18 @@
   if (!key.Valid())
     return false;
 
-  bool result;
+  LONG result = ERROR_SUCCESS;
   if (set) {
     result = key.WriteValue(L"ChromeTab.ChromeActiveDocument", 1);
-    result = key.WriteValue(L"ChromeTab.ChromeActiveDocument.1", 1) && result;
+    if (result == ERROR_SUCCESS)

[grt (UTC plus 2), 2011/01/11 03:51:30]

is this change in behavior intentional?

[amit, 2011/01/12 04:11:23]

It's not. good catch!

+      result = key.WriteValue(L"ChromeTab.ChromeActiveDocument.1", 1);
   } else {
     result = key.DeleteValue(L"ChromeTab.ChromeActiveDocument");
-    result = key.DeleteValue(L"ChromeTab.ChromeActiveDocument.1") && result;
+    if (result == ERROR_SUCCESS)
+      result = key.DeleteValue(L"ChromeTab.ChromeActiveDocument.1");

[grt (UTC plus 2), 2011/01/11 03:51:30]

Don't we want to do the second delete even if the first one fails?  Uninstall is
usually best-effort rather than fail-fast.

[amit, 2011/01/12 04:11:23]

Done.

   }
 
-  return result;
+  return result == ERROR_SUCCESS;
 }
 
 bool RegisterSecuredMimeHandler(bool enable, bool is_system) {