Start session fail causes restart chrome

BUG=244628
TEST=manual check on Acer C720

Review URL: https://codereview.chromium.org/228703004

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@266791 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/chromeos/login/crash_restore_browsertest.cc

@@ -33,7 +33,7 @@ const char kUserId3[] = "user3@example.com";
 
 class CrashRestoreSimpleTest : public InProcessBrowserTest {
  protected:
-  CrashRestoreSimpleTest() {}
+  CrashRestoreSimpleTest() : session_started_count_(0) {}
 
   virtual ~CrashRestoreSimpleTest() {}
 
@@ -53,10 +53,27 @@ class CrashRestoreSimpleTest : public InProcessBrowserTest {
     dbus_thread_manager->SetSessionManagerClient(
         scoped_ptr<SessionManagerClient>(session_manager_client_));
     DBusThreadManager::SetInstanceForTesting(dbus_thread_manager);
-    session_manager_client_->StartSession(kUserId1);
+    // We need to create MessageLoop here to process callback from
+    // session_manager
+    base::MessageLoop msg_loop;
+    session_manager_client_->StartSession(
+        kUserId1,
+        base::Bind(&CrashRestoreSimpleTest::OnSessionStarted,
+                   base::Unretained(this),
+                   kUserId1));
+    base::RunLoop().RunUntilIdle();
+    ASSERT_EQ(1, session_started_count_);
   }
 
+ public:
+  void OnSessionStarted(const std::string& user_email, bool success) {
+    ASSERT_TRUE(success);
+    ++session_started_count_;
+  }
+
+ protected:
   FakeSessionManagerClient* session_manager_client_;
+  int session_started_count_;
 };
 
 IN_PROC_BROWSER_TEST_F(CrashRestoreSimpleTest, RestoreSessionForOneUser) {
@@ -118,8 +135,21 @@ class CrashRestoreComplexTest : public CrashRestoreSimpleTest {
 
   virtual void SetUpInProcessBrowserTestFixture() OVERRIDE {
     CrashRestoreSimpleTest::SetUpInProcessBrowserTestFixture();
-    session_manager_client_->StartSession(kUserId2);
-    session_manager_client_->StartSession(kUserId3);
+    // We need to create MessageLoop here to process callback from
+    // session_manager
+    base::MessageLoop msg_loop;
+    session_manager_client_->StartSession(
+        kUserId2,
+        base::Bind(&CrashRestoreSimpleTest::OnSessionStarted,
+                   base::Unretained(this),
+                   kUserId2));
+    session_manager_client_->StartSession(
+        kUserId3,
+        base::Bind(&CrashRestoreSimpleTest::OnSessionStarted,
+                   base::Unretained(this),
+                   kUserId3));
+    base::RunLoop().RunUntilIdle();
+    ASSERT_EQ(3, CrashRestoreSimpleTest::session_started_count_);
   }
 };
 

chrome/browser/chromeos/login/login_utils.cc

@@ -200,6 +200,19 @@ class LoginUtilsImpl
   // the authentication profile.
   void CompleteProfileCreate(Profile* user_profile);
 
+  // Callback to resume profile preparing after start session.
+  void OnSessionStarted(const UserContext& user_context,
+                        const std::string& display_email,
+                        bool has_cookies,
+                        LoginUtils::Delegate* delegate,
+                        bool success);
+
+  // Complete profile preparation with having active session.
+  void CompletePrepareProfileWithActiveSession(const UserContext& user_context,
+                                               const std::string& display_email,
+                                               bool has_cookies,
+                                               LoginUtils::Delegate* delegate);
+
   // Finalized profile preparation.
   void FinalizePrepareProfile(Profile* user_profile);
 
@@ -400,10 +413,42 @@ void LoginUtilsImpl::PrepareProfile(
   if (!has_active_session) {
     btl->AddLoginTimeMarker("StartSession-Start", false);
     DBusThreadManager::Get()->GetSessionManagerClient()->StartSession(
-        user_context.username);
-    btl->AddLoginTimeMarker("StartSession-End", false);
+        user_context.username,
+        base::Bind(&LoginUtilsImpl::OnSessionStarted,
+                   AsWeakPtr(),
+                   user_context,
+                   display_email,
+                   has_cookies,
+                   delegate));
+    return;
+  }
+  CompletePrepareProfileWithActiveSession(
+      user_context, display_email, has_cookies, delegate);
+}
+
+void LoginUtilsImpl::OnSessionStarted(const UserContext& user_context,
+                                      const std::string& display_email,
+                                      bool has_cookies,
+                                      LoginUtils::Delegate* delegate,
+                                      bool success) {
+  BootTimesLoader* btl = BootTimesLoader::Get();
+  btl->AddLoginTimeMarker("StartSession-End", false);
+
+  if (!success) {
+    LOG(ERROR) << "StartSession failed";
+    chrome::AttemptUserExit();
+    return;
   }
+  CompletePrepareProfileWithActiveSession(
+      user_context, display_email, has_cookies, delegate);
+}
 
