Making profile avatars and names sync.

Making profile avatars and names sync.

BUG=87658
TEST=Have two machines with chrome, both with multiple profiles and each with one profile signed in to the same account. Change avatar/name on one, and it will change on the other.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=112856

[jwd, 2011-12-02 02:35:43]

A change made to the unit tests will cause all unit tests to fail. I am waiting
for another issue to be resolved for those tests to work. See the comment in
profile_manager_unittest.cc for more details.

http://codereview.chromium.org/8771024/diff/3006/chrome/browser/profiles/prof...
File chrome/browser/profiles/profile_manager_unittest.cc (right):

http://codereview.chromium.org/8771024/diff/3006/chrome/browser/profiles/prof...
chrome/browser/profiles/profile_manager_unittest.cc:69: new
ProfileManagerWithoutInit(temp_dir_.path()));
This change, and the other related changes, will cause all of these tests to
fail. Once crbug.com/101434 is resolved, this should work.

[jwd, 2011-12-02 18:56:47]

Tests are working now, thanks to rlp.

[sail, 2011-12-02 19:07:10]

http://codereview.chromium.org/8771024/diff/5015/chrome/browser/profiles/prof...
File chrome/browser/profiles/profile_info_cache.cc (right):

http://codereview.chromium.org/8771024/diff/5015/chrome/browser/profiles/prof...
chrome/browser/profiles/profile_info_cache.cc:633: return
index<kDefaultAvatarIconsCount;
space before and after "<"

http://codereview.chromium.org/8771024/diff/5015/chrome/browser/profiles/prof...
File chrome/browser/profiles/profile_manager.cc (right):

