cros: Override stub paths before fake cryptohome client created

Now Chrome OS linux build is broken - fake cryptohome client can't read
install attributes.
Also fixed ActiveDirectoryLoginTest to detect it. It relies on
fake cryptohome client reading correct install attributes.

Bug: 945344, 946024
Change-Id: I55f55c9e0445c3f1c3578445aaec19362db44483
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1538503
Commit-Queue: Roman Sorokin [CET] <rsorokin@chromium.org>
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Reviewed-by: Pavol Marko <pmarko@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Toni Baržić <tbarzic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#644733}

chrome/browser/chromeos/dbus/dbus_helper.cc

@@ -4,7 +4,11 @@
 
 #include "chrome/browser/chromeos/dbus/dbus_helper.h"
 
+#include "base/path_service.h"
+#include "base/system/sys_info.h"
 #include "chrome/browser/chromeos/settings/device_settings_service.h"
+#include "chrome/common/chrome_paths.h"
+#include "chromeos/constants/chromeos_paths.h"
 #include "chromeos/cryptohome/system_salt_getter.h"
 #include "chromeos/dbus/auth_policy/auth_policy_client.h"
 #include "chromeos/dbus/biod/biod_client.h"
@@ -18,9 +22,23 @@
 #include "chromeos/dbus/upstart/upstart_client.h"
 #include "chromeos/tpm/install_attributes.h"
 
