Tray message changed for child accounts. Warning bubble removed for child accounts.

Tray message changed for child accounts. Warning bubble removed for child accounts.

BUG=435130, 439941
CL based on https://chromereviews.googleplex.com/

TBR=jennyz@chromium.org

Committed: https://crrev.com/73b32fca8ed133af29af871d2108f9ea5de7f0bf
Cr-Commit-Position: refs/heads/master@{#309427}

[merkulova, 2014-12-08 13:36:21]

merkulova@chromium.org changed reviewers:
+ jennyz@chromium.org, oshima@chromium.org, treib@chromium.org

[merkulova, 2014-12-08 13:37:14]

merkulova@chromium.org changed reviewers:
- jennyz@chromium.org, oshima@chromium.org

[merkulova, 2014-12-08 13:37:48]

Marc, please have a look.

[Marc Treib, 2014-12-08 14:19:49]

https://codereview.chromium.org/783963002/diff/1/chrome/app/chromeos_strings....
File chrome/app/chromeos_strings.grdp (right):

https://codereview.chromium.org/783963002/diff/1/chrome/app/chromeos_strings....
chrome/app/chromeos_strings.grdp:1682: <message
name="IDS_USER_IS_MANAGED_BY_ONE_NOTICE" desc="Text for notifications showing
that this user is managed by other account.">
nit: The ID and description could mention "child", I think that'd be clearer.

https://codereview.chromium.org/783963002/diff/1/chrome/browser/ui/ash/system...
File chrome/browser/ui/ash/system_tray_delegate_chromeos.h (right):

https://codereview.chromium.org/783963002/diff/1/chrome/browser/ui/ash/system...
chrome/browser/ui/ash/system_tray_delegate_chromeos.h:81: bool
IsUserRegularSupervised() const override;
Please rename: IsUserChild (or just IsChild, or IsChildUser, whichever you
prefer).
Also in all other places that mention "RegularSupervised"

[merkulova, 2014-12-08 15:09:00]

merkulova@chromium.org changed reviewers:
+ oshima@chromium.org

[merkulova, 2014-12-08 15:09:56]

merkulova@chromium.org changed reviewers:
- oshima@chromium.org

[merkulova, 2014-12-08 15:10:13]

https://codereview.chromium.org/783963002/diff/1/chrome/app/chromeos_strings....
File chrome/app/chromeos_strings.grdp (right):

https://codereview.chromium.org/783963002/diff/1/chrome/app/chromeos_strings....
chrome/app/chromeos_strings.grdp:1682: <message
name="IDS_USER_IS_MANAGED_BY_ONE_NOTICE" desc="Text for notifications showing
that this user is managed by other account.">
On 2014/12/08 14:19:49, Marc Treib wrote:
> nit: The ID and description could mention "child", I think that'd be clearer.

Done.

https://codereview.chromium.org/783963002/diff/1/chrome/browser/ui/ash/system...
File chrome/browser/ui/ash/system_tray_delegate_chromeos.h (right):

https://codereview.chromium.org/783963002/diff/1/chrome/browser/ui/ash/system...
chrome/browser/ui/ash/system_tray_delegate_chromeos.h:81: bool
IsUserRegularSupervised() const override;
On 2014/12/08 14:19:49, Marc Treib wrote:
> Please rename: IsUserChild (or just IsChild, or IsChildUser, whichever you
> prefer).
> Also in all other places that mention "RegularSupervised"

Another CL addresses that https://codereview.chromium.org/790503002/

Landing will be managed accurately.

[Marc Treib, 2014-12-08 15:26:54]

lgtm if you want to do the renaming in another CL.

General question: Do you know why all the SU strings are defined separately for
CrOS and desktop, even though their values seem to be mostly the same?

Please fix the "based on" link in the description.

https://codereview.chromium.org/783963002/diff/1/chrome/browser/ui/ash/system...
File chrome/browser/ui/ash/system_tray_delegate_chromeos.h (right):

https://codereview.chromium.org/783963002/diff/1/chrome/browser/ui/ash/system...
chrome/browser/ui/ash/system_tray_delegate_chromeos.h:81: bool
IsUserRegularSupervised() const override;
On 2014/12/08 15:10:13, merkulova wrote:
> On 2014/12/08 14:19:49, Marc Treib wrote:
> > Please rename: IsUserChild (or just IsChild, or IsChildUser, whichever you
> > prefer).
> > Also in all other places that mention "RegularSupervised"
> 
> Another CL addresses that https://codereview.chromium.org/790503002/
> 
> Landing will be managed accurately.

It doesn't (yet?) rename the methods that were newly added here. Are you
planning to add that?
Either way, you can also do the renaming in a followup CL if you prefer.

[merkulova, 2014-12-11 10:50:05]

merkulova@chromium.org changed reviewers:
+ oshima@chromium.org

[merkulova, 2014-12-11 10:50:28]

+oshima@ as ../ui/ash/.* files owner

[oshima, 2014-12-11 19:21:31]

https://codereview.chromium.org/783963002/diff/40001/chrome/browser/ui/ash/sy...
File chrome/browser/ui/ash/system_tray_delegate_chromeos.cc (right):

https://codereview.chromium.org/783963002/diff/40001/chrome/browser/ui/ash/sy...
chrome/browser/ui/ash/system_tray_delegate_chromeos.cc:406: else
nuke else

https://codereview.chromium.org/783963002/diff/40001/chrome/browser/ui/ash/sy...
chrome/browser/ui/ash/system_tray_delegate_chromeos.cc:413: }
we should move this to ash at some point.