http://codereview.chromium.org/8771024/diff/5015/chrome/browser/profiles/prof...
chrome/browser/profiles/profile_manager.cc:636: string16 profile_name =
UTF8ToUTF16(profile->GetPrefs()->GetString(
can you add a comment to explain why you're doing this

http://codereview.chromium.org/8771024/diff/5015/chrome/browser/profiles/prof...
chrome/browser/profiles/profile_manager.cc:654: if
(HasAnyDefaultUserPrefs(profile)) {
can you add a comment to explain why you're doing this

http://codereview.chromium.org/8771024/diff/5015/chrome/browser/ui/webui/opti...
File chrome/browser/ui/webui/options/manage_profile_handler.cc (right):

http://codereview.chromium.org/8771024/diff/5015/chrome/browser/ui/webui/opti...
chrome/browser/ui/webui/options/manage_profile_handler.cc:150: Profile* prof =
abbreviating names is bad, profile would be better

http://codereview.chromium.org/8771024/diff/5015/chrome/browser/ui/webui/opti...
chrome/browser/ui/webui/options/manage_profile_handler.cc:183:
pref_service->SetString(prefs::kProfileName,UTF16ToUTF8(new_profile_name));
I don't think this is a good place to set this pref. Doing it here means that
it'll only get update if user changes it from the settings UI.

It would be better to add code to profile_impl.cc to watch for the profile name
and icon to change and set the pref there.

I think the code to publish desktop shortcuts for profiles does something
similar.

[jwd, 2011-12-02 21:10:16]

http://codereview.chromium.org/8771024/diff/5015/chrome/browser/profiles/prof...
File chrome/browser/profiles/profile_info_cache.cc (right):

http://codereview.chromium.org/8771024/diff/5015/chrome/browser/profiles/prof...
chrome/browser/profiles/profile_info_cache.cc:633: return
index<kDefaultAvatarIconsCount;
On 2011/12/02 19:07:10, sail wrote:
> space before and after "<"

Done.

http://codereview.chromium.org/8771024/diff/5015/chrome/browser/profiles/prof...
File chrome/browser/profiles/profile_manager.cc (right):

http://codereview.chromium.org/8771024/diff/5015/chrome/browser/profiles/prof...
chrome/browser/profiles/profile_manager.cc:636: string16 profile_name =
UTF8ToUTF16(profile->GetPrefs()->GetString(
On 2011/12/02 19:07:10, sail wrote:
> can you add a comment to explain why you're doing this

Done.

http://codereview.chromium.org/8771024/diff/5015/chrome/browser/profiles/prof...
chrome/browser/profiles/profile_manager.cc:654: if
(HasAnyDefaultUserPrefs(profile)) {
On 2011/12/02 19:07:10, sail wrote:
> can you add a comment to explain why you're doing this

Done.

http://codereview.chromium.org/8771024/diff/5015/chrome/browser/ui/webui/opti...
File chrome/browser/ui/webui/options/manage_profile_handler.cc (right):

http://codereview.chromium.org/8771024/diff/5015/chrome/browser/ui/webui/opti...
chrome/browser/ui/webui/options/manage_profile_handler.cc:150: Profile* prof =
On 2011/12/02 19:07:10, sail wrote:
> abbreviating names is bad, profile would be better

Done.

http://codereview.chromium.org/8771024/diff/5015/chrome/browser/ui/webui/opti...
chrome/browser/ui/webui/options/manage_profile_handler.cc:183:
pref_service->SetString(prefs::kProfileName,UTF16ToUTF8(new_profile_name));
On 2011/12/02 19:07:10, sail wrote:
> I don't think this is a good place to set this pref. Doing it here means that
> it'll only get update if user changes it from the settings UI.
> 
> It would be better to add code to profile_impl.cc to watch for the profile
name
> and icon to change and set the pref there.
> 
> I think the code to publish desktop shortcuts for profiles does something
> similar.

This code was actually in the wrong place. In the non GAIA case, the cache
shouldn't be touched directly. The values are set in the profile prefs and the
profile pushes the changes to the cache. This is done because incoming sync
changes will be set in the profile, so I decided to make the profile the place
where these values are set and changed. The init code in profile manager is an
exception to this.

[sail, 2011-12-02 23:29:56]

LGTM

I think the manage_profile_handler.cc code is getting very very messy. Hopefully
we can clean this up after the branch point.

http://codereview.chromium.org/8771024/diff/8001/chrome/browser/ui/webui/opti...
File chrome/browser/ui/webui/options/manage_profile_handler.cc (right):

http://codereview.chromium.org/8771024/diff/8001/chrome/browser/ui/webui/opti...
chrome/browser/ui/webui/options/manage_profile_handler.cc:190:
pref_service->SetInteger(prefs::kProfileAvatarIndex, new_icon_index);
can you add a comment that the profile will update the cache

[csilv, 2011-12-02 23:40:18]

lgtm

http://codereview.chromium.org/8771024/diff/8001/chrome/browser/ui/webui/opti...
File chrome/browser/ui/webui/options/manage_profile_handler.cc (right):

http://codereview.chromium.org/8771024/diff/8001/chrome/browser/ui/webui/opti...
chrome/browser/ui/webui/options/manage_profile_handler.cc:165:
pref_service->SetString(prefs::kProfileName,UTF16ToUTF8(new_profile_name));
nit: space after comma

[jwd, 2011-12-03 00:13:10]

http://codereview.chromium.org/8771024/diff/8001/chrome/browser/ui/webui/opti...
File chrome/browser/ui/webui/options/manage_profile_handler.cc (right):

http://codereview.chromium.org/8771024/diff/8001/chrome/browser/ui/webui/opti...
chrome/browser/ui/webui/options/manage_profile_handler.cc:165:
pref_service->SetString(prefs::kProfileName,UTF16ToUTF8(new_profile_name));
On 2011/12/02 23:40:18, csilv wrote:
> nit: space after comma

Done.

http://codereview.chromium.org/8771024/diff/8001/chrome/browser/ui/webui/opti...
chrome/browser/ui/webui/options/manage_profile_handler.cc:190:
pref_service->SetInteger(prefs::kProfileAvatarIndex, new_icon_index);
On 2011/12/02 23:29:57, sail wrote:
> can you add a comment that the profile will update the cache

Done.

[commit-bot: I haz the power, 2011-12-03 00:13:27]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jwd@chromium.org/8771024/14013

[commit-bot: I haz the power, 2011-12-03 03:16:40]

Change committed as 112856