chromeos: Avoid bogus child account warning at startup.

chromeos: Avoid bogus child account warning at startup.

I'm seeing a "User instance wasn't found while setting child
account flag." warning at boot. Avoid logging anything if
the current profile is the signin profile.

BUG=636175
Committed: https://crrev.com/41f175f8b846a87f277fbb891522ec86e24a12d5
Cr-Commit-Position: refs/heads/master@{#411330}

[Daniel Erat, 2016-08-10 00:28:44]

derat@chromium.org changed reviewers:
+ bauerb@chromium.org

[Daniel Erat, 2016-08-10 00:28:57]

The CQ bit was checked by derat@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-10 00:29:24]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-10 01:52:34]

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

[commit-bot: I haz the power, 2016-08-10 01:52:35]

Dry run: This issue passed the CQ dry run.

[Bernhard Bauer, 2016-08-10 14:23:57]

lgtm

https://codereview.chromium.org/2232653002/diff/1/chrome/browser/supervised_u...
File chrome/browser/supervised_user/child_accounts/child_account_service.cc
(right):

https://codereview.chromium.org/2232653002/diff/1/chrome/browser/supervised_u...
chrome/browser/supervised_user/child_accounts/child_account_service.cc:302:
LOG(WARNING)
Actually, would it make sense to have a DCHECK/CHECK here? Could this
legitimately happen outside of the signin profile?

[Daniel Erat, 2016-08-10 15:08:41]

https://codereview.chromium.org/2232653002/diff/1/chrome/browser/supervised_u...
File chrome/browser/supervised_user/child_accounts/child_account_service.cc
(right):

https://codereview.chromium.org/2232653002/diff/1/chrome/browser/supervised_u...
chrome/browser/supervised_user/child_accounts/child_account_service.cc:302:
LOG(WARNING)
On 2016/08/10 14:23:57, Bernhard Bauer wrote:
> Actually, would it make sense to have a DCHECK/CHECK here? Could this
> legitimately happen outside of the signin profile?

sorry, i have no idea -- i've never looked at this code before yesterday.

re switching it to a check, is it a situation that means that we should expect
undefined behavior or security issues? is it likely that restarting chrome will
clear it up? if not, i don't think a check is appropriate.

[Bernhard Bauer, 2016-08-10 17:17:29]

https://codereview.chromium.org/2232653002/diff/1/chrome/browser/supervised_u...
File chrome/browser/supervised_user/child_accounts/child_account_service.cc
(right):

https://codereview.chromium.org/2232653002/diff/1/chrome/browser/supervised_u...
chrome/browser/supervised_user/child_accounts/child_account_service.cc:302:
LOG(WARNING)
On 2016/08/10 15:08:40, Daniel Erat wrote:
> On 2016/08/10 14:23:57, Bernhard Bauer wrote:
> > Actually, would it make sense to have a DCHECK/CHECK here? Could this
> > legitimately happen outside of the signin profile?
> 
> sorry, i have no idea -- i've never looked at this code before yesterday.
> 
> re switching it to a check, is it a situation that means that we should expect
> undefined behavior or security issues? is it likely that restarting chrome
will
> clear it up? if not, i don't think a check is appropriate.

Hm, I'm thinking this is a situation that is _supposed_ to never happen (at
least I don't know when it would legitimately happen), so a DCHECK would be
appropriate (cf.
https://chromium.googlesource.com/chromium/src/+/master/styleguide/c++/c++.md...).

[Daniel Erat, 2016-08-10 20:12:13]

https://codereview.chromium.org/2232653002/diff/1/chrome/browser/supervised_u...
File chrome/browser/supervised_user/child_accounts/child_account_service.cc
(right):

https://codereview.chromium.org/2232653002/diff/1/chrome/browser/supervised_u...
chrome/browser/supervised_user/child_accounts/child_account_service.cc:302:
LOG(WARNING)
On 2016/08/10 17:17:29, Bernhard Bauer wrote:
> On 2016/08/10 15:08:40, Daniel Erat wrote:
> > On 2016/08/10 14:23:57, Bernhard Bauer wrote:
> > > Actually, would it make sense to have a DCHECK/CHECK here? Could this
> > > legitimately happen outside of the signin profile?
> > 
> > sorry, i have no idea -- i've never looked at this code before yesterday.
> > 
> > re switching it to a check, is it a situation that means that we should
expect
> > undefined behavior or security issues? is it likely that restarting chrome
> will
> > clear it up? if not, i don't think a check is appropriate.
> 
> Hm, I'm thinking this is a situation that is _supposed_ to never happen (at
> least I don't know when it would legitimately happen), so a DCHECK would be
> appropriate (cf.
>
https://chromium.googlesource.com/chromium/src/+/master/styleguide/c++/c++.md...).

how about LOG(DFATAL)? then we get a crash in debug builds as if we used
DCHECK() and an error message in release builds.

[Bernhard Bauer, 2016-08-11 11:07:17]

OK, LGTM.

[Daniel Erat, 2016-08-11 14:10:56]

The CQ bit was checked by derat@chromium.org

[commit-bot: I haz the power, 2016-08-11 14:11:24]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-11 15:04:00]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-08-11 15:05:32]

Description was changed from

==========
chromeos: Avoid bogus child account warning at startup.

I'm seeing a "User instance wasn't found while setting child
account flag." warning at boot. Avoid logging anything if
the current profile is the signin profile.

BUG=636175
==========

to

==========
chromeos: Avoid bogus child account warning at startup.

I'm seeing a "User instance wasn't found while setting child
account flag." warning at boot. Avoid logging anything if
the current profile is the signin profile.

BUG=636175

Committed: https://crrev.com/41f175f8b846a87f277fbb891522ec86e24a12d5
Cr-Commit-Position: refs/heads/master@{#411330}
==========

[commit-bot: I haz the power, 2016-08-11 15:05:32]

Patchset 2 (id:??) landed as
https://crrev.com/41f175f8b846a87f277fbb891522ec86e24a12d5
Cr-Commit-Position: refs/heads/master@{#411330}