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

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(

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);
 

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_;
 

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,

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