Refactor ProfileInfoCache in c/b/profiles

chrome/browser/profiles/profile_manager.cc

 // Copyright (c) 2012 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/profiles/profile_manager.h"
 
 #include <stdint.h>
 
 #include <set>
 #include <string>
@@ skipping 19 matching lines @@
 #include "chrome/browser/content_settings/host_content_settings_map_factory.h"
 #include "chrome/browser/download/download_service.h"
 #include "chrome/browser/download/download_service_factory.h"
 #include "chrome/browser/invalidation/profile_invalidation_provider_factory.h"
 #include "chrome/browser/net/spdyproxy/data_reduction_proxy_chrome_settings.h"
 #include "chrome/browser/net/spdyproxy/data_reduction_proxy_chrome_settings_factory.h"
 #include "chrome/browser/password_manager/password_manager_setting_migrator_service_factory.h"
 #include "chrome/browser/password_manager/password_store_factory.h"
 #include "chrome/browser/prefs/incognito_mode_prefs.h"
 #include "chrome/browser/profiles/bookmark_model_loaded_observer.h"
+#include "chrome/browser/profiles/profile_attributes_entry.h"
 #include "chrome/browser/profiles/profile_attributes_storage.h"
 #include "chrome/browser/profiles/profile_avatar_icon_util.h"
 #include "chrome/browser/profiles/profile_destroyer.h"
 #include "chrome/browser/profiles/profile_info_cache.h"
 #include "chrome/browser/profiles/profile_metrics.h"
 #include "chrome/browser/profiles/profile_statistics.h"
 #include "chrome/browser/profiles/profiles_state.h"
 #include "chrome/browser/sessions/session_service_factory.h"
 #include "chrome/browser/signin/account_fetcher_service_factory.h"
 #include "chrome/browser/signin/account_reconcilor_factory.h"