+namespace {
+
+void OverrideStubPathsIfNeeded() {
+  base::FilePath user_data_dir;
+  if (!base::SysInfo::IsRunningOnChromeOS() &&
+      base::PathService::Get(chrome::DIR_USER_DATA, &user_data_dir)) {
+    chromeos::RegisterStubPathOverrides(user_data_dir);
+  }
+}
+
+}  // namespace
+
 namespace chromeos {
 
 void InitializeDBus() {
+  OverrideStubPathsIfNeeded();
+
   SystemSaltGetter::Initialize();
 
   // Initialize DBusThreadManager for the browser.

chrome/browser/chromeos/login/active_directory_login_browsertest.cc

@@ -26,7 +26,6 @@
 #include "chrome/test/base/interactive_test_utils.h"
 #include "chromeos/constants/chromeos_switches.h"
 #include "chromeos/dbus/auth_policy/fake_auth_policy_client.h"
-#include "chromeos/dbus/cryptohome/fake_cryptohome_client.h"
 #include "chromeos/dbus/cryptohome/tpm_util.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
 #include "chromeos/login/auth/authpolicy_login_helper.h"
@@ -44,7 +43,7 @@ namespace {
 
 const char kPassword[] = "password";
 
-constexpr char kGaiaSigninId[] = "signin-frame";
+constexpr char kGaiaSigninId[] = "signin-frame-dialog";
 constexpr char kAdOfflineAuthId[] = "offline-ad-auth";
 
 constexpr char kTestActiveDirectoryUser[] = "test-user";
@@ -88,9 +87,6 @@ class ActiveDirectoryLoginTest : public LoginManagerTest {
     // other ChromeBrowserMain initialization occurs.
     AuthPolicyClient::InitializeFake();
     FakeAuthPolicyClient::Get()->DisableOperationDelayForTesting();
-
-    // Note: FakeCryptohomeClient needs paths to be set to load install attribs.
-    active_directory_test_helper::OverridePaths();
   }
 
   void SetUpCommandLine(base::CommandLine* command_line) override {
@@ -325,6 +321,7 @@ class ActiveDirectoryLoginAutocompleteTest : public ActiveDirectoryLoginTest {
 
 // Test successful Active Directory login.
 IN_PROC_BROWSER_TEST_F_WITH_PRE(ActiveDirectoryLoginTest, LoginSuccess) {
+  ASSERT_TRUE(tpm_util::IsActiveDirectoryLocked());
   TestNoError();
   TestDomainHidden();
   content::WindowedNotificationObserver session_start_waiter(
@@ -336,6 +333,7 @@ IN_PROC_BROWSER_TEST_F_WITH_PRE(ActiveDirectoryLoginTest, LoginSuccess) {
 
 // Test different UI errors for Active Directory login.
 IN_PROC_BROWSER_TEST_F_WITH_PRE(ActiveDirectoryLoginTest, LoginErrors) {
+  ASSERT_TRUE(tpm_util::IsActiveDirectoryLocked());
   SetupActiveDirectoryJSNotifications();
   TestNoError();
   TestDomainHidden();
@@ -379,6 +377,7 @@ IN_PROC_BROWSER_TEST_F_WITH_PRE(ActiveDirectoryLoginTest, LoginErrors) {
 // Test successful Active Directory login from the password change screen.
 IN_PROC_BROWSER_TEST_F_WITH_PRE(ActiveDirectoryLoginTest,
                                 PasswordChange_LoginSuccess) {
+  ASSERT_TRUE(tpm_util::IsActiveDirectoryLocked());
   TestLoginVisible();
   TestDomainHidden();
 
@@ -397,6 +396,7 @@ IN_PROC_BROWSER_TEST_F_WITH_PRE(ActiveDirectoryLoginTest,
 // Test different UI errors for Active Directory password change screen.
 IN_PROC_BROWSER_TEST_F_WITH_PRE(ActiveDirectoryLoginTest,
                                 PasswordChange_UIErrors) {
+  ASSERT_TRUE(tpm_util::IsActiveDirectoryLocked());
   TestLoginVisible();
   TestDomainHidden();
 
@@ -429,6 +429,7 @@ IN_PROC_BROWSER_TEST_F_WITH_PRE(ActiveDirectoryLoginTest,
 // Test reopening Active Directory password change screen clears errors.
 IN_PROC_BROWSER_TEST_F_WITH_PRE(ActiveDirectoryLoginTest,
                                 PasswordChange_ReopenClearErrors) {
+  ASSERT_TRUE(tpm_util::IsActiveDirectoryLocked());
   TestLoginVisible();
   TestDomainHidden();
 
@@ -446,6 +447,7 @@ IN_PROC_BROWSER_TEST_F_WITH_PRE(ActiveDirectoryLoginTest,
 // Tests that autocomplete works. Submits username without domain.
 IN_PROC_BROWSER_TEST_F_WITH_PRE(ActiveDirectoryLoginAutocompleteTest,
                                 LoginSuccess) {
+  ASSERT_TRUE(tpm_util::IsActiveDirectoryLocked());
   TestNoError();
   TestDomainVisible();
 
@@ -459,6 +461,7 @@ IN_PROC_BROWSER_TEST_F_WITH_PRE(ActiveDirectoryLoginAutocompleteTest,
 // Tests that user could override autocomplete domain.
 IN_PROC_BROWSER_TEST_F_WITH_PRE(ActiveDirectoryLoginAutocompleteTest,
                                 TestAutocomplete) {
+  ASSERT_TRUE(tpm_util::IsActiveDirectoryLocked());
   SetupActiveDirectoryJSNotifications();
 
   TestLoginVisible();

chrome/browser/chromeos/login/active_directory_test_helper.cc

@@ -61,7 +61,6 @@ void PrepareLogin(const std::string& user_principal_name) {
 
   // Lock the device to AD mode. Paths need to be set here, so install attribs
   // can be saved to disk.
-  OverridePaths();
   ASSERT_TRUE(
       tpm_util::LockDeviceActiveDirectoryForTesting(user_and_domain[1]));
 

chrome/browser/chromeos/policy/user_affiliation_browsertest.cc

@@ -23,7 +23,6 @@
 #include "chromeos/cryptohome/cryptohome_parameters.h"
 #include "chromeos/dbus/auth_policy/fake_auth_policy_client.h"
 #include "chromeos/dbus/cryptohome/cryptohome_client.h"
-#include "chromeos/dbus/cryptohome/fake_cryptohome_client.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
 #include "chromeos/dbus/session_manager/fake_session_manager_client.h"
 #include "chromeos/dbus/session_manager/session_manager_client.h"
@@ -156,6 +155,8 @@ class UserAffiliationBrowserTest
   // InProcessBrowserTest
   void SetUpInProcessBrowserTestFixture() override {
     InProcessBrowserTest::SetUpInProcessBrowserTestFixture();
+    // Some DBus services rely on paths, so override it here.
+    chromeos::active_directory_test_helper::OverridePaths();
     chromeos::FakeSessionManagerClient* fake_session_manager_client =
         new chromeos::FakeSessionManagerClient;
     chromeos::DBusThreadManager::GetSetterForTesting()->SetSessionManagerClient(
@@ -171,10 +172,6 @@ class UserAffiliationBrowserTest
       chromeos::AuthPolicyClient::InitializeFake();
       fake_auth_policy_client = chromeos::FakeAuthPolicyClient::Get();
       fake_auth_policy_client->DisableOperationDelayForTesting();
-      // PrepareLogin requires a message loop, which isn't available yet here.
-      base::MessageLoop message_loop;
-      chromeos::active_directory_test_helper::PrepareLogin(
-          account_id_.GetUserEmail());
     }
 
     DevicePolicyCrosTestHelper test_helper;
@@ -291,6 +288,10 @@ class UserAffiliationBrowserTest
 
 IN_PROC_BROWSER_TEST_P(UserAffiliationBrowserTest, PRE_PRE_TestAffiliation) {
   AffiliationTestHelper::PreLoginUser(account_id_);
+  if (GetParam().active_directory) {
+    chromeos::active_directory_test_helper::PrepareLogin(
+        account_id_.GetUserEmail());
+  }
 }
 
 // This part of the test performs a regular sign-in through the login manager.

chrome/browser/metrics/chrome_feature_list_creator.cc

@@ -14,7 +14,6 @@
 #include "base/memory/scoped_refptr.h"
 #include "base/path_service.h"
 #include "base/strings/utf_string_conversions.h"
-#include "base/system/sys_info.h"
 #include "base/time/time.h"
 #include "base/trace_event/trace_event.h"
 #include "cc/base/switches.h"
@@ -47,32 +46,9 @@
 
 #if defined(OS_CHROMEOS)
 #include "chrome/browser/chromeos/policy/browser_policy_connector_chromeos.h"
-#include "chromeos/constants/chromeos_paths.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
 #endif  // defined(OS_CHROMEOS)
 
-namespace {
-
-#if defined(OS_CHROMEOS)
-void RegisterStubPathOverridesIfNecessary() {
-  // These overrides need to occur before BrowserPolicyConnectorChromeOS
-  // (for one) is created. The DCHECK ensures that is the case.
-  DCHECK(!g_browser_process);
-
-  base::FilePath user_data_dir;
-  if (base::SysInfo::IsRunningOnChromeOS() ||
-      !base::PathService::Get(chrome::DIR_USER_DATA, &user_data_dir)) {
-    return;
-  }
-
-  // Override some paths with stub locations so that cloud policy and enterprise
-  // enrollment work on desktop builds, for ease of development.
-  chromeos::RegisterStubPathOverrides(user_data_dir);
-}
-#endif  // defined(OS_CHROMEOS)
-
-}  // namespace
-
 ChromeFeatureListCreator::ChromeFeatureListCreator() = default;
 
 ChromeFeatureListCreator::~ChromeFeatureListCreator() = default;
@@ -135,7 +111,6 @@ void ChromeFeatureListCreator::CreatePrefService() {
   RegisterLocalState(pref_registry.get());
 
 #if defined(OS_CHROMEOS)
-  RegisterStubPathOverridesIfNecessary();
   // DBus must be initialized before constructing the policy connector.
   CHECK(chromeos::DBusThreadManager::IsInitialized());
   browser_policy_connector_ =