Fix sign-in error strings.

Move auth errors from the hotdog menu to the new profile menu.

See UX changes in this video:
https://drive.google.com/a/google.com/file/d/0BzdR97pL_sHWaW5aM0EzS2JYbU0/edit?usp=sharing

Video of clicking icon and doing re-auth:
https://docs.google.com/a/google.com/file/d/0BzdR97pL_sHWQ0NMYWdJekFRVHc/edit

BUG=311235, 374771
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=272092

[noms (inactive), 2014-05-11 19:21:25]

Some initial comments. I think they're mostly nits. :)

https://codereview.chromium.org/240453006/diff/520001/chrome/browser/ui/toolb...
File chrome/browser/ui/toolbar/wrench_menu_model.cc (left):

https://codereview.chromium.org/240453006/diff/520001/chrome/browser/ui/toolb...
chrome/browser/ui/toolbar/wrench_menu_model.cc:281: command_id ==
IDC_SHOW_SIGNIN;
Hmm, can we delete this already? Shouldn't we still use it for
pre-new-profile-management?

https://codereview.chromium.org/240453006/diff/520001/chrome/browser/ui/views...
File chrome/browser/ui/views/profiles/new_avatar_button.cc (right):

https://codereview.chromium.org/240453006/diff/520001/chrome/browser/ui/views...
chrome/browser/ui/views/profiles/new_avatar_button.cc:161: // If there is an
error, use a special background.
nit: I think the comment is wrong. You're only changing the menu marker, not the
background.

https://codereview.chromium.org/240453006/diff/520001/chrome/browser/ui/views...
File chrome/browser/ui/views/profiles/profile_chooser_view.h (right):

https://codereview.chromium.org/240453006/diff/520001/chrome/browser/ui/views...
chrome/browser/ui/views/profiles/profile_chooser_view.h:99: typedef
std::map<views::View*, std::string> AccountButtonIndexes;
Why View? It's still a Button* that's getting added, I think.

[Roger Tawa OOO till Jul 10th, 2014-05-13 21:23:54]

Thanks Monica.

https://codereview.chromium.org/240453006/diff/520001/chrome/browser/ui/toolb...
File chrome/browser/ui/toolbar/wrench_menu_model.cc (left):

https://codereview.chromium.org/240453006/diff/520001/chrome/browser/ui/toolb...
chrome/browser/ui/toolbar/wrench_menu_model.cc:281: command_id ==
IDC_SHOW_SIGNIN;
On 2014/05/11 19:21:25, Monica (OOO until May 15) wrote:
> Hmm, can we delete this already? Shouldn't we still use it for
> pre-new-profile-management?

Nope.  All changes in this file must be kept for pre-mirror.

https://codereview.chromium.org/240453006/diff/520001/chrome/browser/ui/views...
File chrome/browser/ui/views/profiles/new_avatar_button.cc (right):

https://codereview.chromium.org/240453006/diff/520001/chrome/browser/ui/views...
chrome/browser/ui/views/profiles/new_avatar_button.cc:161: // If there is an
error, use a special background.
On 2014/05/11 19:21:25, Monica (OOO until May 15) wrote:
> nit: I think the comment is wrong. You're only changing the menu marker, not
the
> background.

Done.

https://codereview.chromium.org/240453006/diff/520001/chrome/browser/ui/views...
File chrome/browser/ui/views/profiles/profile_chooser_view.h (right):

https://codereview.chromium.org/240453006/diff/520001/chrome/browser/ui/views...
chrome/browser/ui/views/profiles/profile_chooser_view.h:99: typedef
std::map<views::View*, std::string> AccountButtonIndexes;
On 2014/05/11 19:21:25, Monica (OOO until May 15) wrote:
> Why View? It's still a Button* that's getting added, I think.

Done.

[noms (inactive), 2014-05-13 22:56:59]

https://codereview.chromium.org/240453006/diff/520001/chrome/browser/ui/toolb...
File chrome/browser/ui/toolbar/wrench_menu_model.cc (left):

