Installer support for Windows 10 inactive user toast.

Installer support for Windows 10 inactive user toast.

BUG=717091
TBR=thakis@chromium.org

Review-Url: https://codereview.chromium.org/2933043002
Cr-Commit-Position: refs/heads/master@{#479735}
Committed: https://chromium.googlesource.com/chromium/src/+/42c01cb631e9a3de4a9bbff3efd50ab2d028915d

[grt (UTC plus 2), 2017-06-11 21:22:34]

The CQ bit was checked by grt@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-11 21:22:40]

Dry run: CQ is trying da patch.

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

[grt (UTC plus 2), 2017-06-11 21:28:11]

grt@chromium.org changed reviewers:
+ nikunjb@chromium.org, pmonette@chromium.org

[grt (UTC plus 2), 2017-06-11 21:28:11]

This is the skeleton for the experiment code in the installer. It won't do
anything for the time being (see TODO in IsSelectedForStudy), but I'd like to
get the framework reviewed and landed so we can iterate on it. Before enabling
this, I hope to have bits in place so that we can easily execute stage 2 of the
study, where we ship some variant(s) of the toast to the majority of the
population based on the results of stage 1. I'll work on this in parallel to
landing this piece. Thanks for taking a look.

[commit-bot: I haz the power, 2017-06-11 21:30:50]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-06-11 21:30:51]

Dry run: Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[grt (UTC plus 2), 2017-06-12 07:42:46]

The CQ bit was checked by grt@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-12 07:42:51]

Dry run: 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, 2017-06-12 07:50:40]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-06-12 07:50:41]

Dry run: Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[grt (UTC plus 2), 2017-06-12 09:12:28]

The CQ bit was checked by grt@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-12 09:12:40]

Dry run: 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, 2017-06-12 09:21:37]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-06-12 09:21:37]

Dry run: Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[grt (UTC plus 2), 2017-06-12 13:14:22]

The CQ bit was checked by grt@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-12 13:14:28]

Dry run: 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, 2017-06-12 14:54:29]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-06-12 14:54:30]

Dry run: This issue passed the CQ dry run.

[grt (UTC plus 2), 2017-06-13 11:22:33]

The CQ bit was checked by grt@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-13 11:22:41]

Dry run: CQ is trying da patch.

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

[grt (UTC plus 2), 2017-06-13 12:25:13]

PTAL!

[commit-bot: I haz the power, 2017-06-13 13:36:08]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-06-13 13:36:10]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[Patrick Monette, 2017-06-13 19:13:42]

I am still a bit confused with the experiment code.

why study 1 vs study2?

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

https://codereview.chromium.org/2933043002/diff/80001/chrome/installer/setup/...
chrome/installer/setup/user_experiment.cc:423: if (brand == L"GGRV")
NIt: Is there any comment or link that could be added to make these brand checks
look a bit less random?

https://codereview.chromium.org/2933043002/diff/80001/chrome/installer/util/e...
File chrome/installer/util/experiment_storage.cc (right):

https://codereview.chromium.org/2933043002/diff/80001/chrome/installer/util/e...
chrome/installer/util/experiment_storage.cc:93: *path =
install_details.GetClientStateMediumKeyPath()
Looks weird at first glance. Why not also move the second L"\\" into the
constant?

I suggest kRegKeyRetentionFormat[] and use StringPrintf().

https://codereview.chromium.org/2933043002/diff/80001/chrome/tools/test/exper...
File chrome/tools/test/experiment_tool_win.py (right):

https://codereview.chromium.org/2933043002/diff/80001/chrome/tools/test/exper...
chrome/tools/test/experiment_tool_win.py:77: """Deletes the RetntionStudy value
for the install."""
Typo

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

https://codereview.chromium.org/2933043002/diff/100001/chrome/installer/setup...
chrome/installer/setup/install_worker.cc:665: // Chrome's installer. Do it here
as well so that they key exists for manual
.. so that the key ..

