UserPolicyManagerFactoryChromeOS: Elaborate comment

chrome/browser/chromeos/policy/user_policy_manager_factory_chromeos.cc

Index: chrome/browser/chromeos/policy/user_policy_manager_factory_chromeos.cc
diff --git a/chrome/browser/chromeos/policy/user_policy_manager_factory_chromeos.cc b/chrome/browser/chromeos/policy/user_policy_manager_factory_chromeos.cc
index 835f51160734be5a8533f7ff97ce02bd1ed42c63..cbf3e75c425f1d7fd57891954b1c8d665d655d0d 100644
--- a/chrome/browser/chromeos/policy/user_policy_manager_factory_chromeos.cc
+++ b/chrome/browser/chromeos/policy/user_policy_manager_factory_chromeos.cc
@@ -176,9 +176,10 @@ UserPolicyManagerFactoryChromeOS::CreateManagerForProfile(
   bool is_active_directory = false;
   switch (account_id.GetAccountType()) {
     case AccountType::UNKNOWN:
+      // At the time of login, the account type must be well-defined.
+      CHECK(false);

[Andrew T Wilson (Slow), 2017/05/18 15:26:03]

Is CHECK the right thing here, nor AttemptUserExit() or NOTREACHED? I'm somewhat
unclear as to what NOTREACHED does in production code.

Is there any chance of a reboot loop if you just CHECK here?

[Thiemo Nagel, 2017/05/18 16:00:24]

I guess AttemptUserExit() is just as good, except maybe that CHECK() acts with
greater prejudice (I'm slightly put off by the "Attempt" wording) and also
conceptually this is something that (supposedly) is prevented by program logic
and thus CHECK() would seem like the correct way to express that.
In production it just logs an error.
 
I don't think so because for the login profile we bail above.

+      break;
     case AccountType::GOOGLE:
-      // TODO(tnagel): Return nullptr for unknown accounts once AccountId
-      // migration is finished.
       if (!user->HasGaiaAccount()) {
         return {};
       }