Don't use path expansion for profile dictionaries in LocalState and

ProfileInfoCache.

Otherwise if the usernames are only being hashed by appending "-hash", extra
dictionaries are created and written out to the LocalState file. In particular,
the ProfileInfoCache stores some extraneous information.

This change should have no impact when running on real machines.

BUG=289772

Review URL: https://chromiumcodereview.appspot.com/23528008

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

5 files changed, 71 insertions, 19 deletions

diff --git a/chrome/browser/prefs/pref_metrics_service.cc b/chrome/browser/prefs/pref_metrics_service.cc
index 4cf8e60..82af366 100644
--- a/chrome/browser/prefs/pref_metrics_service.cc
+++ b/chrome/browser/prefs/pref_metrics_service.cc
@@ -104,6 +104,7 @@ PrefMetricsService::PrefMetricsService(Profile* profile,
     : profile_(profile),
       prefs_(profile->GetPrefs()),
       local_state_(local_state),
+      profile_name_(profile_->GetPath().AsUTF8Unsafe()),
       pref_hash_seed_(kSHA256DigestSize, 0),
       device_id_(device_id),
       tracked_pref_paths_(tracked_pref_paths),
@@ -284,7 +285,8 @@ void PrefMetricsService::CheckTrackedPreferences() {
   // Get the hashed prefs dictionary if it exists. If it doesn't, it will be
   // created if we set preference values below.
   const base::DictionaryValue* hashed_prefs = NULL;
-  pref_hash_dicts->GetDictionary(profile_name_, &hashed_prefs);
+  pref_hash_dicts->GetDictionaryWithoutPathExpansion(profile_name_,
+                                                     &hashed_prefs);
   for (int i = 0; i < tracked_pref_path_count_; ++i) {
     // Skip prefs that haven't been registered.
     if (!prefs_->FindPreference(tracked_pref_paths_[i]))
@@ -351,21 +353,28 @@ void PrefMetricsService::UpdateTrackedPreference(const char* path) {
     RemoveTrackedPreference(path);
   } else {
     DictionaryPrefUpdate update(local_state_, prefs::kProfilePreferenceHashes);
-    update->SetString(GetHashedPrefPath(path),
-                      GetHashedPrefValue(path, value));
+    DictionaryValue* child_dictionary = NULL;
+
+    // Get the dictionary corresponding to the profile name,
+    // which may have a '.'
+    if (!update->GetDictionaryWithoutPathExpansion(profile_name_,
+                                                   &child_dictionary)) {
+      child_dictionary = new DictionaryValue;
+      update->SetWithoutPathExpansion(profile_name_, child_dictionary);
+    }
+    child_dictionary->SetString(path, GetHashedPrefValue(path, value));
   }
 }
 
 bool PrefMetricsService::RemoveTrackedPreference(const char* path) {
   DictionaryPrefUpdate update(local_state_, prefs::kProfilePreferenceHashes);
-  return update->Remove(GetHashedPrefPath(path), NULL);
-}
+  DictionaryValue* child_dictionary = NULL;
 
-std::string PrefMetricsService::GetHashedPrefPath(const char* path) {
-  std::string hash_pref_path(profile_name_);
-  hash_pref_path.append(".");
-  hash_pref_path.append(path);
-  return hash_pref_path;
+  if (!update->GetDictionaryWithoutPathExpansion(profile_name_,
+                                                 &child_dictionary)) {
+    return false;
+  }
+  return child_dictionary->Remove(path, NULL);
 }
 
 std::string PrefMetricsService::GetHashedPrefValue(
diff --git a/chrome/browser/prefs/pref_metrics_service.h b/chrome/browser/prefs/pref_metrics_service.h
index b2657c1..0badbf3 100644
--- a/chrome/browser/prefs/pref_metrics_service.h
+++ b/chrome/browser/prefs/pref_metrics_service.h
@@ -108,9 +108,6 @@ class PrefMetricsService : public BrowserContextKeyedService {
   // value was present.
   bool RemoveTrackedPreference(const char* path);
 
-  // Gets the path to the preference value hash in local state.
-  std::string GetHashedPrefPath(const char* path);
-
   // Computes an MD5 hash for the given preference value.
   std::string GetHashedPrefValue(const char* path, const base::Value* value);
 
diff --git a/chrome/browser/prefs/pref_metrics_service_unittest.cc b/chrome/browser/prefs/pref_metrics_service_unittest.cc
index 9e05c49..df8fd5d 100644
--- a/chrome/browser/prefs/pref_metrics_service_unittest.cc
+++ b/chrome/browser/prefs/pref_metrics_service_unittest.cc
@@ -9,8 +9,10 @@
 #include "base/values.h"
 #include "chrome/browser/prefs/pref_metrics_service.h"
 #include "chrome/browser/prefs/scoped_user_pref_update.h"
+#include "chrome/test/base/testing_browser_process.h"
 #include "chrome/test/base/testing_pref_service_syncable.h"
 #include "chrome/test/base/testing_profile.h"
+#include "chrome/test/base/testing_profile_manager.h"
 #include "components/user_prefs/pref_registry_syncable.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -41,7 +43,15 @@ class PrefMetricsServiceTest : public testing::Test {
 
     base::StatisticsRecorder::Initialize();
 
-    prefs_ = profile_.GetTestingPrefService();
+    // Reset and set up the profile manager.
+    profile_manager_.reset(new TestingProfileManager(
+        TestingBrowserProcess::GetGlobal()));
+    ASSERT_TRUE(profile_manager_->SetUp());
+
+    // Check that PrefMetricsService behaves with a '.' in the profile name.
+    profile_ = profile_manager_->CreateTestingProfile("test@example.com");
+
+    prefs_ = profile_->GetTestingPrefService();
 
     // Register our test-only tracked prefs as string values.
     for (int i = 0; i < kTrackedPrefCount; ++i) {
@@ -60,7 +70,7 @@ class PrefMetricsServiceTest : public testing::Test {
 
   scoped_ptr<PrefMetricsService> CreatePrefMetricsService() {
     return scoped_ptr<PrefMetricsService>(
-        new PrefMetricsService(&profile_,
+        new PrefMetricsService(profile_,
                                &local_state_,
                                "test_device_id",
                                kTrackedPrefs,
@@ -115,7 +125,8 @@ class PrefMetricsServiceTest : public testing::Test {
     pref2_unchanged_total = unchanged2;
   }
 
-  TestingProfile profile_;
+  TestingProfile* profile_;
+  scoped_ptr<TestingProfileManager> profile_manager_;
   TestingPrefServiceSyncable* prefs_;
   TestingPrefServiceSimple local_state_;
 
diff --git a/chrome/browser/profiles/profile_info_cache.cc b/chrome/browser/profiles/profile_info_cache.cc
index 3d6fe8e..1f38167 100644
--- a/chrome/browser/profiles/profile_info_cache.cc
+++ b/chrome/browser/profiles/profile_info_cache.cc
@@ -216,7 +216,7 @@ void ProfileInfoCache::AddProfileToCache(const base::FilePath& profile_path,
   // Default value for whether background apps are running is false.
   info->SetBoolean(kBackgroundAppsKey, false);
   info->SetString(kManagedUserId, managed_user_id);
-  cache->Set(key, info.release());
+  cache->SetWithoutPathExpansion(key, info.release());
 
   sorted_keys_.insert(FindPositionForProfile(key, name), key);
 
@@ -807,7 +807,7 @@ const DictionaryValue* ProfileInfoCache::GetInfoForProfileAtIndex(
   const DictionaryValue* cache =
       prefs_->GetDictionary(prefs::kProfileInfoCache);
   const DictionaryValue* info = NULL;
-  cache->GetDictionary(sorted_keys_[index], &info);
+  cache->GetDictionaryWithoutPathExpansion(sorted_keys_[index], &info);
   return info;
 }
 
@@ -815,7 +815,7 @@ void ProfileInfoCache::SetInfoForProfileAtIndex(size_t index,
                                                 DictionaryValue* info) {
   DictionaryPrefUpdate update(prefs_, prefs::kProfileInfoCache);
   DictionaryValue* cache = update.Get();
-  cache->Set(sorted_keys_[index], info);
+  cache->SetWithoutPathExpansion(sorted_keys_[index], info);
 
   content::NotificationService::current()->Notify(
       chrome::NOTIFICATION_PROFILE_CACHED_INFO_CHANGED,
diff --git a/chrome/browser/profiles/profile_info_cache_unittest.cc b/chrome/browser/profiles/profile_info_cache_unittest.cc
index 51e1fd45..1d5e6df 100644
--- a/chrome/browser/profiles/profile_info_cache_unittest.cc
+++ b/chrome/browser/profiles/profile_info_cache_unittest.cc
@@ -4,6 +4,8 @@
 
 #include "chrome/browser/profiles/profile_info_cache_unittest.h"
 
+#include <vector>
+
 #include "base/prefs/testing_pref_service.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
@@ -459,4 +461,37 @@ TEST_F(ProfileInfoCacheTest, CreateManagedTestingProfile) {
   }
 }
 
+TEST_F(ProfileInfoCacheTest, AddStubProfile) {
+  EXPECT_EQ(0u, GetCache()->GetNumberOfProfiles());
+
+  // Add some profiles with and without a '.' in their paths.
+  const struct {
+    const char* profile_path;
+    const char* profile_name;
+  } kTestCases[] = {
+    { "path.test0", "name_0" },
+    { "path_test1", "name_1" },
+    { "path.test2", "name_2" },
+    { "path_test3", "name_3" },
+  };
+
+  for (size_t i = 0; i < ARRAYSIZE_UNSAFE(kTestCases); ++i) {
+    base::FilePath profile_path = GetProfilePath(kTestCases[i].profile_path);
+    string16 profile_name = ASCIIToUTF16(kTestCases[i].profile_name);
+
+    GetCache()->AddProfileToCache(profile_path, profile_name, string16(), i,
+                                  "");
+
+    EXPECT_EQ(profile_path, GetCache()->GetPathOfProfileAtIndex(i));
+    EXPECT_EQ(profile_name, GetCache()->GetNameOfProfileAtIndex(i));
+  }
+
+  ASSERT_EQ(4U, GetCache()->GetNumberOfProfiles());
+
+  // Check that the profiles can be extracted from the local state.
+  std::vector<string16> names = ProfileInfoCache::GetProfileNames();
+  for (size_t i = 0; i < 4; i++)
+    ASSERT_FALSE(names[i].empty());
+}
+
 }  // namespace