https://codereview.chromium.org/2933043002/diff/100001/chrome/installer/setup...
File chrome/installer/setup/user_experiment.cc (right):

https://codereview.chromium.org/2933043002/diff/100001/chrome/installer/setup...
chrome/installer/setup/user_experiment.cc:110: // windowstation.
space?

https://codereview.chromium.org/2933043002/diff/100001/chrome/installer/setup...
chrome/installer/setup/user_experiment.cc:150: return state == QUNS_NOT_PRESENT
||          // Locked/screensaver running.
Whats the rationale for showing the notification when the lock screen is on?

What if during a presentation, the screensaver starts because the presenter
didn't touch
the computer for a while?

The name of QUNS_ACCEPTS_NOTIFICATIONS pretty much suggest it should be the only
one who
allows notifications

https://codereview.chromium.org/2933043002/diff/100001/chrome/installer/setup...
chrome/installer/setup/user_experiment.cc:160: return session_length >=
base::TimeDelta::FromMinutes(5);
Give a name to this 5 minutes constant

https://codereview.chromium.org/2933043002/diff/100001/chrome/installer/setup...
chrome/installer/setup/user_experiment.cc:229: // NOTE: No clients will
participate while this under development.
Nit: Missing a word?

https://codereview.chromium.org/2933043002/diff/100001/chrome/installer/setup...
chrome/installer/setup/user_experiment.cc:236: if (!lock->LoadMetrics(&metrics)
|| metrics.InTerminalState())
Should you explicitly check that you are in an initial state?

https://codereview.chromium.org/2933043002/diff/100001/chrome/installer/setup...
chrome/installer/setup/user_experiment.cc:469: participation =
base::RandDouble() <= 0.02756962532
Why is this not 0.025?

https://codereview.chromium.org/2933043002/diff/100001/chrome/installer/setup...
chrome/installer/setup/user_experiment.cc:490: return base::RandDouble() < 0.01
? kHoldbackGroup : kStudyTwoGroup;
Why split non-participating (participation != ExperimentStorage::kStudyOne)
users into 2 groups? Holdback from what?

https://codereview.chromium.org/2933043002/diff/100001/chrome/installer/util/...
File chrome/installer/util/experiment_storage.h (right):

https://codereview.chromium.org/2933043002/diff/100001/chrome/installer/util/...
chrome/installer/util/experiment_storage.h:47: static constexpr Study kStudyTwo
= 2;
Does study 2 still means not participating?

[grt (UTC plus 2), 2017-06-14 20:43:43]

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

[grt (UTC plus 2), 2017-06-14 20:43:44]

I've updated the "Study selection" section of the design doc. I hope it clears
things up a little bit.

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

https://codereview.chromium.org/2933043002/diff/80001/chrome/installer/setup/...
chrome/installer/setup/user_experiment.cc:423: if (brand == L"GGRV")
On 2017/06/13 19:13:40, Patrick Monette wrote:
> NIt: Is there any comment or link that could be added to make these brand
checks
> look a bit less random?

+georgesak to confirm that I have the correct set of brand codes and to suggest
a comment/link.

https://codereview.chromium.org/2933043002/diff/80001/chrome/installer/util/e...
File chrome/installer/util/experiment_storage.cc (right):

https://codereview.chromium.org/2933043002/diff/80001/chrome/installer/util/e...
chrome/installer/util/experiment_storage.cc:93: *path =
install_details.GetClientStateMediumKeyPath()
On 2017/06/13 19:13:40, Patrick Monette wrote:
> Looks weird at first glance. Why not also move the second L"\\" into the
> constant?
> 
> I suggest kRegKeyRetentionFormat[] and use StringPrintf().

Acknowledged.

https://codereview.chromium.org/2933043002/diff/80001/chrome/tools/test/exper...
File chrome/tools/test/experiment_tool_win.py (right):

