Support --delete-old-versions in setup.exe.

Support --delete-old-versions in setup.exe.

setup.exe --delete-old-versions repetitively attempts to delete all
files that belong to old versions of Chrome. It exits when no files
that belong to an old version of Chrome remain or when another process
tries to acquire the SetupSingleton.

setup.exe --delete-old-versions is launched in two occasions:
- When the installer fails to delete old files after a not-in-use
  update.
- When executables are successfully renamed on Chrome startup or
  shutdown.

BUG=451546
Committed: https://crrev.com/1d37e596b979152e8f413dadb43de44cc41740d0
Cr-Commit-Position: refs/heads/master@{#422938}

[fdoray, 2016-09-12 20:45:22]

fdoray@chromium.org changed reviewers:
+ grt@chromium.org

[fdoray, 2016-09-12 20:45:23]

PTAL

[grt (UTC plus 2), 2016-09-13 08:10:41]

awesome!

https://codereview.chromium.org/2333853002/diff/1/chrome/installer/setup/inst...
File chrome/installer/setup/install.cc (right):

https://codereview.chromium.org/2333853002/diff/1/chrome/installer/setup/inst...
chrome/installer/setup/install.cc:691: !base::PathExists(
rather than checking for new_chrome.exe again, you could check for the result
codes that represent that state:
(result == IN_USE_UPDATED || result == IN_USE_DOWNGRADE)

https://codereview.chromium.org/2333853002/diff/1/chrome/installer/setup/inst...
chrome/installer/setup/install.cc:702: launch_options.start_hidden = true;
i think it's prudent to use:
  launch_options.force_breakaway_from_job_ = true;
so that anything running the installer in a job object (msiexec may do this)
won't wait around for the deleter process to complete.

https://codereview.chromium.org/2333853002/diff/1/chrome/installer/setup/inst...
chrome/installer/setup/install.cc:707:
base::CommandLine::ForCurrentProcess()->GetProgram()));
the currently running setup.exe during an update is in a temp dir created by
mini_installer.exe somewheres, so it isn't the right choice. since different
callers of this function know which setup.exe to use (InstallOrUpdateProduct
knows where the new one is, and the rename dance knows that the currently
running one is correct), how about changing this fn to take a FilePath to the
setup.exe that should do the work? InstallOrUpdateProduct should formulate one
with:

installer_state.GetInstallerDirectory(new_version).Append(setup_path.BaseName())

and the rename dance could have it passed down from HandleNonInstallBlahBlah
which already has the value from the path service.

for future reference, note that
base::CommandLine::ForCurrentProcess()->GetProgram() may not accurately name the
current executable. using the PathService to get FILE_EXE is preferable.

https://codereview.chromium.org/2333853002/diff/1/chrome/installer/setup/inst...
chrome/installer/setup/install.cc:708:
command_line.AppendSwitch(switches::kDeleteOldVersions);
we generally try to use the command-line args that specify which product is
being operated on, at which level, and propagate verbose logging. to get the
right product flags, it's probably easiest for this function to suss out whether
the binaries are being updated or just chrome (either single-install or canary):

  // Deleting old versions is relevant if multi-install binaries are being
  // updated or if single-install Chrome is.
  const Product* product =
    installer_state.FindProduct(BrowserDistribution::CHROME_BINARIES);
  if (!product)
    product = installer_state.FindProduct(BrowserDistribution::CHROME_BROWSER);
  if (!product)
    return;

from there, you can ask the |product| to add its special switches (i.e.,
--chrome-sxs or --multi-install) to command_line:

  product->AppendProductFlags(&command_line);

and then add in --system-level and --verbose-logging (which could be conditional
on installer_state.verbose_logging(), but i suggest always having it on for
starters):

  if (installer_state.system_install())
    command_line.AppendSwitch(switches::kSystemLevel);
  // Unconditionally enable verbose logging for now to make diagnosing potential
  // failures possible.
  command_line.AppendSwitch(switches::kVerboseLogging);

https://codereview.chromium.org/2333853002/diff/1/chrome/installer/setup/inst...
chrome/installer/setup/install.cc:709: base::LaunchProcess(command_line,
launch_options);
logging around this would be useful. maybe:
  VLOG(1) << "Launching \"" << command_line.GetCommandLineString() << "\" to
