Move reusable part of LoginPolicyTestBase into a helper class.

Move reusable part of LoginPolicyTestBase into a helper class.

The user policy part can be reused independent of the login simulation. Additionally functions to refresh and to wait for the policy application are added.

BUG=460232
Committed: https://crrev.com/ae14fbdb694429ecbd5f95794a9beff65880c40e
Cr-Commit-Position: refs/heads/master@{#334577}

[pneubeck (no reviews), 2015-06-12 08:08:21]

Patchset #1 (id:1) has been deleted

[pneubeck (no reviews), 2015-06-12 08:08:26]

Patchset #1 (id:20001) has been deleted

[pneubeck (no reviews), 2015-06-12 08:09:23]

pneubeck@chromium.org changed reviewers:
+ bartfab@chromium.org

[pneubeck (no reviews), 2015-06-12 08:09:24]

addressed all comments from https://codereview.chromium.org/1150373002/
, cleaned up and ready for review.

[pneubeck (no reviews), 2015-06-12 08:44:45]

Patchset #1 (id:40001) has been deleted

[pneubeck (no reviews), 2015-06-12 08:50:18]

Patchset #1 (id:40001) has been deleted

[bartfab (slow), 2015-06-15 17:12:28]

lgtm

https://codereview.chromium.org/1185593002/diff/60001/chrome/browser/chromeos...
File chrome/browser/chromeos/policy/user_policy_test_helper.cc (right):

https://codereview.chromium.org/1185593002/diff/60001/chrome/browser/chromeos...
chrome/browser/chromeos/policy/user_policy_test_helper.cc:35: const std::string&
account) {
Nit: s/account/account_id/ for consistency

https://codereview.chromium.org/1185593002/diff/60001/chrome/browser/chromeos...
chrome/browser/chromeos/policy/user_policy_test_helper.cc:93:
ASSERT_TRUE(policy_manager->core()->client());
Nit: #include "components/policy/core/common/cloud/cloud_policy_core.h"

https://codereview.chromium.org/1185593002/diff/60001/chrome/browser/chromeos...
chrome/browser/chromeos/policy/user_policy_test_helper.cc:94:
ASSERT_FALSE(policy_manager->core()->client()->is_registered());
Nit: #include "components/policy/core/common/cloud/cloud_policy_client.h"

https://codereview.chromium.org/1185593002/diff/60001/chrome/browser/chromeos...
chrome/browser/chromeos/policy/user_policy_test_helper.cc:103:
policy::ProfilePolicyConnectorFactory::GetForBrowserContext(profile);
Nit: Why do you ASSERT_TRUE() the |policy_manager| but not the
|profile_connector| and |policy_service|?

https://codereview.chromium.org/1185593002/diff/60001/chrome/browser/chromeos...
chrome/browser/chromeos/policy/user_policy_test_helper.cc:119:
policy::ProfilePolicyConnectorFactory::GetForBrowserContext(profile);
Nit: As above: Should this method ASSERT_TRUE() to be on the safe side?

https://codereview.chromium.org/1185593002/diff/60001/chrome/browser/chromeos...
File chrome/browser/chromeos/policy/user_policy_test_helper.h (right):

https://codereview.chromium.org/1185593002/diff/60001/chrome/browser/chromeos...
chrome/browser/chromeos/policy/user_policy_test_helper.h:30: explicit
UserPolicyTestHelper(const std::string& user_email);
Nit: "email" is incorrect. The ID may not be an e-mail at all (e.g. for
device-local accounts). The correct term is "account_id".

https://codereview.chromium.org/1185593002/diff/60001/chrome/browser/chromeos...
chrome/browser/chromeos/policy/user_policy_test_helper.h:42: // profile.
Alternatively, a login can be simulated making this function
Nit 1: s/simulated/simulated, /
Nit 2: A login does not make this function obsolete in general. Rather, it makes
it unnecessary to call the function.

https://codereview.chromium.org/1185593002/diff/60001/chrome/browser/chromeos...
chrome/browser/chromeos/policy/user_policy_test_helper.h:47: // applied to
|profile|.
Nit: Add a note indicating whether the method waits for the policy to be applied
before returning.

https://codereview.chromium.org/1185593002/diff/60001/chrome/browser/chromeos...
chrome/browser/chromeos/policy/user_policy_test_helper.h:57: const std::string
user_email_;
Nit: As above, s/user_email_/account_id_/

[pneubeck (no reviews), 2015-06-16 09:16:21]

https://codereview.chromium.org/1185593002/diff/60001/chrome/browser/chromeos...
File chrome/browser/chromeos/policy/user_policy_test_helper.cc (right):

https://codereview.chromium.org/1185593002/diff/60001/chrome/browser/chromeos...
chrome/browser/chromeos/policy/user_policy_test_helper.cc:35: const std::string&
account) {
On 2015/06/15 17:12:27, bartfab wrote:
> Nit: s/account/account_id/ for consistency

Done.

https://codereview.chromium.org/1185593002/diff/60001/chrome/browser/chromeos...
chrome/browser/chromeos/policy/user_policy_test_helper.cc:93:
ASSERT_TRUE(policy_manager->core()->client());
On 2015/06/15 17:12:27, bartfab wrote:
> Nit: #include "components/policy/core/common/cloud/cloud_policy_core.h"

Removed this assert.

https://codereview.chromium.org/1185593002/diff/60001/chrome/browser/chromeos...
chrome/browser/chromeos/policy/user_policy_test_helper.cc:94:
ASSERT_FALSE(policy_manager->core()->client()->is_registered());
On 2015/06/15 17:12:27, bartfab wrote:
> Nit: #include "components/policy/core/common/cloud/cloud_policy_client.h"

Done.

https://codereview.chromium.org/1185593002/diff/60001/chrome/browser/chromeos...
chrome/browser/chromeos/policy/user_policy_test_helper.cc:103:
policy::ProfilePolicyConnectorFactory::GetForBrowserContext(profile);
On 2015/06/15 17:12:27, bartfab wrote:
> Nit: Why do you ASSERT_TRUE() the |policy_manager| but not the
> |profile_connector| and |policy_service|?

Removed the assert(..manager..)

https://codereview.chromium.org/1185593002/diff/60001/chrome/browser/chromeos...
chrome/browser/chromeos/policy/user_policy_test_helper.cc:119:
policy::ProfilePolicyConnectorFactory::GetForBrowserContext(profile);
On 2015/06/15 17:12:27, bartfab wrote:
> Nit: As above: Should this method ASSERT_TRUE() to be on the safe side?

I removed the asserts above, as I don't think that this really helps.
The tests will fail anyways. With test isolation the impact should be small. In
the worst case, one has to make a debug build to find out where it crashes.

https://codereview.chromium.org/1185593002/diff/60001/chrome/browser/chromeos...
File chrome/browser/chromeos/policy/user_policy_test_helper.h (right):

https://codereview.chromium.org/1185593002/diff/60001/chrome/browser/chromeos...
chrome/browser/chromeos/policy/user_policy_test_helper.h:30: explicit
UserPolicyTestHelper(const std::string& user_email);
On 2015/06/15 17:12:27, bartfab wrote:
> Nit: "email" is incorrect. The ID may not be an e-mail at all (e.g. for
> device-local accounts). The correct term is "account_id".

Done.

https://codereview.chromium.org/1185593002/diff/60001/chrome/browser/chromeos...
chrome/browser/chromeos/policy/user_policy_test_helper.h:42: // profile.
Alternatively, a login can be simulated making this function
On 2015/06/15 17:12:27, bartfab wrote:
> Nit 1: s/simulated/simulated, /
> Nit 2: A login does not make this function obsolete in general. Rather, it
makes
> it unnecessary to call the function.

Done.

https://codereview.chromium.org/1185593002/diff/60001/chrome/browser/chromeos...
chrome/browser/chromeos/policy/user_policy_test_helper.h:47: // applied to
|profile|.
On 2015/06/15 17:12:27, bartfab wrote:
> Nit: Add a note indicating whether the method waits for the policy to be
applied
> before returning.

Done.

https://codereview.chromium.org/1185593002/diff/60001/chrome/browser/chromeos...
chrome/browser/chromeos/policy/user_policy_test_helper.h:57: const std::string
user_email_;
On 2015/06/15 17:12:27, bartfab wrote:
> Nit: As above, s/user_email_/account_id_/

Done.

[pneubeck (no reviews), 2015-06-16 09:16:26]

The CQ bit was checked by pneubeck@chromium.org

[pneubeck (no reviews), 2015-06-16 09:16:26]

The patchset sent to the CQ was uploaded after l-g-t-m from bartfab@chromium.org
Link to the patchset: https://codereview.chromium.org/1185593002/#ps80001
(title: "Addressed comments")

[commit-bot: I haz the power, 2015-06-16 09:17:02]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1185593002/80001

[commit-bot: I haz the power, 2015-06-16 10:09:19]

Committed patchset #2 (id:80001)

[commit-bot: I haz the power, 2015-06-16 10:10:16]

Patchset 2 (id:??) landed as
https://crrev.com/ae14fbdb694429ecbd5f95794a9beff65880c40e
Cr-Commit-Position: refs/heads/master@{#334577}