https://codereview.chromium.org/2933043002/diff/80001/chrome/tools/test/exper...
chrome/tools/test/experiment_tool_win.py:77: """Deletes the RetntionStudy value
for the install."""
On 2017/06/13 19:13:40, Patrick Monette wrote:
> Typo

Done.

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

https://codereview.chromium.org/2933043002/diff/100001/chrome/installer/setup...
chrome/installer/setup/install_worker.cc:665: // Chrome's installer. Do it here
as well so that they key exists for manual
On 2017/06/13 19:13:41, Patrick Monette wrote:
> .. so that the key ..

Done.

https://codereview.chromium.org/2933043002/diff/100001/chrome/installer/setup...
File chrome/installer/setup/user_experiment.cc (right):

https://codereview.chromium.org/2933043002/diff/100001/chrome/installer/setup...
chrome/installer/setup/user_experiment.cc:110: // windowstation.
On 2017/06/13 19:13:42, Patrick Monette wrote:
> space?

Done.

https://codereview.chromium.org/2933043002/diff/100001/chrome/installer/setup...
chrome/installer/setup/user_experiment.cc:150: return state == QUNS_NOT_PRESENT
||          // Locked/screensaver running.
On 2017/06/13 19:13:41, Patrick Monette wrote:
> Whats the rationale for showing the notification when the lock screen is on?

So that the toast is there on the desktop when you unlock.

> What if during a presentation, the screensaver starts because the presenter
> didn't touch
> the computer for a while?

Hmm. That would be sub-optimal. I wonder if the fullscreen check would succeed
in that case?

> The name of QUNS_ACCEPTS_NOTIFICATIONS pretty much suggest it should be the
only
> one who
> allows notifications

It does, but maybe it's not what we want. :-)

I'll experiment with fullscreen+locked to make sure the behavior makes sense.

https://codereview.chromium.org/2933043002/diff/100001/chrome/installer/setup...
chrome/installer/setup/user_experiment.cc:160: return session_length >=
base::TimeDelta::FromMinutes(5);
On 2017/06/13 19:13:41, Patrick Monette wrote:
> Give a name to this 5 minutes constant

Done.

https://codereview.chromium.org/2933043002/diff/100001/chrome/installer/setup...
chrome/installer/setup/user_experiment.cc:229: // NOTE: No clients will
participate while this under development.
On 2017/06/13 19:13:41, Patrick Monette wrote:
> Nit: Missing a word?

Done.

https://codereview.chromium.org/2933043002/diff/100001/chrome/installer/setup...
chrome/installer/setup/user_experiment.cc:236: if (!lock->LoadMetrics(&metrics)
|| metrics.InTerminalState())
On 2017/06/13 19:13:41, Patrick Monette wrote:
> Should you explicitly check that you are in an initial state?

No -- it's okay to pick up where we left off so long as the toast was never
shown.

https://codereview.chromium.org/2933043002/diff/100001/chrome/installer/setup...
chrome/installer/setup/user_experiment.cc:469: participation =
base::RandDouble() <= 0.02756962532
On 2017/06/13 19:13:41, Patrick Monette wrote:
> Why is this not 0.025?

This is the value we reached via maths in a doc.

https://codereview.chromium.org/2933043002/diff/100001/chrome/installer/setup...
chrome/installer/setup/user_experiment.cc:490: return base::RandDouble() < 0.01
? kHoldbackGroup : kStudyTwoGroup;
On 2017/06/13 19:13:41, Patrick Monette wrote:
> Why split non-participating (participation != ExperimentStorage::kStudyOne)
> users into 2 groups? Holdback from what?

The holdback group is the group that participates in the study, but never sees
the UX. We can evaluate retention in other groups in relation to the holdback
group to understand how the UX impacts retention.

