Implementation of Device End of Life Notification

chrome/browser/chromeos/login/existing_user_controller.h

Index: chrome/browser/chromeos/login/existing_user_controller.h
diff --git a/chrome/browser/chromeos/login/existing_user_controller.h b/chrome/browser/chromeos/login/existing_user_controller.h
index e33bf6710179309edff4de55afb1207e35e11787..456b38d5b9c50278df8b1b735fdf35ef450c1718 100644
--- a/chrome/browser/chromeos/login/existing_user_controller.h
+++ b/chrome/browser/chromeos/login/existing_user_controller.h
@@ -19,12 +19,14 @@
 #include "base/time/time.h"
 #include "base/timer/timer.h"
 #include "chrome/browser/chromeos/app_mode/kiosk_app_manager.h"
+#include "chrome/browser/chromeos/eol_notification.h"

[xiyuan, 2016/06/16 22:41:28]

Forward clear in line 49 is enough, move the include to cc file.

[xiaoyinh(OOO Sep 11-29), 2016/06/17 04:07:35]

Done.

 #include "chrome/browser/chromeos/hats/hats_notification_controller.h"
 #include "chrome/browser/chromeos/login/session/user_session_manager.h"
 #include "chrome/browser/chromeos/login/signin/token_handle_util.h"
 #include "chrome/browser/chromeos/login/ui/login_display.h"
 #include "chrome/browser/chromeos/settings/cros_settings.h"
 #include "chrome/browser/chromeos/settings/device_settings_service.h"
+#include "chromeos/dbus/update_engine_client.h"

[xiyuan, 2016/06/16 22:41:28]

Do we need this?

[xiaoyinh(OOO Sep 11-29), 2016/06/17 04:07:35]

Removed.

 #include "chromeos/login/auth/login_performer.h"
 #include "chromeos/login/auth/user_context.h"
 #include "components/signin/core/account_id/account_id.h"
@@ -44,6 +46,7 @@ class BootstrapUserContextInitializer;
 class CrosSettings;
 class LoginDisplayHost;
 class OAuth2TokenInitializer;
+class EolNotification;
 
 namespace login {
 class NetworkStateHelper;
@@ -252,6 +255,9 @@ class ExistingUserController : public LoginDisplay::Delegate,
       const AccountId&,
       TokenHandleUtil::TokenHandleStatus token_handle_status);
 
+  // Used to Add/Update Eol notification
+  std::unique_ptr<EolNotification> eol_notification_;

[xiyuan, 2016/06/16 22:41:28]

This does not feel belong here. It is user session/profile specific. Doing it
like this does not work for multi profile case.

Think it should be part of UserSessionManager and associated with its profile
similar to |default_ime_states_|.

Or better yet, make this a KeyedService so that every profile could have an
instance of it.

[xiaoyinh(OOO Sep 11-29), 2016/06/17 04:07:35]

Hi Xiyuan,

Could you elaborate what could go wrong? My observation is In the
UserSessionManger after a profile is prepared it will call
ExistingUserController::OnProfilePrepared() with an associating profile.

And ExistingUserController maintains it's own life cycle and deletes itself when
the user logs in, so every login we will have a new ExistingUserController to
reset the EolNotification with the current profile.

I've tested that have multiple profiles in the chromebook and switch account by
login and logout, so far it looks fine.

Let me know if I've missed anything.

[xiyuan, 2016/06/17 15:21:27]

ExistingUserController is part of the login screen (owned by
LoginDisplayHostImpl). It will be gone soon after the login screen is dismissed.
However, EolNotification calls update engine and wait for the eol status, which
is not coupled with the login screen life time. So we have a race here. If login
screen is disposed sooner than update engine's call returns, we will never see
the notification.

+
   // Public session auto-login timer.
   std::unique_ptr<base::OneShotTimer> auto_login_timer_;