@@ skipping 344 matching lines @@
 
   // If the profile is already loaded (e.g., chrome.exe launched twice), just
   // return it.
   Profile* profile = GetProfileByPath(profile_dir);
   if (profile)
     return profile;
   return CreateAndInitializeProfile(profile_dir);
 }
 
 size_t ProfileManager::GetNumberOfProfiles() {
-  return GetProfileInfoCache().GetNumberOfProfiles();
+  return GetProfileAttributesStorage().GetNumberOfProfiles();
 }
 
 void ProfileManager::CreateProfileAsync(
     const base::FilePath& profile_path,
     const CreateCallback& callback,
     const base::string16& name,
     const std::string& icon_url,
     const std::string& supervised_user_id) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   TRACE_EVENT1("browser,startup",
@@ skipping 10 matching lines @@
 
   // Create the profile if needed and collect its ProfileInfo.
   ProfilesInfoMap::iterator iter = profiles_info_.find(profile_path);
   ProfileInfo* info = NULL;
 
   if (iter != profiles_info_.end()) {
     info = iter->second.get();
   } else {
     // Initiate asynchronous creation process.
     info = RegisterProfile(CreateProfileAsyncHelper(profile_path, this), false);
-    ProfileInfoCache& cache = GetProfileInfoCache();
-    // Get the icon index from the user's icon url
     size_t icon_index;
     DCHECK(base::IsStringASCII(icon_url));
     if (profiles::IsDefaultAvatarIconUrl(icon_url, &icon_index)) {
       // add profile to cache with user selected name and avatar
-      cache.AddProfileToCache(profile_path, name, std::string(),
-                              base::string16(), icon_index, supervised_user_id);
+      GetProfileAttributesStorage().AddProfile(profile_path, name,
+          std::string(), base::string16(), icon_index, supervised_user_id);
     }
 
     if (!supervised_user_id.empty()) {
       content::RecordAction(
           UserMetricsAction("ManagedMode_LocallyManagedUserCreated"));
     }
 
     ProfileMetrics::UpdateReportedProfilesStatistics(this);
   }
 
@@ skipping 226 matching lines @@
   // Cancel all in-progress downloads before deleting the profile to prevent a
   // "Do you want to exit Google Chrome and cancel the downloads?" prompt
   // (crbug.com/336725).
   Profile* profile = GetProfileByPath(profile_dir);
   if (profile) {
     DownloadService* service =
         DownloadServiceFactory::GetForBrowserContext(profile);
     service->CancelDownloads();
   }
 
-  ProfileInfoCache& cache = GetProfileInfoCache();
-
+  ProfileAttributesStorage& storage = GetProfileAttributesStorage();
   // If we're deleting the last (non-legacy-supervised) profile, then create a
   // new profile in its place.
   base::FilePath last_non_supervised_profile_path;
-  for (size_t i = 0; i < cache.GetNumberOfProfiles(); ++i) {
-    base::FilePath cur_path = cache.GetPathOfProfileAtIndex(i);
+  std::vector<ProfileAttributesEntry*> entries =
+      storage.GetAllProfilesAttributes();
+  for (ProfileAttributesEntry* entry : entries) {

[Mike Lerman, 2016/03/18 17:10:44]

why assign storage.GetAllProfilesAttribures() to a temporary variable, why not
iterate over it directly?
for (ProfileAttributesEntry* entry : storage.GetAllProfilesAttributes())

[lwchkg, 2016/03/19 18:48:46]

Did think of this, because in other places the code is
g_browser_process->GetProfileManager()->GetProfileAttributesStorage().
    GetAllProfilesAttributes();
which is too long to be included in the for loop.

Sounds good anyway.

[Mike Lerman, 2016/03/30 01:18:10]

And really, I'm sure the compiler optimizes this anyways.
I'm fine with either one.

[lwchkg, 2016/03/31 19:32:26]

Stupid typo. Should be "Didn't think of this" (i.e. the opposite) instead.

+    base::FilePath cur_path = entry->GetPath();
     // Make sure that this profile is not pending deletion, and is not
     // legacy-supervised.
     if (cur_path != profile_dir &&
-        !cache.ProfileIsLegacySupervisedAtIndex(i) &&
+        !entry->IsLegacySupervised() &&
         !IsProfileMarkedForDeletion(cur_path)) {
       last_non_supervised_profile_path = cur_path;
       break;
     }
   }
 
   if (last_non_supervised_profile_path.empty()) {
     std::string new_avatar_url;
     base::string16 new_profile_name;
 
 #if !defined(OS_CHROMEOS) && !defined(OS_ANDROID)
     int avatar_index = profiles::GetPlaceholderAvatarIndex();
     new_avatar_url = profiles::GetDefaultAvatarIconUrl(avatar_index);
-    new_profile_name = cache.ChooseNameForNewProfile(avatar_index);
+    new_profile_name = storage.ChooseNameForNewProfile(avatar_index);
 #endif
 
     base::FilePath new_path(GenerateNextProfileDirectoryPath());
     CreateProfileAsync(new_path,
                        base::Bind(&ProfileManager::OnNewActiveProfileLoaded,
                                   base::Unretained(this),
                                   profile_dir,
                                   new_path,
                                   callback),
                        new_profile_name,
@@ skipping 33 matching lines @@
 void ProfileManager::AutoloadProfiles() {
   // If running in the background is disabled for the browser, do not autoload
   // any profiles.
   PrefService* local_state = g_browser_process->local_state();
   DCHECK(local_state);
   if (!local_state->HasPrefPath(prefs::kBackgroundModeEnabled) ||
       !local_state->GetBoolean(prefs::kBackgroundModeEnabled)) {
     return;
   }
 
-  ProfileInfoCache& cache = GetProfileInfoCache();
-  size_t number_of_profiles = cache.GetNumberOfProfiles();
-  for (size_t p = 0; p < number_of_profiles; ++p) {
-    if (cache.GetBackgroundStatusOfProfileAtIndex(p)) {
+  std::vector<ProfileAttributesEntry*> entries =
+      GetProfileAttributesStorage().GetAllProfilesAttributes();
+  for (ProfileAttributesEntry* entry : entries) {
+    if (entry->GetBackgroundStatus()) {
       // If status is true, that profile is running background apps. By calling
       // GetProfile, we automatically cause the profile to be loaded which will
       // register it with the BackgroundModeManager.
-      GetProfile(cache.GetPathOfProfileAtIndex(p));
+      GetProfile(entry->GetPath());
     }
   }
 }
 
 void ProfileManager::CleanUpEphemeralProfiles() {
   const std::string last_used_profile = GetLastUsedProfileName();
 
   bool last_active_profile_deleted = false;
   base::FilePath new_profile_path;
   std::vector<base::FilePath> profiles_to_delete;
-  ProfileInfoCache& profile_cache = GetProfileInfoCache();
-  size_t profiles_count = profile_cache.GetNumberOfProfiles();
-  for (size_t i = 0; i < profiles_count; ++i) {
-    base::FilePath profile_path = profile_cache.GetPathOfProfileAtIndex(i);
-    if (profile_cache.ProfileIsEphemeralAtIndex(i)) {
+  ProfileAttributesStorage& storage = GetProfileAttributesStorage();
+  std::vector<ProfileAttributesEntry*> entries =
+      storage.GetAllProfilesAttributes();
+  for (ProfileAttributesEntry* entry : entries) {
+    base::FilePath profile_path = entry->GetPath();
+    if (entry->IsEphemeral()) {
       profiles_to_delete.push_back(profile_path);
       if (profile_path.BaseName().MaybeAsASCII() == last_used_profile)
         last_active_profile_deleted = true;
     } else if (new_profile_path.empty()) {
       new_profile_path = profile_path;
     }
   }
 
   // If the last active profile was ephemeral, set a new one.
   if (last_active_profile_deleted) {
     if (new_profile_path.empty())
       new_profile_path = GenerateNextProfileDirectoryPath();
 
     profiles::SetLastUsedProfile(new_profile_path.BaseName().MaybeAsASCII());
   }
 
   // This uses a separate loop, because deleting the profile from the
   // ProfileInfoCache will modify indices.
   for (const base::FilePath& profile_path : profiles_to_delete) {
     BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE,
                             base::Bind(&NukeProfileFromDisk, profile_path));
 
-    profile_cache.DeleteProfileFromCache(profile_path);
+    storage.RemoveProfile(profile_path);
   }
 }
 
 void ProfileManager::InitProfileUserPrefs(Profile* profile) {
   TRACE_EVENT0("browser", "ProfileManager::InitProfileUserPrefs");
-  ProfileInfoCache& cache = GetProfileInfoCache();
+  ProfileAttributesStorage& storage = GetProfileAttributesStorage();
 
-  if (profile->GetPath().DirName() != cache.GetUserDataDir())
+  if (profile->GetPath().DirName() != user_data_dir())

[lwchkg, 2016/03/14 15:49:10]

I want to change this code (and a few similar code) into DCHECK, but I'm not
sure if this is 100% safe.

Anyway, there is no discussion about this line in the related code review:
https://codereview.chromium.org/8771024/

[Mike Lerman, 2016/03/18 17:10:44]

Maybe this helps with testing?
It seems safe to leave as is.

[lwchkg, 2016/03/19 18:48:46]

Not sure, but very possible. I guess it *helped* testing. Too bad the author
didn't add a comment for us.

If we want to know, we can try adding LOG(WARNING) and run all tests.
Agree. And despite my wants to change, leaving the code as is is the only safe
option unless we have more evidence. (And this won't happen soon.)

[Mike Lerman, 2016/03/30 01:18:10]

We can add a UserCount and then a Googler can go and check if we ever actually
hit this case.

[lwchkg, 2016/03/31 19:32:26]

Sorry... what is a UserCount?

[Mike Lerman, 2016/04/04 15:45:31]

It's a way to gather statistics from the Chrome userbase (for those who opt in).
The way to do so is with a call to UMA_HISTOGRAM_BOOLEAN(). You can set up a new
histogram in histograms.xml. You can verify locally by looking at
chrome://histograms and seeing what Chrome's taking note of (note: it always
tracks locally whether you've opted in or not, and uploads to Google only if
you've opted in)

     return;
 
   size_t avatar_index;
   std::string profile_name;
   std::string supervised_user_id;
   if (profile->IsGuestSession()) {
     profile_name = l10n_util::GetStringUTF8(IDS_PROFILES_GUEST_PROFILE_NAME);
     avatar_index = 0;
   } else {
-    size_t profile_cache_index =
-        cache.GetIndexOfProfileWithPath(profile->GetPath());
-    // If the cache has an entry for this profile, use the cache data.
-    if (profile_cache_index != std::string::npos) {
-      avatar_index =
-          cache.GetAvatarIconIndexOfProfileAtIndex(profile_cache_index);
-      profile_name =
-          base::UTF16ToUTF8(cache.GetNameOfProfileAtIndex(profile_cache_index));
-      supervised_user_id =
-          cache.GetSupervisedUserIdOfProfileAtIndex(profile_cache_index);
+    ProfileAttributesEntry* entry;
+    bool has_entry = storage.GetProfileAttributesWithPath(profile->GetPath(),
+                                                          &entry);
+    // If the profile attributes storage has an entry for this profile, use the
+    // data in the profile attributes storage.
+    if (has_entry) {
+      avatar_index = entry->GetAvatarIconIndex();
+      profile_name = base::UTF16ToUTF8(entry->GetName());
+      supervised_user_id = entry->GetSupervisedUserId();
     } else if (profile->GetPath() ==
-               profiles::GetDefaultProfileDir(cache.GetUserDataDir())) {
+                   profiles::GetDefaultProfileDir(user_data_dir())) {
       avatar_index = profiles::GetPlaceholderAvatarIndex();
 #if !defined(OS_CHROMEOS) && !defined(OS_ANDROID)
       profile_name =
-          base::UTF16ToUTF8(cache.ChooseNameForNewProfile(avatar_index));
+          base::UTF16ToUTF8(storage.ChooseNameForNewProfile(avatar_index));
 #else
       profile_name = l10n_util::GetStringUTF8(IDS_DEFAULT_PROFILE_NAME);
 #endif
     } else {
-      avatar_index = cache.ChooseAvatarIconIndexForNewProfile();
+      avatar_index = storage.ChooseAvatarIconIndexForNewProfile();
       profile_name =
-          base::UTF16ToUTF8(cache.ChooseNameForNewProfile(avatar_index));
+          base::UTF16ToUTF8(storage.ChooseNameForNewProfile(avatar_index));
     }
   }
 
   if (!profile->GetPrefs()->HasPrefPath(prefs::kProfileAvatarIndex))
     profile->GetPrefs()->SetInteger(prefs::kProfileAvatarIndex, avatar_index);
 
   if (!profile->GetPrefs()->HasPrefPath(prefs::kProfileName))
     profile->GetPrefs()->SetString(prefs::kProfileName, profile_name);
 
   base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
   bool force_supervised_user_id =
 #if defined(OS_CHROMEOS)
       g_browser_process->platform_part()
               ->profile_helper()
               ->GetSigninProfileDir() != profile->GetPath() &&
 #endif
       command_line->HasSwitch(switches::kSupervisedUserId);
   if (force_supervised_user_id) {
     supervised_user_id =
         command_line->GetSwitchValueASCII(switches::kSupervisedUserId);
   }
   if (force_supervised_user_id ||
       !profile->GetPrefs()->HasPrefPath(prefs::kSupervisedUserId)) {
     profile->GetPrefs()->SetString(prefs::kSupervisedUserId,
                                    supervised_user_id);
   }
 }
 
 void ProfileManager::RegisterTestingProfile(Profile* profile,
-                                            bool add_to_cache,
+                                            bool add_to_storage,
                                             bool start_deferred_task_runners) {
   RegisterProfile(profile, true);
-  if (add_to_cache) {
+  if (add_to_storage) {
     InitProfileUserPrefs(profile);
-    AddProfileToCache(profile);
+    AddProfileToStorage(profile);
   }
   if (start_deferred_task_runners) {
     StartupTaskRunnerServiceFactory::GetForProfile(profile)->
         StartDeferredTaskRunners();
   }
 }
 
 void ProfileManager::Observe(
     int type,
     const content::NotificationSource& source,
@@ skipping 151 matching lines @@
   RunCallbacks(callbacks, profile,
                profile ? Profile::CREATE_STATUS_INITIALIZED :
                          Profile::CREATE_STATUS_LOCAL_FAIL);
 }
 
 void ProfileManager::DoFinalInit(Profile* profile, bool go_off_the_record) {
   TRACE_EVENT0("browser", "ProfileManager::DoFinalInit");
   TRACK_SCOPED_REGION("Startup", "ProfileManager::DoFinalInit");
 
   DoFinalInitForServices(profile, go_off_the_record);
-  AddProfileToCache(profile);
+  AddProfileToStorage(profile);
   DoFinalInitLogging(profile);
 
   ProfileMetrics::LogNumberOfProfiles(this);
   content::NotificationService::current()->Notify(
       chrome::NOTIFICATION_PROFILE_ADDED,
       content::Source<Profile>(profile),
       content::NotificationService::NoDetails());
 
-  // Record statistics to ProfileInfoCache if statistics were not recorded
-  // during shutdown, i.e. the last shutdown was a system shutdown or a crash.
-  if (!profile->IsGuestSession() && !profile->IsSystemProfile() &&
+  // Record statistics to ProfileAttributesStorage if statistics were not
+  // recorded during shutdown, i.e. the last shutdown was a system shutdown or a
+  // crash.
+    if (!profile->IsGuestSession() && !profile->IsSystemProfile() &&
       !profile->IsNewProfile() && !go_off_the_record &&
       profile->GetLastSessionExitType() != Profile::EXIT_NORMAL) {
     profiles::GatherProfileStatistics(
         profile, profiles::ProfileStatisticsCallback(), nullptr);
   }
 }
 
 void ProfileManager::DoFinalInitForServices(Profile* profile,
                                             bool go_off_the_record) {
   TRACE_EVENT0("browser", "ProfileManager::DoFinalInitForServices");
   TRACK_SCOPED_REGION("Startup", "ProfileManager::DoFinalInitForServices");
 
 #if defined(ENABLE_EXTENSIONS)
-  ProfileInfoCache& cache = GetProfileInfoCache();
-
   // Ensure that the HostContentSettingsMap has been created before the
   // ExtensionSystem is initialized otherwise the ExtensionSystem will be
   // registered twice
   HostContentSettingsMap* content_settings_map =
     HostContentSettingsMapFactory::GetForProfile(profile);
 
   extensions::ExtensionSystem::Get(profile)->InitForRegularProfile(
       !go_off_the_record);
   // During tests, when |profile| is an instance of TestingProfile,
   // ExtensionSystem might not create an ExtensionService.
   // This block is duplicated in the HostContentSettingsMapFactory
   // ::BuildServiceInstanceFor method, it should be called once when both the
   // HostContentSettingsMap and the extension_service are set up.
   if (extensions::ExtensionSystem::Get(profile)->extension_service()) {
     extensions::ExtensionSystem::Get(profile)->extension_service()->
         RegisterContentSettings(content_settings_map);
   }
   // Set the block extensions bit on the ExtensionService. There likely are no
   // blockable extensions to block.
-  size_t profile_index = cache.GetIndexOfProfileWithPath(profile->GetPath());
-  if (profile_index != std::string::npos &&
-      cache.ProfileIsSigninRequiredAtIndex(profile_index)) {
+  ProfileAttributesEntry* entry;
+  bool has_entry = GetProfileAttributesStorage().
+                       GetProfileAttributesWithPath(profile->GetPath(), &entry);
+  if (has_entry && entry->IsSigninRequired()) {
     extensions::ExtensionSystem::Get(profile)
         ->extension_service()
         ->BlockAllExtensions();
   }
 
 #endif
 #if defined(ENABLE_SUPERVISED_USERS)
   // Initialization needs to happen after extension system initialization (for
   // extension::ManagementPolicy) and InitProfileUserPrefs (for setting the
   // initializing the supervised flag if necessary).
@@ skipping 156 matching lines @@
 
 #if !defined(OS_ANDROID)
 void ProfileManager::FinishDeletingProfile(
     const base::FilePath& profile_dir,
     const base::FilePath& new_active_profile_dir) {
   // Update the last used profile pref before closing browser windows. This
   // way the correct last used profile is set for any notification observers.
   profiles::SetLastUsedProfile(
       new_active_profile_dir.BaseName().MaybeAsASCII());
 
-  ProfileInfoCache& cache = GetProfileInfoCache();
+  ProfileAttributesStorage& storage = GetProfileAttributesStorage();
   // TODO(sail): Due to bug 88586 we don't delete the profile instance. Once we
   // start deleting the profile instance we need to close background apps too.
   Profile* profile = GetProfileByPath(profile_dir);
 
   if (profile) {
     // TODO: Migrate additional code in this block to observe this notification
     // instead of being implemented here.
     content::NotificationService::current()->Notify(
         chrome::NOTIFICATION_PROFILE_DESTRUCTION_STARTED,
         content::Source<Profile>(profile),
         content::NotificationService::NoDetails());
 
     // By this point, all in-progress downloads for the profile being deleted
     // must have been canceled (crbug.com/336725).
     DCHECK(DownloadServiceFactory::GetForBrowserContext(profile)->
-           NonMaliciousDownloadCount() == 0);
+               NonMaliciousDownloadCount() == 0);
     BrowserList::CloseAllBrowsersWithProfile(profile);
 
     // Disable sync for doomed profile.
     if (ProfileSyncServiceFactory::GetInstance()->HasProfileSyncService(
-        profile)) {
+            profile)) {
       ProfileSyncService* sync_service =
           ProfileSyncServiceFactory::GetInstance()->GetForProfile(profile);
       if (sync_service->IsSyncRequested()) {
         // Record sync stopped by profile destruction if it was on before.
         UMA_HISTOGRAM_ENUMERATION("Sync.StopSource",
                                   syncer::PROFILE_DESTRUCTION,
                                   syncer::STOP_SOURCE_LIMIT);
       }
       // Ensure data is cleared even if sync was already off.
       ProfileSyncServiceFactory::GetInstance()->GetForProfile(
           profile)->RequestStop(ProfileSyncService::CLEAR_DATA);
     }
 
-    ProfileMetrics::LogProfileDelete(cache.ProfileIsAuthenticatedAtIndex(
-        cache.GetIndexOfProfileWithPath(profile_dir)));
+    ProfileAttributesEntry* entry;
+    bool has_entry = storage.GetProfileAttributesWithPath(profile_dir, &entry);
+    DCHECK(has_entry);
+    ProfileMetrics::LogProfileDelete(entry->IsAuthenticated());
     // Some platforms store passwords in keychains. They should be removed.
     scoped_refptr<password_manager::PasswordStore> password_store =
         PasswordStoreFactory::GetForProfile(
             profile, ServiceAccessType::EXPLICIT_ACCESS).get();
     if (password_store.get()) {
       password_store->RemoveLoginsCreatedBetween(
           base::Time(), base::Time::Max(), base::Closure());
     }
 
     // The Profile Data doesn't get wiped until Chrome closes. Since we promised
     // that the user's data would be removed, do so immediately.
     profiles::RemoveBrowsingDataForProfile(profile_dir);
   } else {
     // It is safe to delete a not yet loaded Profile from disk.
     BrowserThread::PostTask(
         BrowserThread::FILE, FROM_HERE,
         base::Bind(&NukeProfileFromDisk, profile_dir));
   }
 
   // Queue even a profile that was nuked so it will be MarkedForDeletion and so
   // CreateProfileAsync can't create it.
   QueueProfileDirectoryForDeletion(profile_dir);
-  cache.DeleteProfileFromCache(profile_dir);
+  storage.RemoveProfile(profile_dir);
   ProfileMetrics::UpdateReportedProfilesStatistics(this);
 }
 #endif  // !defined(OS_ANDROID)
 
 ProfileManager::ProfileInfo* ProfileManager::RegisterProfile(
     Profile* profile,
     bool created) {
   TRACE_EVENT0("browser", "ProfileManager::RegisterProfile");
   ProfileInfo* info = new ProfileInfo(profile, created);
   profiles_info_.insert(
       std::make_pair(profile->GetPath(), linked_ptr<ProfileInfo>(info)));
   return info;
 }
 
 ProfileManager::ProfileInfo* ProfileManager::GetProfileInfoByPath(
     const base::FilePath& path) const {
   ProfilesInfoMap::const_iterator iter = profiles_info_.find(path);
   return (iter == profiles_info_.end()) ? NULL : iter->second.get();
 }
 
-void ProfileManager::AddProfileToCache(Profile* profile) {
+void ProfileManager::AddProfileToStorage(Profile* profile) {
   TRACE_EVENT0("browser", "ProfileManager::AddProfileToCache");
   if (profile->IsGuestSession() || profile->IsSystemProfile())
     return;
-  ProfileInfoCache& cache = GetProfileInfoCache();
-  if (profile->GetPath().DirName() != cache.GetUserDataDir())
+  if (profile->GetPath().DirName() != user_data_dir())
     return;
 
   SigninManagerBase* signin_manager =
       SigninManagerFactory::GetForProfile(profile);
   AccountTrackerService* account_tracker =
       AccountTrackerServiceFactory::GetForProfile(profile);
   AccountInfo account_info = account_tracker->GetAccountInfo(
       signin_manager->GetAuthenticatedAccountId());
   base::string16 username = base::UTF8ToUTF16(account_info.email);
 
-  size_t profile_index = cache.GetIndexOfProfileWithPath(profile->GetPath());
-  if (profile_index != std::string::npos) {
-    // The ProfileInfoCache's info must match the Signin Manager.
-    cache.SetAuthInfoOfProfileAtIndex(profile_index, account_info.gaia,
-                                      username);
-    return;
+  ProfileAttributesStorage& storage = GetProfileAttributesStorage();
+  {

[Mike Lerman, 2016/03/18 17:10:44]

you left a set of parens here

[lwchkg, 2016/03/19 18:48:46]

Actually I deliberately added the braces for scoping. I feel uncomfortable to
let entry and has_entry to survive the full function, because another set of
values is assigned to them in line 1409.

WDYT?

[Mike Lerman, 2016/03/30 01:18:10]

Sure, leave a comment for that please.

[lwchkg, 2016/03/31 19:32:26]

Acknowledged.

+    ProfileAttributesEntry* entry;
+    bool has_entry = storage.GetProfileAttributesWithPath(profile->GetPath(),
+                                                          &entry);
+    if (has_entry) {
+      // The ProfileAttributesStorage's info must match the Signin Manager.
+      entry->SetAuthInfo(account_info.gaia, username);
+      return;
+    }
   }
 
   // Profile name and avatar are set by InitProfileUserPrefs and stored in the
-  // profile. Use those values to setup the cache entry.
+  // profile. Use those values to setup the entry in profile attributes storage.
   base::string16 profile_name =
       base::UTF8ToUTF16(profile->GetPrefs()->GetString(prefs::kProfileName));
 
-  size_t icon_index = profile->GetPrefs()->GetInteger(
-      prefs::kProfileAvatarIndex);
+  size_t icon_index =
+      profile->GetPrefs()->GetInteger(prefs::kProfileAvatarIndex);
 
   std::string supervised_user_id =
       profile->GetPrefs()->GetString(prefs::kSupervisedUserId);
 
-  cache.AddProfileToCache(profile->GetPath(),
-                          profile_name,
-                          account_info.gaia,
-                          username,
-                          icon_index,
-                          supervised_user_id);
+  storage.AddProfile(profile->GetPath(), profile_name, account_info.gaia,
+                     username, icon_index, supervised_user_id);
 
   if (profile->GetPrefs()->GetBoolean(prefs::kForceEphemeralProfiles)) {
-    cache.SetProfileIsEphemeralAtIndex(
-        cache.GetIndexOfProfileWithPath(profile->GetPath()), true);
+    ProfileAttributesEntry* entry;
+    bool has_entry = storage.GetProfileAttributesWithPath(profile->GetPath(),
+                                                          &entry);
+    DCHECK(has_entry);
+    entry->SetIsEphemeral(true);
   }
 }
 
 void ProfileManager::SetNonPersonalProfilePrefs(Profile* profile) {
   PrefService* prefs = profile->GetPrefs();
   prefs->SetBoolean(prefs::kSigninAllowed, false);
   prefs->SetBoolean(bookmarks::prefs::kEditBookmarksEnabled, false);
   prefs->SetBoolean(bookmarks::prefs::kShowBookmarkBar, false);
   prefs->ClearPref(DefaultSearchManager::kDefaultSearchProviderDataPrefName);
 }
@@ skipping 31 matching lines @@
   DCHECK(local_state);
   // Only keep track of profiles that we are managing; tests may create others.
   // Also never consider the SystemProfile as "active".
   if (profiles_info_.find(last_active->GetPath()) != profiles_info_.end() &&
       !last_active->IsSystemProfile()) {
     std::string profile_path_base =
         last_active->GetPath().BaseName().MaybeAsASCII();
     if (profile_path_base != GetLastUsedProfileName())
       profiles::SetLastUsedProfile(profile_path_base);
 
-    ProfileInfoCache& cache = GetProfileInfoCache();
-    size_t profile_index =
-        cache.GetIndexOfProfileWithPath(last_active->GetPath());
-    if (profile_index != std::string::npos) {
+    ProfileAttributesEntry* entry;
+    if (GetProfileAttributesStorage().
+            GetProfileAttributesWithPath(last_active->GetPath(), &entry)) {
 #if !defined(OS_CHROMEOS)
       // Incognito Profiles don't have ProfileKeyedServices.
       if (!last_active->IsOffTheRecord()) {
-        CrossDevicePromoFactory::GetForProfile(last_active)
-            ->MaybeBrowsingSessionStarted(
-                cache.GetProfileActiveTimeAtIndex(profile_index));
+        CrossDevicePromoFactory::GetForProfile(last_active)->
+            MaybeBrowsingSessionStarted(entry->GetActiveTime());
       }
 #endif
-      cache.SetProfileActiveTimeAtIndex(profile_index);
+      entry->SetActiveTimeToNow();
     }
   }
 }
 
 ProfileManager::BrowserListObserver::BrowserListObserver(
     ProfileManager* manager)
     : profile_manager_(manager) {
   BrowserList::AddObserver(this);
 }
 
@@ skipping 15 matching lines @@
   }
 
   base::FilePath path = profile->GetPath();
   if (IsProfileMarkedForDeletion(path)) {
     // Do nothing if the profile is already being deleted.
   } else if (profile->GetPrefs()->GetBoolean(prefs::kForceEphemeralProfiles)) {
     // Delete if the profile is an ephemeral profile.
     g_browser_process->profile_manager()->ScheduleProfileForDeletion(
         path, ProfileManager::CreateCallback());
   } else if (!profile->IsSystemProfile()) {
-    // Gather statistics and store into ProfileInfoCache. For incognito profile
-    // we gather the statistics of its parent profile instead, because a window
-    // of the parent profile was open.
+    // Gather statistics and store into ProfileAttributesStorage. For incognito
+    // profile we gather the statistics of its parent profile instead, because a
+    // window of the parent profile was open.
     profiles::GatherProfileStatistics(
         original_profile, profiles::ProfileStatisticsCallback(), nullptr);
   }
 }
 
 void ProfileManager::BrowserListObserver::OnBrowserSetLastActive(
     Browser* browser) {
   // If all browsers are being closed (e.g. the user is in the process of
   // shutting down), this event will be fired after each browser is
   // closed. This does not represent a user intention to change the active
@@ skipping 34 matching lines @@
 
   FinishDeletingProfile(profile_to_delete_path, new_active_profile_path);
   if (!original_callback.is_null())
     original_callback.Run(loaded_profile, status);
 }
 #endif  // !defined(OS_ANDROID)
 
 ProfileManagerWithoutInit::ProfileManagerWithoutInit(
     const base::FilePath& user_data_dir) : ProfileManager(user_data_dir) {
 }