We'll pick the actual value for kStudyTwoGroup once we have results from the
first phase of the study. We may choose to change this logic as well, in case we
decide to split the population differently. For now, this is placeholder code so
that the eventual changes for the 2nd phase of the study can be merged.

https://codereview.chromium.org/2933043002/diff/100001/chrome/installer/util/...
File chrome/installer/util/experiment_storage.h (right):

https://codereview.chromium.org/2933043002/diff/100001/chrome/installer/util/...
chrome/installer/util/experiment_storage.h:47: static constexpr Study kStudyTwo
= 2;
On 2017/06/13 19:13:42, Patrick Monette wrote:
> Does study 2 still means not participating?

Not any more. It means not participating in the current change (~2.8% of the
population participates now). Once we have data, we'll send the rest of the
population through in study 2. I rejiggered things in this patch set so that the
code change for study 2 is easy to merge.

[grt (UTC plus 2), 2017-06-14 21:15:13]

The CQ bit was checked by grt@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-14 21:15:39]

Dry run: 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, 2017-06-14 21:27:23]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-06-14 21:27:24]

Dry run: Try jobs failed on following builders:
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)

[Patrick Monette, 2017-06-14 22:28:08]

lgtm

https://codereview.chromium.org/2933043002/diff/140001/chrome/installer/setup...
File chrome/installer/setup/user_experiment.cc (right):

https://codereview.chromium.org/2933043002/diff/140001/chrome/installer/setup...
chrome/installer/setup/user_experiment.cc:543:
base::win::ScopedProcessInformation process_info(temp_process_info);
This is used only to close handles?

[grt (UTC plus 2), 2017-06-15 09:19:02]

The CQ bit was checked by grt@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-15 09:19:17]

Dry run: CQ is trying da patch.

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

[grt (UTC plus 2), 2017-06-15 09:27:37]

Thanks.

https://codereview.chromium.org/2933043002/diff/140001/chrome/installer/setup...
File chrome/installer/setup/user_experiment.cc (right):

https://codereview.chromium.org/2933043002/diff/140001/chrome/installer/setup...
chrome/installer/setup/user_experiment.cc:543:
base::win::ScopedProcessInformation process_info(temp_process_info);
On 2017/06/14 22:28:08, Patrick Monette wrote:
> This is used only to close handles?

Yup; comment added.

[commit-bot: I haz the power, 2017-06-15 10:41:49]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-06-15 10:41:52]

Dry run: This issue passed the CQ dry run.

[grt (UTC plus 2), 2017-06-15 11:11:20]

Description was changed from

==========
Installer support for Windows 10 inactive user toast.

BUG=717091
==========

to

==========
Installer support for Windows 10 inactive user toast.

BUG=717091
TBR=jochen@chromium.org
==========

[grt (UTC plus 2), 2017-06-15 11:11:48]

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

[grt (UTC plus 2), 2017-06-15 11:19:07]

+TBR jochen for chrome/tools/test addition

[grt (UTC plus 2), 2017-06-15 11:19:19]

The CQ bit was checked by grt@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-15 11:19:30]

Dry run: CQ is trying da patch.

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

[grt (UTC plus 2), 2017-06-15 11:38:18]

Description was changed from

==========
Installer support for Windows 10 inactive user toast.

BUG=717091
TBR=jochen@chromium.org
==========

to

==========
Installer support for Windows 10 inactive user toast.

BUG=717091
TBR=thakis@chromium.org
==========

[grt (UTC plus 2), 2017-06-15 11:38:18]

grt@chromium.org changed reviewers:
+ thakis@chromium.org
- jochen@chromium.org

[grt (UTC plus 2), 2017-06-15 11:38:39]

jochen -> thakis for chrome/tools/test OWNERS stamp

[commit-bot: I haz the power, 2017-06-15 12:55:09]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-06-15 12:55:10]

Dry run: This issue passed the CQ dry run.

[Georges Khalil, 2017-06-15 14:43:49]

Couple of comments for the enterprise specific checks.

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

