We had a problem where if chrome crashed on cros and was relaunched

it would always run with the Default profile. This meant that two
different users could see the same data...a big problem.
We patched in the OS by deleting the profile directory each time
but this is the right fix.
When the session_manager reruns Chrome on a crash it will now
pass a new flag (--login-user). Chrome uses this and ensures that
the profile dir (specified by --login-profile) is mounted as an
encrypted drive. If this flag isn't specified then Chrome uses the
Default profile, but in incognito mode so no data is written.

BUG=chromiumos:1967
TEST=Login to chromeos as user1, in a terminal kill the browser process, chrome relaunches, log into gmail, sign out (using menu or power button). Login as user2, kill browser process, chrome relaunches, go to gmail. Ensure that user1 isn't logged in.

Review URL: http://codereview.chromium.org/1094009

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

13 files changed, 94 insertions, 45 deletions

diff --git a/chrome/browser/browser_main.cc b/chrome/browser/browser_main.cc
index 0971b4a..6941bae 100644
--- a/chrome/browser/browser_main.cc
+++ b/chrome/browser/browser_main.cc
@@ -132,6 +132,7 @@
 #if defined(OS_CHROMEOS)
 #include "chrome/browser/chromeos/cros/cros_library.h"
 #include "chrome/browser/chromeos/external_metrics.h"
+#include "chrome/browser/chromeos/login/user_manager.h"
 #include "chrome/browser/views/browser_dialogs.h"
 #endif
 
@@ -755,6 +756,15 @@ int BrowserMain(const MainFunctionParams& parameters) {
 
   // Try to create/load the profile.
   ProfileManager* profile_manager = browser_process->profile_manager();
+#if defined(OS_CHROMEOS)
+  if (parsed_command_line.HasSwitch(switches::kLoginUser)) {
+    std::string username =
+        parsed_command_line.GetSwitchValueASCII(switches::kLoginUser);
+    LOG(INFO) << "Relaunching browser for user: " << username;
+    chromeos::UserManager::Get()->UserLoggedIn(username);
+  }
+#endif
+
   Profile* profile = profile_manager->GetDefaultProfile(user_data_dir);
 
 #if defined(OS_WIN)
diff --git a/chrome/browser/chromeos/cros/cryptohome_library.cc b/chrome/browser/chromeos/cros/cryptohome_library.cc
index 4be406c..d74d4e0 100644
--- a/chrome/browser/chromeos/cros/cryptohome_library.cc
+++ b/chrome/browser/chromeos/cros/cryptohome_library.cc
@@ -23,4 +23,8 @@ bool CryptohomeLibrary::Mount(const std::string& user_email,
   return chromeos::CryptohomeMount(user_email.c_str(), passhash.c_str());
 }
 
+bool CryptohomeLibrary::IsMounted() {
+  return chromeos::CryptohomeIsMounted();
+}
+
 }  // namespace chromeos
