Always persist histograms from setup.exe.

Always persist histograms from setup.exe.

Before this CL, setup.exe did not always persist histograms (e.g. when
launched with --rename-chrome-exe, when SetupSingleton acquisition
failed during an install or update).

With this CL, RAII is used to ensure that a setup.exe process always
persists histograms (except when the histogram storage directory does
not exist).

BUG=451546
Committed: https://crrev.com/10c71cf7cd97b4d893cd1fff74a1bd086fc46405
Cr-Commit-Position: refs/heads/master@{#418555}

[fdoray, 2016-09-13 18:48:22]

Description was changed from

==========
Always persist histograms from setup.exe.

Before this CL, setup.exe did not always persist histograms (e.g. when
launched with --rename-chrome-exe, when SetupSingleton acquisition
failed during an install or update).

With this CL, a RAII is used to ensure that a setup.exe process always
persists histograms (except when the histogram storage directory does
not exist).

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

to

==========
Always persist histograms from setup.exe.

Before this CL, setup.exe did not always persist histograms (e.g. when
launched with --rename-chrome-exe, when SetupSingleton acquisition
failed during an install or update).

With this CL, RAII is used to ensure that a setup.exe process always
persists histograms (except when the histogram storage directory does
not exist).

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

[fdoray, 2016-09-13 19:12:13]

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

[fdoray, 2016-09-13 19:12:14]

PTAL

[grt (UTC plus 2), 2016-09-13 19:43:00]

lgtm with nits. please wait for bcwhite's review.

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

https://codereview.chromium.org/2335313002/diff/40001/chrome/installer/setup/...
chrome/installer/setup/persistent_histogram_storage.cc:25:
installer::kSetupHistogramAllocatorName);
nit: remove "installer::"

https://codereview.chromium.org/2335313002/diff/40001/chrome/installer/setup/...
chrome/installer/setup/persistent_histogram_storage.cc:63: base::FilePath
PersistentHistogramStorage::GetStorageDir(
nit:
// static
before this

[bcwhite, 2016-09-13 19:53:53]

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

https://codereview.chromium.org/2335313002/diff/40001/chrome/installer/setup/...
chrome/installer/setup/install_worker.cc:397: //
https://msdn.microsoft.com/en-us/library/windows/desktop/aa379283.aspx This
Don't reflow this comment.  It's clearer if the URL is the only thing on this
line.

https://codereview.chromium.org/2335313002/diff/40001/chrome/installer/setup/...
File chrome/installer/setup/persistent_histogram_storage.h (right):

https://codereview.chromium.org/2335313002/diff/40001/chrome/installer/setup/...
chrome/installer/setup/persistent_histogram_storage.h:27: void
set_storage_dir(const base::FilePath& storage_dir) {
You have set_storage_dir and GetStorageDir, only one of which is static.  Please
be consistent (or completely, rather than partially, inconsistent).

[fdoray, 2016-09-13 20:18:26]

bcwhite@: PTAnL

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

https://codereview.chromium.org/2335313002/diff/40001/chrome/installer/setup/...
chrome/installer/setup/install_worker.cc:397: //
https://msdn.microsoft.com/en-us/library/windows/desktop/aa379283.aspx This
On 2016/09/13 19:53:53, bcwhite wrote:
> Don't reflow this comment.  It's clearer if the URL is the only thing on this
> line.

Done.

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

https://codereview.chromium.org/2335313002/diff/40001/chrome/installer/setup/...
chrome/installer/setup/persistent_histogram_storage.cc:25:
installer::kSetupHistogramAllocatorName);
On 2016/09/13 19:42:59, grt (UTC plus 2) wrote:
> nit: remove "installer::"

Done.

https://codereview.chromium.org/2335313002/diff/40001/chrome/installer/setup/...
chrome/installer/setup/persistent_histogram_storage.cc:63: base::FilePath
PersistentHistogramStorage::GetStorageDir(
On 2016/09/13 19:42:59, grt (UTC plus 2) wrote:
> nit:
> // static
> before this

Done.

https://codereview.chromium.org/2335313002/diff/40001/chrome/installer/setup/...
File chrome/installer/setup/persistent_histogram_storage.h (right):

https://codereview.chromium.org/2335313002/diff/40001/chrome/installer/setup/...
chrome/installer/setup/persistent_histogram_storage.h:27: void
set_storage_dir(const base::FilePath& storage_dir) {
On 2016/09/13 19:53:53, bcwhite wrote:
> You have set_storage_dir and GetStorageDir, only one of which is static. 
Please
> be consistent (or completely, rather than partially, inconsistent).

Done.

[bcwhite, 2016-09-14 13:23:06]

lgtm

[fdoray, 2016-09-14 13:26:57]

The CQ bit was checked by fdoray@chromium.org

[fdoray, 2016-09-14 13:26:58]

The patchset sent to the CQ was uploaded after l-g-t-m from grt@chromium.org
Link to the patchset: https://codereview.chromium.org/2335313002/#ps60001
(title: "CR grt/bcwhite #4-5")

[commit-bot: I haz the power, 2016-09-14 13:27:12]

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-09-14 14:23:07]

Description was changed from

==========
Always persist histograms from setup.exe.

Before this CL, setup.exe did not always persist histograms (e.g. when
launched with --rename-chrome-exe, when SetupSingleton acquisition
failed during an install or update).

With this CL, RAII is used to ensure that a setup.exe process always
persists histograms (except when the histogram storage directory does
not exist).

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

to

==========
Always persist histograms from setup.exe.

Before this CL, setup.exe did not always persist histograms (e.g. when
launched with --rename-chrome-exe, when SetupSingleton acquisition
failed during an install or update).

With this CL, RAII is used to ensure that a setup.exe process always
persists histograms (except when the histogram storage directory does
not exist).

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

[commit-bot: I haz the power, 2016-09-14 14:23:08]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-09-14 14:25:15]

Description was changed from

==========
Always persist histograms from setup.exe.

Before this CL, setup.exe did not always persist histograms (e.g. when
launched with --rename-chrome-exe, when SetupSingleton acquisition
failed during an install or update).

With this CL, RAII is used to ensure that a setup.exe process always
persists histograms (except when the histogram storage directory does
not exist).

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

to

==========
Always persist histograms from setup.exe.

Before this CL, setup.exe did not always persist histograms (e.g. when
launched with --rename-chrome-exe, when SetupSingleton acquisition
failed during an install or update).

With this CL, RAII is used to ensure that a setup.exe process always
persists histograms (except when the histogram storage directory does
not exist).

BUG=451546

Committed: https://crrev.com/10c71cf7cd97b4d893cd1fff74a1bd086fc46405
Cr-Commit-Position: refs/heads/master@{#418555}
==========

[commit-bot: I haz the power, 2016-09-14 14:25:17]

Patchset 4 (id:??) landed as
https://crrev.com/10c71cf7cd97b4d893cd1fff74a1bd086fc46405
Cr-Commit-Position: refs/heads/master@{#418555}