Collect the custodian's full name when a supervised user is created.

Collect the custodian's full name when a supervised user is created.

Collect the custodian's GAIA account display name using the
ProfileDownloader and save it in the supervised user's prefs
so it can be shown in that profile's UI.

BUG=249691
TEST=manual

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

[Pam (message me for reviews), 2013-06-21 14:47:35]

Please review.

Note that this contains the changes from the big string CL
(https://codereview.chromium.org/17320005/) too, because they're small and it
was much easier not to extract them for separate review. (This only affects
managed_user_service.*; the MURS wasn't in the string-change CL.)

I'll wait for the string-change CL to land before this one.

Thanks,
- Pam

[Bernhard Bauer, 2013-06-21 15:52:07]

LGTM with a suggestion:

https://chromiumcodereview.appspot.com/16950018/diff/1/chrome/browser/managed...
File chrome/browser/managed_mode/managed_user_registration_service.cc (right):

https://chromiumcodereview.appspot.com/16950018/diff/1/chrome/browser/managed...
chrome/browser/managed_mode/managed_user_registration_service.cc:150: if
(profile_downloader_)
Should it be a DCHECK to try to download a profile more than once (or trying to
download a different profile)?

https://chromiumcodereview.appspot.com/16950018/diff/1/chrome/browser/managed...
File chrome/browser/managed_mode/managed_user_registration_service.h (right):

https://chromiumcodereview.appspot.com/16950018/diff/1/chrome/browser/managed...
chrome/browser/managed_mode/managed_user_registration_service.h:90: void
DownloadProfile(Profile* profile,
It's a bit weird that we have to pass in the profile here, but for now I think
it's okay. Eventually we should probably integrate this with
GAIAInfoUpdateService.

[Pam (message me for reviews), 2013-06-21 16:19:09]

https://chromiumcodereview.appspot.com/16950018/diff/1/chrome/browser/managed...
File chrome/browser/managed_mode/managed_user_registration_service.cc (right):

https://chromiumcodereview.appspot.com/16950018/diff/1/chrome/browser/managed...
chrome/browser/managed_mode/managed_user_registration_service.cc:150: if
(profile_downloader_)
On 2013/06/21 15:52:07, Bernhard Bauer wrote:
> Should it be a DCHECK to try to download a profile more than once (or trying
to
> download a different profile)?

Well, since the downloader isn't guaranteed to ever return either success or
failure, it seemed like there could be one hanging around without it being a
programming error. It didn't look like just destroying a leftover was safe
either, but maybe I'm missing something there.

https://chromiumcodereview.appspot.com/16950018/diff/1/chrome/browser/managed...
File chrome/browser/managed_mode/managed_user_registration_service.h (right):

https://chromiumcodereview.appspot.com/16950018/diff/1/chrome/browser/managed...
chrome/browser/managed_mode/managed_user_registration_service.h:90: void
DownloadProfile(Profile* profile,
On 2013/06/21 15:52:07, Bernhard Bauer wrote:
> It's a bit weird that we have to pass in the profile here, but for now I think
> it's okay. Eventually we should probably integrate this with
> GAIAInfoUpdateService.

Yeah, I don't know why the downloader doesn't take a profile as a parameter into
Start(), but I didn't want to get into refactoring its whole API here.

GAIAInfoUpdateService uses the same downloader, of course, and fills the
ProfileInfoCache. If the GAIAInfoUpdateService is ever enabled, this can all
come out and we can grab what we need from the ProfileInfoCache instead. There's
a TODO in here about that, but I'll make it more clear.

[Bernhard Bauer, 2013-06-21 16:46:57]

https://chromiumcodereview.appspot.com/16950018/diff/1/chrome/browser/managed...
File chrome/browser/managed_mode/managed_user_registration_service.cc (right):

https://chromiumcodereview.appspot.com/16950018/diff/1/chrome/browser/managed...
chrome/browser/managed_mode/managed_user_registration_service.cc:150: if
(profile_downloader_)
On 2013/06/21 16:19:09, Pam - also send IM for reviews wrote:
> On 2013/06/21 15:52:07, Bernhard Bauer wrote:
> > Should it be a DCHECK to try to download a profile more than once (or trying
> to
> > download a different profile)?
> 
> Well, since the downloader isn't guaranteed to ever return either success or
> failure, it seemed like there could be one hanging around without it being a
> programming error. 

But I don't think silently returning is the correct behavior either.

> It didn't look like just destroying a leftover was safe
> either, but maybe I'm missing something there.

Why is destroying the ProfileDownloader not safe?

[Pam (message me for reviews), 2013-06-21 19:20:25]

New patch! I know you already approved it, but in case you want a look.

https://codereview.chromium.org/16950018/diff/1/chrome/browser/managed_mode/m...
File chrome/browser/managed_mode/managed_user_registration_service.cc (right):

https://codereview.chromium.org/16950018/diff/1/chrome/browser/managed_mode/m...
chrome/browser/managed_mode/managed_user_registration_service.cc:150: if
(profile_downloader_)
On 2013/06/21 16:46:57, Bernhard Bauer wrote:
> On 2013/06/21 16:19:09, Pam - also send IM for reviews wrote:
> > On 2013/06/21 15:52:07, Bernhard Bauer wrote:
> > > Should it be a DCHECK to try to download a profile more than once (or
trying
> > to
> > > download a different profile)?
> > 
> > Well, since the downloader isn't guaranteed to ever return either success or
> > failure, it seemed like there could be one hanging around without it being a
> > programming error. 
> 
> But I don't think silently returning is the correct behavior either.
> 
> > It didn't look like just destroying a leftover was safe
> > either, but maybe I'm missing something there.
> 
> Why is destroying the ProfileDownloader not safe?

Fair enough. On a second look, as far as I can tell it's not a problem to dump
the old downloader.

[commit-bot: I haz the power, 2013-06-22 00:16:33]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/pam@chromium.org/16950018/17001

[commit-bot: I haz the power, 2013-06-22 02:32:59]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/pam@chromium.org/16950018/17001

[commit-bot: I haz the power, 2013-06-22 03:00:35]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/pam@chromium.org/16950018/17001

[commit-bot: I haz the power, 2013-06-22 03:01:00]

Failed to request the patch to try. Please note that binary files are still
unsupported at the moment, this is being worked on.

Thanks for your patience.

HTTP Error 500: 
<html><head>
<meta http-equiv="content-type" content="text/html;charset=utf-8">
<title>500 Server Error</title>
</head>
<body text=#000000 bgcolor=#ffffff>
<h1>Error: Server Error</h1>
<h2>The server encountered an error and could not complete your request.<p>If
the problem persists, please <A
HREF="http://code.google.com/appengine/community.html">report</A> your problem
and mention this error message and the query that caused it.</h2>
<h2></h2>
</body></html>

[commit-bot: I haz the power, 2013-06-22 04:41:17]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/pam@chromium.org/16950018/17001

[commit-bot: I haz the power, 2013-06-22 07:05:56]

Change committed as 208013