From 1da9b909780a229f530a325440613b124eff9041 Mon Sep 17 00:00:00 2001
From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com>
Date: Tue, 15 Sep 2026 11:48:38 -0700
Subject: [PATCH] fix: correct SUCCEEDED/FAILED usage on non-HRESULT return
 values (#53937)

base::win::RegKey methods return a LONG Win32 error code (ERROR_SUCCESS
or a positive error), not an HRESULT. SUCCEEDED()/FAILED() only test the
sign bit, so every registry failure in browser_win.cc read as success.
Compare against ERROR_SUCCESS instead.

In jump_list.cc, the closing parenthesis of SUCCEEDED() wrapped the whole
`GetCount(...) && (removed_count > 0)` expression, so the macro was
applied to a bool and always evaluated to true. Move the parenthesis so
SUCCEEDED() wraps only the HRESULT-returning call.


Claude-Session: https://claude.ai/code/session_013QkZj2JgXtCzU9uXNpneem

Co-authored-by: Claude <noreply@anthropic.com>
---
 shell/browser/browser_win.cc      | 26 +++++++++++++-------------
 shell/browser/ui/win/jump_list.cc |  2 +-
 2 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/shell/browser/browser_win.cc b/shell/browser/browser_win.cc
index 32c92b0b1d..f2981ed285 100644
--- a/shell/browser/browser_win.cc
+++ b/shell/browser/browser_win.cc
@@ -440,17 +440,17 @@ bool Browser::RemoveAsDefaultProtocolClient(const std::string& protocol,
   base::win::RegKey classesKey;
   base::win::RegKey commandKey;
 
-  if (FAILED(classesKey.Open(root, keyPath.c_str(), KEY_ALL_ACCESS)))
+  if (classesKey.Open(root, keyPath.c_str(), KEY_ALL_ACCESS) != ERROR_SUCCESS)
     // Classes key doesn't exist, that's concerning, but I guess
     // we're not the default handler
     return true;
 
-  if (FAILED(commandKey.Open(root, cmdPath.c_str(), KEY_ALL_ACCESS)))
+  if (commandKey.Open(root, cmdPath.c_str(), KEY_ALL_ACCESS) != ERROR_SUCCESS)
     // Key doesn't even exist, we can confirm that it is not set
     return true;
 
   std::wstring keyVal;
-  if (FAILED(commandKey.ReadValue(L"", &keyVal)))
+  if (commandKey.ReadValue(L"", &keyVal) != ERROR_SUCCESS)
     // Default value not set, we can confirm that it is not set
     return true;
 
@@ -460,15 +460,15 @@ bool Browser::RemoveAsDefaultProtocolClient(const std::string& protocol,
 
   if (keyVal == exe) {
     // Let's kill the key
-    if (FAILED(classesKey.DeleteKey(shellPath.c_str())))
+    if (classesKey.DeleteKey(shellPath.c_str()) != ERROR_SUCCESS)
       return false;
 
     // Let's clean up after ourselves
     base::win::RegKey protocolKey;
     std::wstring protocolPath = keyPath + wprotocol;
 
-    if (SUCCEEDED(
-            protocolKey.Open(root, protocolPath.c_str(), KEY_ALL_ACCESS))) {
+    if (protocolKey.Open(root, protocolPath.c_str(), KEY_ALL_ACCESS) ==
+        ERROR_SUCCESS) {
       protocolKey.DeleteValue(L"URL Protocol");
 
       // Overwrite the default value to be empty, we can't delete it right away
@@ -521,12 +521,12 @@ bool Browser::SetAsDefaultProtocolClient(const std::string& protocol,
 
   // Write information to registry
   base::win::RegKey key(root, keyPath.c_str(), KEY_ALL_ACCESS);
-  if (FAILED(key.WriteValue(L"URL Protocol", L"")) ||
-      FAILED(key.WriteValue(L"", urlDecl.c_str())))
+  if (key.WriteValue(L"URL Protocol", L"") != ERROR_SUCCESS ||
+      key.WriteValue(L"", urlDecl.c_str()) != ERROR_SUCCESS)
     return false;
 
   base::win::RegKey commandKey(root, cmdPath.c_str(), KEY_ALL_ACCESS);
-  if (FAILED(commandKey.WriteValue(L"", exe.c_str())))
+  if (commandKey.WriteValue(L"", exe.c_str()) != ERROR_SUCCESS)
     return false;
 
   return true;
@@ -550,16 +550,16 @@ bool Browser::IsDefaultProtocolClient(const std::string& protocol,
 
   base::win::RegKey key;
   base::win::RegKey commandKey;
-  if (FAILED(key.Open(root, keyPath.c_str(), KEY_ALL_ACCESS)))
+  if (key.Open(root, keyPath.c_str(), KEY_ALL_ACCESS) != ERROR_SUCCESS)
     // Key doesn't exist, we can confirm that it is not set
     return false;
 
-  if (FAILED(commandKey.Open(root, cmdPath.c_str(), KEY_ALL_ACCESS)))
+  if (commandKey.Open(root, cmdPath.c_str(), KEY_ALL_ACCESS) != ERROR_SUCCESS)
     // Key doesn't exist, we can confirm that it is not set
     return false;
 
   std::wstring keyVal;
-  if (FAILED(commandKey.ReadValue(L"", &keyVal)))
+  if (commandKey.ReadValue(L"", &keyVal) != ERROR_SUCCESS)
     // Default value not set, we can confirm that it is not set
     return false;
 
@@ -722,7 +722,7 @@ v8::Local<v8::Value> Browser::GetLoginItemSettings(
   std::wstring keyVal;
 
   // keep old openAtLogin behaviour
-  if (!FAILED(key.ReadValue(GetAppUserModelID(), &keyVal))) {
+  if (key.ReadValue(GetAppUserModelID(), &keyVal) == ERROR_SUCCESS) {
     std::wstring exe = base::UTF16ToWide(options.path);
     if (FormatCommandLineString(&exe, options.args)) {
       settings.open_at_login = keyVal == exe;
diff --git a/shell/browser/ui/win/jump_list.cc b/shell/browser/ui/win/jump_list.cc
index ef910119be..a5ea4339b5 100644
--- a/shell/browser/ui/win/jump_list.cc
+++ b/shell/browser/ui/win/jump_list.cc
@@ -138,7 +138,7 @@ void ConvertRemovedJumpListItems(IObjectArray* in,
   DCHECK(out);
 
   UINT removed_count;
-  if (SUCCEEDED(in->GetCount(&removed_count) && (removed_count > 0))) {
+  if (SUCCEEDED(in->GetCount(&removed_count)) && removed_count > 0) {
     out->reserve(removed_count);
     JumpListItem item;
     IShellItem* shell_item;