platformKeys: Add policy and corporate key tagging.

chrome/browser/chromeos/policy/login_policy_test_base.cc

Index: chrome/browser/chromeos/policy/login_policy_test_base.cc
diff --git a/chrome/browser/chromeos/policy/login_policy_test_base.cc b/chrome/browser/chromeos/policy/login_policy_test_base.cc
index 7fb6adac12b75160a4d9b39c8850ab8da2bf540b..cb088652bb12189c5b1a1ec75a52065f6b2b4ef1 100644
--- a/chrome/browser/chromeos/policy/login_policy_test_base.cc
+++ b/chrome/browser/chromeos/policy/login_policy_test_base.cc
@@ -6,6 +6,7 @@
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
 #include "base/json/json_writer.h"
+#include "base/logging.h"
 #include "base/values.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/chromeos/login/ui/webui_login_display.h"
@@ -23,6 +24,18 @@
 #include "testing/gtest/include/gtest/gtest.h"
 #include "url/gurl.h"
 
+#include "chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.h"
+#include "chrome/browser/chromeos/policy/user_cloud_policy_manager_factory_chromeos.h"
+#include "components/policy/core/common/cloud/cloud_policy_client.h"
+#include "components/policy/core/common/cloud/cloud_policy_constants.h"
+#include "components/policy/core/common/cloud/mock_cloud_policy_client.h"
+#include "components/policy/core/browser/browser_policy_connector.h"
+#include "chrome/browser/browser_process.h"
+#include "chrome/browser/policy/profile_policy_connector.h"
+#include "chrome/browser/policy/profile_policy_connector_factory.h"
+#include "components/policy/core/common/policy_service.h"
+
+
 namespace policy {
 
 namespace {
@@ -37,13 +50,13 @@ const char kTestSessionSIDCookie[] = "fake-session-SID-cookie";
 const char kTestSessionLSIDCookie[] = "fake-session-LSID-cookie";
 const char kTestUserinfoToken[] = "fake-userinfo-token";
 
-std::string GetPolicy(scoped_ptr<base::DictionaryValue> mandatory,
-                      scoped_ptr<base::DictionaryValue> recommended,
+std::string GetPolicy(const base::DictionaryValue& mandatory,
+                      const base::DictionaryValue& recommended,
                       const std::string& policyType,
                       const std::string& account) {
   scoped_ptr<base::DictionaryValue> policy_type_dict(new base::DictionaryValue);
-  policy_type_dict->Set("mandatory", mandatory.Pass());
-  policy_type_dict->Set("recommended", recommended.Pass());
+  policy_type_dict->Set("mandatory", mandatory.CreateDeepCopy());

[bartfab (slow), 2015/06/11 23:33:09]

I wonder whether the JSON will be slightly different now in case one of the
policy dictionaries is empty. Previously, we would have done a Set() with a
|nullptr| as argument. Now we always pass a dictionary - an empty one in that
case. It should not make a semantic difference... and it is well possible that a
|nullptr| would have actually caused a crash.

[pneubeck (no reviews), 2015/06/12 08:55:18]

good point and indeed nullptr would lead to a crash.

+  policy_type_dict->Set("recommended", recommended.CreateDeepCopy());
 
   scoped_ptr<base::ListValue> managed_users_list(new base::ListValue);
   managed_users_list->AppendString("*");
@@ -74,24 +87,97 @@ LoginPolicyTestBase::LoginPolicyTestBase() {
 LoginPolicyTestBase::~LoginPolicyTestBase() {
 }
 
-void LoginPolicyTestBase::SetUp() {
-  ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
-  SetServerPolicy();
+PolicyTestHelper::PolicyTestHelper(const std::string& user_email,
+                                   const base::DictionaryValue& mandatory,
+                                   const base::DictionaryValue& recommended)
+    : user_email_(user_email) {
+  CHECK(temp_dir_.CreateUniqueTempDir());

[bartfab (slow), 2015/06/11 23:33:09]

Since this will be used in tests only, I think an ASSERT() would be better. You
cannot use ASSERT() in the constructor, but you could move this initialization
to an Init() method and use it there.

[pneubeck (no reviews), 2015/06/12 08:55:18]

Done.

+  SetServerPolicy(mandatory, recommended);
 
   test_server_.reset(new LocalPolicyTestServer(PolicyFilePath()));
-  ASSERT_TRUE(test_server_->Start());
+  CHECK(test_server_->Start());
+}
+
+void PolicyTestHelper::UpdateCommandLine(base::CommandLine* command_line) {
+  command_line->AppendSwitchASCII(policy::switches::kDeviceManagementUrl,
+                                  test_server_->GetServiceURL().spec());
+}
+void PolicyTestHelper::UpdatePolicy(Profile* profile,

[bartfab (slow), 2015/06/11 23:33:09]

Nit: Missing blank line.

[pneubeck (no reviews), 2015/06/12 08:55:18]

Done.

+                                    const base::DictionaryValue& mandatory,
+                                    const base::DictionaryValue& recommended) {
+  SetServerPolicy(mandatory, recommended);
+
+  policy::ProfilePolicyConnector* profile_connector =

[bartfab (slow), 2015/06/11 23:33:09]

Nit: Here and below: const pointers.

[pneubeck (no reviews), 2015/06/12 08:55:18]

Done.

+      policy::ProfilePolicyConnectorFactory::GetForBrowserContext(profile);
+  policy::PolicyService* policy_service = profile_connector->policy_service();
+
+  base::RunLoop run_loop;
+  policy_service->RefreshPolicies(run_loop.QuitClosure());
+  run_loop.Run();
+}
+
+void PolicyTestHelper::WaitForInitialPolicy(Profile* profile) {
+  BrowserPolicyConnector* connector =
+      g_browser_process->browser_policy_connector();
+  connector->ScheduleServiceInitialization(0);
+
+  UserCloudPolicyManagerChromeOS* policy_manager =
+      UserCloudPolicyManagerFactoryChromeOS::GetForProfile(profile);
+  ASSERT_TRUE(policy_manager);
+
+  ASSERT_TRUE(policy_manager->core()->client());
+
+  // Give a bogus OAuth token to the |policy_manager|. This should make its
+  // CloudPolicyClient fetch the DMToken.
+  ASSERT_FALSE(policy_manager->core()->client()->is_registered());
+  enterprise_management::DeviceRegisterRequest::Type registration_type =

[bartfab (slow), 2015/06/11 23:33:09]

Nit: const.

[pneubeck (no reviews), 2015/06/12 08:55:18]

Done.

+      enterprise_management::DeviceRegisterRequest::USER;
+  policy_manager->core()->client()->Register(
+      registration_type,
+      enterprise_management::DeviceRegisterRequest::FLAVOR_USER_REGISTRATION,
+      "bogus", std::string(), std::string(), std::string());
+
+  policy::ProfilePolicyConnector* profile_connector =
+      policy::ProfilePolicyConnectorFactory::GetForBrowserContext(profile);
+  policy::PolicyService* policy_service = profile_connector->policy_service();
+
+  base::RunLoop run_loop;
+  policy_service->RefreshPolicies(run_loop.QuitClosure());
+  run_loop.Run();
+}
+
+PolicyTestHelper::~PolicyTestHelper() {

[bartfab (slow), 2015/06/11 23:33:09]

Nit: Reorder methods so that declaration and definition orders match.

[pneubeck (no reviews), 2015/06/12 08:55:18]

Done.

+}
+
+void PolicyTestHelper::SetServerPolicy(
+    const base::DictionaryValue& mandatory,
+    const base::DictionaryValue& recommended) {
+  const std::string policy = GetPolicy(
+      mandatory, recommended, dm_protocol::kChromeUserPolicyType, user_email_);
+  const int bytes_written =
+      base::WriteFile(PolicyFilePath(), policy.data(), policy.size());
+  CHECK_EQ(static_cast<int>(policy.size()), bytes_written);
+}
+
+base::FilePath PolicyTestHelper::PolicyFilePath() const {
+  return temp_dir_.path().AppendASCII("policy.json");
+}
 
+void LoginPolicyTestBase::SetUp() {
+  scoped_ptr<base::DictionaryValue> mandatory(GetMandatoryPoliciesValue());

[bartfab (slow), 2015/06/11 23:33:09]

If you are switching from scoped_ptrs to deep copies in the helper, why not do
it throughout?

[pneubeck (no reviews), 2015/06/12 08:55:18]

Done.

+  scoped_ptr<base::DictionaryValue> recommended(GetRecommendedPoliciesValue());
+  policy_helper_.reset(
+      new PolicyTestHelper(kAccountId, *mandatory, *recommended));
   OobeBaseTest::SetUp();
 }
 
 void LoginPolicyTestBase::SetUpCommandLine(base::CommandLine* command_line) {
-  command_line->AppendSwitchASCII(policy::switches::kDeviceManagementUrl,
-                                  test_server_->GetServiceURL().spec());
+  policy_helper_->UpdateCommandLine(command_line);
   OobeBaseTest::SetUpCommandLine(command_line);
 }
 
 void LoginPolicyTestBase::SetUpOnMainThread() {
-  SetMergeSessionParams(kAccountId);
+  SetMergeSessionParams();
   SetUpGaiaServerWithAccessTokens();
   OobeBaseTest::SetUpOnMainThread();
 }
@@ -116,7 +202,7 @@ void LoginPolicyTestBase::SetUpGaiaServerWithAccessTokens() {
   fake_gaia_->IssueOAuthToken(kTestRefreshToken, token_info);
 }
 
-void LoginPolicyTestBase::SetMergeSessionParams(const std::string& email) {
+void LoginPolicyTestBase::SetMergeSessionParams() {
   FakeGaia::MergeSessionParams params;
   params.auth_sid_cookie = kTestAuthSIDCookie;
   params.auth_lsid_cookie = kTestAuthLSIDCookie;
@@ -126,7 +212,7 @@ void LoginPolicyTestBase::SetMergeSessionParams(const std::string& email) {
   params.gaia_uber_token = kTestGaiaUberToken;
   params.session_sid_cookie = kTestSessionSIDCookie;
   params.session_lsid_cookie = kTestSessionLSIDCookie;
-  params.email = email;
+  params.email = kAccountId;
   fake_gaia_->SetMergeSessionParams(params);
 }
 
@@ -151,18 +237,4 @@ void LoginPolicyTestBase::LogIn(const std::string& user_id,
       content::NotificationService::AllSources()).Wait();
 }
 
-void LoginPolicyTestBase::SetServerPolicy() {
-  const std::string policy =
-      GetPolicy(GetMandatoryPoliciesValue(), GetRecommendedPoliciesValue(),
-                dm_protocol::kChromeUserPolicyType, kAccountId);
-
-  const int bytes_written =
-      base::WriteFile(PolicyFilePath(), policy.data(), policy.size());
-  ASSERT_EQ(static_cast<int>(policy.size()), bytes_written);
-}
-
-base::FilePath LoginPolicyTestBase::PolicyFilePath() const {
-  return temp_dir_.path().AppendASCII("policy.json");
-}
-
 }  // namespace policy