Make the policy fetch for first time login blocking

chrome/browser/chromeos/login/login_utils_browsertest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/chromeos/login/login_utils.h"
 
 #include "base/basictypes.h"
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/files/scoped_temp_dir.h"
@@ skipping 514 matching lines @@
   std::string device_policy_;
   std::string user_policy_;
 
   DISALLOW_COPY_AND_ASSIGN(LoginUtilsTest);
 };
 
 class LoginUtilsBlockingLoginTest
     : public LoginUtilsTest,
       public testing::WithParamInterface<int> {};
 
-TEST_F(LoginUtilsTest, NormalLoginDoesntBlock) {
+TEST_F(LoginUtilsTest, NormalLoginDoesBlock) {
   UserManager* user_manager = UserManager::Get();
   EXPECT_FALSE(user_manager->IsUserLoggedIn());
   EXPECT_FALSE(connector_->IsEnterpriseManaged());
   EXPECT_FALSE(prepared_profile_);
   EXPECT_EQ(policy::USER_AFFILIATION_NONE,
             connector_->GetUserAffiliation(kUsername));
 
-  // The profile will be created without waiting for a policy response.
+  // First login in a unmanaged device would still block to verify if there is

[bartfab (slow), 2014/07/08 17:40:33]

Nit: Use indicative, not subjunctive.

[kaliamoorthi, 2014/07/11 10:05:19]

Removed the whole test

+  // a user cloud policy for the user.
   PrepareProfile(kUsername);
 
-  EXPECT_TRUE(prepared_profile_);
-  ASSERT_TRUE(user_manager->IsUserLoggedIn());
-  EXPECT_EQ(kUsername, user_manager->GetLoggedInUser()->email());
+  EXPECT_FALSE(prepared_profile_);
 }
 
-TEST_F(LoginUtilsTest, EnterpriseLoginDoesntBlockForNormalUser) {
+TEST_F(LoginUtilsTest, EnterpriseLoginDoesBlockForNormalUser) {
   UserManager* user_manager = UserManager::Get();
   EXPECT_FALSE(user_manager->IsUserLoggedIn());
   EXPECT_FALSE(connector_->IsEnterpriseManaged());
   EXPECT_FALSE(prepared_profile_);
 
   // Enroll the device.
   EnrollDevice(kUsername);
 
   EXPECT_FALSE(user_manager->IsUserLoggedIn());
   EXPECT_TRUE(connector_->IsEnterpriseManaged());
   EXPECT_EQ(kDomain, connector_->GetEnterpriseDomain());
   EXPECT_FALSE(prepared_profile_);
   EXPECT_EQ(policy::USER_AFFILIATION_NONE,
             connector_->GetUserAffiliation(kUsernameOtherDomain));
 
-  // Login with a non-enterprise user shouldn't block.
+  // First login with a non-enterprise user would still block to verify if
+  // there is a user cloud policy for it.
   PrepareProfile(kUsernameOtherDomain);

[bartfab (slow), 2014/07/08 17:40:33]

This is not necessarily a non-enterprise user. The difference between this and
the previous one is whether the device is enrolled or not. So the tests should
be named something like:

LoginBlocksForConsumerUser
LoginBlocksForUnaffiliatedEnterpriseUser

and then EnterpriseLoginBlocksForEnterpriseUser should be something like:

LoginBlocksForAffiliatedEnterpriseUser

[kaliamoorthi, 2014/07/11 10:05:19]

Removed these and folded them into the existing parametrized test

 
-  EXPECT_TRUE(prepared_profile_);
-  ASSERT_TRUE(user_manager->IsUserLoggedIn());
-  EXPECT_EQ(kUsernameOtherDomain, user_manager->GetLoggedInUser()->email());
+  EXPECT_FALSE(prepared_profile_);
 }
 
 #if defined(ENABLE_RLZ)
 TEST_F(LoginUtilsTest, RlzInitialized) {
   // No RLZ brand code set initially.
   EXPECT_FALSE(local_state_.Get()->HasPrefPath(prefs::kRLZBrand));
 
   base::RunLoop wait_for_rlz_init;
   rlz_initialized_cb_ = wait_for_rlz_init.QuitClosure();
 
@@ skipping 28 matching lines @@
   EXPECT_TRUE(connector_->IsEnterpriseManaged());
   EXPECT_EQ(kDomain, connector_->GetEnterpriseDomain());
   EXPECT_FALSE(prepared_profile_);
   EXPECT_EQ(policy::USER_AFFILIATION_MANAGED,
             connector_->GetUserAffiliation(kUsername));
   EXPECT_FALSE(user_manager->IsKnownUser(kUsername));
 
   // Login with a user of the enterprise domain waits for policy.
   PrepareProfile(kUsername);
 
   EXPECT_FALSE(prepared_profile_);

[bartfab (slow), 2014/07/08 17:40:33]

This test actually verifies that the blocking policy fetch works. The
NormalLoginDoesBlock and EnterpriseLoginDoesBlockForNormalUser tests should do
the same.

[kaliamoorthi, 2014/07/11 10:05:19]

Done.

   ASSERT_TRUE(user_manager->IsUserLoggedIn());
   EXPECT_TRUE(user_manager->IsCurrentUserNew());
 
   GaiaUrls* gaia_urls = GaiaUrls::GetInstance();
   net::TestURLFetcher* fetcher;
 
   // |steps| is the test parameter, and is the number of successful fetches.
   // The first incomplete fetch will fail. In any case, the profile creation
   // should resume.
   int steps = GetParam();
@@ skipping 72 matching lines @@
 }
 
 INSTANTIATE_TEST_CASE_P(
     LoginUtilsBlockingLoginTestInstance,
     LoginUtilsBlockingLoginTest,
     testing::Values(0, 1, 2, 3, 4, 5));
 
 }  // namespace
 
 }  // namespace chromeos