diff --git a/chrome/browser/chromeos/cros/cryptohome_library.h b/chrome/browser/chromeos/cros/cryptohome_library.h
index 1bec9fa..7fab7e6 100644
--- a/chrome/browser/chromeos/cros/cryptohome_library.h
+++ b/chrome/browser/chromeos/cros/cryptohome_library.h
@@ -31,6 +31,9 @@ class CryptohomeLibrary {
   virtual bool CheckKey(const std::string& user_email,
                         const std::string& passhash);
 
+  // Asks cryptohomed if a drive is currently mounted.
+  virtual bool IsMounted();
+
  private:
   friend struct DefaultSingletonTraits<CryptohomeLibrary>;
   friend class MockCryptohomeLibrary;
diff --git a/chrome/browser/chromeos/login/account_screen.cc b/chrome/browser/chromeos/login/account_screen.cc
index 5a046ac..2e8a2bf 100644
--- a/chrome/browser/chromeos/login/account_screen.cc
+++ b/chrome/browser/chromeos/login/account_screen.cc
@@ -76,7 +76,7 @@ void AccountScreen::CreateView() {
   view()->SetAccountCreationViewDelegate(this);
 
   GURL url(kCreateAccountPageUrl);
-  Profile* profile = ProfileManager::GetLoginWizardProfile();
+  Profile* profile = ProfileManager::GetDefaultProfile();
   view()->InitDOM(profile,
                   SiteInstance::CreateSiteInstanceForURL(profile, url));
   view()->SetTabContentsDelegate(this);
diff --git a/chrome/browser/chromeos/login/google_authenticator.cc b/chrome/browser/chromeos/login/google_authenticator.cc
index 347f5e9..f14ba97 100644
--- a/chrome/browser/chromeos/login/google_authenticator.cc
+++ b/chrome/browser/chromeos/login/google_authenticator.cc
@@ -60,7 +60,7 @@ bool GoogleAuthenticator::Authenticate(const std::string& username,
   PathService::Get(chrome::DIR_USER_DATA, &user_data_dir);
   ProfileManager* profile_manager = g_browser_process->profile_manager();
   Profile* profile = profile_manager->GetDefaultProfile(user_data_dir);
-  getter_ = profile->GetOffTheRecordProfile()->GetRequestContext();
+  getter_ = profile->GetRequestContext();
   fetcher_.reset(new URLFetcher(GURL(AuthResponseHandler::kClientLoginUrl),
                                 URLFetcher::POST,
                                 this));
diff --git a/chrome/browser/process_singleton_linux.cc b/chrome/browser/process_singleton_linux.cc
index 0ff9a57..10d6321 100644
--- a/chrome/browser/process_singleton_linux.cc
+++ b/chrome/browser/process_singleton_linux.cc
@@ -505,23 +505,7 @@ void ProcessSingleton::LinuxWatcher::HandleMessage(
   PrefService* prefs = g_browser_process->local_state();
   DCHECK(prefs);
 
-  FilePath user_data_dir;
-  PathService::Get(chrome::DIR_USER_DATA, &user_data_dir);
-  ProfileManager* profile_manager = g_browser_process->profile_manager();
-
-#if defined(OS_CHROMEOS)
-  Profile* profile;
-  if (parsed_command_line.HasSwitch(switches::kProfile)) {
-    std::wstring profile_dir =
-        parsed_command_line.GetSwitchValue(switches::kProfile);
-    profile = profile_manager->GetProfile(
-        user_data_dir.Append(FilePath::FromWStringHack(profile_dir)));
-  } else {
-    profile = profile_manager->GetDefaultProfile(user_data_dir);
-  }
-#else
-  Profile* profile = profile_manager->GetDefaultProfile(user_data_dir);
-#endif
+  Profile* profile = ProfileManager::GetDefaultProfile();
 
   if (!profile) {
     // We should only be able to get here if the profile already exists and
diff --git a/chrome/browser/process_singleton_win.cc b/chrome/browser/process_singleton_win.cc
index 68844235..4e8de16 100644
--- a/chrome/browser/process_singleton_win.cc
+++ b/chrome/browser/process_singleton_win.cc
@@ -264,10 +264,7 @@ LRESULT ProcessSingleton::OnCopyData(HWND hwnd, const COPYDATASTRUCT* cds) {
     PrefService* prefs = g_browser_process->local_state();
     DCHECK(prefs);
 
-    FilePath user_data_dir;
-    PathService::Get(chrome::DIR_USER_DATA, &user_data_dir);
-    ProfileManager* profile_manager = g_browser_process->profile_manager();
-    Profile* profile = profile_manager->GetDefaultProfile(user_data_dir);
+    Profile* profile = ProfileManager::GetDefaultProfile();
     if (!profile) {
       // We should only be able to get here if the profile already exists and
       // has been created.
diff --git a/chrome/browser/profile_manager.cc b/chrome/browser/profile_manager.cc
index 4e59066..6699c7a 100644
--- a/chrome/browser/profile_manager.cc
+++ b/chrome/browser/profile_manager.cc
@@ -31,6 +31,11 @@
 #include "net/url_request/url_request_job.h"
 #include "net/url_request/url_request_job_tracker.h"
 
+#if defined(OS_CHROMEOS)
+#include "chrome/browser/chromeos/cros/cros_library.h"
+#include "chrome/browser/chromeos/cros/cryptohome_library.h"
+#endif
+
 // static
 void ProfileManager::ShutdownSessionServices() {
   ProfileManager* pm = g_browser_process->profile_manager();
@@ -39,12 +44,11 @@ void ProfileManager::ShutdownSessionServices() {
 }
 
 // static
-Profile* ProfileManager::GetLoginWizardProfile() {
+Profile* ProfileManager::GetDefaultProfile() {
   FilePath user_data_dir;
   PathService::Get(chrome::DIR_USER_DATA, &user_data_dir);
   ProfileManager* profile_manager = g_browser_process->profile_manager();
-  return profile_manager->GetDefaultProfile(
-      user_data_dir)->GetOffTheRecordProfile();
+  return profile_manager->GetDefaultProfile(user_data_dir);
 }
 
 ProfileManager::ProfileManager() : logged_in_(false) {
@@ -91,22 +95,45 @@ FilePath ProfileManager::GetProfilePrefsPath(
 
 Profile* ProfileManager::GetDefaultProfile(const FilePath& user_data_dir) {
   FilePath default_profile_dir(user_data_dir);
-  std::wstring profile = chrome::kNotSignedInProfile;
 #if defined(OS_CHROMEOS)
-  // If the user has logged in, pick up the new profile.
   const CommandLine& command_line = *CommandLine::ForCurrentProcess();
-  // TODO(davemoore) Delete this once chromium os has started using
-  // "--login-profile" instead of "--profile".
-  if (logged_in_ && command_line.HasSwitch(switches::kProfile)) {
-    profile = command_line.GetSwitchValue(switches::kProfile);
-  }
-  if (logged_in_ && command_line.HasSwitch(switches::kLoginProfile)) {
-    profile = command_line.GetSwitchValue(switches::kLoginProfile);
+  if (logged_in_) {
+    std::wstring profile_dir;
+    // If the user has logged in, pick up the new profile.
+    // TODO(davemoore) Delete this once chromium os has started using
+    // "--login-profile" instead of "--profile".
+    if (command_line.HasSwitch(switches::kLoginProfile)) {
+      profile_dir = command_line.GetSwitchValue(switches::kLoginProfile);
+    } else if (command_line.HasSwitch(switches::kProfile)) {
+      profile_dir = command_line.GetSwitchValue(switches::kProfile);
+    } else {
+      // We should never be logged in with no profile dir.
+      NOTREACHED();
+      return NULL;
+    }
+    default_profile_dir = default_profile_dir.Append(
+        FilePath::FromWStringHack(profile_dir));
+    return GetProfile(default_profile_dir);
+  } else {
+    // If not logged in on cros, always return the incognito profile
+    default_profile_dir = default_profile_dir.Append(
+        FilePath::FromWStringHack(chrome::kNotSignedInProfile));
+    Profile*profile = GetProfile(default_profile_dir);
+
+    // For cros, return the OTR profile so we never accidentally keep
+    // user data in an unencrypted profile. But doing this makes
+    // many of the browser and ui tests fail.
+    // TODO(davemoore) Fix the tests so they allow OTR profiles.
+    if (!command_line.HasSwitch(switches::kTestType))
+      profile = profile->GetOffTheRecordProfile();
+
+    return profile;
   }
-#endif
+#else
   default_profile_dir = default_profile_dir.Append(
-      FilePath::FromWStringHack(profile));
+      FilePath::FromWStringHack(chrome::kNotSignedInProfile));
   return GetProfile(default_profile_dir);
+#endif
 }
 
 Profile* ProfileManager::GetProfile(const FilePath& profile_dir) {
@@ -200,6 +227,12 @@ void ProfileManager::Observe(
     const NotificationDetails& details) {
 #if defined(OS_CHROMEOS)
   if (type == NotificationType::LOGIN_USER_CHANGED) {
+    CHECK(chromeos::CrosLibrary::EnsureLoaded());
+    // If we don't have a mounted profile directory we're in trouble.
+    // TODO(davemoore) Once we have better api this check should ensure that
+    // our profile directory is the one that's mounted, and that it's mounted
+    // as the current user.
+    CHECK(chromeos::CryptohomeLibrary::Get()->IsMounted());
     logged_in_ = true;
   }
 #endif
diff --git a/chrome/browser/profile_manager.h b/chrome/browser/profile_manager.h
index 0e6e8db..2981225 100644
--- a/chrome/browser/profile_manager.h
+++ b/chrome/browser/profile_manager.h
@@ -74,15 +74,15 @@ class ProfileManager : public NonThreadSafe,
   // Invokes ShutdownSessionService() on all profiles.
   static void ShutdownSessionServices();
 
-  // Returns profile to use for Chrome in login wizard mode.
-  static Profile* GetLoginWizardProfile();
-
   // Returns the default profile.  This adds the profile to the
   // ProfileManager if it doesn't already exist.  This method returns NULL if
   // the profile doesn't exist and we can't create it.
-  // The profile used can be overridden by using --profile on
+  // The profile used can be overridden by using --login-profile on cros.
   Profile* GetDefaultProfile(const FilePath& user_data_dir);
 
+  // Same as instance method but provides the default user_data_dir as well.
+  static Profile* GetDefaultProfile();
+
   // Returns a profile for a specific profile directory within the user data
   // dir. This will return an existing profile it had already been created,
   // otherwise it will create and manage it.
diff --git a/chrome/common/chrome_switches.cc b/chrome/common/chrome_switches.cc
index 5dade42..07e9433 100644
--- a/chrome/common/chrome_switches.cc
+++ b/chrome/common/chrome_switches.cc
@@ -685,6 +685,9 @@ const char kTestName[]                      = "test-name";
 // Runs the security test for the sandbox.
 const char kTestSandbox[]                   = "test-sandbox";
 
+// Pass the type of the current test harness ("browser" or "ui")
+const char kTestType[]                      = "test-type";
+
 // The value of this switch tells the app to listen for and broadcast
 // testing-related messages on IPC channel with the given ID.
 const char kTestingChannelID[]              = "testing-channel";
@@ -804,6 +807,9 @@ const char kProfile[]                       = "profile";
 // Specifies the profile to use once a chromeos user is logged in.
 const char kLoginProfile[]                  = "login-profile";
 
+// Specifies the user which is already logged in.
+const char kLoginUser[]                     = "login-user";
+
 // Use the frame layout used in chromeos.
 const char kChromeosFrame[]                 = "chromeos-frame";
 #endif
diff --git a/chrome/common/chrome_switches.h b/chrome/common/chrome_switches.h
index acd52f5..e6c541e 100644
--- a/chrome/common/chrome_switches.h
+++ b/chrome/common/chrome_switches.h
@@ -192,6 +192,7 @@ extern const char kSyncerThreadTimedStop[];
 extern const char kTabCountToLoadOnSessionRestore[];
 extern const char kTestName[];
 extern const char kTestSandbox[];
+extern const char kTestType[];
 extern const char kTestingChannelID[];
 extern const char kThumbnailStore[];
 extern const char kTrustedPlugins[];
@@ -230,6 +231,7 @@ extern const char kLoginScreenSize[];
 extern const char kTestLoadLibcros[];
 extern const char kProfile[];
 extern const char kLoginProfile[];
+extern const char kLoginUser[];
 extern const char kChromeosFrame[];
 #endif
 
diff --git a/chrome/test/in_process_browser_test.cc b/chrome/test/in_process_browser_test.cc
index 80c0ce9..3f4ce39 100644
--- a/chrome/test/in_process_browser_test.cc
+++ b/chrome/test/in_process_browser_test.cc
@@ -63,6 +63,9 @@ extern int BrowserMain(const MainFunctionParams&);
 
 const wchar_t kUnitTestShowWindows[] = L"show-windows";
 
+// Passed as value of kTestType.
+static const char kBrowserTestType[] = "browser";
+
 // Default delay for the time-out at which we stop the
 // inner-message loop the first time.
 const int kInitialTimeoutInMS = 30000;
@@ -132,6 +135,10 @@ void InProcessBrowserTest::SetUp() {
   // Don't show the first run ui.
   command_line->AppendSwitch(switches::kNoFirstRun);
 
+  // This is a Browser test.
+  command_line->AppendSwitchWithValue(switches::kTestType,
+                                      ASCIIToWide(kBrowserTestType));
+
   // Single-process mode is not set in BrowserMain so it needs to be processed
   // explicitlty.
   original_single_process_ = RenderProcessHost::run_renderer_in_process();
@@ -244,10 +251,7 @@ void InProcessBrowserTest::RunTestOnMainThreadLoop() {
   // In the long term it would be great if we could use a TestingProfile
   // here and only enable services you want tested, but that requires all
   // consumers of Profile to handle NULL services.
-  FilePath user_data_dir;
-  PathService::Get(chrome::DIR_USER_DATA, &user_data_dir);
-  ProfileManager* profile_manager = g_browser_process->profile_manager();
-  Profile* profile = profile_manager->GetDefaultProfile(user_data_dir);
+  Profile* profile = ProfileManager::GetDefaultProfile();
   if (!profile) {
     // We should only be able to get here if the profile already exists and
     // has been created.
diff --git a/chrome/test/ui/ui_test.cc b/chrome/test/ui/ui_test.cc
index 83803ed..05d332c 100644
--- a/chrome/test/ui/ui_test.cc
+++ b/chrome/test/ui/ui_test.cc
@@ -49,6 +49,8 @@ static const int kWaitForActionMaxMsec = 10000;
 static const int kCommandExecutionTimeout = 30000;
 // Delay to let the browser shut down before trying more brutal methods.
 static const int kWaitForTerminateMsec = 30000;
+// Passed as value of kTestType.
+static const char kUITestType[] = "ui";
 
 const wchar_t UITestBase::kFailedNoCrashService[] =
 #if defined(OS_WIN)
@@ -1077,6 +1079,10 @@ bool UITestBase::LaunchBrowserHelper(const CommandLine& arguments,
   // default browser) that could conflicts with some tests expectations.
   command_line.AppendSwitch(switches::kNoDefaultBrowserCheck);
 
+  // This is a UI test.
+  command_line.AppendSwitchWithValue(switches::kTestType,
+                                     ASCIIToWide(kUITestType));
+
   // We need cookies on file:// for things like the page cycler.
   if (enable_file_cookies_)
     command_line.AppendSwitch(switches::kEnableFileCookies);