https://codereview.chromium.org/240453006/diff/520001/chrome/browser/ui/toolb...
chrome/browser/ui/toolbar/wrench_menu_model.cc:281: command_id ==
IDC_SHOW_SIGNIN;
I guess what I was trying to say was: it looks like we're deleting the 'old way'
of showing global errors. Doesn't this break pre-mirror? 

On 2014/05/13 21:23:54, Roger Tawa wrote:
> On 2014/05/11 19:21:25, Monica (OOO until May 15) wrote:
> > Hmm, can we delete this already? Shouldn't we still use it for
> > pre-new-profile-management?
> 
> Nope.  All changes in this file must be kept for pre-mirror.

[Roger Tawa OOO till Jul 10th, 2014-05-14 14:02:13]

https://codereview.chromium.org/240453006/diff/520001/chrome/browser/ui/toolb...
File chrome/browser/ui/toolbar/wrench_menu_model.cc (left):

https://codereview.chromium.org/240453006/diff/520001/chrome/browser/ui/toolb...
chrome/browser/ui/toolbar/wrench_menu_model.cc:281: command_id ==
IDC_SHOW_SIGNIN;
On 2014/05/13 22:57:00, Monica (OOO until May 15) wrote:
> I guess what I was trying to say was: it looks like we're deleting the 'old
way'
> of showing global errors. Doesn't this break pre-mirror? 
> 
> On 2014/05/13 21:23:54, Roger Tawa wrote:
> > On 2014/05/11 19:21:25, Monica (OOO until May 15) wrote:
> > > Hmm, can we delete this already? Shouldn't we still use it for
> > > pre-new-profile-management?
> > 
> > Nope.  All changes in this file must be kept for pre-mirror.
> 

Correct, with mirror auth errors are shown differently (but other global errors
are unchanged).  The first patchset did break pre-mirror because it did not
check for IsNewProfileManagement().  I added the checks in patchset 2.

[Roger Tawa OOO till Jul 10th, 2014-05-15 14:54:43]

Hi Monica,

I broke the original CL into three parts, this one now being much smaller.  The
other CLs are:

https://codereview.chromium.org/284033002/
https://codereview.chromium.org/288813003/

Can you want to take another look?  Thanks.

[noms (inactive), 2014-05-16 13:37:03]

https://codereview.chromium.org/240453006/diff/680001/chrome/app/generated_re...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/240453006/diff/680001/chrome/app/generated_re...
chrome/app/generated_resources.grd:10987: +      <message
name="IDS_PROFILES_GAIA_REAUTH_TITLE" desc="Title of the account
reauthentication view in the avatar menu bubble.">
There's already IDS_ONE_CLICK_SIGNIN_CONFIRM_EMAIL_DIALOG_TITLE. Maybe this
should be a more genericly named string?

https://codereview.chromium.org/240453006/diff/680001/chrome/browser/ui/views...
File chrome/browser/ui/views/profiles/new_avatar_button.cc (right):

https://codereview.chromium.org/240453006/diff/680001/chrome/browser/ui/views...
chrome/browser/ui/views/profiles/new_avatar_button.cc:128: rect.Offset(0, -3);
This change gets my spider senses tingly. Does it still work in RTL?  Also, why
-3? Does this still work with the normal, non-error state drop down arrow?

https://codereview.chromium.org/240453006/diff/680001/chrome/browser/ui/views...
chrome/browser/ui/views/profiles/new_avatar_button.cc:166: SetIcon(icon);
Should you maybe just move the icon bit in the if check? it doesn't seem to be
used unless there is an error.

https://codereview.chromium.org/240453006/diff/680001/chrome/browser/ui/views...
chrome/browser/ui/views/profiles/new_avatar_button.cc:178: if (parent())
Can parent() ever even return null?

https://codereview.chromium.org/240453006/diff/680001/chrome/browser/ui/views...
File chrome/browser/ui/views/profiles/profile_chooser_view.cc (right):

https://codereview.chromium.org/240453006/diff/680001/chrome/browser/ui/views...
chrome/browser/ui/views/profiles/profile_chooser_view.cc:1139:
SigninErrorController* error =
I think it would be nicer if you had a helper function that returned a boolean.
Something like HasSigninError(Profile*) ?

