[Profiles] Notify observers if a profile switches bw/ default and custom names.

[Profiles] Notify observers if a profile switches bw/ default and custom names.

BUG=462246
TEST=Start Chrome with --enable-new-avatar-menu and a single, local profile.
Sign in. The avatar button should correctly update and display the synced name.

Committed: https://crrev.com/20f07ace8550e333cac446b4c4fa347fa6dbb1f2
Cr-Commit-Position: refs/heads/master@{#319691}

[noms (inactive), 2015-03-04 19:24:14]

noms@chromium.org changed reviewers:
+ mlerman@chromium.org

[noms (inactive), 2015-03-04 19:24:15]

This fixes a bug introduced by the giant refactor in
https://codereview.chromium.org/895803003/. Please take a look. Thanks!

[Mike Lerman, 2015-03-04 19:25:24]

lgtm

[Mike Lerman, 2015-03-04 19:25:26]

lgtm

[noms (inactive), 2015-03-04 19:27:41]

The CQ bit was checked by noms@chromium.org

[commit-bot: I haz the power, 2015-03-04 19:29:20]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/974373002/1

[commit-bot: I haz the power, 2015-03-04 22:27:44]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-03-04 22:27:45]

Try jobs failed on following builders:
  android_rel_tests_recipe on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/android_rel_tes...)

[noms (inactive), 2015-03-05 00:21:41]

I made a change to the ProfileInfoCache test to match the new notification. Let
me know if you think it looks okay (I didn't really want to send a new kind of
notification for this property, since it really affects the profile name, but
now it's a bit annoying because you might get notified when the profile name is
the same).

[Mike Lerman, 2015-03-05 13:49:41]

On 2015/03/05 00:21:41, noms (OOO-slow to respond) wrote:
> I made a change to the ProfileInfoCache test to match the new notification.
Let
> me know if you think it looks okay (I didn't really want to send a new kind of
> notification for this property, since it really affects the profile name, but
> now it's a bit annoying because you might get notified when the profile name
is
> the same).

Should we then only send out the notification if the name changes?

[chromium-reviews, 2015-03-05 15:37:34]

So that's kind of the whole point of the CL. The problem is that the logic
in GetNameOfProfileAtIndex() includes a check for IsUsingDefaultProfile(),
which means that when the isUsingDefaultName preference changes, what
GetName... returns also changes (and hence why we notify observers that the
name changes. It technically has, but we cant tell the difference between
old/new at that point)

On Thu, 5 Mar 2015 05:49 null <mlerman@chromium.org> wrote:

> On 2015/03/05 00:21:41, noms (OOO-slow to respond) wrote:
> > I made a change to the ProfileInfoCache test to match the new
> > notification.
> Let
> > me know if you think it looks okay (I didn't really want to send a new
> > kind of
> > notification for this property, since it really affects the profile name,
> > but
> > now it's a bit annoying because you might get notified when the profile
> > name
> is
> > the same).
>
> Should we then only send out the notification if the name changes?
>
> https://codereview.chromium.org/974373002/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Mike Lerman, 2015-03-05 16:39:24]

On 2015/03/05 15:37:34, chromium-reviews wrote:
> So that's kind of the whole point of the CL. The problem is that the logic
> in GetNameOfProfileAtIndex() includes a check for IsUsingDefaultProfile(),
> which means that when the isUsingDefaultName preference changes, what
> GetName... returns also changes (and hence why we notify observers that the
> name changes. It technically has, but we cant tell the difference between
> old/new at that point)
> 
> On Thu, 5 Mar 2015 05:49 null <mailto:mlerman@chromium.org> wrote:
> 
> > On 2015/03/05 00:21:41, noms (OOO-slow to respond) wrote:
> > > I made a change to the ProfileInfoCache test to match the new
> > > notification.
> > Let
> > > me know if you think it looks okay (I didn't really want to send a new
> > > kind of
> > > notification for this property, since it really affects the profile name,
> > > but
> > > now it's a bit annoying because you might get notified when the profile
> > > name
> > is
> > > the same).
> >
> > Should we then only send out the notification if the name changes?
> >
> > https://codereview.chromium.org/974373002/
> >
> 
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

Can't we get the ProfileNameAtIndex at line 741 in a std::string, set the
IsUsingDefaultNamePref, then call ProfileNameAtIndex again and compare if it
changed?

[noms (inactive), 2015-03-09 17:30:46]

On 2015/03/05 16:39:24, Mike Lerman wrote:
> On 2015/03/05 15:37:34, chromium-reviews wrote:
> > So that's kind of the whole point of the CL. The problem is that the logic
> > in GetNameOfProfileAtIndex() includes a check for IsUsingDefaultProfile(),
> > which means that when the isUsingDefaultName preference changes, what
> > GetName... returns also changes (and hence why we notify observers that the
> > name changes. It technically has, but we cant tell the difference between
> > old/new at that point)
> > 
> > On Thu, 5 Mar 2015 05:49 null <mailto:mlerman@chromium.org> wrote:
> > 
> > > On 2015/03/05 00:21:41, noms (OOO-slow to respond) wrote:
> > > > I made a change to the ProfileInfoCache test to match the new
> > > > notification.
> > > Let
> > > > me know if you think it looks okay (I didn't really want to send a new
> > > > kind of
> > > > notification for this property, since it really affects the profile
name,
> > > > but
> > > > now it's a bit annoying because you might get notified when the profile
> > > > name
> > > is
> > > > the same).
> > >
> > > Should we then only send out the notification if the name changes?
> > >
> > > https://codereview.chromium.org/974373002/
> > >
> > 
> > To unsubscribe from this group and stop receiving emails from it, send an
> email
> > to mailto:chromium-reviews+unsubscribe@chromium.org.
> 
> Can't we get the ProfileNameAtIndex at line 741 in a std::string, set the
> IsUsingDefaultNamePref, then call ProfileNameAtIndex again and compare if it
> changed?

Oh yeah, you're totally right. Thanks for spotting it. Done!

[noms (inactive), 2015-03-09 17:30:53]

The CQ bit was checked by noms@chromium.org

[noms (inactive), 2015-03-09 17:30:53]

The patchset sent to the CQ was uploaded after l-g-t-m from mlerman@chromium.org
Link to the patchset: https://codereview.chromium.org/974373002/#ps40001 (title:
"moar better code")

[commit-bot: I haz the power, 2015-03-09 17:31:59]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/974373002/40001

[commit-bot: I haz the power, 2015-03-09 18:24:45]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-03-09 18:25:36]

Patchset 3 (id:??) landed as
https://crrev.com/20f07ace8550e333cac446b4c4fa347fa6dbb1f2
Cr-Commit-Position: refs/heads/master@{#319691}