Skip to content

Commit

Permalink
Misc. cleanup. No functional change.
Browse files Browse the repository at this point in the history
* Use `expected` monadic ops.
* `absl::Cleanup` is lighter-weight than `base::ScopedClosureRunner` and
  allows binding.
* Shorten code.
* No `else` necessary after `return`.
* Make use of move ops.

Bug: none
Change-Id: Ifb0b6c9a5db41cf96a0b0a492f38fa8bf8740e00
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4464198
Reviewed-by: Xiaoling Bao <[email protected]>
Auto-Submit: Peter Kasting <[email protected]>
Commit-Queue: Xiaoling Bao <[email protected]>
Code-Coverage: Findit <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1134684}
  • Loading branch information
pkasting authored and Chromium LUCI CQ committed Apr 24, 2023
1 parent ea20dc3 commit 0f0c952
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 33 deletions.
2 changes: 1 addition & 1 deletion chrome/updater/app/server/win/com_classes_legacy.cc
Original file line number Diff line number Diff line change
Expand Up @@ -852,7 +852,7 @@ HRESULT LegacyAppCommandWebImpl::RuntimeClassInitialize(
const std::wstring& command_id) {
app_command_runner_ =
AppCommandRunner::LoadAppCommand(scope, app_id, command_id);
return app_command_runner_.has_value() ? S_OK : app_command_runner_.error();
return app_command_runner_.error_or(S_OK);
}

STDMETHODIMP LegacyAppCommandWebImpl::get_status(UINT* status) {
Expand Down
41 changes: 16 additions & 25 deletions chrome/updater/util/win_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@
#include "chrome/updater/win/scoped_handle.h"
#include "chrome/updater/win/user_info.h"
#include "chrome/updater/win/win_constants.h"
#include "third_party/abseil-cpp/absl/cleanup/cleanup.h"
#include "third_party/abseil-cpp/absl/types/optional.h"

namespace updater {
Expand Down Expand Up @@ -437,8 +438,7 @@ HResultOr<bool> IsTokenAdmin(HANDLE token) {
&administrators_group)) {
return base::unexpected(HRESULTFromLastError());
}
base::ScopedClosureRunner free_sid(
base::BindOnce([](PSID sid) { ::FreeSid(sid); }, administrators_group));
absl::Cleanup free_sid = [&] { ::FreeSid(administrators_group); };
BOOL is_member = false;
if (!::CheckTokenMembership(token, administrators_group, &is_member))
return base::unexpected(HRESULTFromLastError());
Expand Down Expand Up @@ -482,9 +482,7 @@ HResultOr<bool> IsCOMCallerAdmin() {
return base::unexpected(hr);
}

base::ScopedClosureRunner co_revert_to_self(
base::BindOnce([]() { ::CoRevertToSelf(); }));

absl::Cleanup co_revert_to_self = [] { ::CoRevertToSelf(); };
if (!::OpenThreadToken(::GetCurrentThread(), TOKEN_QUERY, TRUE,
ScopedKernelHANDLE::Receiver(token).get())) {
hr = HRESULTFromLastError();
Expand All @@ -507,18 +505,13 @@ bool IsUACOn() {
// The presence of a split token definitively indicates that UAC is on. But
// the absence of the token does not necessarily indicate that UAC is off.
HResultOr<bool> is_split_token = IsUserRunningSplitToken();
if (is_split_token.has_value() && is_split_token.value())
return true;

return IsExplorerRunningAtMediumOrLower();
return (is_split_token.has_value() && is_split_token.value()) ||
IsExplorerRunningAtMediumOrLower();
}

bool IsElevatedWithUACOn() {
HResultOr<bool> is_user_admin = IsUserAdmin();
if (is_user_admin.has_value() && !is_user_admin.value())
return false;

return IsUACOn();
return (!is_user_admin.has_value() || is_user_admin.value()) && IsUACOn();
}

std::string GetUACState() {
Expand All @@ -529,13 +522,13 @@ std::string GetUACState() {
base::StringAppendF(&s, "IsUserAdmin: %d, ", is_user_admin.value());

HResultOr<bool> is_user_non_elevated_admin = IsUserNonElevatedAdmin();
if (is_user_non_elevated_admin.has_value())
if (is_user_non_elevated_admin.has_value()) {
base::StringAppendF(&s, "IsUserNonElevatedAdmin: %d, ",
is_user_non_elevated_admin.value());
}

base::StringAppendF(&s, "IsUACOn: %d, ", IsUACOn());
base::StringAppendF(&s, "IsElevatedWithUACOn: %d", IsElevatedWithUACOn());

base::StringAppendF(&s, "IsUACOn: %d, IsElevatedWithUACOn: %d", IsUACOn(),
IsElevatedWithUACOn());
return s;
}

Expand Down Expand Up @@ -567,12 +560,11 @@ HResultOr<DWORD> ShellExecuteAndWait(const base::FilePath& file_path,
CHECK(!file_path.empty());

const HWND hwnd = CreateForegroundParentWindowForUAC();
const base::ScopedClosureRunner destroy_window(base::BindOnce(
[](HWND hwnd) {
if (hwnd)
::DestroyWindow(hwnd);
},
hwnd));
const absl::Cleanup destroy_window = [&] {
if (hwnd) {
::DestroyWindow(hwnd);
}
};

SHELLEXECUTEINFO shell_execute_info = {};
shell_execute_info.cbSize = sizeof(SHELLEXECUTEINFO);
Expand Down Expand Up @@ -859,9 +851,8 @@ absl::optional<base::ScopedTempDir> CreateSecureTempDir() {
base::ScopedTempDir temp_dir_owner;
if (temp_dir_owner.Set(temp_dir)) {
return temp_dir_owner;
} else {
return absl::nullopt;
}
return absl::nullopt;
}

base::ScopedClosureRunner SignalShutdownEvent(UpdaterScope scope) {
Expand Down
3 changes: 2 additions & 1 deletion chrome/updater/win/app_command_runner.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include <windows.h>

#include <string>
#include <utility>
#include <vector>

#include "base/base_paths_win.h"
Expand Down Expand Up @@ -181,7 +182,7 @@ AppCommandRunner::LoadAutoRunOnOsUpgradeAppCommands(
HResultOr<AppCommandRunner> runner =
LoadAppCommand(scope, app_id, it.Name());
if (runner.has_value()) {
app_command_runners.push_back(*runner);
app_command_runners.push_back(*std::move(runner));
}
}

Expand Down
10 changes: 4 additions & 6 deletions chrome/updater/win/installer/installer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -284,12 +284,10 @@ ProcessExitResult HandleRunElevated(const base::CommandLine& command_line) {

// The metainstaller is elevated because unpacking its files and running
// updater.exe must happen from a secure directory.
HResultOr<DWORD> result =
RunElevated(command_line.GetProgram(), [&command_line]() {
base::CommandLine elevate_command_line = command_line;
elevate_command_line.AppendSwitchASCII(kCmdLineExpectElevated, {});
return elevate_command_line.GetArgumentsString();
}());
base::CommandLine elevated_command_line = command_line;
elevated_command_line.AppendSwitchASCII(kCmdLineExpectElevated, {});
HResultOr<DWORD> result = RunElevated(
command_line.GetProgram(), elevated_command_line.GetArgumentsString());

return result.has_value()
? ProcessExitResult(result.value())
Expand Down

0 comments on commit 0f0c952

Please sign in to comment.