Silence logging at uninstall if the ClientState key doesn't exist.

chrome/installer/util/product.cc

 // Copyright (c) 2012 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/product.h"
 
 #include <algorithm>
 
 #include "base/command_line.h"
 #include "base/logging.h"
@@ skipping 99 matching lines @@
   }
 
   return success;
 }
 
 bool Product::SetMsiMarker(bool system_install, bool set) const {
   HKEY reg_root = system_install ? HKEY_LOCAL_MACHINE : HKEY_CURRENT_USER;
   RegKey client_state_key;
   LONG result = client_state_key.Open(reg_root,
                                       distribution_->GetStateKey().c_str(),
-                                      KEY_READ | KEY_WRITE | KEY_WOW64_32KEY);
+                                      KEY_SET_VALUE | KEY_WOW64_32KEY);
   if (result == ERROR_SUCCESS) {
     result = client_state_key.WriteValue(google_update::kRegMSIField,
                                          set ? 1 : 0);
   }
 
-  LOG_IF(ERROR, result != ERROR_SUCCESS) << "Failed to Open or Write MSI value"
-      "to client state key. error: " << result;
+  if (result == ERROR_SUCCESS || result == ERROR_FILE_NOT_FOUND)

[gab, 2014/09/08 16:29:58]

nit: I'd prefer handling the error in scope and having the regular (non-scoped)
flow be the expected flow, i.e.:

----------------------------------------------------------------------
if (result != ERROR_SUCESS && result != ERROR_FILE_NOT_FOUND) {
  LOG(ERROR) << ...;
  return false;
}

return true;
----------------------------------------------------------------------

makes it such that any other code can be added after handling the error without
having to flip the logic.

[grt (UTC plus 2), 2014/09/08 16:47:56]

yeah. my urge to avoid braces made me do it this way. it's prettier. i can't
imagine this function having more code after this. how strongly do you feel
about this?

[gab, 2014/09/08 16:58:41]

Mildly. I don't think having to add braces matters, but I do find it weird for
the success condition to be in the  middle while the end of the method is the
error case.

[grt (UTC plus 2), 2014/09/08 17:03:29]

Done.

+    return true;
 
-  return (result == ERROR_SUCCESS);
+  LOG(ERROR) << "Failed to Open or Write MSI value to client state key. error: "
+             << result;
+  return false;
 }
 
 bool Product::ShouldCreateUninstallEntry() const {
   return operations_->ShouldCreateUninstallEntry(options_);
 }
 
 void Product::AddKeyFiles(std::vector<base::FilePath>* key_files) const {
   operations_->AddKeyFiles(options_, key_files);
 }
 
@@ skipping 26 matching lines @@
   if (distribution_->HasUserExperiments()) {
     VLOG(1) << "LaunchUserExperiment status: " << status << " product: "
             << distribution_->GetDisplayName()
             << " system_level: " << system_level;
     operations_->LaunchUserExperiment(
         setup_path, options_, status, system_level);
   }
 }
 
 }  // namespace installer