Removing FRE UI for unicorn accounts.

chrome/browser/chromeos/login/session/user_session_manager.cc

Index: chrome/browser/chromeos/login/session/user_session_manager.cc
diff --git a/chrome/browser/chromeos/login/session/user_session_manager.cc b/chrome/browser/chromeos/login/session/user_session_manager.cc
index 246f4880e0b3262da320c88309bf6d33c15c55fb..fc91cf68074e8c953c1843fb400a92044898f99c 100644
--- a/chrome/browser/chromeos/login/session/user_session_manager.cc
+++ b/chrome/browser/chromeos/login/session/user_session_manager.cc
@@ -65,6 +65,8 @@
 #include "chrome/browser/signin/account_tracker_service_factory.h"
 #include "chrome/browser/signin/easy_unlock_service.h"
 #include "chrome/browser/signin/signin_manager_factory.h"
+#include "chrome/browser/supervised_user/child_accounts/child_account_service.h"
+#include "chrome/browser/supervised_user/child_accounts/child_account_service_factory.h"
 #include "chrome/browser/ui/app_list/start_page_service.h"
 #include "chrome/browser/ui/startup/startup_browser_creator.h"
 #include "chrome/common/chrome_switches.h"
@@ -325,7 +327,10 @@ UserSessionManager::UserSessionManager()
       session_restore_strategy_(
           OAuth2LoginManager::RESTORE_FROM_SAVED_OAUTH2_REFRESH_TOKEN),
       running_easy_unlock_key_ops_(false),
