Don't use path expansion for profile dictionaries in LocalState

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
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=223448

[michaelpg, 2013-09-11 23:46:17]

Fix for extra profiles in profile cache, would like to get committed before CrOS
multiprofile avatar menu changes.

[battre, 2013-09-12 08:38:46]

LGTM if you add unit tests (they can be reviewed by somebody else) (no need for
another OWNER's approval from me).

https://codereview.chromium.org/23528008/diff/1/chrome/browser/prefs/pref_met...
File chrome/browser/prefs/pref_metrics_service.cc (right):

https://codereview.chromium.org/23528008/diff/1/chrome/browser/prefs/pref_met...
chrome/browser/prefs/pref_metrics_service.cc:287:
pref_hash_dicts->GetDictionaryWithoutPathExpansion(profile_name_,
This is pretty error prone. Can you please update the unit tests to use a
directory that contains a period?

[xiyuan, 2013-09-12 16:18:37]

lgtm

https://codereview.chromium.org/23528008/diff/1/chrome/browser/prefs/pref_met...
File chrome/browser/prefs/pref_metrics_service.cc (right):

https://codereview.chromium.org/23528008/diff/1/chrome/browser/prefs/pref_met...
chrome/browser/prefs/pref_metrics_service.cc:351: // which may have a .
nit: which may have a '.'.

[Elliot Glaysher, 2013-09-12 17:48:19]

lgtm

[michaelpg, 2013-09-14 18:18:32]

Updated unit tests - Xiyuan, could you take a look? Thanks.

https://codereview.chromium.org/23528008/diff/1/chrome/browser/prefs/pref_met...
File chrome/browser/prefs/pref_metrics_service.cc (right):

https://codereview.chromium.org/23528008/diff/1/chrome/browser/prefs/pref_met...
chrome/browser/prefs/pref_metrics_service.cc:287:
pref_hash_dicts->GetDictionaryWithoutPathExpansion(profile_name_,
On 2013/09/12 08:38:47, battre wrote:
> This is pretty error prone. Can you please update the unit tests to use a
> directory that contains a period?

Done.

https://codereview.chromium.org/23528008/diff/1/chrome/browser/prefs/pref_met...
chrome/browser/prefs/pref_metrics_service.cc:351: // which may have a .
On 2013/09/12 16:18:37, xiyuan wrote:
> nit: which may have a '.'.

Done.

[xiyuan, 2013-09-14 18:39:55]

SLGTM

https://codereview.chromium.org/23528008/diff/17001/chrome/browser/prefs/pref...
File chrome/browser/prefs/pref_metrics_service_unittest.cc (right):

https://codereview.chromium.org/23528008/diff/17001/chrome/browser/prefs/pref...
chrome/browser/prefs/pref_metrics_service_unittest.cc:51: // Check that
PrefMetricsService behaves with a  '.' in the profile name.
nit: one space between 'a' and '.'

https://codereview.chromium.org/23528008/diff/17001/chrome/browser/profiles/p...
File chrome/browser/profiles/profile_info_cache_unittest.cc (right):

https://codereview.chromium.org/23528008/diff/17001/chrome/browser/profiles/p...
chrome/browser/profiles/profile_info_cache_unittest.cc:473: string16
profile_name = ASCIIToUTF16(base::StringPrintf("name_%u", i));
nit: It's easier to read if we put the profile_path and profile_name into a test
case struct array.

e.g.

struct  {
  char* profile_path;
  char* profile_name;
} kTestCases[] = {
  { "path.ctest0", "name_0" },
  { "path_ctest1", "name_1" },
  { "path.ctest2", "name_2" },
  { "path_ctest3", "name_3" },
};

for (size_t i = 0; i < ARRAYSIZE_UNSAFE(kTestCases); ++i) {
  base::FilePath profile_path(kTestCases[i].profile_path);
  string16 profile_name = ASCIIToUTF16(kTestCases[i].profile_name);
  ...
}

[michaelpg, 2013-09-14 19:30:23]

Waiting for tree to open.

https://codereview.chromium.org/23528008/diff/17001/chrome/browser/prefs/pref...
File chrome/browser/prefs/pref_metrics_service_unittest.cc (right):

https://codereview.chromium.org/23528008/diff/17001/chrome/browser/prefs/pref...
chrome/browser/prefs/pref_metrics_service_unittest.cc:51: // Check that
PrefMetricsService behaves with a  '.' in the profile name.
On 2013/09/14 18:39:56, xiyuan wrote:
> nit: one space between 'a' and '.'

Done.

https://codereview.chromium.org/23528008/diff/17001/chrome/browser/profiles/p...
File chrome/browser/profiles/profile_info_cache_unittest.cc (right):

https://codereview.chromium.org/23528008/diff/17001/chrome/browser/profiles/p...
chrome/browser/profiles/profile_info_cache_unittest.cc:473: string16
profile_name = ASCIIToUTF16(base::StringPrintf("name_%u", i));
On 2013/09/14 18:39:56, xiyuan wrote:
> nit: It's easier to read if we put the profile_path and profile_name into a
test
> case struct array.
> 
> e.g.
> 
> struct  {
>   char* profile_path;
>   char* profile_name;
> } kTestCases[] = {
>   { "path.ctest0", "name_0" },
>   { "path_ctest1", "name_1" },
>   { "path.ctest2", "name_2" },
>   { "path_ctest3", "name_3" },
> };
> 
> for (size_t i = 0; i < ARRAYSIZE_UNSAFE(kTestCases); ++i) {
>   base::FilePath profile_path(kTestCases[i].profile_path);
>   string16 profile_name = ASCIIToUTF16(kTestCases[i].profile_name);
>   ...
> }

Done.

[commit-bot: I haz the power, 2013-09-16 16:53:26]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/michaelpg@chromium.org/23528008/32001

[commit-bot: I haz the power, 2013-09-16 19:42:05]

Retried try job too often on win7_aura for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win7_aura&...

[commit-bot: I haz the power, 2013-09-16 21:10:44]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/michaelpg@chromium.org/23528008/32001

[commit-bot: I haz the power, 2013-09-16 22:26:40]

Change committed as 223448