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 70917)
+++ 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);

[robertshield, 2011/01/12 15:01:34]

nit: 
if (run_once.Create(hive, kRunOnce, KEY_READ | KEY_WRITE) == ERROR_SUCCESS) {
would fit on one line, allowing you to do away with ret, at least at this scope
level.

[amit, 2011/01/14 05:23:33]

Actually, it does not fit on one line but that's not the point here.
SetupRunOnce now returns a proper return code in case of failure.

+      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,17 @@
   if (!key.Valid())
     return false;
 
-  bool result;
+  LONG result1 = ERROR_SUCCESS;
+  LONG result2 = ERROR_SUCCESS;
   if (set) {
-    result = key.WriteValue(L"ChromeTab.ChromeActiveDocument", 1);
-    result = key.WriteValue(L"ChromeTab.ChromeActiveDocument.1", 1) && result;
+    result1 = key.WriteValue(L"ChromeTab.ChromeActiveDocument", 1);
+    result2 = key.WriteValue(L"ChromeTab.ChromeActiveDocument.1", 1);
   } else {
-    result = key.DeleteValue(L"ChromeTab.ChromeActiveDocument");
-    result = key.DeleteValue(L"ChromeTab.ChromeActiveDocument.1") && result;
+    result1 = key.DeleteValue(L"ChromeTab.ChromeActiveDocument");
+    result2 = key.DeleteValue(L"ChromeTab.ChromeActiveDocument.1");
   }
 
-  return result;
+  return (result2 == ERROR_SUCCESS) || (result2 == ERROR_SUCCESS);

[robertshield, 2011/01/12 15:01:34]

The || should be an &&: previously both writes/deletes had to succeed for this
to return true, now only one of them has to succeed.

[amit, 2011/01/14 05:23:33]

Done.

 }
 
 bool RegisterSecuredMimeHandler(bool enable, bool is_system) {