-      should_launch_browser_(true) {
+      should_launch_browser_(true),
+      requesting_start_urls_(false),
+      profile_fetching_flags_(nullptr),
+      weak_factory_(this) {
   net::NetworkChangeNotifier::AddConnectionTypeObserver(this);
   user_manager::UserManager::Get()->AddSessionStateObserver(this);
 }
@@ -340,6 +345,9 @@ UserSessionManager::~UserSessionManager() {
   net::NetworkChangeNotifier::RemoveConnectionTypeObserver(this);
 }
 
+// static
+const int UserSessionManager::kFlagsFetchingLoginTimeoutMs = 1000;

[Marc Treib, 2015/03/04 16:31:48]

Can we make this large enough so that we get a second try if the first one
fails? Something like 2 * the initial delay in the CAService. (You can also
decrease the initial delay in the CAService a bit, so this timeout doesn't get
too large.)

[merkulova, 2015/03/04 19:06:49]

That's a tricky question: we want the FRE UI to appear quick enough so that user
wouldn't be able to start the browsing or other activities on his own before the
FRE comes out. I think that even 1 sec might be too long.
We can't make the request for CAService much earlier as the user profile is not
ready then. When it's ready, the session starts and user is able to interact
with it.

Denis, any comments on that from your side?

+
 void UserSessionManager::CompleteGuestSessionLogin(const GURL& start_url) {
   VLOG(1) << "Completing guest session login";
 
@@ -779,6 +787,39 @@ void UserSessionManager::OnProfilePrepared(Profile* profile,
   RestorePendingUserSessions();
 }
 
+void UserSessionManager::OnChildAccountStatusChanged() {
+  ChildAccountService* child_service =
+      ChildAccountServiceFactory::GetForProfile(profile_fetching_flags_);
+  StopStatusObserving();
+
+  if (!child_service)

[Marc Treib, 2015/03/04 16:31:48]

Can this actually happen? If so, add a comment please.

[merkulova, 2015/03/04 19:06:49]

I hope not, this all relies on CAServiceFactory. If it's fine, then I can remove
it. WDYT, Marc?

[Marc Treib, 2015/03/05 10:08:07]

It might (or might not) happen in some unit tests; it should not ever happen in
actual usage. I'd remove all the null checks and see if any tests break.

[merkulova, 2015/03/10 09:18:22]

Done.

+    return;
+ if (!child_service->IsChildAccountStatusKnown()) {

[Marc Treib, 2015/03/04 16:31:48]

DCHECK(child_service->IsChildAccountStatusKnown()) << "message";
if (requesting_start_urls) {
  ...

Or alternatively, remove the whole thing - you don't actually need the CAService
here.

[merkulova, 2015/03/04 19:06:49]

Done.

+    NOTREACHED() << "Child account status was changed to UNKNOWN";
+ } else if (requesting_start_urls_) {

[Marc Treib, 2015/03/04 16:31:48]

I think you can move this block into StopStatusObserving, then you don't need to
repeat it.

[merkulova, 2015/03/04 19:06:49]

Done.

+  InitializeStartUrls();
+  requesting_start_urls_ = false;
+  }
+}
+
+void UserSessionManager::OnStatusObservingCancelled() {
+  StopStatusObserving();
+  if (requesting_start_urls_) {

[Marc Treib, 2015/03/04 16:31:48]

^^

[merkulova, 2015/03/04 19:06:49]

Done.

+    InitializeStartUrls();
+    requesting_start_urls_ = false;
+  }
+}
+
+void UserSessionManager::StopStatusObserving() {
+  if (!profile_fetching_flags_)
+    return;
+  ChildAccountService* child_service =
+      ChildAccountServiceFactory::GetForProfile(profile_fetching_flags_);
+  if (child_service)
+    child_service->RemoveStatusObserver(this);
+  profile_fetching_flags_ = nullptr;
+}
+
 void UserSessionManager::CreateUserSession(const UserContext& user_context,
                                            bool has_auth_cookies) {
   user_context_ = user_context;
@@ -1045,10 +1086,12 @@ void UserSessionManager::InitializeStartUrls() const {
   std::vector<std::string> start_urls;
 
   user_manager::UserManager* user_manager = user_manager::UserManager::Get();
+
   bool can_show_getstarted_guide =
       user_manager->GetActiveUser()->GetType() ==
           user_manager::USER_TYPE_REGULAR &&
       !user_manager->IsCurrentUserNonCryptohomeDataEphemeral();
+
   // Skip the default first-run behavior for public accounts.
   if (!user_manager->IsLoggedInAsPublicAccount()) {
     if (AccessibilityManager::Get()->IsSpokenFeedbackEnabled()) {
@@ -1080,6 +1123,17 @@ void UserSessionManager::InitializeStartUrls() const {
 }
 
 bool UserSessionManager::InitializeUserSession(Profile* profile) {
+  ChildAccountService* child_service =
+      ChildAccountServiceFactory::GetForProfile(profile);
+  if (child_service)

[Marc Treib, 2015/03/04 16:31:48]

Here too: Can this actually happen?

[merkulova, 2015/03/04 19:06:49]

I hope no. It all relies on CAServiceFactory. If it's fine, then this can be
removed.

[Marc Treib, 2015/03/05 10:08:07]

Yup, please remove :)

[merkulova, 2015/03/10 09:18:22]

Done.

+      child_service->AddStatusObserver(this);
+  base::MessageLoopProxy::current()->PostDelayedTask(
+      FROM_HERE,
+      base::Bind(&UserSessionManager::OnStatusObservingCancelled,
+                 weak_factory_.GetWeakPtr()),
+      base::TimeDelta::FromMilliseconds(kFlagsFetchingLoginTimeoutMs));
+  profile_fetching_flags_ = profile;
+
   user_manager::UserManager* user_manager = user_manager::UserManager::Get();
 
   // Kiosk apps has their own session initialization pipeline.
@@ -1098,8 +1152,12 @@ bool UserSessionManager::InitializeUserSession(Profile* profile) {
     if (user_manager->IsCurrentUserNew() && !skip_post_login_screens) {
       // Don't specify start URLs if the administrator has configured the start
       // URLs via policy.
-      if (!SessionStartupPref::TypeIsManaged(profile->GetPrefs()))
-        InitializeStartUrls();
+      if (!SessionStartupPref::TypeIsManaged(profile->GetPrefs())) {
+        if (child_service && child_service->IsChildAccountStatusKnown())
+          InitializeStartUrls();
+        else
+          requesting_start_urls_ = true;
+      }
 
       // Mark the device as registered., i.e. the second part of OOBE as
       // completed.