cros: Defer primary user meta update until active

chrome/browser/ui/ash/session_controller_client.cc

 // Copyright 2016 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/ui/ash/session_controller_client.h"
 
 #include <algorithm>
 #include <utility>
 
 #include "ash/public/cpp/session_types.h"
@@ skipping 20 matching lines @@
 #include "content/public/browser/notification_service.h"
 #include "content/public/common/service_manager_connection.h"
 #include "content/public/common/service_names.mojom.h"
 #include "mojo/public/cpp/bindings/equals_traits.h"
 #include "services/service_manager/public/cpp/connector.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/gfx/image/image_skia.h"
 
 using session_manager::Session;
 using session_manager::SessionManager;
+using session_manager::SessionState;
 using user_manager::UserManager;
 using user_manager::User;
 using user_manager::UserList;
 
 namespace {
 
 SessionControllerClient* g_instance = nullptr;
 
 // Returns the session id of a given user or 0 if user has no session.
 uint32_t GetSessionId(const User& user) {
@@ skipping 130 matching lines @@
 }
 
 void SessionControllerClient::CycleActiveUser(
     ash::CycleUserDirection direction) {
   DoCycleActiveUser(direction);
 }
 
 void SessionControllerClient::ActiveUserChanged(const User* active_user) {
   SendSessionInfoIfChanged();
 
-  // UserAddedToSession is not called for the primary user session so send its
-  // meta data here once.
+  // UserAddedToSession is not called for the primary user session so its meta

[James Cook, 2017/05/08 16:23:02]

It would be very nice if it was called. Hopefully that is part of the work for
crbug.com/657149

The state changes in this class are very confusing right now.

[xiyuan, 2017/05/08 16:46:19]

Agree. I don't know why we send out ActiveUserChanged so early for the primary
user but not for secondary users. Probably because this was a simple replacement
for UserLoggedIn when we introduce multiprofile feature. This makes the
dependent code confusing and difficult to get right. The refactor should address
this.

+  // data here needs to be sent to ash before setting user session order.
+  // However, ActiveUserChanged happens at different timing for primary user
+  // and secondary users. For primary user, it happens before user profile load.
+  // For secondary users, it happens after user profile load. This caused
+  // confusing down the path. Bail out here to defer the primary user session
+  // meta sent until it becomes active so that ash side could expect a

[James Cook, 2017/05/08 16:23:02]

super nit: "meta" -> "metadata"?

[xiyuan, 2017/05/08 16:46:20]

Done.

+  // consistent state.
+  // TODO(xiyuan): Get rid of this after http://crbug.com/657149 refactoring.
   if (!primary_user_session_sent_ &&
       UserManager::Get()->GetPrimaryUser() == active_user) {
-    primary_user_session_sent_ = true;
-    SendUserSession(*active_user);
+    return;
   }
 
   SendUserSessionOrder();
 }
 
 void SessionControllerClient::UserAddedToSession(const User* added_user) {
   SendSessionInfoIfChanged();
   SendUserSession(*added_user);
 }
 
@@ skipping 101 matching lines @@
 
   DoSwitchActiveUser(account_id);
 }
 
 // static
 void SessionControllerClient::FlushForTesting() {
   g_instance->session_controller_.FlushForTesting();
 }
 
 void SessionControllerClient::OnSessionStateChanged() {
+  // Sent the primary user meta and order that are deferred from

[James Cook, 2017/05/08 16:23:02]

super nit: "meta" -> "metadata"?

[xiyuan, 2017/05/08 16:46:19]

Done.

+  // ActiveUserChanged before update session state.
+  if (!primary_user_session_sent_ &&
+      SessionManager::Get()->session_state() == SessionState::ACTIVE) {
+    DCHECK_EQ(UserManager::Get()->GetPrimaryUser(),
+              UserManager::Get()->GetActiveUser());
+    primary_user_session_sent_ = true;
+    SendUserSession(*UserManager::Get()->GetPrimaryUser());
+    SendUserSessionOrder();
+  }
+
   SendSessionInfoIfChanged();
 }
 
 void SessionControllerClient::OnCustodianInfoChanged() {
   DCHECK(supervised_user_profile_);
   User* user = chromeos::ProfileHelper::Get()->GetUserByProfile(
       supervised_user_profile_);
   if (user)
     SendUserSession(*user);
 }
@@ skipping 92 matching lines @@
   const UserList logged_in_users = user_manager->GetLoggedInUsers();
   std::vector<uint32_t> user_session_ids;
   for (auto* user : user_manager->GetLRULoggedInUsers()) {
     const uint32_t user_session_id = GetSessionId(*user);
     DCHECK_NE(0u, user_session_id);
     user_session_ids.push_back(user_session_id);
   }
 
   session_controller_->SetUserSessionOrder(user_session_ids);
 }