Emulate StartAuthPolicyService from FakeUpstartClient

Emulate StartAuthPolicyService from FakeUpstartClient

BUG=688378
TEST=FakeAuthPolicyClientTest.*

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

[Roman Sorokin (ftl), 2017-02-03 14:41:56]

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

[Roman Sorokin (ftl), 2017-02-03 14:41:57]

Hey Thiemo, Ryo, PTAL. Thanks!

[hashimoto, 2017-02-06 04:15:49]

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

https://codereview.chromium.org/2673813003/diff/1/chromeos/dbus/fake_auth_pol...
chromeos/dbus/fake_auth_policy_client.h:37: void Start() { started = true; }
Please add a comment to describe what |started_| means.
Also, I think void set_started(bool) would look more consistent with existing
code.

https://codereview.chromium.org/2673813003/diff/1/chromeos/dbus/fake_auth_pol...
chromeos/dbus/fake_auth_policy_client.h:40: bool started = false;
Please follow the naming rule.
https://google.github.io/styleguide/cppguide.html#Variable_Names

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

https://codereview.chromium.org/2673813003/diff/1/chromeos/dbus/fake_auth_pol...
chromeos/dbus/fake_auth_policy_client_unittest.cc:85:
base::Bind(&JoinAdCallback, kCorrectMachineName, kCorrectUserName,
How about using lambdas?

Instead of base::Bind(&JoinAdCallback, ...), you can write
 base::Bind([](authpolicy::ErrorType error){
   EXPECT_EQ(authpolicy::ERROR_DBUS_FAILURE, error);
 });

The same goes for other callbacks.

https://codereview.chromium.org/2673813003/diff/1/chromeos/dbus/fake_upstart_...
File chromeos/dbus/fake_upstart_client.cc (right):

https://codereview.chromium.org/2673813003/diff/1/chromeos/dbus/fake_upstart_...
chromeos/dbus/fake_upstart_client.cc:20:
chromeos::DBusThreadManager::Get()->GetAuthPolicyClient())
nit: No need to have "chromeos::".
You are already in chromeos namespace.

[Roman Sorokin (ftl), 2017-02-06 10:30:40]

Thanks for the review! PTAL.

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

https://codereview.chromium.org/2673813003/diff/1/chromeos/dbus/fake_auth_pol...
chromeos/dbus/fake_auth_policy_client.h:37: void Start() { started = true; }
On 2017/02/06 04:15:49, hashimoto wrote:
> Please add a comment to describe what |started_| means.
> Also, I think void set_started(bool) would look more consistent with existing
> code.

Done.

https://codereview.chromium.org/2673813003/diff/1/chromeos/dbus/fake_auth_pol...
chromeos/dbus/fake_auth_policy_client.h:40: bool started = false;
On 2017/02/06 04:15:49, hashimoto wrote:
> Please follow the naming rule.
> https://google.github.io/styleguide/cppguide.html#Variable_Names

Done.

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

https://codereview.chromium.org/2673813003/diff/1/chromeos/dbus/fake_auth_pol...
chromeos/dbus/fake_auth_policy_client_unittest.cc:85:
base::Bind(&JoinAdCallback, kCorrectMachineName, kCorrectUserName,
On 2017/02/06 04:15:49, hashimoto wrote:
> How about using lambdas?
> 
> Instead of base::Bind(&JoinAdCallback, ...), you can write
>  base::Bind([](authpolicy::ErrorType error){
>    EXPECT_EQ(authpolicy::ERROR_DBUS_FAILURE, error);
>  });
> 
> The same goes for other callbacks.

Thanks for the hint! Changed for the all calls.

https://codereview.chromium.org/2673813003/diff/1/chromeos/dbus/fake_upstart_...
File chromeos/dbus/fake_upstart_client.cc (right):

https://codereview.chromium.org/2673813003/diff/1/chromeos/dbus/fake_upstart_...
chromeos/dbus/fake_upstart_client.cc:20:
chromeos::DBusThreadManager::Get()->GetAuthPolicyClient())
On 2017/02/06 04:15:49, hashimoto wrote:
> nit: No need to have "chromeos::".
> You are already in chromeos namespace.

Done.

[hashimoto, 2017-02-06 10:35:45]

lgtm

[Thiemo Nagel, 2017-02-15 16:18:59]

lgtm

[Roman Sorokin (ftl), 2017-02-15 16:19:48]

The CQ bit was checked by rsorokin@chromium.org

[commit-bot: I haz the power, 2017-02-15 16:20:24]

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-02-15 16:22:45]

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

[commit-bot: I haz the power, 2017-02-15 16:22:46]

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

[Roman Sorokin (ftl), 2017-02-15 16:31:53]

The CQ bit was checked by rsorokin@chromium.org

[Roman Sorokin (ftl), 2017-02-15 16:31:53]

The patchset sent to the CQ was uploaded after l-g-t-m from tnagel@chromium.org,
hashimoto@chromium.org
Link to the patchset: https://codereview.chromium.org/2673813003/#ps40001
(title: "rebase")

[commit-bot: I haz the power, 2017-02-15 16:32:46]

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-02-15 17:19:32]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1487176313175760,
"parent_rev": "b5d3c8f9e11b4a149345c1ead38132a0f6677cf0", "commit_rev":
"42f2438c1a1992481959a592d9e6391a0aac69b6"}

[commit-bot: I haz the power, 2017-02-15 17:20:12]

Description was changed from

==========
Emulate StartAuthPolicyService from FakeUpstartClient

BUG=688378
TEST=FakeAuthPolicyClientTest.*
==========

to

==========
Emulate StartAuthPolicyService from FakeUpstartClient

BUG=688378
TEST=FakeAuthPolicyClientTest.*

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

[commit-bot: I haz the power, 2017-02-15 17:20:14]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/42f2438c1a1992481959a592d9e6...