Update managed user registration to allow updating the avatar

Update managed user registration to allow updating the avatar

Add a method ManagedUserSyncService::UpdateManagedUserAvatar to update the avatar of an existing managed user.

Update ManagedUserRegistartionUtility::Register to call the above method when a managed user is being imported and updating his avatar is required.

BUG=292415
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=223333

[ibra, 2013-09-11 14:57:01]

Hey Sergiu, can you please take a look? Thanks!

[Sergiu, 2013-09-13 11:14:18]

+Bernhard

lgtm. Also, could we add a test for this somewhere?

[Bernhard Bauer, 2013-09-13 11:24:20]

https://codereview.chromium.org/23691045/diff/14001/chrome/browser/managed_mo...
File chrome/browser/managed_mode/managed_user_registration_utility.cc (right):

https://codereview.chromium.org/23691045/diff/14001/chrome/browser/managed_mo...
chrome/browser/managed_mode/managed_user_registration_utility.cc:218: if
(update_avatar) {
Couldn't we infer this from the information we already have? I.e. if the locally
stored data does not have an avatar yet, we know that we have to update it?

[ibra, 2013-09-13 14:16:50]

PTAL, thanks!

https://codereview.chromium.org/23691045/diff/14001/chrome/browser/managed_mo...
File chrome/browser/managed_mode/managed_user_registration_utility.cc (right):

https://codereview.chromium.org/23691045/diff/14001/chrome/browser/managed_mo...
chrome/browser/managed_mode/managed_user_registration_utility.cc:218: if
(update_avatar) {
On 2013/09/13 11:24:20, Bernhard Bauer wrote:
> Couldn't we infer this from the information we already have? I.e. if the
locally
> stored data does not have an avatar yet, we know that we have to update it?

Done.

[Bernhard Bauer, 2013-09-13 20:17:24]

https://codereview.chromium.org/23691045/diff/24001/chrome/browser/managed_mo...
File chrome/browser/managed_mode/managed_user_registration_utility.cc (right):

https://codereview.chromium.org/23691045/diff/24001/chrome/browser/managed_mo...
chrome/browser/managed_mode/managed_user_registration_utility.cc:214:
avatar_updated_ =
Nit: two spaces less.

https://codereview.chromium.org/23691045/diff/24001/chrome/browser/managed_mo...
File chrome/browser/managed_mode/managed_user_sync_service.h (right):

https://codereview.chromium.org/23691045/diff/24001/chrome/browser/managed_mo...
chrome/browser/managed_mode/managed_user_sync_service.h:41: static const int
kNoAvatar;
Some grouping comments would be nice (in particular to distinguish the
dictionary keys from the avatar ID constant).

https://codereview.chromium.org/23691045/diff/24001/chrome/browser/managed_mo...
chrome/browser/managed_mode/managed_user_sync_service.h:73: // managed user has
an avatar, it will be cleared.
Hm, this makes ClearManagedUserAvatar() basically a convenience method. Can you
add a comment to that effect?

[ibra, 2013-09-16 08:59:09]

https://codereview.chromium.org/23691045/diff/24001/chrome/browser/managed_mo...
File chrome/browser/managed_mode/managed_user_registration_utility.cc (right):

https://codereview.chromium.org/23691045/diff/24001/chrome/browser/managed_mo...
chrome/browser/managed_mode/managed_user_registration_utility.cc:214:
avatar_updated_ =
On 2013/09/13 20:17:24, Bernhard Bauer wrote:
> Nit: two spaces less.

Done.

https://codereview.chromium.org/23691045/diff/24001/chrome/browser/managed_mo...
File chrome/browser/managed_mode/managed_user_sync_service.h (right):

https://codereview.chromium.org/23691045/diff/24001/chrome/browser/managed_mo...
chrome/browser/managed_mode/managed_user_sync_service.h:41: static const int
kNoAvatar;
On 2013/09/13 20:17:24, Bernhard Bauer wrote:
> Some grouping comments would be nice (in particular to distinguish the
> dictionary keys from the avatar ID constant).

Done.

https://codereview.chromium.org/23691045/diff/24001/chrome/browser/managed_mo...
chrome/browser/managed_mode/managed_user_sync_service.h:73: // managed user has
an avatar, it will be cleared.
On 2013/09/13 20:17:24, Bernhard Bauer wrote:
> Hm, this makes ClearManagedUserAvatar() basically a convenience method. Can
you
> add a comment to that effect?

Done.

[Bernhard Bauer, 2013-09-16 09:03:12]

lgtm

[Bernhard Bauer, 2013-09-16 09:03:49]

Can you add a bug number for this please?

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

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ibraaaa@chromium.org/23691045/35001

[commit-bot: I haz the power, 2013-09-16 12:40:27]

Change committed as 223333