commit ec3aded00c036c3f142a194c42d5df8108cc1070
author A Olsen <olsen@chromium.org> Thu Sep 27 11:10:03 2018
committer Commit Bot <commit-bot@chromium.org> Thu Sep 27 11:10:03 2018
tree b358e858ecd9a99d221485c0db3452a54ef69121
parent 16ac55732413ca3caf862bcc688d0ba61b82c0ba

Remove most of chrome_user_manager_util

chrome_user_manager_util is a static helper class that mostly helps
implement ChromeUserManagerImpl and FakeChromeUserManager. A few other
classes also call it directly. The different functions in it make use
of four different global singletons -

1. chromeos::LoginState::Get()
2. user_manager::UserManager::Get()
3. g_browser_process->platform_part()
        ->browser_policy_connector_chromeos()
        ->GetMinimumVersionPolicyHandler()
4. g_browser_process->platform_part()
          ->browser_policy_connector_chromeos()
          ->GetDeviceLocalAccountPolicyService()

This changes moves most of the code out of chrome_user_manager_util,
and makes sure that the remaining code doesn't depend on these
singletons. This is done as follows:

1. Code common to ChromeUserManagerImpl and FakeChromeUserManager now
goes in ChromeUserManager.
2. Code only used by ChromeUserManagerImpl is now there.
3. Direct calls to chrome_user_manager_util by GaiaScreenHandler or
SigninScreenHandler are now replaced by the equivalent calls to the
ChromeUserManager singleton
4. The only code which remains is the completely static function
IsUserAllowed - only contains logic, not state, since all policy data
is passed in - and a variant which takes the policy data as a
ChromeDeviceSettingsProto. (This variant is still called directly
from DeviceOffHoursController.)

The previous design - static functions with dependencies on global
singletons - had some disadvantages, which this CL tries to address:

1. Unwanted dependencies on g_browser_process in chromeos code -
removing these is part of http://crbug.com/446937
2. Static cling - go/staticcling - it is harder to setup tests for
static utilities if they depend on global singletons.
3. Strange dependency cycles. When calling IsUserAllowed on a
ChromeUserManager, it delegated to chrome_user_manager_util, which
calls user_manager::UserManager::Get(), which could be a different
ChromeUserManager, or not be initialized. This also makes tests harder
to set up. Now, all the code is part of the one ChromeUserManager
instance.

Bug: 446937
Change-Id: Ifb6c7aa5bf7081b4c6dd9f5f4043a07d51c79a96
Reviewed-on: https://chromium-review.googlesource.com/1243074
Commit-Queue: A Olsen <olsen@chromium.org>
Reviewed-by: Julian Pastarmov <pastarmovj@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#594666}