+void LoginUtilsImpl::CompletePrepareProfileWithActiveSession(
+    const UserContext& user_context,
+    const std::string& display_email,
+    bool has_cookies,
+    LoginUtils::Delegate* delegate) {
+  BootTimesLoader* btl = BootTimesLoader::Get();
   btl->AddLoginTimeMarker("UserLoggedIn-Start", false);
   UserManager* user_manager = UserManager::Get();
   user_manager->UserLoggedIn(user_context.username,

chrome/browser/chromeos/settings/device_settings_test_helper.cc

@@ -116,7 +116,10 @@ void DeviceSettingsTestHelper::EmitLoginPromptVisible() {}
 void DeviceSettingsTestHelper::RestartJob(int pid,
                                           const std::string& command_line) {}
 
-void DeviceSettingsTestHelper::StartSession(const std::string& user_email) {}
+void DeviceSettingsTestHelper::StartSession(
+    const std::string& user_email,
+    const StartSessionCallback& callback) {
+}
 
 void DeviceSettingsTestHelper::StopSession() {}
 

chrome/browser/chromeos/settings/device_settings_test_helper.h

@@ -88,7 +88,8 @@ class DeviceSettingsTestHelper : public SessionManagerClient {
   virtual bool HasObserver(Observer* observer) OVERRIDE;
   virtual void EmitLoginPromptVisible() OVERRIDE;
   virtual void RestartJob(int pid, const std::string& command_line) OVERRIDE;
-  virtual void StartSession(const std::string& user_email) OVERRIDE;
+  virtual void StartSession(const std::string& user_email,
+                            const StartSessionCallback& callback) OVERRIDE;
   virtual void StopSession() OVERRIDE;
   virtual void StartDeviceWipe() OVERRIDE;
   virtual void RequestLockScreen() OVERRIDE;

chromeos/dbus/fake_session_manager_client.cc

@@ -46,11 +46,14 @@ void FakeSessionManagerClient::RestartJob(int pid,
                                           const std::string& command_line) {
 }
 
-void FakeSessionManagerClient::StartSession(const std::string& user_email) {
+void FakeSessionManagerClient::StartSession(
+    const std::string& user_email,
+    const StartSessionCallback& callback) {
   DCHECK_EQ(0UL, user_sessions_.count(user_email));
   std::string user_id_hash =
       CryptohomeClient::GetStubSanitizedUsername(user_email);
   user_sessions_[user_email] = user_id_hash;
+  base::MessageLoop::current()->PostTask(FROM_HERE, base::Bind(callback, true));
 }
 
 void FakeSessionManagerClient::StopSession() {

chromeos/dbus/fake_session_manager_client.h

@@ -30,7 +30,8 @@ class FakeSessionManagerClient : public SessionManagerClient {
   virtual bool HasObserver(Observer* observer) OVERRIDE;
   virtual void EmitLoginPromptVisible() OVERRIDE;
   virtual void RestartJob(int pid, const std::string& command_line) OVERRIDE;
-  virtual void StartSession(const std::string& user_email) OVERRIDE;
+  virtual void StartSession(const std::string& user_email,
+                            const StartSessionCallback& callback) OVERRIDE;
   virtual void StopSession() OVERRIDE;
   virtual void StartDeviceWipe() OVERRIDE;
   virtual void RequestLockScreen() OVERRIDE;

chromeos/dbus/mock_session_manager_client.h

@@ -24,7 +24,8 @@ class MockSessionManagerClient : public SessionManagerClient {
   MOCK_METHOD1(HasObserver, bool(Observer*));
   MOCK_METHOD0(EmitLoginPromptVisible, void(void));
   MOCK_METHOD2(RestartJob, void(int, const std::string&));
-  MOCK_METHOD1(StartSession, void(const std::string&));
+  MOCK_METHOD2(StartSession,
+               void(const std::string&, const StartSessionCallback&));
   MOCK_METHOD0(StopSession, void(void));
   MOCK_METHOD0(StartDeviceWipe, void(void));
   MOCK_METHOD0(RequestLockScreen, void(void));

chromeos/dbus/session_manager_client.cc

@@ -71,7 +71,8 @@ class SessionManagerClientImpl : public SessionManagerClient {
                    weak_ptr_factory_.GetWeakPtr()));
   }
 
-  virtual void StartSession(const std::string& user_email) OVERRIDE {
+  virtual void StartSession(const std::string& user_email,
+                            const StartSessionCallback& callback) OVERRIDE {
     dbus::MethodCall method_call(login_manager::kSessionManagerInterface,
                                  login_manager::kSessionManagerStartSession);
     dbus::MessageWriter writer(&method_call);
@@ -81,7 +82,8 @@ class SessionManagerClientImpl : public SessionManagerClient {
         &method_call,
         dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
         base::Bind(&SessionManagerClientImpl::OnStartSession,
-                   weak_ptr_factory_.GetWeakPtr()));
+                   weak_ptr_factory_.GetWeakPtr(),
+                   callback));
   }
 
   virtual void StopSession() OVERRIDE {
@@ -332,10 +334,18 @@ class SessionManagerClientImpl : public SessionManagerClient {
   }
 
   // Called when kSessionManagerStartSession method is complete.
-  void OnStartSession(dbus::Response* response) {
-    LOG_IF(ERROR, !response)
-        << "Failed to call "
-        << login_manager::kSessionManagerStartSession;
+  void OnStartSession(const StartSessionCallback& callback,
+                      dbus::Response* response) {
+    bool success = false;
+    if (!response) {
+      LOG(ERROR) << "Failed to call "
+                 << login_manager::kSessionManagerStartSession;
+    } else {
+      dbus::MessageReader reader(response);
+      if (!reader.PopBool(&success))
+        LOG(ERROR) << "Invalid response: " << response->ToString();
+    }
+    callback.Run(success);
   }
 
   // Called when kSessionManagerStopSession method is complete.
@@ -518,7 +528,10 @@ class SessionManagerClientStubImpl : public SessionManagerClient {
   }
   virtual void EmitLoginPromptVisible() OVERRIDE {}
   virtual void RestartJob(int pid, const std::string& command_line) OVERRIDE {}
-  virtual void StartSession(const std::string& user_email) OVERRIDE {}
+  virtual void StartSession(const std::string& user_email,
+                            const StartSessionCallback& callback) OVERRIDE {
+    callback.Run(true);
+  }
   virtual void StopSession() OVERRIDE {}
   virtual void StartDeviceWipe() OVERRIDE {}
   virtual void RequestLockScreen() OVERRIDE {

chromeos/dbus/session_manager_client.h

@@ -70,8 +70,13 @@ class CHROMEOS_EXPORT SessionManagerClient : public DBusClient {
   // Restarts a job referenced by |pid| with the provided command line.
   virtual void RestartJob(int pid, const std::string& command_line) = 0;
 
+  // Used for StartSession. Takes a boolean indicating whether the
+  // operation was successful or not.
+  typedef base::Callback<void(bool success)> StartSessionCallback;
+
   // Starts the session for the user.
-  virtual void StartSession(const std::string& user_email) = 0;
+  virtual void StartSession(const std::string& user_email,
+                            const StartSessionCallback& callback) = 0;
 
   // Stops the current session.
   virtual void StopSession() = 0;