https://codereview.chromium.org/783963002/diff/40001/chrome/browser/ui/ash/sy...
chrome/browser/ui/ash/system_tray_delegate_chromeos.cc:418: 
UserManager is componentized so you can use them in ash. no need for delegate.

[merkulova, 2014-12-12 08:54:56]

PTAL

https://codereview.chromium.org/783963002/diff/40001/chrome/browser/ui/ash/sy...
File chrome/browser/ui/ash/system_tray_delegate_chromeos.cc (right):

https://codereview.chromium.org/783963002/diff/40001/chrome/browser/ui/ash/sy...
chrome/browser/ui/ash/system_tray_delegate_chromeos.cc:406: else
On 2014/12/11 19:21:30, oshima wrote:
> nuke else

Done.

https://codereview.chromium.org/783963002/diff/40001/chrome/browser/ui/ash/sy...
chrome/browser/ui/ash/system_tray_delegate_chromeos.cc:413: }
On 2014/12/11 19:21:30, oshima wrote:
> we should move this to ash at some point.

Hm, you mean which place? Inlined for now.

https://codereview.chromium.org/783963002/diff/40001/chrome/browser/ui/ash/sy...
chrome/browser/ui/ash/system_tray_delegate_chromeos.cc:418: 
On 2014/12/11 19:21:30, oshima wrote:
> UserManager is componentized so you can use them in ash. no need for delegate.

Done.

[oshima, 2014-12-12 20:35:22]

lgtm

https://codereview.chromium.org/783963002/diff/40001/chrome/browser/ui/ash/sy...
File chrome/browser/ui/ash/system_tray_delegate_chromeos.cc (right):

https://codereview.chromium.org/783963002/diff/40001/chrome/browser/ui/ash/sy...
chrome/browser/ui/ash/system_tray_delegate_chromeos.cc:413: }
On 2014/12/12 08:54:56, merkulova wrote:
> On 2014/12/11 19:21:30, oshima wrote:
> > we should move this to ash at some point.
> 
> Hm, you mean which place? Inlined for now.

like below, there is no need to delegate this. 
I'll address it in ash refactoring later.

[merkulova, 2014-12-15 08:24:46]

The CQ bit was checked by merkulova@chromium.org

[commit-bot: I haz the power, 2014-12-15 08:25:38]

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

[commit-bot: I haz the power, 2014-12-15 09:25:45]

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

[commit-bot: I haz the power, 2014-12-15 09:25:46]

Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[merkulova, 2014-12-22 13:49:45]

merkulova@chromium.org changed reviewers:
+ dpolukhin@chromium.org

[merkulova, 2014-12-22 13:55:26]

I'm really sorry to undo the inline, Mitsuru.

It appears necessary as even if UserManager is componentized, FakeUserManager
and MockUserManager are still in chromeos namespace. I initiated a refactoring
crbug.com/443119

That means that even if we're able to invoke UserManager at ash, corresponding
tests are crashing.

This one UI feature is a part of Unicorn accounts feature that is going to be
dogfooded through vacations. So I reverted to delegate implementation with TODO
left to be able to commit this CL. On refactoring and tests fix we'll be able to
use the inline.

+dpolukhin@

[Dmitry Polukhin, 2014-12-22 14:18:22]

LGTM

oshima@ I'll make sure that refactoring will be done and final code will look
like you approved it.

[merkulova, 2014-12-22 14:18:45]

The CQ bit was checked by merkulova@chromium.org

[commit-bot: I haz the power, 2014-12-22 14:19:37]

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

[commit-bot: I haz the power, 2014-12-22 15:27:13]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2014-12-22 15:27:57]

Patchset 5 (id:??) landed as
https://crrev.com/73b32fca8ed133af29af871d2108f9ea5de7f0bf
Cr-Commit-Position: refs/heads/master@{#309427}