https://codereview.chromium.org/240453006/diff/680001/chrome/browser/ui/views...
chrome/browser/ui/views/profiles/profile_chooser_view.cc:1231: switch
(view_mode_) {
Nice! <3

[Roger Tawa OOO till Jul 10th, 2014-05-16 19:43:48]

Thanks Monica.  Please take another look.

https://codereview.chromium.org/240453006/diff/680001/chrome/app/generated_re...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/240453006/diff/680001/chrome/app/generated_re...
chrome/app/generated_resources.grd:10987: +      <message
name="IDS_PROFILES_GAIA_REAUTH_TITLE" desc="Title of the account
reauthentication view in the avatar menu bubble.">
On 2014/05/16 13:37:04, Monica Dinculescu wrote:
> There's already IDS_ONE_CLICK_SIGNIN_CONFIRM_EMAIL_DIALOG_TITLE. Maybe this
> should be a more genericly named string?

Confirmed with Travis to use "Sign in again".  I'll keep the separate string. 
Does not makes to use IDS_SYNC_RELOGIN_LINK_LABEL or
IDS_PROFILES_CREATE_MANAGED_ACCOUNT_SIGN_IN_AGAIN_LINK here.

https://codereview.chromium.org/240453006/diff/680001/chrome/browser/ui/views...
File chrome/browser/ui/views/profiles/new_avatar_button.cc (right):

https://codereview.chromium.org/240453006/diff/680001/chrome/browser/ui/views...
chrome/browser/ui/views/profiles/new_avatar_button.cc:128: rect.Offset(0, -3);
On 2014/05/16 13:37:04, Monica Dinculescu wrote:
> This change gets my spider senses tingly. Does it still work in RTL?  Also,
why
> -3? Does this still work with the normal, non-error state drop down arrow?

Oops, I thought GetTextBounds() handled it, but does not.  Fixed.  Also removed
the hacky -3.

https://codereview.chromium.org/240453006/diff/680001/chrome/browser/ui/views...
chrome/browser/ui/views/profiles/new_avatar_button.cc:166: SetIcon(icon);
On 2014/05/16 13:37:04, Monica Dinculescu wrote:
> Should you maybe just move the icon bit in the if check? it doesn't seem to be
> used unless there is an error.

Nope.  In case there is no longer an error, I want to clear the icon.

https://codereview.chromium.org/240453006/diff/680001/chrome/browser/ui/views...
chrome/browser/ui/views/profiles/new_avatar_button.cc:178: if (parent())
On 2014/05/16 13:37:04, Monica Dinculescu wrote:
> Can parent() ever even return null? 

Yup, when called from OnErrorChanged() when this latter one is called from ctor.

https://codereview.chromium.org/240453006/diff/680001/chrome/browser/ui/views...
File chrome/browser/ui/views/profiles/profile_chooser_view.cc (right):

https://codereview.chromium.org/240453006/diff/680001/chrome/browser/ui/views...
chrome/browser/ui/views/profiles/profile_chooser_view.cc:1139:
SigninErrorController* error =
On 2014/05/16 13:37:04, Monica Dinculescu wrote:
> I think it would be nicer if you had a helper function that returned a
boolean.
> Something like HasSigninError(Profile*) ?

Done.

https://codereview.chromium.org/240453006/diff/680001/chrome/browser/ui/views...
chrome/browser/ui/views/profiles/profile_chooser_view.cc:1231: switch
(view_mode_) {
On 2014/05/16 13:37:04, Monica Dinculescu wrote:
> Nice! <3

Done.

[noms (inactive), 2014-05-20 14:52:29]

lgtm, but I don't own any of these files :)

[Roger Tawa OOO till Jul 10th, 2014-05-20 14:58:23]

Hi Scott,

Can you please do a review of the ui/views files?  Thanks.

[sky, 2014-05-20 17:23:17]

LGTM

[Roger Tawa OOO till Jul 10th, 2014-05-21 20:37:30]

The CQ bit was checked by rogerta@chromium.org

[commit-bot: I haz the power, 2014-05-21 20:39:13]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/rogerta@chromium.org/240453006/960001

[commit-bot: I haz the power, 2014-05-22 04:17:26]

Change committed as 272092