Add AuthPolicyLoginHelper

Add AuthPolicyLoginHelper

Allows cancel all pending calls and restart AuthPolicy service. Used
for enrollment and login UI to proper cancel the flows.

Also
Add RestartAuthPolicyService into UpstartClient.
Make UI flows properly cancel pending authpolicy operations.
Add delays in the FakeAuthPolicy clients in JoinAdDomain and
Authenticate user calls.
Move writing password piping into the AuthPolicyLoginHelper.

BUG=677487, 662400, 676337, 675597
TEST=manual
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2794493002
Cr-Commit-Position: refs/heads/master@{#463578}
Committed: https://chromium.googlesource.com/chromium/src/+/d8c46c166645ad80352881e692745cd2c30aa24f

[Roman Sorokin (ftl), 2017-03-31 11:56:24]

Description was changed from

==========
Add AuthPolicyLoginHelper

Allows cancel all pending calls and restart AuthPolicy service. Used
for enrollment and login UI to proper cancel the flows.

Also
Add RestartAuthPolicyService into UpstartClient.
Make UI flows properly cancel pending authpolicy operations.
Add delays in the FakeAuthPolicy clients in JoinAdDomain and
Authenticate user calls.
Move writing password piping into the AuthPolicyLoginHelper.

BUG=677487,662400,676337,675597
TEST=manual
==========

to

==========
Add AuthPolicyLoginHelper

Allows cancel all pending calls and restart AuthPolicy service. Used
for enrollment and login UI to proper cancel the flows.

Also
Add RestartAuthPolicyService into UpstartClient.
Make UI flows properly cancel pending authpolicy operations.
Add delays in the FakeAuthPolicy clients in JoinAdDomain and
Authenticate user calls.
Move writing password piping into the AuthPolicyLoginHelper.

BUG=677487,662400,676337,675597
TEST=manual
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[Roman Sorokin (ftl), 2017-03-31 11:56:31]

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

[commit-bot: I haz the power, 2017-03-31 11:56:51]

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

[Roman Sorokin (ftl), 2017-03-31 11:57:12]

rsorokin@chromium.org changed reviewers:
+ ljusten@chromium.org

[Roman Sorokin (ftl), 2017-03-31 11:57:12]

Hey Lutz, PTAL.

[commit-bot: I haz the power, 2017-03-31 12:50:57]

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

[commit-bot: I haz the power, 2017-03-31 12:50:58]

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

[ljusten (tachyonic), 2017-03-31 15:19:33]

Looks good, code should work, just some simplification suggestions.

https://codereview.chromium.org/2794493002/diff/1/chrome/browser/ui/webui/chr...
File chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.cc (right):

https://codereview.chromium.org/2794493002/diff/1/chrome/browser/ui/webui/chr...
chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.cc:612: 
DCHECK(authpolicy_login_helper_);?

https://codereview.chromium.org/2794493002/diff/1/chromeos/dbus/fake_auth_pol...
File chromeos/dbus/fake_auth_policy_client.cc (right):

https://codereview.chromium.org/2794493002/diff/1/chromeos/dbus/fake_auth_pol...
chromeos/dbus/fake_auth_policy_client.cc:85: if (!started_) {
This would get rid of a bunch of repetitions:

if (!started_) {
  LOG(ERROR) << "authpolicyd not started";
  error = authpolicy::ERROR_DBUS_FAILURE
} else if (...) {
  ...
}

PostDelayedClosure(base::Bind(callback, error));

https://codereview.chromium.org/2794493002/diff/1/chromeos/login/auth/authpol...
File chromeos/login/auth/authpolicy_login_helper.cc (right):

https://codereview.chromium.org/2794493002/diff/1/chromeos/login/auth/authpol...
chromeos/login/auth/authpolicy_login_helper.cc:38:
base::PostTaskWithTraitsAndReplyWithResult(
As discussed offline, this seems overkill. You're writing 20 bytes to a memory
location and the pipes are non-blocking. I don't think this can possibly block.

https://codereview.chromium.org/2794493002/diff/1/chromeos/login/auth/authpol...
chromeos/login/auth/authpolicy_login_helper.cc:53: GetPipeReadEnd(password,
base::Bind(&AuthPolicyLoginHelper::DoJoinAdDomain,
This could be much simpler, see above.

https://codereview.chromium.org/2794493002/diff/1/chromeos/login/auth/authpol...
chromeos/login/auth/authpolicy_login_helper.cc:58: void
AuthPolicyLoginHelper::DoJoinAdDomain(const std::string& machine_name,
Order of method definition doesn't match declaration order.

https://codereview.chromium.org/2794493002/diff/1/chromeos/login/auth/authpol...
File chromeos/login/auth/authpolicy_login_helper.h (right):

https://codereview.chromium.org/2794493002/diff/1/chromeos/login/auth/authpol...
chromeos/login/auth/authpolicy_login_helper.h:20: // Allows cancel all pending
calls and restart AuthPolicy service. Used for
Allows canceling
and restarting

[Roman Sorokin (ftl), 2017-04-04 10:07:19]

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

[commit-bot: I haz the power, 2017-04-04 10:07:33]

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

[Roman Sorokin (ftl), 2017-04-04 10:10:50]

Hey Lutz, PTAL.

https://codereview.chromium.org/2794493002/diff/1/chrome/browser/ui/webui/chr...
File chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.cc (right):

https://codereview.chromium.org/2794493002/diff/1/chrome/browser/ui/webui/chr...
chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.cc:612: 
On 2017/03/31 15:19:33, ljusten wrote:
> DCHECK(authpolicy_login_helper_);?

Done.

https://codereview.chromium.org/2794493002/diff/1/chromeos/dbus/fake_auth_pol...
File chromeos/dbus/fake_auth_policy_client.cc (right):

https://codereview.chromium.org/2794493002/diff/1/chromeos/dbus/fake_auth_pol...
chromeos/dbus/fake_auth_policy_client.cc:85: if (!started_) {
On 2017/03/31 15:19:33, ljusten wrote:
> This would get rid of a bunch of repetitions:
> 
> if (!started_) {
>   LOG(ERROR) << "authpolicyd not started";
>   error = authpolicy::ERROR_DBUS_FAILURE
> } else if (...) {
>   ...
> }
> 
> PostDelayedClosure(base::Bind(callback, error));

Done.

https://codereview.chromium.org/2794493002/diff/1/chromeos/login/auth/authpol...
File chromeos/login/auth/authpolicy_login_helper.cc (right):

https://codereview.chromium.org/2794493002/diff/1/chromeos/login/auth/authpol...
chromeos/login/auth/authpolicy_login_helper.cc:38:
base::PostTaskWithTraitsAndReplyWithResult(
On 2017/03/31 15:19:33, ljusten wrote:
> As discussed offline, this seems overkill. You're writing 20 bytes to a memory
> location and the pipes are non-blocking. I don't think this can possibly
block.

Done.

https://codereview.chromium.org/2794493002/diff/1/chromeos/login/auth/authpol...
chromeos/login/auth/authpolicy_login_helper.cc:53: GetPipeReadEnd(password,
base::Bind(&AuthPolicyLoginHelper::DoJoinAdDomain,
On 2017/03/31 15:19:33, ljusten wrote:
> This could be much simpler, see above.

Done.

https://codereview.chromium.org/2794493002/diff/1/chromeos/login/auth/authpol...
chromeos/login/auth/authpolicy_login_helper.cc:58: void
AuthPolicyLoginHelper::DoJoinAdDomain(const std::string& machine_name,
On 2017/03/31 15:19:33, ljusten wrote:
> Order of method definition doesn't match declaration order.

Done.

https://codereview.chromium.org/2794493002/diff/1/chromeos/login/auth/authpol...
File chromeos/login/auth/authpolicy_login_helper.h (right):

https://codereview.chromium.org/2794493002/diff/1/chromeos/login/auth/authpol...
chromeos/login/auth/authpolicy_login_helper.h:20: // Allows cancel all pending
calls and restart AuthPolicy service. Used for
On 2017/03/31 15:19:33, ljusten wrote:
> Allows canceling
> and restarting

Done.

[Roman Sorokin (ftl), 2017-04-04 11:36:40]

rsorokin@chromium.org changed reviewers:
+ alemate@chromium.org, hashimoto@chromium.org

[Roman Sorokin (ftl), 2017-04-04 11:36:41]

Hi, hashimoto@chromium.org: Please review changes in 
chromeos/dbus

Hi, alemate@chromium.org: Please review changes in 
chrome/browser/
chromeos/login/auth

[hashimoto, 2017-04-04 12:04:13]

chromeos/dbus lgtm with nits

https://codereview.chromium.org/2794493002/diff/20001/chromeos/dbus/fake_auth...
File chromeos/dbus/fake_auth_policy_client.h (right):

https://codereview.chromium.org/2794493002/diff/20001/chromeos/dbus/fake_auth...
chromeos/dbus/fake_auth_policy_client.h:42: bool get_started() const { return
started_; }
started() instead of get_started().

https://codereview.chromium.org/2794493002/diff/20001/chromeos/dbus/fake_auth...
File chromeos/dbus/fake_auth_policy_client_unittest.cc (right):

https://codereview.chromium.org/2794493002/diff/20001/chromeos/dbus/fake_auth...
chromeos/dbus/fake_auth_policy_client_unittest.cc:23: base::MessageLoop loop;
You can let a test fixture class own this loop and use TEST_F to avoid repeating
this.
https://github.com/google/googletest/blob/master/googletest/docs/Primer.md#te...

[commit-bot: I haz the power, 2017-04-04 12:30:02]

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

[commit-bot: I haz the power, 2017-04-04 12:30:03]

Dry run: This issue passed the CQ dry run.

[ljusten (tachyonic), 2017-04-04 12:51:33]

https://codereview.chromium.org/2794493002/diff/20001/chromeos/login/auth/aut...
File chromeos/login/auth/authpolicy_login_helper.cc (right):

https://codereview.chromium.org/2794493002/diff/20001/chromeos/login/auth/aut...
chromeos/login/auth/authpolicy_login_helper.cc:53: username,
GetDataReadPipe(password).get(),
I was a bit concerned about this, but seems like it's safe.
http://stackoverflow.com/questions/10006891/stdstringc-str-and-temporaries

https://codereview.chromium.org/2794493002/diff/20001/chromeos/login/auth/aut...
chromeos/login/auth/authpolicy_login_helper.cc:58: void
AuthPolicyLoginHelper::CancelRequestsAndRestart() {
Are you aware of
https://cs.chromium.org/chromium/src/base/cancelable_callback.h?
I was wondering whether you could also use that.

[Roman Sorokin (ftl), 2017-04-06 15:00:33]

https://codereview.chromium.org/2794493002/diff/20001/chromeos/dbus/fake_auth...
File chromeos/dbus/fake_auth_policy_client.h (right):

https://codereview.chromium.org/2794493002/diff/20001/chromeos/dbus/fake_auth...
chromeos/dbus/fake_auth_policy_client.h:42: bool get_started() const { return
started_; }
On 2017/04/04 12:04:13, hashimoto wrote:
> started() instead of get_started().

Done.

https://codereview.chromium.org/2794493002/diff/20001/chromeos/dbus/fake_auth...
File chromeos/dbus/fake_auth_policy_client_unittest.cc (right):

https://codereview.chromium.org/2794493002/diff/20001/chromeos/dbus/fake_auth...
chromeos/dbus/fake_auth_policy_client_unittest.cc:23: base::MessageLoop loop;
On 2017/04/04 12:04:13, hashimoto wrote:
> You can let a test fixture class own this loop and use TEST_F to avoid
repeating
> this.
>
https://github.com/google/googletest/blob/master/googletest/docs/Primer.md#te...

Done.

https://codereview.chromium.org/2794493002/diff/20001/chromeos/login/auth/aut...
File chromeos/login/auth/authpolicy_login_helper.cc (right):

https://codereview.chromium.org/2794493002/diff/20001/chromeos/login/auth/aut...
chromeos/login/auth/authpolicy_login_helper.cc:53: username,
GetDataReadPipe(password).get(),
On 2017/04/04 12:51:32, ljusten wrote:
> I was a bit concerned about this, but seems like it's safe.
> http://stackoverflow.com/questions/10006891/stdstringc-str-and-temporaries

Acknowledged.

https://codereview.chromium.org/2794493002/diff/20001/chromeos/login/auth/aut...
chromeos/login/auth/authpolicy_login_helper.cc:58: void
AuthPolicyLoginHelper::CancelRequestsAndRestart() {
On 2017/04/04 12:51:32, ljusten wrote:
> Are you aware of
> https://cs.chromium.org/chromium/src/base/cancelable_callback.h
> I was wondering whether you could also use that.

I think it would be more complicated.

[Roman Sorokin (ftl), 2017-04-06 15:33:47]

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

[commit-bot: I haz the power, 2017-04-06 15:34:25]

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-04-06 16:37:15]

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

[commit-bot: I haz the power, 2017-04-06 16:37:18]

Dry run: This issue passed the CQ dry run.

[Roman Sorokin (ftl), 2017-04-07 12:09:29]

rsorokin@chromium.org changed reviewers:
+ xiyuan@chromium.org
- alemate@chromium.org

[Roman Sorokin (ftl), 2017-04-07 12:09:30]

Hi, xiyuan@chromium.org: Please review changes in 
chrome/browser/
chromeos/login/auth
Thanks!

[xiyuan, 2017-04-07 15:04:24]

https://codereview.chromium.org/2794493002/diff/40001/chrome/browser/chromeos...
File chrome/browser/chromeos/login/enterprise_enrollment_browsertest.cc (right):

https://codereview.chromium.org/2794493002/diff/40001/chrome/browser/chromeos...
chrome/browser/chromeos/login/enterprise_enrollment_browsertest.cc:180:
content::DOMMessageQueue message_queue;
This impl pattern would cause test flakes. This has bite us before. We need to
make sure DOMMessageQueue is created before the DOM message is generated since
any messages sent before the construction is lost.

In our case here, we need to create DOMMessageQueue before
SubmitActiveDirectoryCredentials. JS eval is an IPC and has a nested loop and
its own DOMMessageQueue to wait to JS exec finish. There is no guarantee that
WaitForMessage would be called before JS sends back the message.

Consider move DOMMessageQueue out of here and put it before
SubmitActiveDirectoryCredentials and pass a pointer or reference to this
function.

https://codereview.chromium.org/2794493002/diff/40001/chrome/browser/chromeos...
File chrome/browser/chromeos/login/login_browsertest.cc (right):

https://codereview.chromium.org/2794493002/diff/40001/chrome/browser/chromeos...
chrome/browser/chromeos/login/login_browsertest.cc:298: content::DOMMessageQueue
message_queue;
ditto

https://codereview.chromium.org/2794493002/diff/40001/chromeos/dbus/fake_auth...
File chromeos/dbus/fake_auth_policy_client_unittest.cc (right):

https://codereview.chromium.org/2794493002/diff/40001/chromeos/dbus/fake_auth...
chromeos/dbus/fake_auth_policy_client_unittest.cc:23: FakeAuthPolicyClient
client;
client -> client_

And I would prefer to put this in private and add a getter for it.

https://codereview.chromium.org/2794493002/diff/40001/chromeos/dbus/fake_auth...
chromeos/dbus/fake_auth_policy_client_unittest.cc:26: base::MessageLoop loop;
loop -> loop_

https://codereview.chromium.org/2794493002/diff/40001/chromeos/dbus/fake_auth...
chromeos/dbus/fake_auth_policy_client_unittest.cc:27: };
nit: DISALLOW_COPY_AND_ASSIGN

https://codereview.chromium.org/2794493002/diff/40001/chromeos/login/auth/aut...
File chromeos/login/auth/authpolicy_login_helper.cc (right):

https://codereview.chromium.org/2794493002/diff/40001/chromeos/login/auth/aut...
chromeos/login/auth/authpolicy_login_helper.cc:43: machine_name, username,
GetDataReadPipe(password).get(),
JoinAdDomain is called on UI thread. Would this cause CreateLocalNonBlockingPipe
be called on UI thread?

https://codereview.chromium.org/2794493002/diff/40001/chromeos/login/auth/aut...
File chromeos/login/auth/authpolicy_login_helper.h (right):

https://codereview.chromium.org/2794493002/diff/40001/chromeos/login/auth/aut...
chromeos/login/auth/authpolicy_login_helper.h:29: // See
AuthPolicyClient::JoinAdDomain.
nit: insert an empty line before

https://codereview.chromium.org/2794493002/diff/40001/chromeos/login/auth/aut...
chromeos/login/auth/authpolicy_login_helper.h:34: // See
AuthPolicyClient::AuthenticateUser.
nit: insert an empty line before

https://codereview.chromium.org/2794493002/diff/40001/chromeos/login/auth/aut...
chromeos/login/auth/authpolicy_login_helper.h:38: // Cancel pending requests and
restarts AuthPolicy service.
nit: insert an empty line before

[Roman Sorokin (ftl), 2017-04-10 16:09:00]

Hey Xiyuan, thanks for the fast review! Please take another look.

https://codereview.chromium.org/2794493002/diff/40001/chrome/browser/chromeos...
File chrome/browser/chromeos/login/enterprise_enrollment_browsertest.cc (right):

https://codereview.chromium.org/2794493002/diff/40001/chrome/browser/chromeos...
chrome/browser/chromeos/login/enterprise_enrollment_browsertest.cc:180:
content::DOMMessageQueue message_queue;
On 2017/04/07 15:04:24, xiyuan wrote:
> This impl pattern would cause test flakes. This has bite us before. We need to
> make sure DOMMessageQueue is created before the DOM message is generated since
> any messages sent before the construction is lost.
> 
> In our case here, we need to create DOMMessageQueue before
> SubmitActiveDirectoryCredentials. JS eval is an IPC and has a nested loop and
> its own DOMMessageQueue to wait to JS exec finish. There is no guarantee that
> WaitForMessage would be called before JS sends back the message.
> 
> Consider move DOMMessageQueue out of here and put it before
> SubmitActiveDirectoryCredentials and pass a pointer or reference to this
> function.

Done.

https://codereview.chromium.org/2794493002/diff/40001/chrome/browser/chromeos...
File chrome/browser/chromeos/login/login_browsertest.cc (right):

https://codereview.chromium.org/2794493002/diff/40001/chrome/browser/chromeos...
chrome/browser/chromeos/login/login_browsertest.cc:298: content::DOMMessageQueue
message_queue;
On 2017/04/07 15:04:24, xiyuan wrote:
> ditto

Done.

https://codereview.chromium.org/2794493002/diff/40001/chromeos/dbus/fake_auth...
File chromeos/dbus/fake_auth_policy_client_unittest.cc (right):

https://codereview.chromium.org/2794493002/diff/40001/chromeos/dbus/fake_auth...
chromeos/dbus/fake_auth_policy_client_unittest.cc:23: FakeAuthPolicyClient
client;
On 2017/04/07 15:04:24, xiyuan wrote:
> client -> client_
> 
> And I would prefer to put this in private and add a getter for it.

Done.

https://codereview.chromium.org/2794493002/diff/40001/chromeos/dbus/fake_auth...
chromeos/dbus/fake_auth_policy_client_unittest.cc:26: base::MessageLoop loop;
On 2017/04/07 15:04:24, xiyuan wrote:
> loop -> loop_

Done.

https://codereview.chromium.org/2794493002/diff/40001/chromeos/dbus/fake_auth...
chromeos/dbus/fake_auth_policy_client_unittest.cc:27: };
On 2017/04/07 15:04:24, xiyuan wrote:
> nit: DISALLOW_COPY_AND_ASSIGN

Done.

https://codereview.chromium.org/2794493002/diff/40001/chromeos/login/auth/aut...
File chromeos/login/auth/authpolicy_login_helper.cc (right):

https://codereview.chromium.org/2794493002/diff/40001/chromeos/login/auth/aut...
chromeos/login/auth/authpolicy_login_helper.cc:43: machine_name, username,
GetDataReadPipe(password).get(),
On 2017/04/07 15:04:24, xiyuan wrote:
> JoinAdDomain is called on UI thread. Would this cause
CreateLocalNonBlockingPipe
> be called on UI thread? 

Yes, but this call should not block. Probably overhead of the passing it to a
different thread would be worse.

https://codereview.chromium.org/2794493002/diff/40001/chromeos/login/auth/aut...
File chromeos/login/auth/authpolicy_login_helper.h (right):

https://codereview.chromium.org/2794493002/diff/40001/chromeos/login/auth/aut...
chromeos/login/auth/authpolicy_login_helper.h:29: // See
AuthPolicyClient::JoinAdDomain.
On 2017/04/07 15:04:24, xiyuan wrote:
> nit: insert an empty line before

Done.

https://codereview.chromium.org/2794493002/diff/40001/chromeos/login/auth/aut...
chromeos/login/auth/authpolicy_login_helper.h:34: // See
AuthPolicyClient::AuthenticateUser.
On 2017/04/07 15:04:24, xiyuan wrote:
> nit: insert an empty line before

Done.

https://codereview.chromium.org/2794493002/diff/40001/chromeos/login/auth/aut...
chromeos/login/auth/authpolicy_login_helper.h:38: // Cancel pending requests and
restarts AuthPolicy service.
On 2017/04/07 15:04:24, xiyuan wrote:
> nit: insert an empty line before

Done.

[Roman Sorokin (ftl), 2017-04-10 16:28:40]

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

[commit-bot: I haz the power, 2017-04-10 16:29:05]

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

[xiyuan, 2017-04-10 16:30:29]

lgtm

https://codereview.chromium.org/2794493002/diff/40001/chromeos/login/auth/aut...
File chromeos/login/auth/authpolicy_login_helper.cc (right):

https://codereview.chromium.org/2794493002/diff/40001/chromeos/login/auth/aut...
chromeos/login/auth/authpolicy_login_helper.cc:43: machine_name, username,
GetDataReadPipe(password).get(),
On 2017/04/10 16:09:00, Roman Sorokin (fast) wrote:
> On 2017/04/07 15:04:24, xiyuan wrote:
> > JoinAdDomain is called on UI thread. Would this cause
> CreateLocalNonBlockingPipe
> > be called on UI thread? 
> 
> Yes, but this call should not block. Probably overhead of the passing it to a
> different thread would be worse.

Okay

[commit-bot: I haz the power, 2017-04-10 17:26:26]

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

[commit-bot: I haz the power, 2017-04-10 17:26:27]

Dry run: This issue passed the CQ dry run.

[ljusten (tachyonic), 2017-04-11 08:35:23]

Looks good to me (but I'm not a committer yet).

[Roman Sorokin (ftl), 2017-04-11 08:36:15]

The CQ bit was checked by rsorokin@chromium.org

[Roman Sorokin (ftl), 2017-04-11 08:36:16]

The patchset sent to the CQ was uploaded after l-g-t-m from
hashimoto@chromium.org
Link to the patchset: https://codereview.chromium.org/2794493002/#ps60001
(title: "Update after review")

[commit-bot: I haz the power, 2017-04-11 08:36:47]

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-04-11 08:43:23]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1491899775946290,
"parent_rev": "a8565f4299ea5c705a629363dd63eb0679722bbc", "commit_rev":
"d8c46c166645ad80352881e692745cd2c30aa24f"}

[commit-bot: I haz the power, 2017-04-11 08:44:18]

Description was changed from

==========
Add AuthPolicyLoginHelper

Allows cancel all pending calls and restart AuthPolicy service. Used
for enrollment and login UI to proper cancel the flows.

Also
Add RestartAuthPolicyService into UpstartClient.
Make UI flows properly cancel pending authpolicy operations.
Add delays in the FakeAuthPolicy clients in JoinAdDomain and
Authenticate user calls.
Move writing password piping into the AuthPolicyLoginHelper.

BUG=677487,662400,676337,675597
TEST=manual
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Add AuthPolicyLoginHelper

Allows cancel all pending calls and restart AuthPolicy service. Used
for enrollment and login UI to proper cancel the flows.

Also
Add RestartAuthPolicyService into UpstartClient.
Make UI flows properly cancel pending authpolicy operations.
Add delays in the FakeAuthPolicy clients in JoinAdDomain and
Authenticate user calls.
Move writing password piping into the AuthPolicyLoginHelper.

BUG=677487,662400,676337,675597
TEST=manual
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2794493002
Cr-Commit-Position: refs/heads/master@{#463578}
Committed:
https://chromium.googlesource.com/chromium/src/+/d8c46c166645ad80352881e69274...
==========

[commit-bot: I haz the power, 2017-04-11 08:44:19]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/d8c46c166645ad80352881e69274...

[Roman Sorokin (ftl), 2017-04-11 08:44:31]

Description was changed from

==========
Add AuthPolicyLoginHelper

Allows cancel all pending calls and restart AuthPolicy service. Used
for enrollment and login UI to proper cancel the flows.

Also
Add RestartAuthPolicyService into UpstartClient.
Make UI flows properly cancel pending authpolicy operations.
Add delays in the FakeAuthPolicy clients in JoinAdDomain and
Authenticate user calls.
Move writing password piping into the AuthPolicyLoginHelper.

BUG=677487,662400,676337,675597
TEST=manual
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2794493002
Cr-Commit-Position: refs/heads/master@{#463578}
Committed:
https://chromium.googlesource.com/chromium/src/+/d8c46c166645ad80352881e69274...
==========

to

==========
Add AuthPolicyLoginHelper

Allows cancel all pending calls and restart AuthPolicy service. Used
for enrollment and login UI to proper cancel the flows.

Also
Add RestartAuthPolicyService into UpstartClient.
Make UI flows properly cancel pending authpolicy operations.
Add delays in the FakeAuthPolicy clients in JoinAdDomain and
Authenticate user calls.
Move writing password piping into the AuthPolicyLoginHelper.

BUG=677487,662400,676337,675597
TEST=manual
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2794493002
Cr-Commit-Position: refs/heads/master@{#463578}
Committed:
https://chromium.googlesource.com/chromium/src/+/d8c46c166645ad80352881e69274...
==========

[Roman Sorokin (ftl), 2017-04-11 08:44:31]

rsorokin@chromium.org changed reviewers:
- ljusten@chromium.org