UserPolicyManagerFactoryChromeOS: Elaborate comment

chrome/browser/chromeos/policy/user_policy_manager_factory_chromeos.cc

 // Copyright (c) 2013 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/policy/user_policy_manager_factory_chromeos.h"
 
 #include <utility>
 
 #include "base/bind.h"
 #include "base/command_line.h"
@@ skipping 158 matching lines @@
   if (user->IsSupervised() ||
       BrowserPolicyConnector::IsNonEnterpriseUser(account_id.GetUserEmail())) {
     return {};
   }
 
   policy::BrowserPolicyConnectorChromeOS* connector =
       g_browser_process->platform_part()->browser_policy_connector_chromeos();
   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 {};
       }
       is_active_directory = false;
       break;
     case AccountType::ACTIVE_DIRECTORY:
       // Active Directory users only exist on devices whose install attributes
       // are locked into Active Directory mode.
       CHECK(connector->GetInstallAttributes()->IsActiveDirectoryManaged());
       is_active_directory = true;
@@ skipping 129 matching lines @@
 
 bool UserPolicyManagerFactoryChromeOS::HasTestingFactory(
     content::BrowserContext* context) {
   return false;
 }
 
 void UserPolicyManagerFactoryChromeOS::CreateServiceNow(
     content::BrowserContext* context) {}
 
 }  // namespace policy