GAIAInfoUpdateService: cleanup and extend tests.

GAIAInfoUpdateService: cleanup and extend tests.

This adds a test for the bug fixed in https://codereview.chromium.org/373083002/, and also extends the existing tests to check the GAIA given name.

BUG=none

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

[Marc Treib, 2014-07-11 12:25:26]

https://codereview.chromium.org/386733003/diff/1/chrome/browser/profiles/gaia...
File chrome/browser/profiles/gaia_info_update_service_unittest.cc (left):

https://codereview.chromium.org/386733003/diff/1/chrome/browser/profiles/gaia...
chrome/browser/profiles/gaia_info_update_service_unittest.cc:117:
EXPECT_EQ(name, GetCache()->GetNameOfProfileAtIndex(index));
The GAIAInfoUpdateService does *not* actually update the local profile name (and
imo it shouldn't). This check only worked "accidentally", and I think it should
be removed.

[noms (inactive), 2014-07-11 13:25:08]

https://codereview.chromium.org/386733003/diff/1/chrome/browser/profiles/gaia...
File chrome/browser/profiles/gaia_info_update_service_unittest.cc (left):

https://codereview.chromium.org/386733003/diff/1/chrome/browser/profiles/gaia...
chrome/browser/profiles/gaia_info_update_service_unittest.cc:117:
EXPECT_EQ(name, GetCache()->GetNameOfProfileAtIndex(index));
Hmm. GetNameOfProfile() returns either the Gaia name, or the local profile name
-- so this means that when we download the profile name, GetNameOfProfileAtIndex
should return it. Is this not the case anymore?

On 2014/07/11 12:25:26, treib wrote:
> The GAIAInfoUpdateService does *not* actually update the local profile name
(and
> imo it shouldn't). This check only worked "accidentally", and I think it
should
> be removed.

[Marc Treib, 2014-07-11 13:30:29]

https://codereview.chromium.org/386733003/diff/1/chrome/browser/profiles/gaia...
File chrome/browser/profiles/gaia_info_update_service_unittest.cc (left):

https://codereview.chromium.org/386733003/diff/1/chrome/browser/profiles/gaia...
chrome/browser/profiles/gaia_info_update_service_unittest.cc:117:
EXPECT_EQ(name, GetCache()->GetNameOfProfileAtIndex(index));
Not quite - it uses the local profile name if it's flagged as non-default,
otherwise the GAIA given (!) name, and only if that's empty, falls back to the
GAIA full name. Anyway, that's ProfileInfoCache logic, which imo should not be
tested here.

On 2014/07/11 13:25:08, Monica Dinculescu wrote:
> Hmm. GetNameOfProfile() returns either the Gaia name, or the local profile
name
> -- so this means that when we download the profile name,
GetNameOfProfileAtIndex
> should return it. Is this not the case anymore?
> 
> On 2014/07/11 12:25:26, treib wrote:
> > The GAIAInfoUpdateService does *not* actually update the local profile name
> (and
> > imo it shouldn't). This check only worked "accidentally", and I think it
> should
> > be removed.
>

[noms (inactive), 2014-07-11 13:52:30]

Thanks for the test cleanup! lgtm

https://codereview.chromium.org/386733003/diff/1/chrome/browser/profiles/gaia...
File chrome/browser/profiles/gaia_info_update_service_unittest.cc (left):

https://codereview.chromium.org/386733003/diff/1/chrome/browser/profiles/gaia...
chrome/browser/profiles/gaia_info_update_service_unittest.cc:117:
EXPECT_EQ(name, GetCache()->GetNameOfProfileAtIndex(index));
I agree it should not be tested here. Ok.

On 2014/07/11 13:30:28, treib wrote:
> Not quite - it uses the local profile name if it's flagged as non-default,
> otherwise the GAIA given (!) name, and only if that's empty, falls back to the
> GAIA full name. Anyway, that's ProfileInfoCache logic, which imo should not be
> tested here.
> 
> On 2014/07/11 13:25:08, Monica Dinculescu wrote:
> > Hmm. GetNameOfProfile() returns either the Gaia name, or the local profile
> name
> > -- so this means that when we download the profile name,
> GetNameOfProfileAtIndex
> > should return it. Is this not the case anymore?
> > 
> > On 2014/07/11 12:25:26, treib wrote:
> > > The GAIAInfoUpdateService does *not* actually update the local profile
name
> > (and
> > > imo it shouldn't). This check only worked "accidentally", and I think it
> > should
> > > be removed.
> > 
>

[Marc Treib, 2014-07-11 13:53:34]

The CQ bit was checked by treib@chromium.org

[commit-bot: I haz the power, 2014-07-11 13:54:05]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/treib@chromium.org/386733003/1

[commit-bot: I haz the power, 2014-07-11 15:46:29]

Change committed as 282631