If there is any value in letting the tool run, then it's only marginal - we won't be able to prompt an infected user or collect significant telemetry if Chrome is not open.

I'd be in favor of a more general approach of terminating the reporter process if Chrome is no longer running (I assume uninstall would simply be a specific case). That's what happens to the cleaner running in scanning mode, but as a side effect of the Mojo IPC channel being closed.

Assigning to csharp@ for triage, since I'm no longer in the cleanup tool team.

Moving it into the temp directory to run sounds like an easy fix for this. I'd rather do that to unblock uninstallation and file a separate bug to kill the tool when Chrome exits.

Please prioritize setting CWD (via LaunchOptions::current_directory) when launching the process. This simple step will at least allow the uninstaller to delete the Application directory. Setting it to the directory containing the executable is probably a better choice than %TMP% since the process's CWD is present in the DLL search path.

Addressing the User Data dir can be done in a separate CL.

Thanks!

This is the same issue as  crbug.com/800719 , right? But with the potential quick fix of changing the CWD.

Changing the CWD sounds like a reasonable noogler task, so I'm going to assign it to Olivier.

For the longer term fix, I'm starting to think we should just kill the reporter whenever Chrome exits. I think the would ensure it isn't running when the uninstall starts up, right?

 Issue 800719  has been merged into this issue.

Yup, same issue!

If you wish for the tool to die with the browser process, perhaps it could run within a job object so that it is automagically killed by Windows when the browser goes away. I have no direct experience with job objects, so I'm not sure if there are some caveats here. siggi, wfh, and foreshaw may be able to advise.

crbug.com/840746 was just filed with a report of the tool blocking browser shutdown (because the browser is blocked on WaitForExitWithTimeout, waiting for the tool's process to exit). So just adding it to a job object won't help. We also need to detect that the browser is shutting down and break out of the WaitForExit.

I propose moving to a tmp dir in this bug, and tracking the more complete work to make sure the tool dies with the browser process in crbug.com/840746.

Re #9: It is not necessary to "break out of WaitForExit" if that is happening on a continue-on-shutdown TaskRunner in the browser; then the browser will leave the WaitForExit() running but still continue and exit.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/89b331d3a23dd089ddc23af5a333746f659641cd

commit 89b331d3a23dd089ddc23af5a333746f659641cd
Author: Olivier Li <olivierli@chromium.org>
Date: Fri May 18 21:10:51 2018

Run chrome-cleaner reporter in temp directory

I have inspected the current directory of the reporter tool
before and after this change and it goes from the Chrome install dir
to %TMP%.

I have also uninstalled Chrome while the reporter was running and the
uninstall completed unimpeded.

Bug:  838806 
Change-Id: I1c134abab456f3cfd1265d5b42d9bf47ecf3c2b9
Reviewed-on: https://chromium-review.googlesource.com/1060352
Commit-Queue: Oliver Li <olivierli@chromium.org>
Reviewed-by: Joe Mason <joenotcharles@chromium.org>
Cr-Commit-Position: refs/heads/master@{#560035}
[modify] https://crrev.com/89b331d3a23dd089ddc23af5a333746f659641cd/chrome/browser/safe_browsing/chrome_cleaner/reporter_runner_win.cc