https://codereview.chromium.org/2933043002/diff/80001/chrome/installer/setup/...
chrome/installer/setup/user_experiment.cc:423: if (brand == L"GGRV")
On 2017/06/14 20:43:44, grt (UTC plus 2) wrote:
> On 2017/06/13 19:13:40, Patrick Monette wrote:
> > NIt: Is there any comment or link that could be added to make these brand
> checks
> > look a bit less random?
> 
> +georgesak to confirm that I have the correct set of brand codes and to
suggest
> a comment/link.

These are the correct codes indeed.

But I think it's best if we don't depend on them, if possible, as these could
change eventually. How about using installer_state.is_msi() instead?

https://codereview.chromium.org/2933043002/diff/180001/chrome/installer/setup...
File chrome/installer/setup/user_experiment.cc (right):

https://codereview.chromium.org/2933043002/diff/180001/chrome/installer/setup...
chrome/installer/setup/user_experiment.cc:456: bool IsDomainJoined() {
I suggest using IsOS(OS_DOMAINMEMBER) == ENROLLED for consistency with Chrome,
as per:

https://cs.chromium.org/chromium/src/base/win/win_util.cc?type=cs&q=IsEnterpr...

[Nico, 2017-06-15 14:47:37]

lgtm

https://codereview.chromium.org/2933043002/diff/180001/chrome/installer/util/...
File chrome/installer/util/experiment_storage.h (right):

https://codereview.chromium.org/2933043002/diff/180001/chrome/installer/util/...
chrome/installer/util/experiment_storage.h:47: static constexpr Study kStudyTwo
= 2;
nit: fwiw enum is kind of preferable over static constexpr members because for
static constexpr members you technically need to define storage in the cc, else
you have UB (in practice, this means you'll get a link error if someone takes
the address of the constant -- while with an enum you get a compile error
instead if someone does this). So the lhs version here is imho better -- no UB
and whatnot. (you could just use an enum instead of an enum class of course)

https://codereview.chromium.org/2933043002/diff/180001/chrome/tools/test/expe...
File chrome/tools/test/experiment_tool_win.py (right):

https://codereview.chromium.org/2933043002/diff/180001/chrome/tools/test/expe...
chrome/tools/test/experiment_tool_win.py:14: # retention experiment.
I love this descriptive comment up here. And the whole file reads very well in
general.

You can reduce duplication between this comment and the `description` passed to
argparse below by using a module docstring instead of a comment here and then
passing that in, as described e.g. here:
https://stackoverflow.com/questions/18106327/display-pydocs-description-as-pa...
(you'd not list all the flags up here then but only maybe have an example
invocation)

[grt (UTC plus 2), 2017-06-15 15:07:26]

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

https://codereview.chromium.org/2933043002/diff/80001/chrome/installer/setup/...
chrome/installer/setup/user_experiment.cc:423: if (brand == L"GGRV")
On 2017/06/15 14:43:48, Georges Khalil wrote:
> On 2017/06/14 20:43:44, grt (UTC plus 2) wrote:
> > On 2017/06/13 19:13:40, Patrick Monette wrote:
> > > NIt: Is there any comment or link that could be added to make these brand
> > checks
> > > look a bit less random?
> > 
> > +georgesak to confirm that I have the correct set of brand codes and to
> suggest
> > a comment/link.
> 
> These are the correct codes indeed.
> 
> But I think it's best if we don't depend on them, if possible, as these could
> change eventually. How about using installer_state.is_msi() instead? 

Hmm. Yeah, I can do that. Thanks for the suggestion!

https://codereview.chromium.org/2933043002/diff/180001/chrome/installer/setup...
File chrome/installer/setup/user_experiment.cc (right):

https://codereview.chromium.org/2933043002/diff/180001/chrome/installer/setup...
chrome/installer/setup/user_experiment.cc:456: bool IsDomainJoined() {
On 2017/06/15 14:43:49, Georges Khalil wrote:
> I suggest using IsOS(OS_DOMAINMEMBER) == ENROLLED for consistency with Chrome,
> as per:
> 
>
https://cs.chromium.org/chromium/src/base/win/win_util.cc?type=cs&q=IsEnterpr...

That has a dependency on the shell, which I don't want to introduce in the
installer. As I mentioned in https://codereview.chromium.org/2048083003, I don't
think this was a good change to make in Chrome. It adds a level of indirection
to NetGetJoinInformation and doesn't actually insulate us from needing other
checks.

https://codereview.chromium.org/2933043002/diff/180001/chrome/installer/util/...
File chrome/installer/util/experiment_storage.h (right):

https://codereview.chromium.org/2933043002/diff/180001/chrome/installer/util/...
chrome/installer/util/experiment_storage.h:47: static constexpr Study kStudyTwo
= 2;
On 2017/06/15 14:47:37, Nico wrote:
> nit: fwiw enum is kind of preferable over static constexpr members because for
> static constexpr members you technically need to define storage in the cc,
else
> you have UB (in practice, this means you'll get a link error if someone takes
> the address of the constant -- while with an enum you get a compile error
> instead if someone does this). So the lhs version here is imho better -- no UB
> and whatnot. (you could just use an enum instead of an enum class of course)

Hmm. Makes sense. I think I made the switch because I didn't want to deal with
casting, but it seems to work just fine as a non-class enum. Thanks.

https://codereview.chromium.org/2933043002/diff/180001/chrome/tools/test/expe...
File chrome/tools/test/experiment_tool_win.py (right):

https://codereview.chromium.org/2933043002/diff/180001/chrome/tools/test/expe...
chrome/tools/test/experiment_tool_win.py:14: # retention experiment.
On 2017/06/15 14:47:37, Nico wrote:
> I love this descriptive comment up here. And the whole file reads very well in
> general.
> 
> You can reduce duplication between this comment and the `description` passed
to
> argparse below by using a module docstring instead of a comment here and then
> passing that in, as described e.g. here:
>
https://stackoverflow.com/questions/18106327/display-pydocs-description-as-pa...
> (you'd not list all the flags up here then but only maybe have an example
> invocation)

Nice!

[grt (UTC plus 2), 2017-06-15 15:07:37]

The CQ bit was checked by grt@chromium.org

[grt (UTC plus 2), 2017-06-15 15:07:38]

The patchset sent to the CQ was uploaded after l-g-t-m from
pmonette@chromium.org, thakis@chromium.org
Link to the patchset: https://codereview.chromium.org/2933043002/#ps200001
(title: "review feedback")

[commit-bot: I haz the power, 2017-06-15 15:07:50]

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, 2017-06-15 16:45:00]

CQ is committing da patch.

Bot data: {"patchset_id": 200001, "attempt_start_ts": 1497539257559070,
"parent_rev": "33db34fd4ded402c3c83dbfe299940a3c8652bed", "commit_rev":
"42c01cb631e9a3de4a9bbff3efd50ab2d028915d"}

[commit-bot: I haz the power, 2017-06-15 16:45:12]

Description was changed from

==========
Installer support for Windows 10 inactive user toast.

BUG=717091
TBR=thakis@chromium.org
==========

to

==========
Installer support for Windows 10 inactive user toast.

BUG=717091
TBR=thakis@chromium.org

Review-Url: https://codereview.chromium.org/2933043002
Cr-Commit-Position: refs/heads/master@{#479735}
Committed:
https://chromium.googlesource.com/chromium/src/+/42c01cb631e9a3de4a9bbff3efd5...
==========

[commit-bot: I haz the power, 2017-06-15 16:45:13]

Committed patchset #11 (id:200001) as
https://chromium.googlesource.com/chromium/src/+/42c01cb631e9a3de4a9bbff3efd5...