delete old versions.";
  base::Process process = base::LaunchProcess(command_line, launch_options);
  PLOG_IF(ERROR, !process)
      << "Failed to launch \""<< command_line.GetCommandLineString() << "\"";

https://codereview.chromium.org/2333853002/diff/1/chrome/installer/setup/setu...
File chrome/installer/setup/setup_main.cc (right):

https://codereview.chromium.org/2333853002/diff/1/chrome/installer/setup/setu...
chrome/installer/setup/setup_main.cc:395: installer::InstallStatus
RepeatDeleteOldVersionsUntilSuccess(
it looks like we'll be blind to how this performs. how difficult would it be to
have persistent histograms written for this case so that you can use UMA in here
to log things like the # of long sleeps needed before termination, and whether
termination was due to success or interrupt (see the call to
EndPersistentHistogramStorage at the end of wWinMain)?

https://codereview.chromium.org/2333853002/diff/1/chrome/installer/setup/setu...
chrome/installer/setup/setup_main.cc:407: for (;;) {
why not?
  while (true) {

https://codereview.chromium.org/2333853002/diff/1/chrome/installer/setup/setu...
chrome/installer/setup/setup_main.cc:409: LOG(INFO) << "Successfully deleted old
files from --delete-old-versions "
please use VLOG(1) here and below

https://codereview.chromium.org/2333853002/diff/1/chrome/installer/setup/setu...
chrome/installer/setup/setup_main.cc:414: LOG(INFO) << "Failed to delete old
files from --delete-old-versions "
does this case mean that no old files were deleted, or that not all old files
were deleted? if the latter, please change to "Failed to delete all old..."

https://codereview.chromium.org/2333853002/diff/1/chrome/installer/setup/setu...
chrome/installer/setup/setup_main.cc:490:
installer::LaunchDeleteOldVersionsProcess(*installer_state);
in this specific case, the current setup.exe (PathService::Get(base::FILE_EXE))
is the right one. so perhaps fetch that here and pass it in, or have
HandleNonInstallCmdLineOptions pass it into this fn to be passed along.

https://codereview.chromium.org/2333853002/diff/1/chrome/installer/setup/setu...
chrome/installer/setup/setup_main.cc:1893:
installer::SetupSingleton::Acquire(cmd_line, prefs, &original_state,
how about UMA for how much time it takes to acquire the singleton so that we
know if long-running deletes are blocking updates (once a delete starts, it runs
to completion, yes?)?

[fdoray, 2016-09-13 21:34:54]

PTAnL

https://codereview.chromium.org/2333853002/diff/1/chrome/installer/setup/inst...
File chrome/installer/setup/install.cc (right):

https://codereview.chromium.org/2333853002/diff/1/chrome/installer/setup/inst...
chrome/installer/setup/install.cc:691: !base::PathExists(
On 2016/09/13 08:10:40, grt (UTC plus 2) wrote:
> rather than checking for new_chrome.exe again, you could check for the result
> codes that represent that state:
> (result == IN_USE_UPDATED || result == IN_USE_DOWNGRADE)

Done.

https://codereview.chromium.org/2333853002/diff/1/chrome/installer/setup/inst...
chrome/installer/setup/install.cc:702: launch_options.start_hidden = true;
On 2016/09/13 08:10:40, grt (UTC plus 2) wrote:
> i think it's prudent to use:
>   launch_options.force_breakaway_from_job_ = true;
> so that anything running the installer in a job object (msiexec may do this)
> won't wait around for the deleter process to complete.

Done.

https://codereview.chromium.org/2333853002/diff/1/chrome/installer/setup/inst...
chrome/installer/setup/install.cc:707:
base::CommandLine::ForCurrentProcess()->GetProgram()));
On 2016/09/13 08:10:40, grt (UTC plus 2) wrote:
> the currently running setup.exe during an update is in a temp dir created by
> mini_installer.exe somewheres, so it isn't the right choice. since different
> callers of this function know which setup.exe to use (InstallOrUpdateProduct
> knows where the new one is, and the rename dance knows that the currently
> running one is correct), how about changing this fn to take a FilePath to the
> setup.exe that should do the work? InstallOrUpdateProduct should formulate one
> with:
> 
>
installer_state.GetInstallerDirectory(new_version).Append(setup_path.BaseName())
> 
> and the rename dance could have it passed down from HandleNonInstallBlahBlah
> which already has the value from the path service.
> 
> for future reference, note that
> base::CommandLine::ForCurrentProcess()->GetProgram() may not accurately name
the
> current executable. using the PathService to get FILE_EXE is preferable.

Done.

https://codereview.chromium.org/2333853002/diff/1/chrome/installer/setup/inst...
chrome/installer/setup/install.cc:708:
command_line.AppendSwitch(switches::kDeleteOldVersions);
On 2016/09/13 08:10:40, grt (UTC plus 2) wrote:
> we generally try to use the command-line args that specify which product is
> being operated on, at which level, and propagate verbose logging. to get the
> right product flags, it's probably easiest for this function to suss out
whether
> the binaries are being updated or just chrome (either single-install or
canary):
> 
>   // Deleting old versions is relevant if multi-install binaries are being
>   // updated or if single-install Chrome is.
>   const Product* product =
>     installer_state.FindProduct(BrowserDistribution::CHROME_BINARIES);
>   if (!product)
>     product =
installer_state.FindProduct(BrowserDistribution::CHROME_BROWSER);
>   if (!product)
>     return;
> 
> from there, you can ask the |product| to add its special switches (i.e.,
> --chrome-sxs or --multi-install) to command_line:
> 
>   product->AppendProductFlags(&command_line);
> 
> and then add in --system-level and --verbose-logging (which could be
conditional
> on installer_state.verbose_logging(), but i suggest always having it on for
> starters):
> 
>   if (installer_state.system_install())
>     command_line.AppendSwitch(switches::kSystemLevel);
>   // Unconditionally enable verbose logging for now to make diagnosing
potential
>   // failures possible.
>   command_line.AppendSwitch(switches::kVerboseLogging);

Done.

https://codereview.chromium.org/2333853002/diff/1/chrome/installer/setup/inst...
chrome/installer/setup/install.cc:709: base::LaunchProcess(command_line,
launch_options);
On 2016/09/13 08:10:40, grt (UTC plus 2) wrote:
> logging around this would be useful. maybe:
>   VLOG(1) << "Launching \"" << command_line.GetCommandLineString() << "\" to
> delete old versions.";
>   base::Process process = base::LaunchProcess(command_line, launch_options);
>   PLOG_IF(ERROR, !process)
>       << "Failed to launch \""<< command_line.GetCommandLineString() << "\"";

Done.

https://codereview.chromium.org/2333853002/diff/1/chrome/installer/setup/setu...
File chrome/installer/setup/setup_main.cc (right):

https://codereview.chromium.org/2333853002/diff/1/chrome/installer/setup/setu...
chrome/installer/setup/setup_main.cc:395: installer::InstallStatus
RepeatDeleteOldVersionsUntilSuccess(
On 2016/09/13 08:10:40, grt (UTC plus 2) wrote:
> it looks like we'll be blind to how this performs. how difficult would it be
to
> have persistent histograms written for this case so that you can use UMA in
here
> to log things like the # of long sleeps needed before termination, and whether
> termination was due to success or interrupt (see the call to
> EndPersistentHistogramStorage at the end of wWinMain)?

Done.

https://codereview.chromium.org/2333853002/diff/1/chrome/installer/setup/setu...
chrome/installer/setup/setup_main.cc:407: for (;;) {
On 2016/09/13 08:10:40, grt (UTC plus 2) wrote:
> why not?
>   while (true) {

Done.

https://codereview.chromium.org/2333853002/diff/1/chrome/installer/setup/setu...
chrome/installer/setup/setup_main.cc:409: LOG(INFO) << "Successfully deleted old
files from --delete-old-versions "
On 2016/09/13 08:10:40, grt (UTC plus 2) wrote:
> please use VLOG(1) here and below

Done.

https://codereview.chromium.org/2333853002/diff/1/chrome/installer/setup/setu...
chrome/installer/setup/setup_main.cc:414: LOG(INFO) << "Failed to delete old
files from --delete-old-versions "
On 2016/09/13 08:10:40, grt (UTC plus 2) wrote:
> does this case mean that no old files were deleted, or that not all old files
> were deleted? if the latter, please change to "Failed to delete all old..."

Done. It means that not *all* old files were deleted. Some files may have been
deleted.

https://codereview.chromium.org/2333853002/diff/1/chrome/installer/setup/setu...
chrome/installer/setup/setup_main.cc:490:
installer::LaunchDeleteOldVersionsProcess(*installer_state);
On 2016/09/13 08:10:40, grt (UTC plus 2) wrote:
> in this specific case, the current setup.exe
(PathService::Get(base::FILE_EXE))
> is the right one. so perhaps fetch that here and pass it in, or have
> HandleNonInstallCmdLineOptions pass it into this fn to be passed along.

Done.

https://codereview.chromium.org/2333853002/diff/1/chrome/installer/setup/setu...
chrome/installer/setup/setup_main.cc:1893:
installer::SetupSingleton::Acquire(cmd_line, prefs, &original_state,
On 2016/09/13 08:10:41, grt (UTC plus 2) wrote:
> how about UMA for how much time it takes to acquire the singleton so that we
> know if long-running deletes are blocking updates (once a delete starts, it
runs
> to completion, yes?)?

It can't take more than 5+5 seconds = 10 seconds
https://cs.chromium.org/chromium/src/chrome/installer/setup/setup_singleton.c...
and we can already know how often it fails using the installer result written
below. Do we really need another histogram to know exactly how much time we had
to wait?

Note: "once a delete starts, it runs to completion, yes?": NO
When another process tries to acquire the SetupSingleton, it signals an event
https://cs.chromium.org/chromium/src/chrome/installer/setup/setup_singleton.c...
that causes the --delete-old-versions process to exit as soon as it calls
SetupSingleton::WaitForInterrupt().

[grt (UTC plus 2), 2016-09-14 10:42:11]

looks good. please test with:
- canary: --chrome-sxs
- system-level chrome: --system-level
- multi chrome: --multi-install --chrome

https://codereview.chromium.org/2333853002/diff/1/chrome/installer/setup/setu...
File chrome/installer/setup/setup_main.cc (right):

https://codereview.chromium.org/2333853002/diff/1/chrome/installer/setup/setu...
chrome/installer/setup/setup_main.cc:1893:
installer::SetupSingleton::Acquire(cmd_line, prefs, &original_state,
On 2016/09/13 21:34:54, fdoray wrote:
> On 2016/09/13 08:10:41, grt (UTC plus 2) wrote:
> > how about UMA for how much time it takes to acquire the singleton so that we
> > know if long-running deletes are blocking updates (once a delete starts, it
> runs
> > to completion, yes?)?
> 
> It can't take more than 5+5 seconds = 10 seconds
>
https://cs.chromium.org/chromium/src/chrome/installer/setup/setup_singleton.c...
> and we can already know how often it fails using the installer result written
> below. Do we really need another histogram to know exactly how much time we
had
> to wait?

I forgot about the short timeout. No, I think that's enough.

> Note: "once a delete starts, it runs to completion, yes?": NO
> When another process tries to acquire the SetupSingleton, it signals an event
>
https://cs.chromium.org/chromium/src/chrome/installer/setup/setup_singleton.c...
> that causes the --delete-old-versions process to exit as soon as it calls
> SetupSingleton::WaitForInterrupt().

My question is about once installer::DeleteOldVersions is called. On an old
machine with a slow spinning disk where the process is running at low priority
(it should, no?), how long could it possibly take to delete some directory
trees? Hopefully not long, but once this bit of processing starts it won't stop
due to another process trying to acquire the singleton. Should the process not
run at background CPU and IO priority?

https://codereview.chromium.org/2333853002/diff/40001/chrome/installer/setup/...
File chrome/installer/setup/setup_main.cc (right):

https://codereview.chromium.org/2333853002/diff/40001/chrome/installer/setup/...
chrome/installer/setup/setup_main.cc:398: int num_attempts = 0;
hmm. should it give up after some number of tries?

https://codereview.chromium.org/2333853002/diff/40001/chrome/installer/setup/...
chrome/installer/setup/setup_main.cc:420: if
(installer::DeleteOldVersions(install_dir)) {
WDYT of Process::current().SetProcessBackgrounded(true) before this and false
just after it so that the delete doesn't cause peoples' WoW games to stutter?

https://codereview.chromium.org/2333853002/diff/40001/chrome/installer/setup/...
chrome/installer/setup/setup_main.cc:428: VLOG(1) << "Failed to delete all old
files from --delete-old-versions "
since this could potentially repeat for a very long time, i think it is best to
only log this once with a message along the lines of ".. .Will retry every five
minutes." Otherwise it could fill the log with this one message.

[fdoray, 2016-09-27 19:42:14]

fdoray@chromium.org changed reviewers:
+ rkaplow@chromium.org

[fdoray, 2016-09-27 19:42:16]

grt@: PTAnL. It took me a while but I tested every regular and in-use update for
every install configuration (no flags, --chrome-sxs, --system-level,
--multi-install --chrome)

rkaplow@: PTAL at histograms.xml

https://codereview.chromium.org/2333853002/diff/40001/chrome/installer/setup/...
File chrome/installer/setup/setup_main.cc (right):

https://codereview.chromium.org/2333853002/diff/40001/chrome/installer/setup/...
chrome/installer/setup/setup_main.cc:398: int num_attempts = 0;
On 2016/09/14 10:42:10, grt (UTC plus 2) wrote:
> hmm. should it give up after some number of tries?

Done.

https://codereview.chromium.org/2333853002/diff/40001/chrome/installer/setup/...
chrome/installer/setup/setup_main.cc:420: if
(installer::DeleteOldVersions(install_dir)) {
On 2016/09/14 10:42:10, grt (UTC plus 2) wrote:
> WDYT of Process::current().SetProcessBackgrounded(true) before this and false
> just after it so that the delete doesn't cause peoples' WoW games to stutter?

Done.

https://codereview.chromium.org/2333853002/diff/40001/chrome/installer/setup/...
chrome/installer/setup/setup_main.cc:428: VLOG(1) << "Failed to delete all old
files from --delete-old-versions "
On 2016/09/14 10:42:10, grt (UTC plus 2) wrote:
> since this could potentially repeat for a very long time, i think it is best
to
> only log this once with a message along the lines of ".. .Will retry every
five
> minutes." Otherwise it could fill the log with this one message.

Done.

[grt (UTC plus 2), 2016-09-27 20:50:42]

awesome, thanks. lgtm w/ a fix

https://codereview.chromium.org/2333853002/diff/80001/chrome/installer/setup/...
File chrome/installer/setup/setup_main.cc (right):

https://codereview.chromium.org/2333853002/diff/80001/chrome/installer/setup/...
chrome/installer/setup/setup_main.cc:431:
base::Process::Current().SetProcessBackgrounded(was_backgrounded);
if (was_backgrounded)
  base::Process::Current().SetProcessBackgrounded(false);
no?

[rkaplow, 2016-09-28 14:31:30]

lgtm

https://codereview.chromium.org/2333853002/diff/80001/chrome/installer/setup/...
File chrome/installer/setup/setup_main.cc (right):

https://codereview.chromium.org/2333853002/diff/80001/chrome/installer/setup/...
chrome/installer/setup/setup_main.cc:392:
UMA_HISTOGRAM_COUNTS("Setup.Install.NumDeleteOldVersionsAttemptsBeforeAbort",
nit, I've refactored the COUNT histograms which now recommends an explicit max.
recommend _1M instead of this macro if you need this to measure up to 1M

[fdoray, 2016-10-04 18:58:17]

The CQ bit was checked by fdoray@chromium.org

[fdoray, 2016-10-04 18:58:21]

The CQ bit was unchecked by fdoray@chromium.org

[fdoray, 2016-10-04 20:22:44]

https://codereview.chromium.org/2333853002/diff/80001/chrome/installer/setup/...
File chrome/installer/setup/setup_main.cc (right):

https://codereview.chromium.org/2333853002/diff/80001/chrome/installer/setup/...
chrome/installer/setup/setup_main.cc:392:
UMA_HISTOGRAM_COUNTS("Setup.Install.NumDeleteOldVersionsAttemptsBeforeAbort",
On 2016/09/28 14:31:30, rkaplow wrote:
> nit, I've refactored the COUNT histograms which now recommends an explicit
max.
> recommend _1M instead of this macro if you need this to measure up to 1M

Done.

https://codereview.chromium.org/2333853002/diff/80001/chrome/installer/setup/...
chrome/installer/setup/setup_main.cc:431:
base::Process::Current().SetProcessBackgrounded(was_backgrounded);
On 2016/09/27 20:50:42, grt (UTC plus 2) wrote:
> if (was_backgrounded)
>   base::Process::Current().SetProcessBackgrounded(false);
> no?

Done.

if (!was_backgrounded)

[fdoray, 2016-10-04 20:22:51]

The CQ bit was checked by fdoray@chromium.org

[fdoray, 2016-10-04 20:22:52]

The patchset sent to the CQ was uploaded after l-g-t-m from
rkaplow@chromium.org, grt@chromium.org
Link to the patchset: https://codereview.chromium.org/2333853002/#ps100001
(title: "CR grt/rkaplow #8-9")

[commit-bot: I haz the power, 2016-10-04 20:23:40]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-04 21:36:34]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-10-04 21:38:58]

Description was changed from

==========
Support --delete-old-versions in setup.exe.

setup.exe --delete-old-versions repetitively attempts to delete all
files that belong to old versions of Chrome. It exits when no files
that belong to an old version of Chrome remain or when another process
tries to acquire the SetupSingleton.

setup.exe --delete-old-versions is launched in two occasions:
- When the installer fails to delete old files after a not-in-use
  update.
- When executables are successfully renamed on Chrome startup or
  shutdown.

BUG=451546
==========

to

==========
Support --delete-old-versions in setup.exe.

setup.exe --delete-old-versions repetitively attempts to delete all
files that belong to old versions of Chrome. It exits when no files
that belong to an old version of Chrome remain or when another process
tries to acquire the SetupSingleton.

setup.exe --delete-old-versions is launched in two occasions:
- When the installer fails to delete old files after a not-in-use
  update.
- When executables are successfully renamed on Chrome startup or
  shutdown.

BUG=451546

Committed: https://crrev.com/1d37e596b979152e8f413dadb43de44cc41740d0
Cr-Commit-Position: refs/heads/master@{#422938}
==========

[commit-bot: I haz the power, 2016-10-04 21:38:59]

Patchset 6 (id:??) landed as
https://crrev.com/1d37e596b979152e8f413dadb43de44cc41740d0
Cr-Commit-Position: refs/heads/master@{#422938}

[grt (UTC plus 2), 2016-10-05 08:17:33]

https://codereview.chromium.org/2333853002/diff/80001/chrome/installer/setup/...
File chrome/installer/setup/setup_main.cc (right):

https://codereview.chromium.org/2333853002/diff/80001/chrome/installer/setup/...
chrome/installer/setup/setup_main.cc:431:
base::Process::Current().SetProcessBackgrounded(was_backgrounded);
On 2016/10/04 20:22:43, fdoray wrote:
> On 2016/09/27 20:50:42, grt (UTC plus 2) wrote:
> > if (was_backgrounded)
> >   base::Process::Current().SetProcessBackgrounded(false);
> > no?
> 
> Done.
> 
> if (!was_backgrounded)

This is backwards in the same way it was before, no? The doc comment for
SetProcessBackgrounded says it returns true if the process was backgrounded, so
you want to restore it to the foreground if was_backgrounded is set to true, not
false.

[fdoray, 2016-10-05 12:19:25]

https://codereview.chromium.org/2333853002/diff/80001/chrome/installer/setup/...
File chrome/installer/setup/setup_main.cc (right):

https://codereview.chromium.org/2333853002/diff/80001/chrome/installer/setup/...
chrome/installer/setup/setup_main.cc:431:
base::Process::Current().SetProcessBackgrounded(was_backgrounded);
On 2016/10/05 08:17:33, grt (UTC plus 2) wrote:
> On 2016/10/04 20:22:43, fdoray wrote:
> > On 2016/09/27 20:50:42, grt (UTC plus 2) wrote:
> > > if (was_backgrounded)
> > >   base::Process::Current().SetProcessBackgrounded(false);
> > > no?
> > 
> > Done.
> > 
> > if (!was_backgrounded)
> 
> This is backwards in the same way it was before, no? The doc comment for
> SetProcessBackgrounded says it returns true if the process was backgrounded,
so
> you want to restore it to the foreground if was_backgrounded is set to true,
not
> false.

you're right. I wrongly assumed that SetProcessBackgrounded returned the
previous state (similar to
https://cs.chromium.org/chromium/src/base/threading/thread_restrictions.h?l=156)